| Message ID | 20190423130416.68935-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 72813bfbf0276a97c82af038efb5f02dcdd9e310 |
| Headers | show |
| Series | [v2,1/2] xen/pvh: set xen_domain_type to HVM in xen_pvh_init | expand |
On 4/23/19 9:04 AM, 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 <pgnet.dev@gmail.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Hmm.. This seems to be breaking save/restore for me (for domU), and I can't see any obvious reasons. Can you try it? -boris
On Wed, Apr 24, 2019 at 11:36:41AM -0400, Boris Ostrovsky wrote: > On 4/23/19 9:04 AM, 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 <pgnet.dev@gmail.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > Hmm.. This seems to be breaking save/restore for me (for domU), and I > can't see any obvious reasons. > > Can you try it? Sure, thanks for the extra testing! I have to admit I haven't tested save/restore with this patch applied, it didn't come to mind it might affect that functionality. I assume it's save/restore of a PVH domU that's broken (HVM and PV are fine)? Thanks, Roger.
On 4/24/19 11:45 AM, Roger Pau Monné wrote: > On Wed, Apr 24, 2019 at 11:36:41AM -0400, Boris Ostrovsky wrote: >> On 4/23/19 9:04 AM, 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 <pgnet.dev@gmail.com> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> Hmm.. This seems to be breaking save/restore for me (for domU), and I >> can't see any obvious reasons. >> >> Can you try it? > Sure, thanks for the extra testing! I have to admit I haven't tested > save/restore with this patch applied, it didn't come to mind it might > affect that functionality. > > I assume it's save/restore of a PVH domU that's broken (HVM and PV are > fine)? > PV is fine, I haven't tried HVM. -boris
On Wed, Apr 24, 2019 at 11:45:43AM -0400, Boris Ostrovsky wrote: > On 4/24/19 11:45 AM, Roger Pau Monné wrote: > > On Wed, Apr 24, 2019 at 11:36:41AM -0400, Boris Ostrovsky wrote: > >> On 4/23/19 9:04 AM, 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 <pgnet.dev@gmail.com> > >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > >> Hmm.. This seems to be breaking save/restore for me (for domU), and I > >> can't see any obvious reasons. > >> > >> Can you try it? > > Sure, thanks for the extra testing! I have to admit I haven't tested > > save/restore with this patch applied, it didn't come to mind it might > > affect that functionality. > > > > I assume it's save/restore of a PVH domU that's broken (HVM and PV are > > fine)? > > > > > PV is fine, I haven't tried HVM. I've tested live migration and save/restore of this series on top of 5.1.0-rc5 and seems to work fine for me. On top of which Linux version where you testing, and can you paste the guest config file? Thanks, Roger.
On 4/25/19 6:02 AM, Roger Pau Monné wrote: > On Wed, Apr 24, 2019 at 11:45:43AM -0400, Boris Ostrovsky wrote: >> On 4/24/19 11:45 AM, Roger Pau Monné wrote: >>> On Wed, Apr 24, 2019 at 11:36:41AM -0400, Boris Ostrovsky wrote: >>>> On 4/23/19 9:04 AM, 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 <pgnet.dev@gmail.com> >>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>>> Hmm.. This seems to be breaking save/restore for me (for domU), and I >>>> can't see any obvious reasons. >>>> >>>> Can you try it? >>> Sure, thanks for the extra testing! I have to admit I haven't tested >>> save/restore with this patch applied, it didn't come to mind it might >>> affect that functionality. >>> >>> I assume it's save/restore of a PVH domU that's broken (HVM and PV are >>> fine)? >>> >> >> PV is fine, I haven't tried HVM. > I've tested live migration and save/restore of this series on top of > 5.1.0-rc5 and seems to work fine for me. On top of which Linux version > where you testing, and can you paste the guest config file? > It's not your change. I was booting with threadirqs boot option and hvc console apparently doesn't like that. It's not 100% reproducible, but I was able to fail save/restore without your patches as well. I may look at this sometime next week, but in the meantime for both patches Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> and I will add 4.19+ for stable. -boris
diff --git a/arch/x86/platform/pvh/enlighten.c b/arch/x86/platform/pvh/enlighten.c index 62f5c7045944..1861a2ba0f2b 100644 --- a/arch/x86/platform/pvh/enlighten.c +++ b/arch/x86/platform/pvh/enlighten.c @@ -44,8 +44,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; @@ -103,7 +101,7 @@ static void __init init_pvh_bootparams(bool xen_guest) * If we are trying to boot a Xen PVH guest, it is expected that the kernel * will have been configured to provide the required override for this routine. */ -void __init __weak xen_pvh_init(void) +void __init __weak xen_pvh_init(struct boot_params *boot_params) { xen_raw_printk("Error: Missing xen PVH initialization\n"); BUG(); @@ -112,7 +110,7 @@ void __init __weak xen_pvh_init(void) static void hypervisor_specific_init(bool xen_guest) { if (xen_guest) - xen_pvh_init(); + xen_pvh_init(&pvh_bootparams); } /* @@ -131,6 +129,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/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index bbffa409e0e8..80a79db72fcf 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -13,6 +13,8 @@ #include <xen/interface/memory.h> +#include "xen-ops.h" + /* * PVH variables. * @@ -21,7 +23,7 @@ */ bool xen_pvh __attribute__((section(".data"))) = 0; -void __init xen_pvh_init(void) +void __init xen_pvh_init(struct boot_params *boot_params) { u32 msr; u64 pfn; @@ -33,6 +35,8 @@ void __init xen_pvh_init(void) msr = cpuid_ebx(xen_cpuid_base() + 2); pfn = __pa(hypercall_page); wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); + + xen_efi_init(boot_params); } void __init mem_map_via_hcall(struct boot_params *boot_params_p) diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h index 0e60bd918695..2f111f47ba98 100644 --- a/arch/x86/xen/xen-ops.h +++ b/arch/x86/xen/xen-ops.h @@ -122,9 +122,9 @@ 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); +extern void xen_efi_init(struct boot_params *boot_params); #else -static inline void __init xen_efi_init(void) +static inline void __init xen_efi_init(struct boot_params *boot_params) { } #endif
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 <pgnet.dev@gmail.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Juergen Gross <jgross@suse.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Darren Hart <dvhart@infradead.org> Cc: Andy Shevchenko <andy@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: x86@kernel.org Cc: xen-devel@lists.xenproject.org Cc: platform-driver-x86@vger.kernel.org --- Changes since v1: - Call xen_efi_init from xen_pvh_init, this avoids having to move the prototype of xen_efi_init to a different header. --- arch/x86/platform/pvh/enlighten.c | 8 ++++---- arch/x86/xen/efi.c | 12 ++++++------ arch/x86/xen/enlighten_pv.c | 2 +- arch/x86/xen/enlighten_pvh.c | 6 +++++- arch/x86/xen/xen-ops.h | 4 ++-- 5 files changed, 18 insertions(+), 14 deletions(-)