Re: [PATCH v16 14/20] iommu/arm-smmu: Prepare for the adreno-smmu implementation

2020-09-05 Thread Rob Clark
On Fri, Sep 4, 2020 at 9:00 AM Bjorn Andersson
 wrote:
>
> On Tue 01 Sep 11:46 CDT 2020, Rob Clark wrote:
>
> > From: Jordan Crouse 
> >
> > Do a bit of prep work to add the upcoming adreno-smmu implementation.
> >
> > Add an hook to allow the implementation to choose which context banks
> > to allocate.
> >
> > Move some of the common structs to arm-smmu.h in anticipation of them
> > being used by the implementations and update some of the existing hooks
> > to pass more information that the implementation will need.
> >
> > These modifications will be used by the upcoming Adreno SMMU
> > implementation to identify the GPU device and properly configure it
> > for pagetable switching.
> >
> > Co-developed-by: Rob Clark 
> > Signed-off-by: Jordan Crouse 
> > Signed-off-by: Rob Clark 
>
> As I built the handoff support on top of this patch I ended up
> reworking the alloc_context_bank() prototype to something I found a
> little bit cleaner.
>
> So perhaps you would be interested in squashing
> https://lore.kernel.org/linux-arm-msm/20200904155513.282067-2-bjorn.anders...@linaro.org/
> into this patch?

Yeah, I think this looks nicer, thanks

BR,
-R

> Otherwise, feel free to add my:
>
> Reviewed-by: Bjorn Andersson 
>
> Regards,
> Bjorn
>
> > ---
> >  drivers/iommu/arm/arm-smmu/arm-smmu-impl.c |  2 +-
> >  drivers/iommu/arm/arm-smmu/arm-smmu.c  | 69 ++
> >  drivers/iommu/arm/arm-smmu/arm-smmu.h  | 51 +++-
> >  3 files changed, 68 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c 
> > b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> > index a9861dcd0884..88f17cc33023 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> > @@ -69,7 +69,7 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
> >  }
> >
> >  static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
> > - struct io_pgtable_cfg *pgtbl_cfg)
> > + struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
> >  {
> >   struct cavium_smmu *cs = container_of(smmu_domain->smmu,
> > struct cavium_smmu, smmu);
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index 8e884e58f208..68b7b9e6140e 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -65,41 +65,10 @@ module_param(disable_bypass, bool, S_IRUGO);
> >  MODULE_PARM_DESC(disable_bypass,
> >   "Disable bypass streams such that incoming transactions from devices 
> > that are not attached to an iommu domain will report an abort back to the 
> > device and will not be allowed to pass through the SMMU.");
> >
> > -struct arm_smmu_s2cr {
> > - struct iommu_group  *group;
> > - int count;
> > - enum arm_smmu_s2cr_type type;
> > - enum arm_smmu_s2cr_privcfg  privcfg;
> > - u8  cbndx;
> > -};
> > -
> >  #define s2cr_init_val (struct arm_smmu_s2cr){  
> >   \
> >   .type = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS,\
> >  }
> >
> > -struct arm_smmu_smr {
> > - u16 mask;
> > - u16 id;
> > - boolvalid;
> > -};
> > -
> > -struct arm_smmu_cb {
> > - u64 ttbr[2];
> > - u32 tcr[2];
> > - u32 mair[2];
> > - struct arm_smmu_cfg *cfg;
> > -};
> > -
> > -struct arm_smmu_master_cfg {
> > - struct arm_smmu_device  *smmu;
> > - s16 smendx[];
> > -};
> > -#define INVALID_SMENDX   -1
> > -#define cfg_smendx(cfg, fw, i) \
> > - (i >= fw->num_ids ? INVALID_SMENDX : cfg->smendx[i])
> > -#define for_each_cfg_sme(cfg, fw, i, idx) \
> > - for (i = 0; idx = cfg_smendx(cfg, fw, i), i < fw->num_ids; ++i)
> > -
> >  static bool using_legacy_binding, using_generic_binding;
> >
> >  static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> > @@ -234,19 +203,6 @@ static int arm_smmu_register_legacy_master(struct 
> > device *dev,
> >  }
> >  #endif /* CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS */
> >
> > -static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
> > -{
> > - int idx;
> > -
> > - do {
> > - idx = find_next_zero_bit(map, end, start);
> > - if (idx == end)
> > - return -ENOSPC;
> > - } while (test_and_set_bit(idx, map));
> > -
> > - return idx;
> > -}
> > -
> >  static void __arm_smmu_free_bitmap(unsigned long *map, int idx)
> >  {
> >   clear_bit(idx, map);
> > @@ -578,7 +534,7 @@ static void arm_smmu_init_context_bank(struct 
> > arm_smmu_domain *smmu_domain,
> >   }
> >  }
> >
> > -s

Re: [PATCH v16 14/20] iommu/arm-smmu: Prepare for the adreno-smmu implementation

2020-09-04 Thread Bjorn Andersson
On Tue 01 Sep 11:46 CDT 2020, Rob Clark wrote:

> From: Jordan Crouse 
> 
> Do a bit of prep work to add the upcoming adreno-smmu implementation.
> 
> Add an hook to allow the implementation to choose which context banks
> to allocate.
> 
> Move some of the common structs to arm-smmu.h in anticipation of them
> being used by the implementations and update some of the existing hooks
> to pass more information that the implementation will need.
> 
> These modifications will be used by the upcoming Adreno SMMU
> implementation to identify the GPU device and properly configure it
> for pagetable switching.
> 
> Co-developed-by: Rob Clark 
> Signed-off-by: Jordan Crouse 
> Signed-off-by: Rob Clark 

As I built the handoff support on top of this patch I ended up
reworking the alloc_context_bank() prototype to something I found a
little bit cleaner.

So perhaps you would be interested in squashing
https://lore.kernel.org/linux-arm-msm/20200904155513.282067-2-bjorn.anders...@linaro.org/
into this patch?

Otherwise, feel free to add my:

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-impl.c |  2 +-
>  drivers/iommu/arm/arm-smmu/arm-smmu.c  | 69 ++
>  drivers/iommu/arm/arm-smmu/arm-smmu.h  | 51 +++-
>  3 files changed, 68 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index a9861dcd0884..88f17cc33023 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -69,7 +69,7 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
>  }
>  
>  static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
> - struct io_pgtable_cfg *pgtbl_cfg)
> + struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>  {
>   struct cavium_smmu *cs = container_of(smmu_domain->smmu,
> struct cavium_smmu, smmu);
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 8e884e58f208..68b7b9e6140e 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -65,41 +65,10 @@ module_param(disable_bypass, bool, S_IRUGO);
>  MODULE_PARM_DESC(disable_bypass,
>   "Disable bypass streams such that incoming transactions from devices 
> that are not attached to an iommu domain will report an abort back to the 
> device and will not be allowed to pass through the SMMU.");
>  
> -struct arm_smmu_s2cr {
> - struct iommu_group  *group;
> - int count;
> - enum arm_smmu_s2cr_type type;
> - enum arm_smmu_s2cr_privcfg  privcfg;
> - u8  cbndx;
> -};
> -
>  #define s2cr_init_val (struct arm_smmu_s2cr){
> \
>   .type = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS,\
>  }
>  
> -struct arm_smmu_smr {
> - u16 mask;
> - u16 id;
> - boolvalid;
> -};
> -
> -struct arm_smmu_cb {
> - u64 ttbr[2];
> - u32 tcr[2];
> - u32 mair[2];
> - struct arm_smmu_cfg *cfg;
> -};
> -
> -struct arm_smmu_master_cfg {
> - struct arm_smmu_device  *smmu;
> - s16 smendx[];
> -};
> -#define INVALID_SMENDX   -1
> -#define cfg_smendx(cfg, fw, i) \
> - (i >= fw->num_ids ? INVALID_SMENDX : cfg->smendx[i])
> -#define for_each_cfg_sme(cfg, fw, i, idx) \
> - for (i = 0; idx = cfg_smendx(cfg, fw, i), i < fw->num_ids; ++i)
> -
>  static bool using_legacy_binding, using_generic_binding;
>  
>  static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> @@ -234,19 +203,6 @@ static int arm_smmu_register_legacy_master(struct device 
> *dev,
>  }
>  #endif /* CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS */
>  
> -static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
> -{
> - int idx;
> -
> - do {
> - idx = find_next_zero_bit(map, end, start);
> - if (idx == end)
> - return -ENOSPC;
> - } while (test_and_set_bit(idx, map));
> -
> - return idx;
> -}
> -
>  static void __arm_smmu_free_bitmap(unsigned long *map, int idx)
>  {
>   clear_bit(idx, map);
> @@ -578,7 +534,7 @@ static void arm_smmu_init_context_bank(struct 
> arm_smmu_domain *smmu_domain,
>   }
>  }
>  
> -static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int 
> idx)
> +void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
>  {
>   u32 reg;
>   bool stage1;
> @@ -665,7 +621,8 @@ static void arm_smmu_write_context_bank(struct 
> arm_smmu_device *smmu, int idx)
>  }
>  
>  static int arm_smmu_init_domai

[PATCH v16 14/20] iommu/arm-smmu: Prepare for the adreno-smmu implementation

2020-09-01 Thread Rob Clark
From: Jordan Crouse 

Do a bit of prep work to add the upcoming adreno-smmu implementation.

Add an hook to allow the implementation to choose which context banks
to allocate.

Move some of the common structs to arm-smmu.h in anticipation of them
being used by the implementations and update some of the existing hooks
to pass more information that the implementation will need.

These modifications will be used by the upcoming Adreno SMMU
implementation to identify the GPU device and properly configure it
for pagetable switching.

Co-developed-by: Rob Clark 
Signed-off-by: Jordan Crouse 
Signed-off-by: Rob Clark 
---
 drivers/iommu/arm/arm-smmu/arm-smmu-impl.c |  2 +-
 drivers/iommu/arm/arm-smmu/arm-smmu.c  | 69 ++
 drivers/iommu/arm/arm-smmu/arm-smmu.h  | 51 +++-
 3 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index a9861dcd0884..88f17cc33023 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -69,7 +69,7 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
 }
 
 static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
-   struct io_pgtable_cfg *pgtbl_cfg)
+   struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
 {
struct cavium_smmu *cs = container_of(smmu_domain->smmu,
  struct cavium_smmu, smmu);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 8e884e58f208..68b7b9e6140e 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -65,41 +65,10 @@ module_param(disable_bypass, bool, S_IRUGO);
 MODULE_PARM_DESC(disable_bypass,
"Disable bypass streams such that incoming transactions from devices 
that are not attached to an iommu domain will report an abort back to the 
device and will not be allowed to pass through the SMMU.");
 
-struct arm_smmu_s2cr {
-   struct iommu_group  *group;
-   int count;
-   enum arm_smmu_s2cr_type type;
-   enum arm_smmu_s2cr_privcfg  privcfg;
-   u8  cbndx;
-};
-
 #define s2cr_init_val (struct arm_smmu_s2cr){  \
.type = disable_bypass ? S2CR_TYPE_FAULT : S2CR_TYPE_BYPASS,\
 }
 
-struct arm_smmu_smr {
-   u16 mask;
-   u16 id;
-   boolvalid;
-};
-
-struct arm_smmu_cb {
-   u64 ttbr[2];
-   u32 tcr[2];
-   u32 mair[2];
-   struct arm_smmu_cfg *cfg;
-};
-
-struct arm_smmu_master_cfg {
-   struct arm_smmu_device  *smmu;
-   s16 smendx[];
-};
-#define INVALID_SMENDX -1
-#define cfg_smendx(cfg, fw, i) \
-   (i >= fw->num_ids ? INVALID_SMENDX : cfg->smendx[i])
-#define for_each_cfg_sme(cfg, fw, i, idx) \
-   for (i = 0; idx = cfg_smendx(cfg, fw, i), i < fw->num_ids; ++i)
-
 static bool using_legacy_binding, using_generic_binding;
 
 static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
@@ -234,19 +203,6 @@ static int arm_smmu_register_legacy_master(struct device 
*dev,
 }
 #endif /* CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS */
 
-static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
-{
-   int idx;
-
-   do {
-   idx = find_next_zero_bit(map, end, start);
-   if (idx == end)
-   return -ENOSPC;
-   } while (test_and_set_bit(idx, map));
-
-   return idx;
-}
-
 static void __arm_smmu_free_bitmap(unsigned long *map, int idx)
 {
clear_bit(idx, map);
@@ -578,7 +534,7 @@ static void arm_smmu_init_context_bank(struct 
arm_smmu_domain *smmu_domain,
}
 }
 
-static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
+void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx)
 {
u32 reg;
bool stage1;
@@ -665,7 +621,8 @@ static void arm_smmu_write_context_bank(struct 
arm_smmu_device *smmu, int idx)
 }
 
 static int arm_smmu_init_domain_context(struct iommu_domain *domain,
-   struct arm_smmu_device *smmu)
+   struct arm_smmu_device *smmu,
+   struct device *dev)
 {
int irq, start, ret = 0;
unsigned long ias, oas;
@@ -780,10 +737,20 @@ static int arm_smmu_init_domain_context(struct 
iommu_domain *domain,
ret = -EINVAL;
goto out_unlock;
}
-   ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
+
+   smmu_domain->smmu = smmu;
+
+   if (smmu->impl && smmu->impl->alloc_context_bank)
+