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

[Jason Wang <jasowang@redhat.com>]

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