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

[Heinrich Schuchardt]

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?

Best regards

Heinrich

> 
> 
> Alex
>