[U-Boot] [PATCH v2 1/2] x86: Add efi runtime reset

[Alexander Graf]

On 30.01.19 00:48, Heinrich Schuchardt wrote:
> On 1/30/19 12:15 AM, Alexander Graf wrote:
>>
>>
>> On 30.01.19 00:08, Heinrich Schuchardt wrote:
>>> On 1/28/19 4:42 PM, Alexander Graf wrote:
>>>> Our selftest will soon test the actual runtime reset function rather than
>>>> the boot time one. For this, we need to ensure that the runtime version
>>>> actually succeeds on x86 to keep our travis tests work.
>>>>
>>>> So this patch implements an x86 runtime reset function. It is missing
>>>> shutdown functionality today, but OSs usually implement that via ACPI
>>>> and this function does more than the stub from before, so it's at least
>>>> an improvement.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>>> ---
>>>>  drivers/sysreset/sysreset_x86.c | 23 +++++++++++++++++++++++
>>>>  1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/drivers/sysreset/sysreset_x86.c b/drivers/sysreset/sysreset_x86.c
>>>> index 20b958cfd4..efed45ccb7 100644
>>>> --- a/drivers/sysreset/sysreset_x86.c
>>>> +++ b/drivers/sysreset/sysreset_x86.c
>>>> @@ -10,6 +10,29 @@
>>>>  #include <sysreset.h>
>>>>  #include <asm/io.h>
>>>>  #include <asm/processor.h>
>>>> +#include <efi_loader.h>
>>>> +
>>>> +#ifdef CONFIG_EFI_LOADER
>>>> +void __efi_runtime EFIAPI efi_reset_system(
>>>> +			enum efi_reset_type reset_type,
>>>> +			efi_status_t reset_status,
>>>> +			unsigned long data_size, void *reset_data)
>>>> +{
>>>> +	u32 value = 0;
>>>> +
>>>> +	if (reset_type == EFI_RESET_COLD)
>>>> +		value = SYS_RST | RST_CPU | FULL_RST;
>>>> +	else if (reset_type == EFI_RESET_WARM)
>>>> +		value = SYS_RST | RST_CPU;
>>>> +
>>>> +	/* TODO EFI_RESET_PLATFORM_SPECIFIC and EFI_RESET_SHUTDOWN */
>>>> +
>>>> +	if (value)
>>>> +		outb(value, IO_PORT_RESET);
>>>
>>> This does not look ACPI compliant. Shouldn't we read the ACPI table to
>>> identify the reset register?
>>
>> There are about 500 different ways to reset the system, CPU, something
>> on x86. I don't think U-Boot should do anything with ACPI. It's an ACPI
>> producer, not an ACPI consumer.
>>
>>> When we do the reset CPUs in several sockets may be running multiple
>>> cores each. Are all of these stopped via this register? Or do we first
>>> have to halt all but one core before doing the reset?
>>
>> I don't know what the reset protocol dictates here. But it probably also
>> heavily depends on the target platform we're looking at. IIUC this
>> particular reset is the keyboard controller one which just pulls the
>> reset line of the primary CPU socket.
>>
>> I am not aware of any multi-socket targets that U-Boot supports, so it
>> should work in all of today's cases?
>>
>> Realistically, I would not expect anyone to care too much about U-Boot
>> on multi-socket (x86) systems :).
> 
> Your code resembles loosely BOOT_CF9_SAFE in
> native_machine_emergency_restart() in Linux arch/x86/kernel/reboot.c
> which is tried before using the keyboard controller as last resort.
> 
> u8 reboot_code = reboot_mode == REBOOT_WARM ?  0x06 : 0x0E;
> u8 cf9 = inb(0xcf9) & ~reboot_code;
> outb(cf9|2, 0xcf9); /* Request hard reset */
> udelay(50);
> /* Actually do the reset */
> outb(cf9|reboot_code, 0xcf9);
> udelay(50);
> 
> So the Kernel first switches bit 2 off and bit 1 on, waits, and then
> switches bit 2 on, cf.
> http://smackerelofopinion.blogspot.com/2011/02/resetting-pc-using-reset-control.html
> 
> Shouldn't we do it the same way as the Kernel does it?

Not sure, I really just copied the existing code ;). I guess I should
make that more obvious by not duplicating, but calling it.


Alex