2019-04-23 09:31:23

by Roger Pau Monne

[permalink] [raw]
Subject: [PATCH 1/2] xen/pvh: set xen_domain_type to HVM in xen_pvh_init

Or else xen_domain() returns false despite xen_pvh being set.

Signed-off-by: Roger Pau Monné <[email protected]>
---
Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: [email protected]
---
arch/x86/xen/enlighten_pvh.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index 35b7599d2d0b..bbffa409e0e8 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -27,6 +27,7 @@ void __init xen_pvh_init(void)
u64 pfn;

xen_pvh = 1;
+ xen_domain_type = XEN_HVM_DOMAIN;
xen_start_flags = pvh_start_info.flags;

msr = cpuid_ebx(xen_cpuid_base() + 2);
--
2.17.2 (Apple Git-113)


2019-04-23 09:30:34

by Roger Pau Monne

[permalink] [raw]
Subject: [PATCH 2/2] xen/pvh: correctly setup the PV EFI interface for dom0

This involves initializing the boot params EFI related fields and the
efi global variable.

Without this fix a PVH dom0 doesn't detect when booted from EFI, and
thus doesn't support accessing any of the EFI related data.

Reported-by: PGNet Dev <[email protected]>
Signed-off-by: Roger Pau Monné <[email protected]>
---
Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Stefano Stabellini <[email protected]>
Cc: Darren Hart <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/x86/platform/pvh/enlighten.c | 9 ++++++---
arch/x86/xen/efi.c | 12 ++++++------
arch/x86/xen/enlighten_pv.c | 2 +-
arch/x86/xen/xen-ops.h | 8 --------
include/xen/xen-ops.h | 8 +++++++-
5 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c
index 62f5c7045944..a748d2ea2d90 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -11,6 +11,7 @@
#include <asm/xen/interface.h>

#include <xen/xen.h>
+#include <xen/xen-ops.h>
#include <xen/interface/hvm/start_info.h>

/*
@@ -44,8 +45,6 @@ void __init __weak mem_map_via_hcall(struct boot_params *ptr __maybe_unused)

static void __init init_pvh_bootparams(bool xen_guest)
{
- memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
-
if ((pvh_start_info.version > 0) && (pvh_start_info.memmap_entries)) {
struct hvm_memmap_table_entry *ep;
int i;
@@ -111,8 +110,10 @@ void __init __weak xen_pvh_init(void)

static void hypervisor_specific_init(bool xen_guest)
{
- if (xen_guest)
+ if (xen_guest) {
xen_pvh_init();
+ xen_efi_init(&pvh_bootparams);
+ }
}

/*
@@ -131,6 +132,8 @@ void __init xen_prepare_pvh(void)
BUG();
}

+ memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
+
hypervisor_specific_init(xen_guest);

init_pvh_bootparams(xen_guest);
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 1fbb629a9d78..0d3365cb64de 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -158,7 +158,7 @@ static enum efi_secureboot_mode xen_efi_get_secureboot(void)
return efi_secureboot_mode_unknown;
}

-void __init xen_efi_init(void)
+void __init xen_efi_init(struct boot_params *boot_params)
{
efi_system_table_t *efi_systab_xen;

@@ -167,12 +167,12 @@ void __init xen_efi_init(void)
if (efi_systab_xen == NULL)
return;

- strncpy((char *)&boot_params.efi_info.efi_loader_signature, "Xen",
- sizeof(boot_params.efi_info.efi_loader_signature));
- boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
- boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
+ strncpy((char *)&boot_params->efi_info.efi_loader_signature, "Xen",
+ sizeof(boot_params->efi_info.efi_loader_signature));
+ boot_params->efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
+ boot_params->efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);

- boot_params.secure_boot = xen_efi_get_secureboot();
+ boot_params->secure_boot = xen_efi_get_secureboot();

set_bit(EFI_BOOT, &efi.flags);
set_bit(EFI_PARAVIRT, &efi.flags);
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index c54a493e139a..4722ba2966ac 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1403,7 +1403,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
/* We need this for printk timestamps */
xen_setup_runstate_info(0);

- xen_efi_init();
+ xen_efi_init(&boot_params);

/* Start the world */
#ifdef CONFIG_X86_32
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 0e60bd918695..357eb3fa2144 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -121,14 +121,6 @@ static inline void __init xen_init_vga(const struct dom0_vga_console_info *info,

void __init xen_init_apic(void);

-#ifdef CONFIG_XEN_EFI
-extern void xen_efi_init(void);
-#else
-static inline void __init xen_efi_init(void)
-{
-}
-#endif
-
__visible void xen_irq_enable_direct(void);
__visible void xen_irq_disable_direct(void);
__visible unsigned long xen_save_fl_direct(void);
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 4969817124a8..51ef98e96d88 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -209,6 +209,7 @@ int xen_xlate_map_ballooned_pages(xen_pfn_t **pfns, void **vaddr,

bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);

+#ifdef CONFIG_XEN_EFI
efi_status_t xen_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc);
efi_status_t xen_efi_set_time(efi_time_t *tm);
efi_status_t xen_efi_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
@@ -233,7 +234,12 @@ efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
int *reset_type);
void xen_efi_reset_system(int reset_type, efi_status_t status,
unsigned long data_size, efi_char16_t *data);
-
+extern void xen_efi_init(struct boot_params *boot_params);
+#else
+static inline void __init xen_efi_init(struct boot_params *boot_params)
+{
+}
+#endif

#ifdef CONFIG_PREEMPT

--
2.17.2 (Apple Git-113)

2019-04-23 09:37:31

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 2/2] xen/pvh: correctly setup the PV EFI interface for dom0

On 23/04/2019 11:28, Roger Pau Monne wrote:
> This involves initializing the boot params EFI related fields and the
> efi global variable.
>
> Without this fix a PVH dom0 doesn't detect when booted from EFI, and
> thus doesn't support accessing any of the EFI related data.
>
> Reported-by: PGNet Dev <[email protected]>
> Signed-off-by: Roger Pau Monné <[email protected]>
> ---
> Cc: Boris Ostrovsky <[email protected]>
> Cc: Juergen Gross <[email protected]>
> Cc: Stefano Stabellini <[email protected]>
> Cc: Darren Hart <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/x86/platform/pvh/enlighten.c | 9 ++++++---
> arch/x86/xen/efi.c | 12 ++++++------
> arch/x86/xen/enlighten_pv.c | 2 +-
> arch/x86/xen/xen-ops.h | 8 --------
> include/xen/xen-ops.h | 8 +++++++-
> 5 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c
> index 62f5c7045944..a748d2ea2d90 100644
> --- a/arch/x86/platform/pvh/enlighten.c
> +++ b/arch/x86/platform/pvh/enlighten.c
> @@ -11,6 +11,7 @@
> #include <asm/xen/interface.h>
>
> #include <xen/xen.h>
> +#include <xen/xen-ops.h>
> #include <xen/interface/hvm/start_info.h>
>
> /*
> @@ -44,8 +45,6 @@ void __init __weak mem_map_via_hcall(struct boot_params *ptr __maybe_unused)
>
> static void __init init_pvh_bootparams(bool xen_guest)
> {
> - memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
> -
> if ((pvh_start_info.version > 0) && (pvh_start_info.memmap_entries)) {
> struct hvm_memmap_table_entry *ep;
> int i;
> @@ -111,8 +110,10 @@ void __init __weak xen_pvh_init(void)
>
> static void hypervisor_specific_init(bool xen_guest)
> {
> - if (xen_guest)
> + if (xen_guest) {
> xen_pvh_init();
> + xen_efi_init(&pvh_bootparams);
> + }
> }
>
> /*
> @@ -131,6 +132,8 @@ void __init xen_prepare_pvh(void)
> BUG();
> }
>
> + memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
> +
> hypervisor_specific_init(xen_guest);
>
> init_pvh_bootparams(xen_guest);
> diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> index 1fbb629a9d78..0d3365cb64de 100644
> --- a/arch/x86/xen/efi.c
> +++ b/arch/x86/xen/efi.c
> @@ -158,7 +158,7 @@ static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> return efi_secureboot_mode_unknown;
> }
>
> -void __init xen_efi_init(void)
> +void __init xen_efi_init(struct boot_params *boot_params)
> {
> efi_system_table_t *efi_systab_xen;
>
> @@ -167,12 +167,12 @@ void __init xen_efi_init(void)
> if (efi_systab_xen == NULL)
> return;
>
> - strncpy((char *)&boot_params.efi_info.efi_loader_signature, "Xen",
> - sizeof(boot_params.efi_info.efi_loader_signature));
> - boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
> - boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
> + strncpy((char *)&boot_params->efi_info.efi_loader_signature, "Xen",
> + sizeof(boot_params->efi_info.efi_loader_signature));
> + boot_params->efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
> + boot_params->efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
>
> - boot_params.secure_boot = xen_efi_get_secureboot();
> + boot_params->secure_boot = xen_efi_get_secureboot();
>
> set_bit(EFI_BOOT, &efi.flags);
> set_bit(EFI_PARAVIRT, &efi.flags);
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index c54a493e139a..4722ba2966ac 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -1403,7 +1403,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
> /* We need this for printk timestamps */
> xen_setup_runstate_info(0);
>
> - xen_efi_init();
> + xen_efi_init(&boot_params);
>
> /* Start the world */
> #ifdef CONFIG_X86_32
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 0e60bd918695..357eb3fa2144 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -121,14 +121,6 @@ static inline void __init xen_init_vga(const struct dom0_vga_console_info *info,
>
> void __init xen_init_apic(void);
>
> -#ifdef CONFIG_XEN_EFI
> -extern void xen_efi_init(void);
> -#else
> -static inline void __init xen_efi_init(void)
> -{
> -}
> -#endif
> -
> __visible void xen_irq_enable_direct(void);
> __visible void xen_irq_disable_direct(void);
> __visible unsigned long xen_save_fl_direct(void);
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index 4969817124a8..51ef98e96d88 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -209,6 +209,7 @@ int xen_xlate_map_ballooned_pages(xen_pfn_t **pfns, void **vaddr,
>
> bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);
>
> +#ifdef CONFIG_XEN_EFI
> efi_status_t xen_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc);
> efi_status_t xen_efi_set_time(efi_time_t *tm);
> efi_status_t xen_efi_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
> @@ -233,7 +234,12 @@ efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
> int *reset_type);
> void xen_efi_reset_system(int reset_type, efi_status_t status,
> unsigned long data_size, efi_char16_t *data);
> -
> +extern void xen_efi_init(struct boot_params *boot_params);
> +#else
> +static inline void __init xen_efi_init(struct boot_params *boot_params)

Is struct boot_params defined on ARM?


Juergen

2019-04-23 09:44:01

by Roger Pau Monne

[permalink] [raw]
Subject: Re: [PATCH 2/2] xen/pvh: correctly setup the PV EFI interface for dom0

On Tue, Apr 23, 2019 at 11:36:10AM +0200, Juergen Gross wrote:
> On 23/04/2019 11:28, Roger Pau Monne wrote:
> > diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> > index 4969817124a8..51ef98e96d88 100644
> > --- a/include/xen/xen-ops.h
> > +++ b/include/xen/xen-ops.h
> > @@ -209,6 +209,7 @@ int xen_xlate_map_ballooned_pages(xen_pfn_t **pfns, void **vaddr,
> >
> > bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);
> >
> > +#ifdef CONFIG_XEN_EFI
> > efi_status_t xen_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc);
> > efi_status_t xen_efi_set_time(efi_time_t *tm);
> > efi_status_t xen_efi_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
> > @@ -233,7 +234,12 @@ efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
> > int *reset_type);
> > void xen_efi_reset_system(int reset_type, efi_status_t status,
> > unsigned long data_size, efi_char16_t *data);
> > -
> > +extern void xen_efi_init(struct boot_params *boot_params);
> > +#else
> > +static inline void __init xen_efi_init(struct boot_params *boot_params)
>
> Is struct boot_params defined on ARM?

Seems to be defined for x86 and some mips only, so I will have to find
another header. Will post v2 shortly.

Thanks, Roger.