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

Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for scalabl

From: Yi Liu
Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for scalable modern mode
Date: Mon, 4 Nov 2024 15:23:08 +0800
User-agent: Mozilla Thunderbird 
On 2024/11/4 14:25, Duan, Zhenzhong wrote:




-----Original Message-----
From: Liu, Yi L <yi.l.liu@intel.com>
Sent: Monday, November 4, 2024 12:25 PM
Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
scalable modern mode

On 2024/9/30 17:26, Zhenzhong Duan wrote:

Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities
related to scalable mode translation, thus there are multiple combinations.

This vIOMMU implementation wants to simplify it with a new property "x-fls".
When enabled in scalable mode, first stage translation also known as scalable
modern mode is supported. When enabled in legacy mode, throw out error.

With scalable modern mode exposed to user, also accurate the pasid entry
check in vtd_pe_type_check().

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


Maybe a Suggested-by tag can help to understand where this idea comes. :)


Will add:
Suggested-by: Jason Wang <jasowang@redhat.com>




---
   hw/i386/intel_iommu_internal.h |  2 ++
   hw/i386/intel_iommu.c          | 28 +++++++++++++++++++---------
   2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 2702edd27f..f13576d334 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/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 068a08f522..14578655e1 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -803,16 +803,18 @@ static inline bool

vtd_is_fl_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)
   {
       switch (VTD_PE_GET_TYPE(pe)) {
-    case VTD_SM_PASID_ENTRY_SLT:
-        return true;
-    case VTD_SM_PASID_ENTRY_PT:
-        return x86_iommu->pt_supported;
       case VTD_SM_PASID_ENTRY_FLT:
+        return !!(s->ecap & VTD_ECAP_FLTS);
+    case VTD_SM_PASID_ENTRY_SLT:
+        return !!(s->ecap & VTD_ECAP_SLTS);
       case VTD_SM_PASID_ENTRY_NESTED:
+        /* Not support NESTED page table type yet */
+        return false;
+    case VTD_SM_PASID_ENTRY_PT:
+        return !!(s->ecap & VTD_ECAP_PT);
       default:
           /* Unknown type */
           return false;
@@ -861,7 +863,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;
@@ -875,7 +876,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;
       }

@@ -3779,6 +3780,7 @@ static Property vtd_properties[] = {
                         VTD_HOST_AW_AUTO),
       DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,

FALSE),

       DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode,

FALSE),

+    DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
       DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control,

false),

a question: is there any requirement on the layout of this array? Should
new fields added in the end?


Looked over the history, seems we didn't have an explicit rule in 
vtd_properties.
I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of 
scalable mode.
Let me know if you have preference to add in the end.


I don't have a preference for now as long as it does not break any
functionality. BTW. Will x-flt or x-flts better?






       DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
       DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
@@ -4509,7 +4511,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;
       }

@@ -4683,6 +4688,11 @@ static bool vtd_decide_config(IntelIOMMUState *s,

Error **errp)

           }
       }

+    if (!s->scalable_mode && s->scalable_modern) {
+        error_setg(errp, "Legacy mode: not support x-fls=on");
+        return false;
+    }
+
       if (s->aw_bits == VTD_HOST_AW_AUTO) {
           if (s->scalable_modern) {
               s->aw_bits = VTD_HOST_AW_48BIT;


--
Regards,
Yi Liu


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