Re: [Freedreno] [PATCH v2 2/7] iommu/arm-smmu: Add domain attribute for QCOM system cache

2020-01-03 Thread Rob Clark
On Thu, Jan 2, 2020 at 3:02 AM Sharat Masetty  wrote:
>
> From: Vivek Gautam 
>
> Add iommu domain attribute for using system cache aka last level
> cache on QCOM SoCs by client drivers like GPU to set right
> attributes for caching the hardware pagetables into the system cache.
>
> Signed-off-by: Vivek Gautam 
> Co-developed-by: Sai Prakash Ranjan 
> Signed-off-by: Sai Prakash Ranjan 
> ---
>  drivers/iommu/arm-smmu-qcom.c | 10 ++
>  drivers/iommu/arm-smmu.c  | 14 ++
>  drivers/iommu/arm-smmu.h  |  1 +
>  include/linux/iommu.h |  1 +
>  4 files changed, 26 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> index 24c071c..d1d22df 100644
> --- a/drivers/iommu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm-smmu-qcom.c
> @@ -30,7 +30,17 @@ static int qcom_sdm845_smmu500_reset(struct 
> arm_smmu_device *smmu)
> return ret;
>  }
>
> +static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> + struct io_pgtable_cfg *pgtbl_cfg)
> +{
> +   if (smmu_domain->sys_cache)
> +   pgtbl_cfg->coherent_walk = false;

just curious, does coherent walk not work with sys-cache, or is it
just that it is kind of pointless (given that, afaiu, the pagetables
can be cached by the system cache)?

> +
> +   return 0;
> +}
> +
>  static const struct arm_smmu_impl qcom_smmu_impl = {
> +   .init_context = qcom_smmu_init_context,
> .reset = qcom_sdm845_smmu500_reset,
>  };
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 4f7e0c0..055b548 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1466,6 +1466,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain 
> *domain,
> case DOMAIN_ATTR_NESTING:
> *(int *)data = (smmu_domain->stage == 
> ARM_SMMU_DOMAIN_NESTED);
> return 0;
> +   case DOMAIN_ATTR_QCOM_SYS_CACHE:
> +   *((int *)data) = smmu_domain->sys_cache;
> +   return 0;
> default:
> return -ENODEV;
> }
> @@ -1506,6 +1509,17 @@ static int arm_smmu_domain_set_attr(struct 
> iommu_domain *domain,
> else
> smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
> break;
> +   case DOMAIN_ATTR_QCOM_SYS_CACHE:
> +   if (smmu_domain->smmu) {
> +   ret = -EPERM;
> +   goto out_unlock;
> +   }
> +
> +   if (*((int *)data))
> +   smmu_domain->sys_cache = true;
> +   else
> +   smmu_domain->sys_cache = false;
> +   break;
> default:
> ret = -ENODEV;
> }
> diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> index f57cdbe..8aeaaf0 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -322,6 +322,7 @@ struct arm_smmu_domain {
> struct mutexinit_mutex; /* Protects smmu pointer 
> */
> spinlock_t  cb_lock; /* Serialises ATS1* ops and 
> TLB syncs */
> struct iommu_domain domain;
> +   boolsys_cache;
>  };
>
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0c60e75..bd61c60 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -127,6 +127,7 @@ enum iommu_attr {
> DOMAIN_ATTR_FSL_PAMUV1,
> DOMAIN_ATTR_NESTING,/* two stages of translation */
> DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
> +   DOMAIN_ATTR_QCOM_SYS_CACHE,

Given that IOMMU_QCOM_SYS_CACHE was renamed to IOMMU_SYS_CACHE_ONLY, I
wonder if this domain attr should simply be DOMAIN_ATTR_SYS_CACHE?

But that said, the function of this domain attr seems to simply be to
disable coherent walk.. I wonder if naming the domain attr after what
it does would make more sense?

BR,
-R


> DOMAIN_ATTR_MAX,
>  };
>
> --
> 1.9.1
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 3/3] iommu: Enable compile testing for some of drivers

2020-01-03 Thread Krzysztof Kozlowski
On Thu, Jan 02, 2020 at 04:40:16PM -0600, Suman Anna wrote:
> Hi Krzysztof,
> 
> On 12/31/19 2:07 AM, Krzysztof Kozlowski wrote:
> > On Tue, Dec 31, 2019 at 03:43:39PM +0800, kbuild test robot wrote:
> >> Hi Krzysztof,
> >>
> >> I love your patch! Perhaps something to improve:
> >>
> >> [auto build test WARNING on iommu/next]
> >> [also build test WARNING on v5.5-rc4]
> >> [if your patch is applied to the wrong git tree, please drop us a note to 
> >> help
> >> improve the system. BTW, we also suggest to use '--base' option to specify 
> >> the
> >> base tree in git format-patch, please see 
> >> https://stackoverflow.com/a/37406982]
> >>
> >> url:
> >> https://github.com/0day-ci/linux/commits/Krzysztof-Kozlowski/iommu-omap-Fix-pointer-cast-Wpointer-to-int-cast-warnings-on-64-bit/20191231-022212
> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
> >> config: ia64-allmodconfig (attached as .config)
> >> compiler: ia64-linux-gcc (GCC) 7.5.0
> >> reproduce:
> >> wget 
> >> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
> >> -O ~/bin/make.cross
> >> chmod +x ~/bin/make.cross
> >> # save the attached .config to linux build tree
> >> GCC_VERSION=7.5.0 make.cross ARCH=ia64 
> >>
> >> If you fix the issue, kindly add following tag
> >> Reported-by: kbuild test robot 
> > 
> > I saw it already while compile testing my patch. I must admit that I
> > could not find easy/fast fix for it.  Probably the
> > omap_iommu_translate() helper should be made 64-bit friendly but this
> > obfuscates the code. 
> 
> >The driver and hardware supports only 32-bit addresses.
> 
> Yeah, is there a reason why you are trying to enable the build for the
> OMAP IOMMU driver on 64-bit platforms or other architectures - the IP
> and driver is only ever used on ARMv7 platforms, and it should already
> be available for COMPILE_TEST on those.

It's the same reason we enable compile testing on all other drivers
which will never be used outside original platforms. There are many
of such examples because we want to increase the build coverage to as
many different configurations as possible. There could be also another
benefit - easier code reusability - although it is purely theoretical in
this case, I think.

Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu