qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 03/17] intel_iommu: Add a placeholder variable for scalabl

From: Yi Liu
Subject: Re: [PATCH v1 03/17] intel_iommu: Add a placeholder variable for scalable modern mode
Date: Fri, 19 Jul 2024 11:22:46 +0800
User-agent: Mozilla Thunderbird 
On 2024/7/19 10:47, Duan, Zhenzhong wrote:




-----Original Message-----
From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
Subject: Re: [PATCH v1 03/17] intel_iommu: Add a placeholder variable for
scalable modern mode



On 18/07/2024 10:16, Zhenzhong Duan wrote:

Caution: External email. Do not open attachments or click links, unless this

email comes from a known sender and you know the content is safe.



Add an new element scalable_mode in IntelIOMMUState to mark scalable
modern mode, this element will be exposed as an intel_iommu property
finally.

For now, it's only a placehholder and used for cap/ecap initialization,
compatibility check and block host device passthrough until nesting
is supported.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
   hw/i386/intel_iommu_internal.h |  2 ++
   include/hw/i386/intel_iommu.h  |  1 +
   hw/i386/intel_iommu.c          | 34 +++++++++++++++++++++++-----------
   3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/hw/i386/intel_iommu_internal.h

b/hw/i386/intel_iommu_internal.h

index c0ca7b372f..4e0331caba 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -195,6 +195,7 @@
   #define VTD_ECAP_PASID              (1ULL << 40)
   #define VTD_ECAP_SMTS               (1ULL << 43)
   #define VTD_ECAP_SLTS               (1ULL << 46)
+#define VTD_ECAP_FLTS               (1ULL << 47)

   /* CAP_REG */
   /* (offset >> 4) << 24 */
@@ -211,6 +212,7 @@
   #define VTD_CAP_SLLPS               ((1ULL << 34) | (1ULL << 35))
   #define VTD_CAP_DRAIN_WRITE         (1ULL << 54)
   #define VTD_CAP_DRAIN_READ          (1ULL << 55)
+#define VTD_CAP_FS1GP               (1ULL << 56)
   #define VTD_CAP_DRAIN               (VTD_CAP_DRAIN_READ |

VTD_CAP_DRAIN_WRITE)

   #define VTD_CAP_CM                  (1ULL << 7)
   #define VTD_PASID_ID_SHIFT          20
diff --git a/include/hw/i386/intel_iommu.h

b/include/hw/i386/intel_iommu.h

index 1eb05c29fc..788ed42477 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -262,6 +262,7 @@ struct IntelIOMMUState {

       bool caching_mode;              /* RO - is cap CM enabled? */
       bool scalable_mode;             /* RO - is Scalable Mode supported? */
+    bool scalable_modern;           /* RO - is modern SM supported? */
       bool snoop_control;             /* RO - is SNP filed supported? */

       dma_addr_t root;                /* Current root table pointer */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1cff8b00ae..40cbd4a0f4 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -755,16 +755,20 @@ static inline bool

vtd_is_level_supported(IntelIOMMUState *s, uint32_t level)

   }

   /* Return true if check passed, otherwise false */
-static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu,
-                                     VTDPASIDEntry *pe)
+static inline bool vtd_pe_type_check(IntelIOMMUState *s,

VTDPASIDEntry *pe)

   {

What about using the cap/ecap registers to know if the translation types
are supported or not.
Otherwise, we could add a comment to explain why we expect
s->scalable_modern to give us enough information.


What about below:

/*
  *VTD_ECAP_FLTS in ecap is set if s->scalable_modern is true, or else 
VTD_ECAP_SLTS can be set or not depending on s->scalable_mode.
  *So it's simpler to check s->scalable_modern directly for a PASID entry type 
instead ecap bits.
  */


Since this helper is for pasid entry check, so you can just return false
if the pe's PGTT is SS-only.

It might make more sense to check the ecap/cap here as anyhow the
capability is listed in ecap/cap. This may also bring us some convenience.

Say in the future, if we want to add a new mode (e.g. scalable mode 2.0)
that supports both FS and SS for guest, we may need to update this helper
as well if we check the scalable_modern. But if we check the ecap/cap, then
the future change just needs to control the ecap/cap setting at the
beginning of the vIOMMU init. To keep the code aligned, you may need to
check ecap.PT bit for VTD_SM_PASID_ENTRY_PT. :)




+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
+
       switch (VTD_PE_GET_TYPE(pe)) {
+    case VTD_SM_PASID_ENTRY_FLT:
+        return s->scalable_modern;
       case VTD_SM_PASID_ENTRY_SLT:
-        return true;
+        return !s->scalable_modern;
+    case VTD_SM_PASID_ENTRY_NESTED:
+        /* Not support NESTED page table type yet */
+        return false;
       case VTD_SM_PASID_ENTRY_PT:
           return x86_iommu->pt_supported;
-    case VTD_SM_PASID_ENTRY_FLT:
-    case VTD_SM_PASID_ENTRY_NESTED:
       default:
           /* Unknown type */
           return false;
@@ -813,7 +817,6 @@ static int

vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,

       uint8_t pgtt;
       uint32_t index;
       dma_addr_t entry_size;
-    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);

       index = VTD_PASID_TABLE_INDEX(pasid);
       entry_size = VTD_PASID_ENTRY_SIZE;
@@ -827,7 +830,7 @@ static int

vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,

       }

       /* Do translation type check */
-    if (!vtd_pe_type_check(x86_iommu, pe)) {
+    if (!vtd_pe_type_check(s, pe)) {
           return -VTD_FR_PASID_TABLE_ENTRY_INV;
       }

@@ -3861,7 +3864,13 @@ static bool vtd_check_hiod(IntelIOMMUState

*s, HostIOMMUDevice *hiod,

           return false;
       }

-    return true;
+    if (!s->scalable_modern) {
+        /* All checks requested by VTD non-modern mode pass */
+        return true;
+    }
+
+    error_setg(errp, "host device is unsupported in scalable modern mode

yet");

+    return false;
   }

   static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int

devfn,

@@ -4084,7 +4093,10 @@ static void vtd_cap_init(IntelIOMMUState *s)
       }

       /* TODO: read cap/ecap from host to decide which cap to be exposed.

*/

-    if (s->scalable_mode) {
+    if (s->scalable_modern) {
+        s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
+        s->cap |= VTD_CAP_FS1GP;
+    } else if (s->scalable_mode) {
           s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
       }

@@ -4251,9 +4263,9 @@ static bool vtd_decide_config(IntelIOMMUState

*s, Error **errp)

           }
       }

-    /* Currently only address widths supported are 39 and 48 bits */
       if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
-        (s->aw_bits != VTD_HOST_AW_48BIT)) {
+        (s->aw_bits != VTD_HOST_AW_48BIT) &&
+        !s->scalable_modern) {
           error_setg(errp, "Supported values for aw-bits are: %d, %d",
                      VTD_HOST_AW_39BIT, VTD_HOST_AW_48BIT);
           return false;
--
2.34.1



--
Regards,
Yi Liu
reply via email to
[Prev in Thread] Current Thread [Next in Thread]