On Mon, Nov 23, 2020 at 01:40:12PM +0100, Jan Beulich wrote:
> Use of __acpi_map_table() here was at least close to an abuse already
> before, but it will now consistently return NULL here. Drop the layering
> violation and use set_fixmap() directly. Re-use of the ACPI fixmap area
> is hopefully going to remain "fine" for the time being.
>
> Add checks to acpi_enter_sleep(): The vector now needs to be contained
> within a single page, but the ACPI spec requires 64-byte alignment of
> FACS anyway. Also bail if no wakeup vector was determined in the first
> place, in part as preparation for a subsequent relaxation change.
>
> Fixes: 1c4aa69ca1e1 ("xen/acpi: Rework acpi_os_map_memory() and
> acpi_os_unmap_memory()")
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> --- a/xen/arch/x86/acpi/boot.c
> +++ b/xen/arch/x86/acpi/boot.c
> @@ -443,6 +443,11 @@ acpi_fadt_parse_sleep_info(struct acpi_t
> "FACS is shorter than ACPI spec allow: %#x",
> facs->length);
>
> + if (facs_pa % 64)
> + printk(KERN_WARNING PREFIX
> + "FACS is not 64-byte aligned: %#lx",
> + facs_pa);
> +
> acpi_sinfo.wakeup_vector = facs_pa +
> offsetof(struct acpi_table_facs, firmware_waking_vector);
> acpi_sinfo.vector_width = 32;
> --- a/xen/arch/x86/acpi/power.c
> +++ b/xen/arch/x86/acpi/power.c
> @@ -174,17 +174,20 @@ static void acpi_sleep_prepare(u32 state
> if ( state != ACPI_STATE_S3 )
> return;
>
> - wakeup_vector_va = __acpi_map_table(
> - acpi_sinfo.wakeup_vector, sizeof(uint64_t));
> -
> /* TBoot will set resume vector itself (when it is safe to do so). */
> if ( tboot_in_measured_env() )
> return;
>
> + set_fixmap(FIX_ACPI_END, acpi_sinfo.wakeup_vector);
> + wakeup_vector_va = fix_to_virt(FIX_ACPI_END) +
> + PAGE_OFFSET(acpi_sinfo.wakeup_vector);
> +
> if ( acpi_sinfo.vector_width == 32 )
> *(uint32_t *)wakeup_vector_va = bootsym_phys(wakeup_start);
> else
> *(uint64_t *)wakeup_vector_va = bootsym_phys(wakeup_start);
> +
> + clear_fixmap(FIX_ACPI_END);
Why not use vmap here instead of the fixmap?
Thanks, Roger.
