[PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-06-10 Thread Jean-Philippe Brucker
For platform devices that support SubstreamID (SSID), firmware provides
the number of supported SSID bits. Restrict it to what the SMMU supports
and cache it into master->ssid_bits.

Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/arm-smmu-v3.c | 11 +++
 drivers/iommu/of_iommu.c|  6 +-
 include/linux/iommu.h   |  1 +
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d5a694f02c2..3254f473e681 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -604,6 +604,7 @@ struct arm_smmu_master {
struct list_headdomain_head;
u32 *sids;
unsigned intnum_sids;
+   unsigned intssid_bits;
boolats_enabled :1;
 };
 
@@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
}
}
 
+   master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
+
+   /*
+* If the SMMU doesn't support 2-stage CD, limit the linear
+* tables to a reasonable number of contexts, let's say
+* 64kB / sizeof(ctx_desc) = 1024 = 2^10
+*/
+   if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
+   master->ssid_bits = min(master->ssid_bits, 10U);
+
group = iommu_group_get_for_dev(dev);
if (!IS_ERR(group)) {
iommu_group_put(group);
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index f04a6df65eb8..04f4f6b95d82 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device 
*dev,
if (err)
break;
}
-   }
 
+   fwspec = dev_iommu_fwspec_get(dev);
+   if (!err && fwspec)
+   of_property_read_u32(master_np, "pasid-num-bits",
+&fwspec->num_pasid_bits);
+   }
 
/*
 * Two success conditions can be represented by non-negative err here:
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 519e40fb23ce..b91df613385f 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -536,6 +536,7 @@ struct iommu_fwspec {
struct fwnode_handle*iommu_fwnode;
void*iommu_priv;
u32 flags;
+   u32 num_pasid_bits;
unsigned intnum_ids;
u32 ids[1];
 };
-- 
2.21.0



Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-06-11 Thread Jonathan Cameron
On Mon, 10 Jun 2019 19:47:09 +0100
Jean-Philippe Brucker  wrote:

> For platform devices that support SubstreamID (SSID), firmware provides
> the number of supported SSID bits. Restrict it to what the SMMU supports
> and cache it into master->ssid_bits.
> 
> Signed-off-by: Jean-Philippe Brucker 

Missing kernel-doc.

Thanks,

Jonathan

> ---
>  drivers/iommu/arm-smmu-v3.c | 11 +++
>  drivers/iommu/of_iommu.c|  6 +-
>  include/linux/iommu.h   |  1 +
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d5a694f02c2..3254f473e681 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -604,6 +604,7 @@ struct arm_smmu_master {
>   struct list_headdomain_head;
>   u32 *sids;
>   unsigned intnum_sids;
> + unsigned intssid_bits;
>   boolats_enabled :1;
>  };
>  
> @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
>   }
>   }
>  
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> +
> + /*
> +  * If the SMMU doesn't support 2-stage CD, limit the linear
> +  * tables to a reasonable number of contexts, let's say
> +  * 64kB / sizeof(ctx_desc) = 1024 = 2^10
> +  */
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min(master->ssid_bits, 10U);
> +
>   group = iommu_group_get_for_dev(dev);
>   if (!IS_ERR(group)) {
>   iommu_group_put(group);
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index f04a6df65eb8..04f4f6b95d82 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device 
> *dev,
>   if (err)
>   break;
>   }
> - }
>  
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (!err && fwspec)
> + of_property_read_u32(master_np, "pasid-num-bits",
> +  &fwspec->num_pasid_bits);
> + }
>  
>   /*
>* Two success conditions can be represented by non-negative err here:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 519e40fb23ce..b91df613385f 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -536,6 +536,7 @@ struct iommu_fwspec {
>   struct fwnode_handle*iommu_fwnode;
>   void*iommu_priv;
>   u32 flags;
> + u32 num_pasid_bits;

This structure has kernel doc so you need to add something for this.

>   unsigned intnum_ids;
>   u32 ids[1];
>  };


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


Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-06-11 Thread Jean-Philippe Brucker
On 11/06/2019 10:42, Jonathan Cameron wrote:
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 519e40fb23ce..b91df613385f 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -536,6 +536,7 @@ struct iommu_fwspec {
>>  struct fwnode_handle*iommu_fwnode;
>>  void*iommu_priv;
>>  u32 flags;
>> +u32 num_pasid_bits;
> 
> This structure has kernel doc so you need to add something for this.

Good catch

Thanks,
Jean


Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-06-18 Thread Will Deacon
On Mon, Jun 10, 2019 at 07:47:09PM +0100, Jean-Philippe Brucker wrote:
> For platform devices that support SubstreamID (SSID), firmware provides
> the number of supported SSID bits. Restrict it to what the SMMU supports
> and cache it into master->ssid_bits.
> 
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  drivers/iommu/arm-smmu-v3.c | 11 +++
>  drivers/iommu/of_iommu.c|  6 +-
>  include/linux/iommu.h   |  1 +
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d5a694f02c2..3254f473e681 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -604,6 +604,7 @@ struct arm_smmu_master {
>   struct list_headdomain_head;
>   u32 *sids;
>   unsigned intnum_sids;
> + unsigned intssid_bits;
>   boolats_enabled :1;
>  };
>  
> @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
>   }
>   }
>  
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> +
> + /*
> +  * If the SMMU doesn't support 2-stage CD, limit the linear
> +  * tables to a reasonable number of contexts, let's say
> +  * 64kB / sizeof(ctx_desc) = 1024 = 2^10
> +  */
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min(master->ssid_bits, 10U);

Please introduce a #define for the 10, so that it is computed in the way
you describe in the comment (a bit like we do for things like queue sizes).

> +
>   group = iommu_group_get_for_dev(dev);
>   if (!IS_ERR(group)) {
>   iommu_group_put(group);
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index f04a6df65eb8..04f4f6b95d82 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device 
> *dev,
>   if (err)
>   break;
>   }
> - }
>  
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (!err && fwspec)
> + of_property_read_u32(master_np, "pasid-num-bits",
> +  &fwspec->num_pasid_bits);
> + }

Hmm. Do you know if there's anything in ACPI for this?

Otherwise, patch looks fine. Thanks.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-06-19 Thread Jean-Philippe Brucker
On 18/06/2019 19:08, Will Deacon wrote:
>> +/*
>> + * If the SMMU doesn't support 2-stage CD, limit the linear
>> + * tables to a reasonable number of contexts, let's say
>> + * 64kB / sizeof(ctx_desc) = 1024 = 2^10
>> + */
>> +if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
>> +master->ssid_bits = min(master->ssid_bits, 10U);
> 
> Please introduce a #define for the 10, so that it is computed in the way
> you describe in the comment (a bit like we do for things like queue sizes).

Ok

>> +
>>  group = iommu_group_get_for_dev(dev);
>>  if (!IS_ERR(group)) {
>>  iommu_group_put(group);
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index f04a6df65eb8..04f4f6b95d82 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct 
>> device *dev,
>>  if (err)
>>  break;
>>  }
>> -}
>>  
>> +fwspec = dev_iommu_fwspec_get(dev);
>> +if (!err && fwspec)
>> +of_property_read_u32(master_np, "pasid-num-bits",
>> + &fwspec->num_pasid_bits);
>> +}
> 
> Hmm. Do you know if there's anything in ACPI for this?

Yes, IORT version D introduced a "substream width" field for the Named
component node (platform device). I don't think it existed last time I
checked, so I'll see about supporting it.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-07-08 Thread Auger Eric
Hi Jean,

On 6/10/19 8:47 PM, Jean-Philippe Brucker wrote:
> For platform devices that support SubstreamID (SSID), firmware provides
> the number of supported SSID bits. Restrict it to what the SMMU supports
> and cache it into master->ssid_bits.
The commit message may give the impression the master's ssid_bits field
only is used for platform devices.
> 
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  drivers/iommu/arm-smmu-v3.c | 11 +++
>  drivers/iommu/of_iommu.c|  6 +-
>  include/linux/iommu.h   |  1 +
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d5a694f02c2..3254f473e681 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -604,6 +604,7 @@ struct arm_smmu_master {
>   struct list_headdomain_head;
>   u32 *sids;
>   unsigned intnum_sids;
> + unsigned intssid_bits;
>   boolats_enabled :1;
>  };
>  
> @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
>   }
>   }
>  
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
In case the device is a PCI device, what is the value taken by
fwspec->num_pasid_bits?
> +
> + /*
> +  * If the SMMU doesn't support 2-stage CD, limit the linear
> +  * tables to a reasonable number of contexts, let's say
> +  * 64kB / sizeof(ctx_desc) = 1024 = 2^10
ctx_desc is 26B so 11bits would be OK
> +  */
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min(master->ssid_bits, 10U);
> +
>   group = iommu_group_get_for_dev(dev);
>   if (!IS_ERR(group)) {
>   iommu_group_put(group);
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index f04a6df65eb8..04f4f6b95d82 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device 
> *dev,
>   if (err)
>   break;
>   }
> - }
>  
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (!err && fwspec)
> + of_property_read_u32(master_np, "pasid-num-bits",
> +  &fwspec->num_pasid_bits);
> + }
>  
>   /*
>* Two success conditions can be represented by non-negative err here:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 519e40fb23ce..b91df613385f 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -536,6 +536,7 @@ struct iommu_fwspec {
>   struct fwnode_handle*iommu_fwnode;
>   void*iommu_priv;
>   u32 flags;
> + u32 num_pasid_bits;
>   unsigned intnum_ids;
>   u32 ids[1];
>  };
> 
Thanks

Eric


Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

2019-09-19 Thread Jean-Philippe Brucker
Hi Eric,

Sorry for the delay. I'll see if I can resend this for v5.5, although I
can't do much testing at the moment.

On Mon, Jul 08, 2019 at 09:58:22AM +0200, Auger Eric wrote:
> Hi Jean,
> 
> On 6/10/19 8:47 PM, Jean-Philippe Brucker wrote:
> > For platform devices that support SubstreamID (SSID), firmware provides
> > the number of supported SSID bits. Restrict it to what the SMMU supports
> > and cache it into master->ssid_bits.
> The commit message may give the impression the master's ssid_bits field
> only is used for platform devices.

Ok maybe I should add that this field will be used for PCI PASID as
well.

> > @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
> > }
> > }
> >  
> > +   master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> In case the device is a PCI device, what is the value taken by
> fwspec->num_pasid_bits?

It would be zero, as firmware only specifies a value for platform
devices. For a PCI device, patch 8/8 fills master->ssid_bits from the
PCIe PASID capability.

> > +   /*
> > +* If the SMMU doesn't support 2-stage CD, limit the linear
> > +* tables to a reasonable number of contexts, let's say
> > +* 64kB / sizeof(ctx_desc) = 1024 = 2^10
> ctx_desc is 26B so 11bits would be OK

This refers to the size of the hardware context descriptor, not struct
arm_smmu_ctx_desc. Next version moves this to a define and makes it
clearer.

Thanks,
Jean