On Tue, Jan 17, 2023 at 4:57 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 optional acpi callback allowing to skip setting
> the wakeup address, as in the Xen PV case this will be handled by the
> hypervisor anyway.
>
> Fixes: f1e525009493 ("x86/boot: Skip realmode init code when running as Xen PV guest")
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2:
> - new approach, avoid calling acpi_get_wakeup_address()
I'll queue this up for 6.3 if the x86 people don't object.
Thanks!
> ---
> arch/x86/include/asm/acpi.h | 8 ++++++++
> drivers/acpi/sleep.c | 6 +++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> index 65064d9f7fa6..8eb74cf386db 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -14,6 +14,7 @@
> #include <asm/mmu.h>
> #include <asm/mpspec.h>
> #include <asm/x86_init.h>
> +#include <asm/cpufeature.h>
>
> #ifdef CONFIG_ACPI_APEI
> # include <asm/pgtable_types.h>
> @@ -63,6 +64,13 @@ extern int (*acpi_suspend_lowlevel)(void);
> /* Physical address to resume after wakeup */
> unsigned long acpi_get_wakeup_address(void);
>
> +static inline bool acpi_skip_set_wakeup_address(void)
> +{
> + return cpu_feature_enabled(X86_FEATURE_XENPV);
> +}
> +
> +#define acpi_skip_set_wakeup_address acpi_skip_set_wakeup_address
> +
> /*
> * Check if the CPU can handle C2 and deeper
> */
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 0b557c0d405e..4ca667251272 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -60,13 +60,17 @@ static struct notifier_block tts_notifier = {
> .priority = 0,
> };
>
> +#ifndef acpi_skip_set_wakeup_address
> +#define acpi_skip_set_wakeup_address() false
> +#endif
> +
> static int acpi_sleep_prepare(u32 acpi_state)
> {
> #ifdef CONFIG_ACPI_SLEEP
> unsigned long acpi_wakeup_address;
>
> /* do we have a wakeup address for S2 and S3? */
> - if (acpi_state == ACPI_STATE_S3) {
> + if (acpi_state == ACPI_STATE_S3 && !acpi_skip_set_wakeup_address()) {
> acpi_wakeup_address = acpi_get_wakeup_address();
> if (!acpi_wakeup_address)
> return -EFAULT;
> --
