Re: [PATCH 08/14] iommu/amd: Store the nid in io_pgtable_cfg instead of the domain

[Vasant Hegde <vasant.hegde@amd.com>]

Jason,

On 8/21/2024 11:07 PM, Jason Gunthorpe wrote:
> We already have memory in the union here that is being wasted in AMD's
> case, use it to store the nid.
> 
> Putting the nid here further isolates the io_pgtable code from the struct
> protection_domain.
> 
> Fixup protection_domain_alloc so that the NID from the device is provided,
> at this point dev is never NULL for AMD so this will now allocate the
> first table pointer on the correct NUMA node.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/amd/amd_iommu.h       |  2 +-
>  drivers/iommu/amd/amd_iommu_types.h |  1 -
>  drivers/iommu/amd/io_pgtable.c      |  7 ++++---
>  drivers/iommu/amd/io_pgtable_v2.c   |  5 ++---
>  drivers/iommu/amd/iommu.c           | 11 ++++++-----
>  drivers/iommu/amd/pasid.c           |  2 +-
>  include/linux/io-pgtable.h          |  4 ++++
>  7 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 5a050080d2e814..5459f726fb29d6 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -45,7 +45,7 @@ extern enum io_pgtable_fmt amd_iommu_pgtable;
>  extern int amd_iommu_gpt_level;
>  
>  /* Protection domain ops */
> -struct protection_domain *protection_domain_alloc(unsigned int type);
> +struct protection_domain *protection_domain_alloc(unsigned int type, int nid);
>  void protection_domain_free(struct protection_domain *domain);
>  struct iommu_domain *amd_iommu_domain_alloc_sva(struct device *dev,
>  						struct mm_struct *mm);
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index ef4c4887cbbbd5..74dc003f5b7815 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -579,7 +579,6 @@ struct protection_domain {
>  	struct amd_io_pgtable iop;
>  	spinlock_t lock;	/* mostly used to lock the page table*/
>  	u16 id;			/* the domain id written to the device table */
> -	int nid;		/* Node ID */
>  	enum protection_domain_mode pd_mode; /* Track page table type */
>  	bool dirty_tracking;	/* dirty tracking is enabled in the domain */
>  	unsigned dev_cnt;	/* devices assigned to this domain */
> diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
> index dab1cf53b1f3ff..d56be842c0b71e 100644
> --- a/drivers/iommu/amd/io_pgtable.c
> +++ b/drivers/iommu/amd/io_pgtable.c
> @@ -145,7 +145,7 @@ static bool increase_address_space(struct protection_domain *domain,
>  	bool ret = true;
>  	u64 *pte;
>  
> -	pte = iommu_alloc_page_node(domain->nid, gfp);
> +	pte = iommu_alloc_page_node(domain->iop.pgtbl.cfg.amd.nid, gfp);

Too many level and used in multiple places. IMO its worth having helper function
to set/get nid.

-Vasant