Re: [PATCH] x86/acpi: fix suspend with Xen

[Juergen Gross <jgross@xxxxxxxx>]

On 17.01.23 15:09, Rafael J. Wysocki wrote:
> On Mon, Jan 16, 2023 at 7:45 AM Juergen Gross <jgross@xxxxxxxx> wrote:
> > On 13.01.23 20:40, Rafael J. Wysocki wrote:
> > > On Fri, Jan 13, 2023 at 3:06 PM Juergen Gross <jgross@xxxxxxxx> wrote:
> > > > Commit f1e525009493 ("x86/boot: Skip realmode init code when running as
> > > > Xen PV guest") missed one code path accessing real_mode_header, leading
> > > > to dereferencing NULL when suspending the system under Xen:
> > > >
> > > >       [  348.284004] PM: suspend entry (deep)
> > > >       [  348.289532] Filesystems sync: 0.005 seconds
> > > >       [  348.291545] Freezing user space processes ... (elapsed 0.000 seconds) 
> > > > done.
> > > >       [  348.292457] OOM killer disabled.
> > > >       [  348.292462] Freezing remaining freezable tasks ... (elapsed 0.104 
> > > > seconds) done.
> > > >       [  348.396612] printk: Suspending console(s) (use no_console_suspend to 
> > > > debug)
> > > >       [  348.749228] PM: suspend devices took 0.352 seconds
> > > >       [  348.769713] ACPI: EC: interrupt blocked
> > > >       [  348.816077] BUG: kernel NULL pointer dereference, address: 
> > > > 000000000000001c
> > > >       [  348.816080] #PF: supervisor read access in kernel mode
> > > >       [  348.816081] #PF: error_code(0x0000) - not-present page
> > > >       [  348.816083] PGD 0 P4D 0
> > > >       [  348.816086] Oops: 0000 [#1] PREEMPT SMP NOPTI
> > > >       [  348.816089] CPU: 0 PID: 6764 Comm: systemd-sleep Not tainted 
> > > > 6.1.3-1.fc32.qubes.x86_64 #1
> > > >       [  348.816092] Hardware name: Star Labs StarBook/StarBook, BIOS 8.01 
> > > > 07/03/2022
> > > >       [  348.816093] RIP: e030:acpi_get_wakeup_address+0xc/0x20
> > > >
> > > > Fix that by adding an indirection for acpi_get_wakeup_address() which
> > > > Xen PV dom0 can use to return a dummy non-zero wakeup address (this
> > > > address won't ever be used, as the real suspend handling is done by the
> > > > hypervisor).
> > > How exactly does this help?
> > I believed the first sentence of the commit message would make this
> > clear enough.
> That was clear, but the fix part wasn't really.
>
> > I can expand the commit message to go more into detail if you think
> > this is really needed.
> IMO calling acpi_set_waking_vector() with a known-invalid wakeup
> vector address in dom0 is plain confusing.
>
> I'm not sure what to do about it yet, but IMV something needs to be done.
Another possibility would be to modify acpi_sleep_prepare(), e.g. like the
attached patch (compile tested only).


Juergen

Attachment: 0001-acpi-fix-suspend-with-Xen-PV.patch
Description: Text Data

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature