|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v3] x86/nSVM: Check the L1 IOPM_BASE assigned physical address
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 <abdelkareem.abdelsaamad@xxxxxxxxxx>
---
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |