Re: [Xen-devel] [PATCH] xen/shim: stash RSDP address for ACPI driver

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jan 22, 2018 at 12:47:10PM +0000, Wei Liu wrote:
> It used to the case that we placed RSDP under 1MB and let Xen search
> for it. We moved the placement to under 4GB in 4a5733771, so the
> search wouldn't work.
> 
> Stash the RSDP address to solve this problem.
> 
> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> What about PVH + EFI?

PVH guests using EFI firmware will get the RSDP address from the EFI
tables. Ie: EFI firmware will use the PVH entry point, thus fetching
the RSDP from start_info, but the kernel loaded from EFI should be
using the EFI entry point, and thus fetching the RSDP pointer from the
EFI tables. Or at least that was my thinking.

> ---
>  xen/arch/x86/guest/pvh-boot.c        | 4 ++++
>  xen/drivers/acpi/osl.c               | 9 +++++++++
>  xen/include/asm-x86/guest/pvh-boot.h | 1 +
>  3 files changed, 14 insertions(+)
> 
> diff --git a/xen/arch/x86/guest/pvh-boot.c b/xen/arch/x86/guest/pvh-boot.c
> index be3122b16c..427f9ea6b1 100644
> --- a/xen/arch/x86/guest/pvh-boot.c
> +++ b/xen/arch/x86/guest/pvh-boot.c
> @@ -30,6 +30,7 @@
>  /* Initialised in head.S, before .bss is zeroed. */
>  bool __initdata pvh_boot;
>  uint32_t __initdata pvh_start_info_pa;
> +unsigned long __initdata pvh_rsdp_pa;

uint64_t maybe to use the same type as start_info.h.

>  
>  static multiboot_info_t __initdata pvh_mbi;
>  static module_t __initdata pvh_mbi_mods[8];
> @@ -69,6 +70,9 @@ static void __init convert_pvh_info(void)
>          mod[i].mod_end   = entry[i].paddr + entry[i].size;
>          mod[i].string    = entry[i].cmdline_paddr;
>      }
> +
> +    /* Stash RSDP pointer so ACPI driver can get it */
> +    pvh_rsdp_pa = pvh_info->rsdp_paddr;;

Double ';'.

Is this too early to panic? IMHO we should add:

if ( !pvh_info->rsdp_paddr )
    panic("Unable to boot in PVH mode without ACPI tables");

Preferably here or at acpi_os_get_root_pointer.

>  }
>  
>  static void __init get_memory_map(void)
> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
> index 52c9b4ba9a..6a81de1707 100644
> --- a/xen/drivers/acpi/osl.c
> +++ b/xen/drivers/acpi/osl.c
> @@ -38,6 +38,10 @@
>  #include <xen/efi.h>
>  #include <xen/vmap.h>
>  
> +#ifdef CONFIG_PVH_GUEST
> +#include <asm/guest/pvh-boot.h>
> +#endif
> +
>  #define _COMPONENT           ACPI_OS_SERVICES
>  ACPI_MODULE_NAME("osl")
>  
> @@ -74,6 +78,11 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>                              "System description tables not found\n");
>                       return 0;
>               }
> +#ifdef CONFIG_PVH_GUEST
> +     } else if (pvh_boot) {
> +             ASSERT(pvh_rsdp_pa);
> +             return pvh_rsdp_pa;
> +#endif
>       } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
>               acpi_physical_address pa = 0;

Can this be done in a non-PVH specific way?

Can we have a global rsdp_hint variable or similar that would be used
here if set?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel