| Message ID | 20220822213036.21630-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/domain: Fix struct domain memory corruption when building PV guests | expand |
On 22.08.2022 23:30, Andrew Cooper wrote: > arch_domain_create() can't blindly write into d->arch.hvm union. Move the > logic into hvm_domain_initialise(), which involves passing config down. > > Fixes: 2ce11ce249a3 ("x86/HVM: allow per-domain usage of hardware virtualized APIC") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> preferably with a small adjustment (see below). > This does not fix XenServer's wall of red from testing, but I have at least > figured out what's going on. There's a piece of plain RAM in place of a > working LAPIC MMIO mapping (accelerated or otherwise), which causes HVMLoader > to spin in a tight loop waiting for CPU 1 to come up after failing to send an > INIT-SIPI-SIPI. Where's that page of plain RAM coming from? And do you meanwhile understand why XenServer testing is exposing the issue while osstest isn't? > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -576,7 +576,8 @@ static int cf_check hvm_print_line( > return X86EMUL_OKAY; > } > > -int hvm_domain_initialise(struct domain *d) > +int hvm_domain_initialise(struct domain *d, > + struct xen_domctl_createdomain *config) May I ask for const to be added here? Unless you anticipate the function might legitimately modify the config data? Jan
On 23/08/2022 08:21, Jan Beulich wrote: > On 22.08.2022 23:30, Andrew Cooper wrote: >> arch_domain_create() can't blindly write into d->arch.hvm union. Move the >> logic into hvm_domain_initialise(), which involves passing config down. >> >> Fixes: 2ce11ce249a3 ("x86/HVM: allow per-domain usage of hardware virtualized APIC") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. >> This does not fix XenServer's wall of red from testing, but I have at least >> figured out what's going on. There's a piece of plain RAM in place of a >> working LAPIC MMIO mapping (accelerated or otherwise), which causes HVMLoader >> to spin in a tight loop waiting for CPU 1 to come up after failing to send an >> INIT-SIPI-SIPI. > Where's that page of plain RAM coming from? And do you meanwhile > understand why XenServer testing is exposing the issue while osstest > isn't? The sink page is allocated and inserted into the P2M based on hardware capabilities, and the VMCS is (not) configured using the new settings. APIC-V is still fully disabled in XenServer because there are still interrupts lost on migration which take out Windows. Jane's work was the very start of being able to test APIC acceleration rationally, and ultimately find a fix. ~Andrew
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 15e7e772012e..41e1e3f27272 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -853,7 +853,7 @@ int arch_domain_create(struct domain *d, if ( is_hvm_domain(d) ) { - if ( (rc = hvm_domain_initialise(d)) != 0 ) + if ( (rc = hvm_domain_initialise(d, config)) != 0 ) goto fail; } else if ( is_pv_domain(d) ) @@ -885,12 +885,6 @@ int arch_domain_create(struct domain *d, d->arch.msr_relaxed = config->arch.misc_flags & XEN_X86_MSR_RELAXED; - d->arch.hvm.assisted_xapic = - config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; - - d->arch.hvm.assisted_x2apic = - config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; - spec_ctrl_init_domain(d); return 0; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 6b5d585ed4cc..ae8267852013 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -576,7 +576,8 @@ static int cf_check hvm_print_line( return X86EMUL_OKAY; } -int hvm_domain_initialise(struct domain *d) +int hvm_domain_initialise(struct domain *d, + struct xen_domctl_createdomain *config) { unsigned int nr_gsis; int rc; @@ -597,6 +598,12 @@ int hvm_domain_initialise(struct domain *d) INIT_LIST_HEAD(&d->arch.hvm.mmcfg_regions); INIT_LIST_HEAD(&d->arch.hvm.msix_tables); + d->arch.hvm.assisted_xapic = + config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; + + d->arch.hvm.assisted_x2apic = + config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; + rc = create_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0, NULL, NULL); if ( rc ) goto fail; diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h index 03096f31effa..55a53d9cac8f 100644 --- a/xen/arch/x86/include/asm/hvm/hvm.h +++ b/xen/arch/x86/include/asm/hvm/hvm.h @@ -259,7 +259,8 @@ extern s8 hvm_port80_allowed; extern const struct hvm_function_table *start_svm(void); extern const struct hvm_function_table *start_vmx(void); -int hvm_domain_initialise(struct domain *d); +int hvm_domain_initialise(struct domain *d, + struct xen_domctl_createdomain *config); void hvm_domain_relinquish_resources(struct domain *d); void hvm_domain_destroy(struct domain *d);
arch_domain_create() can't blindly write into d->arch.hvm union. Move the logic into hvm_domain_initialise(), which involves passing config down. Fixes: 2ce11ce249a3 ("x86/HVM: allow per-domain usage of hardware virtualized APIC") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Jane Malalane <jane.malalane@citrix.com> This does not fix XenServer's wall of red from testing, but I have at least figured out what's going on. There's a piece of plain RAM in place of a working LAPIC MMIO mapping (accelerated or otherwise), which causes HVMLoader to spin in a tight loop waiting for CPU 1 to come up after failing to send an INIT-SIPI-SIPI. Sadly the fix is not as straightforward as I'd hoped, and needs more testing. --- xen/arch/x86/domain.c | 8 +------- xen/arch/x86/hvm/hvm.c | 9 ++++++++- xen/arch/x86/include/asm/hvm/hvm.h | 3 ++- 3 files changed, 11 insertions(+), 9 deletions(-)