Re: [PATCH v6 13/17] iommu/amd: Refactor GCR3 table helper functions

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Jan 25, 2024 at 12:11:31PM +0000, Vasant Hegde wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> To use the new per-device struct gcr3_tbl_info. Use GFP_KERNEL flag
> instead of GFP_ATOMIC for GCR3 table allocation. Also modify
> set_dte_entry() to use new per device GCR3 table.
> 
> Also in free_gcr3_table() path replace BUG_ON with WARN_ON_ONCE().
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/iommu.c | 53 +++++++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 22 deletions(-)

The code looks OK after this so

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

> @@ -1924,14 +1933,14 @@ static int do_attach(struct iommu_dev_data *dev_data,
>  	/* Init GCR3 table and update device table */
>  	if (domain->pd_mode == PD_MODE_V2) {
>  		/* By default, setup GCR3 table to support single PASID */
> -		ret = setup_gcr3_table(dev_data->domain, 1);
> +		ret = setup_gcr3_table(&dev_data->gcr3_info, iommu, 1);
>  		if (ret)
>  			return ret;

But the patch order is off here. The above showing this was using
dev_data->domain for a few patches is not perfectly bisection-correct.

Patches 11/12/13 are all sort of entangled together so I can see how
it ended up like this. You could probably squash them together and be
fine if someone prefers to be sticky on this.

Jason