linux-kernel - Re: [RFC PATCH 2/6] iommu/amd: Add support for hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Dec 2023 09:27:21 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Suravee Suthikulpanit <suravee.suthikulpanit@....com>
Cc:     linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
        joro@...tes.org, yi.l.liu@...el.com, kevin.tian@...el.com,
        nicolinc@...dia.com, eric.auger@...hat.com, vasant.hegde@....com,
        jon.grimm@....com, santosh.shukla@....com, Dhaval.Giani@....com,
        pandoh@...gle.com, loganodell@...gle.com
Subject: Re: [RFC PATCH 2/6] iommu/amd: Add support for hw_info for iommu
 capability query

On Tue, Dec 12, 2023 at 10:01:35AM -0600, Suravee Suthikulpanit wrote:
> AMD IOMMU Extended Feature (EFR) and Extended Feature 2 (EFR2) registers
> specify features supported by each IOMMU hardware instance.
> The IOMMU driver checks each feature-specific bits before enabling
> each feature at run time.
> 
> For IOMMUFD, the hypervisor determines which IOMMU features to support
> in the guest, and communicates this information to user-space (e.g. QEMU)
> via iommufd IOMMU_DEVICE_GET_HW_INFO ioctl.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
> ---
>  drivers/iommu/amd/amd_iommu.h       |  2 ++
>  drivers/iommu/amd/amd_iommu_types.h |  3 +++
>  drivers/iommu/amd/iommu.c           | 38 +++++++++++++++++++++++++++++
>  include/uapi/linux/iommufd.h        | 13 ++++++++++
>  4 files changed, 56 insertions(+)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 108253edbeb0..4118129f4a24 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -72,6 +72,8 @@ void amd_iommu_dev_flush_pasid_pages(struct iommu_dev_data *dev_data,
>  void amd_iommu_dev_flush_pasid_all(struct iommu_dev_data *dev_data,
>  				   ioasid_t pasid);
>  
> +void amd_iommu_build_efr(u64 *efr, u64 *efr2);
> +
>  #ifdef CONFIG_IRQ_REMAP
>  int amd_iommu_create_irq_domain(struct amd_iommu *iommu);
>  #else
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 14f67a8cf755..956fd4658a4a 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -100,12 +100,15 @@
>  #define FEATURE_HDSUP		BIT_ULL(52)
>  #define FEATURE_SNP		BIT_ULL(63)
>  
> +#define FEATURE_GATS_5LEVEL	1ULL
>  #define FEATURE_GATS_SHIFT	12
>  #define FEATURE_GATS_MASK	(0x03ULL << FEATURE_GATS_SHIFT)
>  
> +#define FEATURE_GLX_3LEVEL	0ULL
>  #define FEATURE_GLX_SHIFT	14
>  #define FEATURE_GLX_MASK	(0x03ULL << FEATURE_GLX_SHIFT)
>  
> +#define FEATURE_PASMAX_16	0xFULL
>  #define FEATURE_PASMAX_SHIFT	32
>  #define FEATURE_PASMAX_MASK	(0x1FULL << FEATURE_PASMAX_SHIFT)
>  
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4e4ff1550cf3..c41932e9f16a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2822,8 +2822,46 @@ static const struct iommu_dirty_ops amd_dirty_ops = {
>  	.read_and_clear_dirty = amd_iommu_read_and_clear_dirty,
>  };
>  
> +void amd_iommu_build_efr(u64 *efr, u64 *efr2)
> +{
> +	if (efr) {
> +		*efr = (FEATURE_GT | FEATURE_GIOSUP | FEATURE_PPR);
> +
> +		/* 5-level v2 page table support */
> +		*efr |= ((FEATURE_GATS_5LEVEL << FEATURE_GATS_SHIFT) &
> +			 FEATURE_GATS_MASK);
> +
> +		/* 3-level GCR3 table support */
> +		*efr |= ((FEATURE_GLX_3LEVEL << FEATURE_GLX_SHIFT) &
> +			 FEATURE_GLX_MASK);
> +
> +		/* 16-bit PASMAX support */
> +		*efr |= ((FEATURE_PASMAX_16 << FEATURE_PASMAX_SHIFT) &
> +			 FEATURE_PASMAX_MASK);
> +	}
> +
> +	if (efr2)
> +		*efr2 = 0;

Why are you checking for null here? It is never called with null

> +/**
> + * struct iommu_hw_info_amd - AMD IOMMU device info
> + *
> + * @efr : Value of AMD IOMMU Extended Feature Register (EFR)
> + * @efr2: Value of AMD IOMMU Extended Feature 2 Register (EFR2)

Please reference a section in the spec for what these are just for
clarity

> + */
> +struct iommu_hw_info_amd {
> +	__u64 efr;
> +	__u64 efr2;
> +};

__aligned_u64

Jason