[PATCH v11 03/11] iommu/arm-smmu-v3: Swap EVTQ_MSI_INDEX and GERROR_MSI_INDEX

[Jason Gunthorpe]

> The nvec trick in arm_smmu_setup_msis() works well on PRIQ since its index
> is the last index in enum arm_smmu_msi_index, but it does not work well on
> the EVTQ as its index is 0.
> 
> A subsequent change will make EVTQ optional so it can be disabled in kdump
> mode. This can skip its MSI vector allocation.
> 
> The index is driver-internal to select arm_smmu_msi_cfg[] register set and
> the corresponding MSI slot. Nothing else.
> 
> Swap EVTQ_MSI_INDEX and GERROR_MSI_INDEX. Then, arm_smmu_setup_msis() will
> simply decrease nvec when EVTQ is disabled.
> 
> No functional change intended.
> 
> Signed-off-by: Nicolin Chen <nicolinc at nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 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 aeace4335abfe9..6ff61565f75838 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -57,8 +57,8 @@ static struct iommu_dirty_ops arm_smmu_dirty_ops;
>  static DEFINE_STATIC_KEY_FALSE(arm_smmu_erratum_repeat_tlbi_cfgi_key);
>  
>  enum arm_smmu_msi_index {
> -	EVTQ_MSI_INDEX,
>  	GERROR_MSI_INDEX,
> +	EVTQ_MSI_INDEX,
>  	PRIQ_MSI_INDEX,
>  	ARM_SMMU_MAX_MSIS,
>  };

This is the only change that is really needed

> @@ -68,16 +68,16 @@ static_assert(sizeof(struct arm_smmu_ste) == NUM_ENTRY_QWORDS * sizeof(u64));
>  static_assert(sizeof(struct arm_smmu_cd) == NUM_ENTRY_QWORDS * sizeof(u64));
>  
>  static phys_addr_t arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = {
> -	[EVTQ_MSI_INDEX] = {
> -		ARM_SMMU_EVTQ_IRQ_CFG0,
> -		ARM_SMMU_EVTQ_IRQ_CFG1,
> -		ARM_SMMU_EVTQ_IRQ_CFG2,
> -	},
>  	[GERROR_MSI_INDEX] = {
>  		ARM_SMMU_GERROR_IRQ_CFG0,
>  		ARM_SMMU_GERROR_IRQ_CFG1,
>  		ARM_SMMU_GERROR_IRQ_CFG2,
>  	},
> +	[EVTQ_MSI_INDEX] = {
> +		ARM_SMMU_EVTQ_IRQ_CFG0,
> +		ARM_SMMU_EVTQ_IRQ_CFG1,
> +		ARM_SMMU_EVTQ_IRQ_CFG2,
> +	},

The arrays don't have to be in order

>  	[PRIQ_MSI_INDEX] = {
>  		ARM_SMMU_PRIQ_IRQ_CFG0,
>  		ARM_SMMU_PRIQ_IRQ_CFG1,
> @@ -4965,15 +4965,15 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
>  		return;
>  	}
>  
> -	/* Allocate MSIs for evtq, gerror and priq. Ignore cmdq */
> +	/* Allocate MSIs for gerror, evtq and priq. Ignore cmdq */
>  	ret = platform_device_msi_init_and_alloc_irqs(dev, nvec, arm_smmu_write_msi_msg);
>  	if (ret) {
>  		dev_warn(dev, "failed to allocate MSIs - falling back to wired irqs\n");
>  		return;
>  	}
>  
> -	smmu->evtq.q.irq = msi_get_virq(dev, EVTQ_MSI_INDEX);
>  	smmu->gerr_irq = msi_get_virq(dev, GERROR_MSI_INDEX);
> +	smmu->evtq.q.irq = msi_get_virq(dev, EVTQ_MSI_INDEX);
>  	smmu->priq.q.irq = msi_get_virq(dev, PRIQ_MSI_INDEX);

Nor do the assignments.

But I'm not really a fan of this design, it is very fragile.

I think if you want to compress the nvec's array it should be done
directly.

The smmu should have a small array
   u8 msi_map[]

Which holds the xx_MSI_INDEX value. Then arm_smmu_write_msi_msg()
would indirect to get the config:

	phys_addr_t *cfg = arm_smmu_msi_cfg[smmu->msi_map[desc->msi_index]];

And we just compress msi_map as we go through the normal flow

   index_msi[EVTQ_MSI_INDEX] = nvec;
   smmu->msi_map[nvec++] = EVTQ_MSI_INDEX

Then it isn't fragile at all.

-- 
Jason