Re: [PATCH v7 05/24] iommu/arm-smmu-v3: Move IDR parsing to common functions

[Jason Gunthorpe]

> [ ... 21 lines skipped ... ]
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-common-lib.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2015 ARM Limited
> + *
> + * Author: Will Deacon <will.deacon@xxxxxxx>
> + * Arm SMMUv3 driver functions shared with hypervisor.
> + */
> +
> +#include "arm-smmu-v3.h"
> +#include <asm-generic/errno-base.h>
> +
> +#include <linux/string.h>
> +
> +u32 smmu_idr0_features(u32 reg)
> +{

I think you should do more to make this shared C file usable by both
compiles with more of the infrastructure available.

For instance, something like this at the top:

 #ifdef __KVM_NVHE_HYPERVISOR__
 #include "pkvm/arm_smmu_v3.h"
 #define arm_smmu_device  hyp_arm_smmu_v3_device
 #else
 #include "arm-smmu-v3.h"
 #endif

(heh you used _ and the driver uses - in file names)

Then the existing code using 'arm_smmu_device' just works fine since
the hyp version has the same structure members. Now just move the code
as-is.

Otherwise we still have alot of cut and paste duplication:

	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR0);

	smmu->features |= smmu_idr0_features(reg);
	if (!(smmu->features & (ARM_SMMU_FEAT_TT_LE | ARM_SMMU_FEAT_TT_BE))) {
		dev_err(smmu->dev, "unknown/unsupported TT endianness!\n");
		return -ENXIO;
	}

vs

	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR0);

	smmu->features |= smmu_idr0_features(reg);
	if (!(smmu->features & (ARM_SMMU_FEAT_TT_LE | ARM_SMMU_FEAT_TT_BE)))
		return -ENXIO;

Then you can put the prior patches tlbi stuff here instead of the
header file, which is important because after I get the CONT and
errata stuff worked in it becomes really quite big.

-- 
Jason