Re: [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Fri, Jun 26, 2015 at 09:33:04AM +0100, Zhen Lei wrote:
> Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
> it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
> prefetch the configuration for a specified StreamID, without this command
> will not impact SMMUv3 function.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d799feb..3971aef 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  	};
> 
>  	int i;
> +	u32 opcode;
>  	u64 cmd[CMDQ_ENT_DWORDS];
>  	struct arm_smmu_queue *q = &smmu->cmdq.q;
>  	u32 cons = readl_relaxed(q->cons_reg);
> @@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  		.opcode = CMDQ_OP_CMD_SYNC,
>  	};
> 
> -	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
> -		cerror_str[idx]);
> -
>  	switch (idx) {
>  	case CMDQ_ERR_CERROR_ILL_IDX:
>  		break;
>  	case CMDQ_ERR_CERROR_ABT_IDX:
> -		dev_err(smmu->dev, "retrying command fetch\n");
>  	case CMDQ_ERR_CERROR_NONE_IDX:
> +		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
> +			cerror_str[idx]);
> +		dev_err(smmu->dev, "retrying command execution\n");

Why are you making this (unrelated) change?

Out of interest, did the gerror code manage to turn the prefetch into
a SYNC, as intended?

>  		return;
>  	}
> 
> @@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  	 * not to touch any of the shadow cmdq state.
>  	 */
>  	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
> +
> +	/*
> +	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
> +	 * command, it's ugly to print error information.
> +	 */
> +	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
> +	if (opcode == CMDQ_OP_PREFETCH_CFG)
> +		goto skip_error_command;
> +

I think a better way to fix this would be to add a quick to the device-tree
along the lines of "hisilicon,broken-prefetch-cmd" which we could check
before issuing the prefetch command in arm_smmu_write_strtab_ent.

Something along the lines of the arm_smmu_options in arm-smmu.c would be
a good basis for quirks.

Will