[PATCH v3] x86/nSVM: Check the L1 IOPM_BASE assigned physical address
Abdelkareem Abdelsaamad <[email protected]> Fri, 7 Aug 2026 16:58:43 +0100
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <f97fdc50622e4db559ce059091d0ebf2028e2f2e.1786114518.git.abdelkareem.abdelsaamad@citrix.com> |
The Xen nested virtualization code maps the physical address assigned by the L1 guests, for IOPM_BASE, directly to valid host address without sanity checks. Add sanity checks to verify the L1 assigned address is a valid guest address. This check also makes the bahavior compliant with the Hardware handling of the assigned addresses. The hardware is expected to trigger VMEXIT_INVALID with IOPM_BASE address greater than or qual to the maximum supported physical address, see the APM volume #2 (40332—Rev. 4.40—July 2026). While at it, clean up the code. Remove the unused bool viopm and the svm_vcpu::ns_oiomap_pa. Change nsvm_vmrun_permissionmap return error from literal 1 to NSVM_ERROR_VVMCB. Signed-off-by: Abdelkareem Abdelsaamad <[email protected]> --- Changes in V3: - Switch to using gfn_valid() instead of domain_get_maximum_gpfn() to align with what is told to the guest in the CPUID. - Change nsvm_vmrun_permissionmap return error from literal 1 to NSVM_ERROR_VVMCB. - Remove parentheses (IOPM_PAGES_COUNT - 1). Changes in V2: - Rename IOPM_MAX_PAGES_DIFF and MSRPM_MAX_PAGES_DIFF constants to IOPM_PAGES_COUNT and MSRPM_PAGES_COUNT. - Drop the 2-pages for domain check. - Change the IOPM and MSRPM boundary checks. - Use gaddr_to_gfn instead of open-coding >> PAGE_SHIFT. - Use __func__ instead of hardcoding raw function names. --- Testing: - Using a locally developed XTF nested virt setup, I manually tested VMRUN instruction handling with the address value (0xffffffffffffffffUL) assigned to VMCB::iopm_base_pa: - Without the changes the address is mapped by the Xen code to the address 0x604b634000 and it completes the execution without any reported errors. - With the changes, the VMRUN execution fails and VMEXIT_INVALID is reported back in the ns_vmexit.exitcode. - CI tests: https://gitlab.com/xen-project/people/aabdelsa/xen/-/pipelines/2741283982 --- xen/arch/x86/hvm/svm/nestedsvm.c | 17 +++++++++++++---- xen/arch/x86/include/asm/hvm/svm-types.h | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c index b06124c2c9..e895cf12c4 100644 --- a/xen/arch/x86/hvm/svm/nestedsvm.c +++ b/xen/arch/x86/hvm/svm/nestedsvm.c @@ -18,6 +18,7 @@ #define NSVM_ERROR_VVMCB 1 #define NSVM_ERROR_VMENTRY 2 +#define IOPM_PAGES_COUNT 3 int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr) { @@ -282,7 +283,7 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs) return 0; } -static int nsvm_vmrun_permissionmap(struct vcpu *v, bool viopm) +static int nsvm_vmrun_permissionmap(struct vcpu *v) { struct svm_vcpu *arch_svm = &v->arch.hvm.svm; struct nestedsvm *svm = &vcpu_nestedsvm(v); @@ -294,6 +295,15 @@ static int nsvm_vmrun_permissionmap(struct vcpu *v, bool viopm) enum hvm_translation_result ret; unsigned long *ns_viomap; bool ioport_80 = true, ioport_ed = true; + gfn_t ns_iopm_end = + gfn_add(gaddr_to_gfn(ns_vmcb->_iopm_base_pa), IOPM_PAGES_COUNT - 1); + + if ( !gfn_valid(v->domain, ns_iopm_end) ) + { + gdprintk(XENLOG_ERR, "%s invalid _iopm_base_pa address (%#"PRIx64")\n", + __func__, ns_vmcb->_iopm_base_pa); + return NSVM_ERROR_VVMCB; + } ns_msrpm_ptr = (unsigned long *)svm->ns_cached_msrpm; @@ -302,13 +312,12 @@ static int nsvm_vmrun_permissionmap(struct vcpu *v, bool viopm) if ( ret != HVMTRANS_okay ) { gdprintk(XENLOG_ERR, "hvm_copy_from_guest_phys msrpm %u\n", ret); - return 1; + return NSVM_ERROR_VVMCB; } /* Check l1 guest io permission map and get a shadow one based on * if l1 guest intercepts io ports 0x80 and/or 0xED. */ - svm->ns_oiomap_pa = svm->ns_iomap_pa; svm->ns_iomap_pa = ns_vmcb->_iopm_base_pa; ns_viomap = hvm_map_guest_frame_ro(svm->ns_iomap_pa >> PAGE_SHIFT, 0); @@ -418,7 +427,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs) n2vmcb->_tsc_offset = n1vmcb->_tsc_offset + ns_vmcb->_tsc_offset; /* Nested IO permission bitmaps */ - rc = nsvm_vmrun_permissionmap(v, clean.iopm); + rc = nsvm_vmrun_permissionmap(v); if ( rc ) return rc; diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h index 8acadb9dcc..beab9a3af2 100644 --- a/xen/arch/x86/include/asm/hvm/svm-types.h +++ b/xen/arch/x86/include/asm/hvm/svm-types.h @@ -51,7 +51,7 @@ struct nestedsvm { unsigned long *ns_merged_msrpm; /* guest physical address of virtual io permission map */ - paddr_t ns_iomap_pa, ns_oiomap_pa; + paddr_t ns_iomap_pa; /* Shadow io permission map */ unsigned long *ns_iomap; -- 2.53.0