Re: [PATCH v5 04/27] iommu/arm-smmu-v3: Add an ops indirection to the STE code

[Nicolin Chen <nicolinc@nvidia.com>]

Hi Michael/Jason,

On Mon, Mar 04, 2024 at 07:43:52PM -0400, Jason Gunthorpe wrote:
> Prepare to put the CD code into the same mechanism. Add an ops indirection
> around all the STE specific code and make the worker functions independent
> of the entry content being processed.
> 
> get_used and sync ops are provided to hook the correct code.
> 
> Signed-off-by: Michael Shavit <mshavit@google.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 172 ++++++++++++--------
>  1 file changed, 104 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index c60b067c1f553e..b7f947e36f596f 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -48,8 +48,20 @@ enum arm_smmu_msi_index {
>  	ARM_SMMU_MAX_MSIS,
>  };
>  
> -static void arm_smmu_sync_ste_for_sid(struct arm_smmu_device *smmu,
> -				      ioasid_t sid);
> +struct arm_smmu_entry_writer_ops;
> +struct arm_smmu_entry_writer {
> +	const struct arm_smmu_entry_writer_ops *ops;
> +	struct arm_smmu_master *master;
> +};
> +
> +struct arm_smmu_entry_writer_ops {
> +	unsigned int num_entry_qwords;

I vaguely remember some related discussion, yet can't find it
out. So sorry for questioning this, if it's already discussed.
Aren't CD and STE having the same num_entry_qwords in terms of
their values? Feels like we can just use NUM_ENTRY_QWORDS?

> +	__le64 v_bit;
> +	void (*get_used)(const __le64 *entry, __le64 *used);
> +	void (*sync)(struct arm_smmu_entry_writer *writer);
> +};
> +
> +#define NUM_ENTRY_QWORDS (sizeof(struct arm_smmu_ste) / sizeof(u64))

And this seems to be just a fixed "8"? Since both are defined
straightforwardly:

struct arm_smmu_ste {
	__le64 data[8];
};
...
struct arm_smmu_cd {
	__le64 data[8];
};

Might be a bit nitpicking, yet maybe the other way around?

#define NUM_ENTRY_QWORDS 8
...
struct arm_smmu_ste {
	__le64 data[NUM_ENTRY_QWORDS];
};
...
struct arm_smmu_cd {
	__le64 data[NUM_ENTRY_QWORDS];
};

Thanks
Nicolin