Re: [Devel] [PATCH v9 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Lorenzo Pieralisi]

On Thu, Jun 22, 2017 at 09:35:35PM +0200, Robert Richter wrote:
> On 22.06.17 19:58:22, Will Deacon wrote:
> > On Thu, Jun 22, 2017 at 07:22:57PM +0100, Will Deacon wrote:
> > > On Thu, Jun 22, 2017 at 05:35:35PM +0530, Geetha sowjanya wrote:
> > > > Cavium ThunderX2 SMMUv3 implementation has two Silicon Erratas.
> > > > 1. Errata ID #74
> > > >    SMMU register alias Page 1 is not implemented
> > > > 2. Errata ID #126
> > > >    SMMU doesnt support unique IRQ lines and also MSI for gerror,
> > > >    eventq and cmdq-sync
> > > > 
> > > > The following patchset does software workaround for these two erratas.
> > > 
> > > I've picked up the first two patches, and left comments on the final patch.
> > 
> > ... except that it doesn't build:
> > 
> > 
> > drivers/acpi/arm64/iort.c: In function âarm_smmu_v3_resource_sizeâ:
> > drivers/acpi/arm64/iort.c:837:21: error: âACPI_IORT_SMMU_V3_CAVIUM_CN99XXâ undeclared (first use in this function)
> >   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> >                      ^
> > drivers/acpi/arm64/iort.c:837:21: note: each undeclared identifier is reported only once for each function it appears in
> > make[4]: *** [drivers/acpi/arm64/iort.o] Error 1
> > 
> > 
> > I don't see ACPI_IORT_SMMU_V3_CAVIUM_CN99XX defined, even in linux-next.
> > 
> > What's the plan here?
> 
> It is defined already in acpica and we actually waiting for the acpi
> maintainers to include it:
> 
>  https://github.com/acpica/acpica/commit/d00a4eb86e64
> 
> We could add
> 
> /* Until ACPICA headers cover IORT rev. C */
> #ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> #endif
> 
> to both files:
> 
>  drivers/acpi/arm64/iort.c
>  drivers/iommu/arm-smmu-v3.c
> 

I thought it was a solved problem (and that the IORT patch was based
on Robin's workaround) but I was clearly wrong and I apologise to
Will about this.

FWIW, you could add the define in include/linux/acpi_iort.h and I will
remove it whenever ACPICA changes make it into the kernel.

Thanks,
Lorenzo

> This is similar to what Robin did.
> 
> (I checked arm64 include files and the closest was
> arch/arm64/include/asm/acpi.h, bug this seems not really suitable to
> me.)
> 
> I have created a separate patch to be applied at first below. We can
> revert it after acpica was updated.
> 
> -Robert
> 
> 
> 
> 
> From ad7f0112a2a71059c32bd315835c33cc7bc660b8 Mon Sep 17 00:00:00 2001
> From: Robert Richter <rrichter@xxxxxxxxxx>
> Date: Thu, 22 Jun 2017 21:20:54 +0200
> Subject: [PATCH] iommu/arm-smmu-v3: Add temporary Cavium SMMU-V3 model nuber
>  definitions
> 
> The model number is already defined in acpica and we actually waiting
> for the acpi maintainers to include it:
> 
>  https://github.com/acpica/acpica/commit/d00a4eb86e64
> 
> Adding those temporary definitions until the change makes it into
> include/acpi/actbl2.h.
> 
> Signed-off-by: Robert Richter <rrichter@xxxxxxxxxx>
> ---
>  drivers/acpi/arm64/iort.c   | 5 +++++
>  drivers/iommu/arm-smmu-v3.c | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 797b28dc7b34..15491237a657 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -31,6 +31,11 @@
>  #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
>  				(1 << ACPI_IORT_NODE_SMMU_V3))
>  
> +/* Until ACPICA headers cover IORT rev. C */
> +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> +#endif
> +
>  struct iort_its_msi_chip {
>  	struct list_head	list;
>  	struct fwnode_handle	*fw_node;
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 380969aa60d5..c759dfa7442d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -412,6 +412,11 @@
>  #define MSI_IOVA_BASE			0x8000000
>  #define MSI_IOVA_LENGTH			0x100000
>  
> +/* Until ACPICA headers cover IORT rev. C */
> +#ifndef ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX		0x2
> +#endif
> +
>  static bool disable_bypass;
>  module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
> -- 
> 2.11.0
>