[Qemu-devel] [PATCH v3 0/2] VT-d supports 48 bits address width

[Qemu-devel] [PATCH v3 0/2] VT-d supports 48 bits address width

[Peter Xu]

V3:
- renaming "aw-bits" to "x-aw-bits", and clarify that this parameter
  is experimental [AlexW]

(V2 is mostly a re-written of V1, so no change log is appended)

The first patch did some check to make sure the CE GAW bits are always
valid.

The second patch extended VT-d GAW (Guest Address Width) from 39 bits
to 48 bits by providing "aw-bits" parameter for intel-iommu.

Test done:

- boot vm without "x-aw-bits" or "x-aw-bits=39": guest boots with 39 bits
- boot vm with "x-aw-bits=48": guest boots with 48 bits
- boot vm with illegal x-aw-bits: guest refuses to boot

Please review. Thanks,

Peter Xu (2):
  intel_iommu: check validity for GAW bits in CE
  intel_iommu: provide "x-aw-bits" parameter

 hw/i386/acpi-build.c           |  8 +++++++-
 hw/i386/intel_iommu.c          | 38 ++++++++++++++++++++++++++++++++------
 hw/i386/intel_iommu_internal.h | 13 ++++++-------
 include/hw/i386/intel_iommu.h  |  1 +
 4 files changed, 46 insertions(+), 14 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 1/2] intel_iommu: check validity for GAW bits in CE

[Peter Xu]

Currently vt-d Context Entry (CE) only allows 39/48 bits address width.
If guest software configured more than that, we complain and report.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c          | 17 ++++++++++++++++-
 hw/i386/intel_iommu_internal.h |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5f3e351..517a2a3 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -599,9 +599,19 @@ static inline uint32_t vtd_get_level_from_context_entry(VTDContextEntry *ce)
     return 2 + (ce->hi & VTD_CONTEXT_ENTRY_AW);
 }
 
+/* Return 0 if failed to fetch valid aw */
 static inline uint32_t vtd_get_agaw_from_context_entry(VTDContextEntry *ce)
 {
-    return 30 + (ce->hi & VTD_CONTEXT_ENTRY_AW) * 9;
+    uint8_t aw = (ce->hi & VTD_CONTEXT_ENTRY_AW);
+    /*
+     * According to vt-d spec 10.4.2 bits 12:8, SAGAW only allows
+     * 39/48 bits.
+     */
+    if (aw > VTD_CE_AW_48BIT) {
+        error_report("Context entry address width not supported (aw=%d)" , aw);
+        return 0;
+    }
+    return 30 + aw * 9;
 }
 
 static const uint64_t vtd_paging_entry_rsvd_field[] = {
@@ -642,6 +652,11 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, bool is_write,
     uint32_t ce_agaw = vtd_get_agaw_from_context_entry(ce);
     uint64_t access_right_check;
 
+    if (!ce_agaw) {
+        error_report("Illegal context entry AGAW");
+        return -VTD_FR_CONTEXT_ENTRY_INV;
+    }
+
     /* Check if @gpa is above 2^X-1, where X is the minimum of MGAW in CAP_REG
      * and AW in context-entry.
      */
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 11abfa2..e808c67 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -406,6 +406,8 @@ typedef struct VTDRootEntry VTDRootEntry;
 #define VTD_CONTEXT_ENTRY_RSVD_LO   (0xff0ULL | ~VTD_HAW_MASK)
 /* hi */
 #define VTD_CONTEXT_ENTRY_AW        7ULL /* Adjusted guest-address-width */
+#define VTD_CE_AW_39BIT             1
+#define VTD_CE_AW_48BIT             2
 #define VTD_CONTEXT_ENTRY_DID(val)  (((val) >> 8) & VTD_DOMAIN_ID_MASK)
 #define VTD_CONTEXT_ENTRY_RSVD_HI   0xffffffffff000080ULL
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 1/2] intel_iommu: check validity for GAW bits in CE

[Michael S. Tsirkin]

On Wed, Dec 14, 2016 at 10:09:04AM +0800, Peter Xu wrote:
> Currently vt-d Context Entry (CE) only allows 39/48 bits address width.
> If guest software configured more than that, we complain and report.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/i386/intel_iommu.c          | 17 ++++++++++++++++-
>  hw/i386/intel_iommu_internal.h |  2 ++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 5f3e351..517a2a3 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -599,9 +599,19 @@ static inline uint32_t vtd_get_level_from_context_entry(VTDContextEntry *ce)
>      return 2 + (ce->hi & VTD_CONTEXT_ENTRY_AW);
>  }
>  
> +/* Return 0 if failed to fetch valid aw */
>  static inline uint32_t vtd_get_agaw_from_context_entry(VTDContextEntry *ce)
>  {
> -    return 30 + (ce->hi & VTD_CONTEXT_ENTRY_AW) * 9;
> +    uint8_t aw = (ce->hi & VTD_CONTEXT_ENTRY_AW);
> +    /*
> +     * According to vt-d spec 10.4.2 bits 12:8, SAGAW only allows
> +     * 39/48 bits.
> +     */
> +    if (aw > VTD_CE_AW_48BIT) {

5-level is almost sure to allow more. I don't see the point of this test.

> +        error_report("Context entry address width not supported (aw=%d)" , aw);
> +        return 0;
> +    }
> +    return 30 + aw * 9;
>  }
>  
>  static const uint64_t vtd_paging_entry_rsvd_field[] = {
> @@ -642,6 +652,11 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, bool is_write,
>      uint32_t ce_agaw = vtd_get_agaw_from_context_entry(ce);
>      uint64_t access_right_check;
>  
> +    if (!ce_agaw) {
> +        error_report("Illegal context entry AGAW");
> +        return -VTD_FR_CONTEXT_ENTRY_INV;
> +    }
> +
>      /* Check if @gpa is above 2^X-1, where X is the minimum of MGAW in CAP_REG
>       * and AW in context-entry.
>       */
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 11abfa2..e808c67 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -406,6 +406,8 @@ typedef struct VTDRootEntry VTDRootEntry;
>  #define VTD_CONTEXT_ENTRY_RSVD_LO   (0xff0ULL | ~VTD_HAW_MASK)
>  /* hi */
>  #define VTD_CONTEXT_ENTRY_AW        7ULL /* Adjusted guest-address-width */
> +#define VTD_CE_AW_39BIT             1
> +#define VTD_CE_AW_48BIT             2
>  #define VTD_CONTEXT_ENTRY_DID(val)  (((val) >> 8) & VTD_DOMAIN_ID_MASK)
>  #define VTD_CONTEXT_ENTRY_RSVD_HI   0xffffffffff000080ULL
>  
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH v3 1/2] intel_iommu: check validity for GAW bits in CE

[Peter Xu]

On Wed, Dec 14, 2016 at 04:48:42AM +0200, Michael S. Tsirkin wrote:
> On Wed, Dec 14, 2016 at 10:09:04AM +0800, Peter Xu wrote:
> > Currently vt-d Context Entry (CE) only allows 39/48 bits address width.
> > If guest software configured more than that, we complain and report.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  hw/i386/intel_iommu.c          | 17 ++++++++++++++++-
> >  hw/i386/intel_iommu_internal.h |  2 ++
> >  2 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 5f3e351..517a2a3 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -599,9 +599,19 @@ static inline uint32_t vtd_get_level_from_context_entry(VTDContextEntry *ce)
> >      return 2 + (ce->hi & VTD_CONTEXT_ENTRY_AW);
> >  }
> >  
> > +/* Return 0 if failed to fetch valid aw */
> >  static inline uint32_t vtd_get_agaw_from_context_entry(VTDContextEntry *ce)
> >  {
> > -    return 30 + (ce->hi & VTD_CONTEXT_ENTRY_AW) * 9;
> > +    uint8_t aw = (ce->hi & VTD_CONTEXT_ENTRY_AW);
> > +    /*
> > +     * According to vt-d spec 10.4.2 bits 12:8, SAGAW only allows
> > +     * 39/48 bits.
> > +     */
> > +    if (aw > VTD_CE_AW_48BIT) {
> 
> 5-level is almost sure to allow more. I don't see the point of this test.

Please check above comment - spec only allow 3/4 level page table.

My version of vt-d spec is 2016 Oct. I suppose that's new enough...

-- peterx

Re: [Qemu-devel] [PATCH v3 1/2] intel_iommu: check validity for GAW bits in CE

[Tian, Kevin]

> From: Peter Xu [mailto:peterx@redhat.com]
> Sent: Wednesday, December 14, 2016 10:54 AM
> 
> On Wed, Dec 14, 2016 at 04:48:42AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Dec 14, 2016 at 10:09:04AM +0800, Peter Xu wrote:
> > > Currently vt-d Context Entry (CE) only allows 39/48 bits address width.
> > > If guest software configured more than that, we complain and report.
> > >
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >  hw/i386/intel_iommu.c          | 17 ++++++++++++++++-
> > >  hw/i386/intel_iommu_internal.h |  2 ++
> > >  2 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > index 5f3e351..517a2a3 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -599,9 +599,19 @@ static inline uint32_t
> vtd_get_level_from_context_entry(VTDContextEntry *ce)
> > >      return 2 + (ce->hi & VTD_CONTEXT_ENTRY_AW);
> > >  }
> > >
> > > +/* Return 0 if failed to fetch valid aw */
> > >  static inline uint32_t vtd_get_agaw_from_context_entry(VTDContextEntry *ce)
> > >  {
> > > -    return 30 + (ce->hi & VTD_CONTEXT_ENTRY_AW) * 9;
> > > +    uint8_t aw = (ce->hi & VTD_CONTEXT_ENTRY_AW);
> > > +    /*
> > > +     * According to vt-d spec 10.4.2 bits 12:8, SAGAW only allows
> > > +     * 39/48 bits.
> > > +     */
> > > +    if (aw > VTD_CE_AW_48BIT) {
> >
> > 5-level is almost sure to allow more. I don't see the point of this test.
> 
> Please check above comment - spec only allow 3/4 level page table.
> 
> My version of vt-d spec is 2016 Oct. I suppose that's new enough...
> 

It is in the works and will be published in January.

Thanks
Kevin

[Qemu-devel] [PATCH v3 2/2] intel_iommu: provide "x-aw-bits" parameter

[Peter Xu]

Previously vt-d codes only supports 39 bits iova address width. This
patch provide a new parameter for Intel IOMMU to extend its address
width to 48 bits (though the default is still 39).

After enabling larger address width (48), we should be able to map
larger iova addresses in the guest. To check whether 48 bits aw is
enabled, we can grep in the guest dmesg with line: "dmar: Host address
width 48".

This parameter is experimental.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/acpi-build.c           |  8 +++++++-
 hw/i386/intel_iommu.c          | 21 ++++++++++++++++-----
 hw/i386/intel_iommu_internal.h | 11 ++++-------
 include/hw/i386/intel_iommu.h  |  1 +
 4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9708cdc..9dd06fd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2576,6 +2576,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
     AcpiTableDmar *dmar;
     AcpiDmarHardwareUnit *drhd;
     uint8_t dmar_flags = 0;
+    uint8_t aw_bits = VTD_HOST_ADDRESS_WIDTH;
     X86IOMMUState *iommu = x86_iommu_get_default();
     AcpiDmarDeviceScope *scope = NULL;
     /* Root complex IOAPIC use one path[0] only */
@@ -2586,8 +2587,13 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
         dmar_flags |= 0x1;      /* Flags: 0x1: INT_REMAP */
     }
 
+    if (iommu->type == TYPE_INTEL) {
+        IntelIOMMUState *intel_iommu = INTEL_IOMMU_DEVICE(iommu);
+        aw_bits = intel_iommu->aw_bits;
+    }
+
     dmar = acpi_data_push(table_data, sizeof(*dmar));
-    dmar->host_address_width = VTD_HOST_ADDRESS_WIDTH - 1;
+    dmar->host_address_width = aw_bits - 1;
     dmar->flags = dmar_flags;
 
     /* DMAR Remapping Hardware Unit Definition structure */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 517a2a3..84149db 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -643,7 +643,7 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level)
  */
 static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, bool is_write,
                             uint64_t *slptep, uint32_t *slpte_level,
-                            bool *reads, bool *writes)
+                            bool *reads, bool *writes, uint8_t aw_bits)
 {
     dma_addr_t addr = vtd_get_slpt_base_from_context(ce);
     uint32_t level = vtd_get_level_from_context_entry(ce);
@@ -660,7 +660,7 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, bool is_write,
     /* Check if @gpa is above 2^X-1, where X is the minimum of MGAW in CAP_REG
      * and AW in context-entry.
      */
-    if (gpa & ~((1ULL << MIN(ce_agaw, VTD_MGAW)) - 1)) {
+    if (gpa & ~((1ULL << MIN(ce_agaw, aw_bits)) - 1)) {
         VTD_DPRINTF(GENERAL, "error: gpa 0x%"PRIx64 " exceeds limits", gpa);
         return -VTD_FR_ADDR_BEYOND_MGAW;
     }
@@ -892,7 +892,7 @@ static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,
     }
 
     ret_fr = vtd_gpa_to_slpte(&ce, addr, is_write, &slpte, &level,
-                              &reads, &writes);
+                              &reads, &writes, s->aw_bits);
     if (ret_fr) {
         ret_fr = -ret_fr;
         if (is_fpd_set && vtd_is_qualified_fault(ret_fr)) {
@@ -2033,6 +2033,7 @@ static Property vtd_properties[] = {
     DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim,
                             ON_OFF_AUTO_AUTO),
     DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim, false),
+    DEFINE_PROP_UINT32("x-aw-bits", IntelIOMMUState, aw_bits, 39),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2395,8 +2396,12 @@ static void vtd_init(IntelIOMMUState *s)
     s->qi_enabled = false;
     s->iq_last_desc_type = VTD_INV_DESC_NONE;
     s->next_frcd_reg = 0;
-    s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND | VTD_CAP_MGAW |
-             VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS;
+    s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND |
+             VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS |
+             VTD_CAP_SAGAW_39bit | VTD_CAP_MGAW(s->aw_bits);
+    if (s->aw_bits == 48) {
+        s->cap |= VTD_CAP_SAGAW_48bit;
+    }
     s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
 
     if (x86_iommu->intr_supported) {
@@ -2518,6 +2523,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
         }
     }
 
+    if (s->aw_bits != 39 && s->aw_bits != 48) {
+        error_setg(errp, "Illegal x-aw-bits %d (allowed values: 39, 48)",
+                   s->aw_bits);
+        return false;
+    }
+
     return true;
 }
 
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index e808c67..035268d 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -131,7 +131,7 @@
 #define VTD_TLB_DID(val)            (((val) >> 32) & VTD_DOMAIN_ID_MASK)
 
 /* IVA_REG */
-#define VTD_IVA_ADDR(val)       ((val) & ~0xfffULL & ((1ULL << VTD_MGAW) - 1))
+#define VTD_IVA_ADDR(val)       ((val) & ~0xfffULL)
 #define VTD_IVA_AM(val)         ((val) & 0x3fULL)
 
 /* GCMD_REG */
@@ -195,8 +195,7 @@
 #define VTD_DOMAIN_ID_SHIFT         16  /* 16-bit domain id for 64K domains */
 #define VTD_DOMAIN_ID_MASK          ((1UL << VTD_DOMAIN_ID_SHIFT) - 1)
 #define VTD_CAP_ND                  (((VTD_DOMAIN_ID_SHIFT - 4) / 2) & 7ULL)
-#define VTD_MGAW                    39  /* Maximum Guest Address Width */
-#define VTD_CAP_MGAW                (((VTD_MGAW - 1) & 0x3fULL) << 16)
+#define VTD_CAP_MGAW(bits)          ((((bits) - 1) & 0x3fULL) << 16)
 #define VTD_MAMV                    18ULL
 #define VTD_CAP_MAMV                (VTD_MAMV << 48)
 #define VTD_CAP_PSI                 (1ULL << 39)
@@ -209,7 +208,6 @@
 #define VTD_CAP_SAGAW_39bit         (0x2ULL << VTD_CAP_SAGAW_SHIFT)
  /* 48-bit AGAW, 4-level page-table */
 #define VTD_CAP_SAGAW_48bit         (0x4ULL << VTD_CAP_SAGAW_SHIFT)
-#define VTD_CAP_SAGAW               VTD_CAP_SAGAW_39bit
 
 /* IQT_REG */
 #define VTD_IQT_QT(val)             (((val) >> 4) & 0x7fffULL)
@@ -248,7 +246,7 @@
 #define VTD_FRCD_SID_MASK       0xffffULL
 #define VTD_FRCD_SID(val)       ((val) & VTD_FRCD_SID_MASK)
 /* For the low 64-bit of 128-bit */
-#define VTD_FRCD_FI(val)        ((val) & (((1ULL << VTD_MGAW) - 1) ^ 0xfffULL))
+#define VTD_FRCD_FI(val)        ((val) & ~0xfffULL)
 
 /* DMA Remapping Fault Conditions */
 typedef enum VTDFaultReason {
@@ -355,8 +353,7 @@ typedef union VTDInvDesc VTDInvDesc;
 #define VTD_INV_DESC_IOTLB_DOMAIN       (2ULL << 4)
 #define VTD_INV_DESC_IOTLB_PAGE         (3ULL << 4)
 #define VTD_INV_DESC_IOTLB_DID(val)     (((val) >> 16) & VTD_DOMAIN_ID_MASK)
-#define VTD_INV_DESC_IOTLB_ADDR(val)    ((val) & ~0xfffULL & \
-                                         ((1ULL << VTD_MGAW) - 1))
+#define VTD_INV_DESC_IOTLB_ADDR(val)    ((val) & ~0xfffULL)
 #define VTD_INV_DESC_IOTLB_AM(val)      ((val) & 0x3fULL)
 #define VTD_INV_DESC_IOTLB_RSVD_LO      0xffffffff0000ff00ULL
 #define VTD_INV_DESC_IOTLB_RSVD_HI      0xf80ULL
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 405c9d1..b0bb53a 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -290,6 +290,7 @@ struct IntelIOMMUState {
     bool intr_eime;                 /* Extended interrupt mode enabled */
     OnOffAuto intr_eim;             /* Toggle for EIM cabability */
     bool buggy_eim;                 /* Force buggy EIM unless eim=off */
+    uint32_t aw_bits;               /* IOVA address width (in bits) */
 };
 
 /* Find the VTD Address space associated with the given bus pointer,
-- 
2.7.4