intel_iommu: Add missed sanity check for 256-bit invalidation queue
According to VTD spec, a 256-bit descriptor will result in an invalid
descriptor error if submitted in an IQ that is setup to provide hardware
with 128-bit descriptors (IQA_REG.DW=0). Meanwhile, there are old inv desc
types (e.g. iotlb_inv_desc) that can be either 128bits or 256bits. If a
128-bit version of this descriptor is submitted into an IQ that is setup
to provide hardware with 256-bit descriptors will also result in an invalid
descriptor error.
The 2nd will be captured by the tail register update. So we only need to
focus on the 1st.
Because the reserved bit check between different types of invalidation desc
are common, so introduce a common function vtd_inv_desc_reserved_check()
to do all the checks and pass the differences as parameters.
With this change, need to replace error_report_once() call with error_report()
to catch different call sites. This isn't an issue as error_report_once()
here is mainly used to help debug guest error, but it only dumps once in
qemu life cycle and doesn't help much, we need error_report() instead.
Fixes: c0c1d351849b ("intel_iommu: add 256 bits qi_desc support")
Suggested-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Message-Id: <20241104125536.1236118-3-zhenzhong.duan@intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1ecfe47..2fc3866 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2532,15 +2532,51 @@
return true;
}
+static bool vtd_inv_desc_reserved_check(IntelIOMMUState *s,
+ VTDInvDesc *inv_desc,
+ uint64_t mask[4], bool dw,
+ const char *func_name,
+ const char *desc_type)
+{
+ if (s->iq_dw) {
+ if (inv_desc->val[0] & mask[0] || inv_desc->val[1] & mask[1] ||
+ inv_desc->val[2] & mask[2] || inv_desc->val[3] & mask[3]) {
+ error_report("%s: invalid %s desc val[3]: 0x%"PRIx64
+ " val[2]: 0x%"PRIx64" val[1]=0x%"PRIx64
+ " val[0]=0x%"PRIx64" (reserved nonzero)",
+ func_name, desc_type, inv_desc->val[3],
+ inv_desc->val[2], inv_desc->val[1],
+ inv_desc->val[0]);
+ return false;
+ }
+ } else {
+ if (dw) {
+ error_report("%s: 256-bit %s desc in 128-bit invalidation queue",
+ func_name, desc_type);
+ return false;
+ }
+
+ if (inv_desc->lo & mask[0] || inv_desc->hi & mask[1]) {
+ error_report("%s: invalid %s desc: hi=%"PRIx64", lo=%"PRIx64
+ " (reserved nonzero)", func_name, desc_type,
+ inv_desc->hi, inv_desc->lo);
+ return false;
+ }
+ }
+
+ return true;
+}
+
static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
{
- if ((inv_desc->hi & VTD_INV_DESC_WAIT_RSVD_HI) ||
- (inv_desc->lo & VTD_INV_DESC_WAIT_RSVD_LO)) {
- error_report_once("%s: invalid wait desc: hi=%"PRIx64", lo=%"PRIx64
- " (reserved nonzero)", __func__, inv_desc->hi,
- inv_desc->lo);
+ uint64_t mask[4] = {VTD_INV_DESC_WAIT_RSVD_LO, VTD_INV_DESC_WAIT_RSVD_HI,
+ VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
+
+ if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false,
+ __func__, "wait")) {
return false;
}
+
if (inv_desc->lo & VTD_INV_DESC_WAIT_SW) {
/* Status Write */
uint32_t status_data = (uint32_t)(inv_desc->lo >>
@@ -2574,13 +2610,14 @@
VTDInvDesc *inv_desc)
{
uint16_t sid, fmask;
+ uint64_t mask[4] = {VTD_INV_DESC_CC_RSVD, VTD_INV_DESC_ALL_ONE,
+ VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
- if ((inv_desc->lo & VTD_INV_DESC_CC_RSVD) || inv_desc->hi) {
- error_report_once("%s: invalid cc inv desc: hi=%"PRIx64", lo=%"PRIx64
- " (reserved nonzero)", __func__, inv_desc->hi,
- inv_desc->lo);
+ if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false,
+ __func__, "cc inv")) {
return false;
}
+
switch (inv_desc->lo & VTD_INV_DESC_CC_G) {
case VTD_INV_DESC_CC_DOMAIN:
trace_vtd_inv_desc_cc_domain(
@@ -2610,12 +2647,11 @@
uint16_t domain_id;
uint8_t am;
hwaddr addr;
+ uint64_t mask[4] = {VTD_INV_DESC_IOTLB_RSVD_LO, VTD_INV_DESC_IOTLB_RSVD_HI,
+ VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
- if ((inv_desc->lo & VTD_INV_DESC_IOTLB_RSVD_LO) ||
- (inv_desc->hi & VTD_INV_DESC_IOTLB_RSVD_HI)) {
- error_report_once("%s: invalid iotlb inv desc: hi=0x%"PRIx64
- ", lo=0x%"PRIx64" (reserved bits unzero)",
- __func__, inv_desc->hi, inv_desc->lo);
+ if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false,
+ __func__, "iotlb inv")) {
return false;
}
@@ -2705,19 +2741,19 @@
hwaddr addr;
uint16_t sid;
bool size;
+ uint64_t mask[4] = {VTD_INV_DESC_DEVICE_IOTLB_RSVD_LO,
+ VTD_INV_DESC_DEVICE_IOTLB_RSVD_HI,
+ VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
+
+ if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false,
+ __func__, "dev-iotlb inv")) {
+ return false;
+ }
addr = VTD_INV_DESC_DEVICE_IOTLB_ADDR(inv_desc->hi);
sid = VTD_INV_DESC_DEVICE_IOTLB_SID(inv_desc->lo);
size = VTD_INV_DESC_DEVICE_IOTLB_SIZE(inv_desc->hi);
- if ((inv_desc->lo & VTD_INV_DESC_DEVICE_IOTLB_RSVD_LO) ||
- (inv_desc->hi & VTD_INV_DESC_DEVICE_IOTLB_RSVD_HI)) {
- error_report_once("%s: invalid dev-iotlb inv desc: hi=%"PRIx64
- ", lo=%"PRIx64" (reserved nonzero)", __func__,
- inv_desc->hi, inv_desc->lo);
- return false;
- }
-
/*
* Using sid is OK since the guest should have finished the
* initialization of both the bus and device.
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 2f9bc01..75ccd50 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -356,6 +356,7 @@
typedef union VTDInvDesc VTDInvDesc;
/* Masks for struct VTDInvDesc */
+#define VTD_INV_DESC_ALL_ONE -1ULL
#define VTD_INV_DESC_TYPE(val) ((((val) >> 5) & 0x70ULL) | \
((val) & 0xfULL))
#define VTD_INV_DESC_CC 0x1 /* Context-cache Invalidate Desc */
