[PATCH v3 16/17] intel_iommu: Introduce a property to control FS1GP cap bit setting

Zhenzhong Duan posted 17 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH v3 16/17] intel_iommu: Introduce a property to control FS1GP cap bit setting
Posted by Zhenzhong Duan 1 month, 4 weeks ago
This gives user flexibility to turn off FS1GP for debug purpose.

It is also useful for future nesting feature. When host IOMMU doesn't
support FS1GP but vIOMMU does, nested page table on host side works
after turn FS1GP off in vIOMMU.

This property has no effect when vIOMMU isn't in scalable modern
mode.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
---
 include/hw/i386/intel_iommu.h | 1 +
 hw/i386/intel_iommu.c         | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 650641544c..f6d9b41b80 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -308,6 +308,7 @@ struct IntelIOMMUState {
     bool dma_drain;                 /* Whether DMA r/w draining enabled */
     bool dma_translation;           /* Whether DMA translation supported */
     bool pasid;                     /* Whether to support PASID */
+    bool fs1gp;                     /* First Stage 1-GByte Page Support */
 
     /*
      * Protects IOMMU states in general.  Currently it protects the
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index bb3ed48281..8b40aace8b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3779,6 +3779,7 @@ static Property vtd_properties[] = {
     DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
     DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
     DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation, true),
+    DEFINE_PROP_BOOL("x-cap-fs1gp", IntelIOMMUState, fs1gp, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -4507,7 +4508,9 @@ static void vtd_cap_init(IntelIOMMUState *s)
     /* TODO: read cap/ecap from host to decide which cap to be exposed. */
     if (s->scalable_modern) {
         s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
-        s->cap |= VTD_CAP_FS1GP;
+        if (s->fs1gp) {
+            s->cap |= VTD_CAP_FS1GP;
+        }
     } else if (s->scalable_mode) {
         s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
     }
-- 
2.34.1


Re: [PATCH v3 16/17] intel_iommu: Introduce a property to control FS1GP cap bit setting
Posted by Jason Wang 1 month, 1 week ago
On Wed, Sep 11, 2024 at 1:27 PM Zhenzhong Duan <zhenzhong.duan@intel.com> wrote:
>
> This gives user flexibility to turn off FS1GP for debug purpose.
>
> It is also useful for future nesting feature. When host IOMMU doesn't
> support FS1GP but vIOMMU does, nested page table on host side works
> after turn FS1GP off in vIOMMU.
>
> This property has no effect when vIOMMU isn't in scalable modern
> mode.

It looks to me there's no need to have an "x" prefix for this.

Other looks good.

>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
> ---
>  include/hw/i386/intel_iommu.h | 1 +
>  hw/i386/intel_iommu.c         | 5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 650641544c..f6d9b41b80 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -308,6 +308,7 @@ struct IntelIOMMUState {
>      bool dma_drain;                 /* Whether DMA r/w draining enabled */
>      bool dma_translation;           /* Whether DMA translation supported */
>      bool pasid;                     /* Whether to support PASID */
> +    bool fs1gp;                     /* First Stage 1-GByte Page Support */
>
>      /*
>       * Protects IOMMU states in general.  Currently it protects the
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index bb3ed48281..8b40aace8b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3779,6 +3779,7 @@ static Property vtd_properties[] = {
>      DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
>      DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>      DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation, true),
> +    DEFINE_PROP_BOOL("x-cap-fs1gp", IntelIOMMUState, fs1gp, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -4507,7 +4508,9 @@ static void vtd_cap_init(IntelIOMMUState *s)
>      /* TODO: read cap/ecap from host to decide which cap to be exposed. */
>      if (s->scalable_modern) {
>          s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
> -        s->cap |= VTD_CAP_FS1GP;
> +        if (s->fs1gp) {
> +            s->cap |= VTD_CAP_FS1GP;
> +        }
>      } else if (s->scalable_mode) {
>          s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
>      }
> --
> 2.34.1
>

Thanks
RE: [PATCH v3 16/17] intel_iommu: Introduce a property to control FS1GP cap bit setting
Posted by Duan, Zhenzhong 1 month, 1 week ago

>-----Original Message-----
>From: Jason Wang <jasowang@redhat.com>
>Subject: Re: [PATCH v3 16/17] intel_iommu: Introduce a property to control
>FS1GP cap bit setting
>
>On Wed, Sep 11, 2024 at 1:27 PM Zhenzhong Duan
><zhenzhong.duan@intel.com> wrote:
>>
>> This gives user flexibility to turn off FS1GP for debug purpose.
>>
>> It is also useful for future nesting feature. When host IOMMU doesn't
>> support FS1GP but vIOMMU does, nested page table on host side works
>> after turn FS1GP off in vIOMMU.
>>
>> This property has no effect when vIOMMU isn't in scalable modern
>> mode.
>
>It looks to me there's no need to have an "x" prefix for this.

Will remove "x" prefix.

Thanks
Zhenzhong

>
>Other looks good.
>
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
>> ---
>>  include/hw/i386/intel_iommu.h | 1 +
>>  hw/i386/intel_iommu.c         | 5 ++++-
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/i386/intel_iommu.h
>b/include/hw/i386/intel_iommu.h
>> index 650641544c..f6d9b41b80 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -308,6 +308,7 @@ struct IntelIOMMUState {
>>      bool dma_drain;                 /* Whether DMA r/w draining enabled */
>>      bool dma_translation;           /* Whether DMA translation supported */
>>      bool pasid;                     /* Whether to support PASID */
>> +    bool fs1gp;                     /* First Stage 1-GByte Page Support */
>>
>>      /*
>>       * Protects IOMMU states in general.  Currently it protects the
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index bb3ed48281..8b40aace8b 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3779,6 +3779,7 @@ static Property vtd_properties[] = {
>>      DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
>>      DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>>      DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState,
>dma_translation, true),
>> +    DEFINE_PROP_BOOL("x-cap-fs1gp", IntelIOMMUState, fs1gp, true),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>> @@ -4507,7 +4508,9 @@ static void vtd_cap_init(IntelIOMMUState *s)
>>      /* TODO: read cap/ecap from host to decide which cap to be exposed.
>*/
>>      if (s->scalable_modern) {
>>          s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
>> -        s->cap |= VTD_CAP_FS1GP;
>> +        if (s->fs1gp) {
>> +            s->cap |= VTD_CAP_FS1GP;
>> +        }
>>      } else if (s->scalable_mode) {
>>          s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
>>      }
>> --
>> 2.34.1
>>
>
>Thanks