Re: [PATCH] x86/shutdown: change default reboot method preference

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 15, 2023 at 09:11:50AM +0200, Roger Pau Monné wrote:
> On Thu, Sep 14, 2023 at 06:42:03PM +0100, Andrew Cooper wrote:
> > On 14/09/2023 4:21 pm, Roger Pau Monne wrote:
> > > The current logic to chose the preferred reboot method is based on the 
> > > mode Xen
> > > has been booted into, so if the box is booted from UEFI, the preferred 
> > > reboot
> > > method will be to use the ResetSystem() run time service call.
> > >
> > > However, that call seems to be widely untested once the firmware has 
> > > exited
> > > boot services, and quite often leads to a result similar to:
> > 
> > I don't know how true this is.  What Xen does differently to most of the
> > rest of the world is not calling SetVirtualAddressMap()
> 
> Hm, but that doesn't seem to affect the rest of the run-time services
> (at least on this box), it's (just?) ResetSystem() that misbehaves.

I've seen (too) many firmwares where also GetVariable crashes without
SetVirtualAddressMap(). That's why we have a build-time option to
actually call SetVirtualAddressMap().

Anyway, I agree that preferring ACPI reboot even when booted through
UEFI is a good idea.

> I can remove that sentence however, it is more of a guess rather than
> a fact.
> 
> > 
> > >
> > > Hardware Dom0 shutdown: rebooting machine
> > > ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:   C    ]----
> > > CPU:    0
> > > RIP:    e008:[<0000000000000017>] 0000000000000017
> > > RFLAGS: 0000000000010202   CONTEXT: hypervisor
> > > [...]
> > > Xen call trace:
> > >    [<0000000000000017>] R 0000000000000017
> > >    [<ffff83207eff7b50>] S ffff83207eff7b50
> > >    [<ffff82d0403525aa>] F machine_restart+0x1da/0x261
> > >    [<ffff82d04035263c>] F apic_wait_icr_idle+0/0x37
> > >    [<ffff82d040233689>] F smp_call_function_interrupt+0xc7/0xcb
> > >    [<ffff82d040352f05>] F call_function_interrupt+0x20/0x34
> > >    [<ffff82d04033b0d5>] F do_IRQ+0x150/0x6f3
> > >    [<ffff82d0402018c2>] F common_interrupt+0x132/0x140
> > >    [<ffff82d040283d33>] F 
> > > arch/x86/acpi/cpu_idle.c#acpi_idle_do_entry+0x113/0x129
> > >    [<ffff82d04028436c>] F 
> > > arch/x86/acpi/cpu_idle.c#acpi_processor_idle+0x3eb/0x5f7
> > >    [<ffff82d04032a549>] F arch/x86/domain.c#idle_loop+0xec/0xee
> > >
> > > ****************************************
> > > Panic on CPU 0:
> > > FATAL TRAP: vector = 6 (invalid opcode)
> > > ****************************************
> > >
> > > Which in most cases does lead to a reboot, however that's unreliable.
> > >
> > > Change the default reboot preference to prefer ACPI over UEFI if 
> > > available and
> > > not in reduced hardware mode.
> > >
> > > This is in line to what Linux does, so it's unlikely to cause issues on 
> > > current
> > > and future hardware, since there's a much higher chance of vendors testing
> > > hardware with Linux rather than Xen.
> > >
> > > I'm not aware of using ACPI reboot causing issues on boxes that do have
> > > properly implemented ResetSystem() methods.
> > >
> > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > 
> > Wording aside, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > 
> > This is a giant embarrassment to Xen, and needs fixing.
> 
> Looking at Linux I think we need to add an override for Acer
> TravelMate X514-51T to force it to use UEFI, will send v2 with it.
> 
> I do wonder if we need to keep the reboot_dmi_table[] entries that
> force systems to use ACPI, as it would now be the default?  My
> thinking is that it's likely better to keep it just in case we change
> the default again in the future.
> 
> Thanks, Roger.
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature