Re: [PATCH v14 13/13] iommu/smmuv3: Accept configs with more than one context descriptor
Hi Shameer,
On 4/1/21 2:38 PM, Shameerali Kolothum Thodi wrote:
>
>
>> -Original Message-
>> From: Auger Eric [mailto:eric.au...@redhat.com]
>> Sent: 01 April 2021 12:49
>> To: yuzenghui
>> Cc: eric.auger@gmail.com; iommu@lists.linux-foundation.org;
>> linux-ker...@vger.kernel.org; k...@vger.kernel.org;
>> kvm...@lists.cs.columbia.edu; w...@kernel.org; m...@kernel.org;
>> robin.mur...@arm.com; j...@8bytes.org; alex.william...@redhat.com;
>> t...@semihalf.com; zhukeqian ;
>> jacob.jun@linux.intel.com; yi.l@intel.com; wangxingang
>> ; jiangkunkun ;
>> jean-phili...@linaro.org; zhangfei@linaro.org; zhangfei@gmail.com;
>> vivek.gau...@arm.com; Shameerali Kolothum Thodi
>> ; nicoleots...@gmail.com;
>> lushenming ; vse...@nvidia.com; Wanghaibin (D)
>>
>> Subject: Re: [PATCH v14 13/13] iommu/smmuv3: Accept configs with more than
>> one context descriptor
>>
>> Hi Zenghui,
>>
>> On 3/30/21 11:23 AM, Zenghui Yu wrote:
>>> Hi Eric,
>>>
>>> On 2021/2/24 4:56, Eric Auger wrote:
>>>> In preparation for vSVA, let's accept userspace provided configs
>>>> with more than one CD. We check the max CD against the host iommu
>>>> capability and also the format (linear versus 2 level).
>>>>
>>>> Signed-off-by: Eric Auger
>>>> Signed-off-by: Shameer Kolothum
>>
>>>> ---
>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 -
>>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> index 332d31c0680f..ab74a0289893 100644
>>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>>> @@ -3038,14 +3038,17 @@ static int
>> arm_smmu_attach_pasid_table(struct
>>>> iommu_domain *domain,
>>>> if (smmu_domain->s1_cfg.set)
>>>> goto out;
>>>> - /*
>>>> - * we currently support a single CD so s1fmt and s1dss
>>>> - * fields are also ignored
>>>> - */
>>>> - if (cfg->pasid_bits)
>>>> + list_for_each_entry(master, _domain->devices,
>>>> domain_head) {
>>>> + if (cfg->pasid_bits > master->ssid_bits)
>>>> + goto out;
>>>> + }
>>>> + if (cfg->vendor_data.smmuv3.s1fmt ==
>>>> STRTAB_STE_0_S1FMT_64K_L2 &&
>>>> + !(smmu->features &
>> ARM_SMMU_FEAT_2_LVL_CDTAB))
>>>> goto out;
>>>> smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
>>>> + smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
>>>> + smmu_domain->s1_cfg.s1fmt =
>> cfg->vendor_data.smmuv3.s1fmt;
>>>
>>> And what about the SIDSS field?
>>>
>> I added this patch upon Shameer's request, to be more vSVA friendly.
>> Hower this series does not really target multiple CD support. At the
>> moment the driver only supports STRTAB_STE_1_S1DSS_SSID0 (0x2) I think.
>> At this moment maybe I can only check the s1dss field is 0x2. Or simply
>> removes this patch?
>>
>> Thoughts?
>
> Right. This was useful for vSVA tests. But yes, to properly support multiple
> CDs
> we need to pass the S1DSS from Qemu. And that requires further changes.
> So I think it's better to remove this patch and reject S1CDMAX != 0 cases.
OK I will remove it
Thanks
Eric
>
> Thanks,
> Shameer
>
>>
>> Eric
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v14 13/13] iommu/smmuv3: Accept configs with more than one context descriptor
> -Original Message-
> From: Auger Eric [mailto:eric.au...@redhat.com]
> Sent: 01 April 2021 12:49
> To: yuzenghui
> Cc: eric.auger@gmail.com; iommu@lists.linux-foundation.org;
> linux-ker...@vger.kernel.org; k...@vger.kernel.org;
> kvm...@lists.cs.columbia.edu; w...@kernel.org; m...@kernel.org;
> robin.mur...@arm.com; j...@8bytes.org; alex.william...@redhat.com;
> t...@semihalf.com; zhukeqian ;
> jacob.jun@linux.intel.com; yi.l@intel.com; wangxingang
> ; jiangkunkun ;
> jean-phili...@linaro.org; zhangfei@linaro.org; zhangfei@gmail.com;
> vivek.gau...@arm.com; Shameerali Kolothum Thodi
> ; nicoleots...@gmail.com;
> lushenming ; vse...@nvidia.com; Wanghaibin (D)
>
> Subject: Re: [PATCH v14 13/13] iommu/smmuv3: Accept configs with more than
> one context descriptor
>
> Hi Zenghui,
>
> On 3/30/21 11:23 AM, Zenghui Yu wrote:
> > Hi Eric,
> >
> > On 2021/2/24 4:56, Eric Auger wrote:
> >> In preparation for vSVA, let's accept userspace provided configs
> >> with more than one CD. We check the max CD against the host iommu
> >> capability and also the format (linear versus 2 level).
> >>
> >> Signed-off-by: Eric Auger
> >> Signed-off-by: Shameer Kolothum
>
> >> ---
> >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 -
> >> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >> index 332d31c0680f..ab74a0289893 100644
> >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> >> @@ -3038,14 +3038,17 @@ static int
> arm_smmu_attach_pasid_table(struct
> >> iommu_domain *domain,
> >> if (smmu_domain->s1_cfg.set)
> >> goto out;
> >> - /*
> >> - * we currently support a single CD so s1fmt and s1dss
> >> - * fields are also ignored
> >> - */
> >> - if (cfg->pasid_bits)
> >> + list_for_each_entry(master, _domain->devices,
> >> domain_head) {
> >> + if (cfg->pasid_bits > master->ssid_bits)
> >> + goto out;
> >> + }
> >> + if (cfg->vendor_data.smmuv3.s1fmt ==
> >> STRTAB_STE_0_S1FMT_64K_L2 &&
> >> + !(smmu->features &
> ARM_SMMU_FEAT_2_LVL_CDTAB))
> >> goto out;
> >> smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
> >> + smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
> >> + smmu_domain->s1_cfg.s1fmt =
> cfg->vendor_data.smmuv3.s1fmt;
> >
> > And what about the SIDSS field?
> >
> I added this patch upon Shameer's request, to be more vSVA friendly.
> Hower this series does not really target multiple CD support. At the
> moment the driver only supports STRTAB_STE_1_S1DSS_SSID0 (0x2) I think.
> At this moment maybe I can only check the s1dss field is 0x2. Or simply
> removes this patch?
>
> Thoughts?
Right. This was useful for vSVA tests. But yes, to properly support multiple CDs
we need to pass the S1DSS from Qemu. And that requires further changes.
So I think it's better to remove this patch and reject S1CDMAX != 0 cases.
Thanks,
Shameer
>
> Eric
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v14 13/13] iommu/smmuv3: Accept configs with more than one context descriptor
Hi Zenghui,
On 3/30/21 11:23 AM, Zenghui Yu wrote:
> Hi Eric,
>
> On 2021/2/24 4:56, Eric Auger wrote:
>> In preparation for vSVA, let's accept userspace provided configs
>> with more than one CD. We check the max CD against the host iommu
>> capability and also the format (linear versus 2 level).
>>
>> Signed-off-by: Eric Auger
>> Signed-off-by: Shameer Kolothum
>> ---
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 -
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index 332d31c0680f..ab74a0289893 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -3038,14 +3038,17 @@ static int arm_smmu_attach_pasid_table(struct
>> iommu_domain *domain,
>> if (smmu_domain->s1_cfg.set)
>> goto out;
>> - /*
>> - * we currently support a single CD so s1fmt and s1dss
>> - * fields are also ignored
>> - */
>> - if (cfg->pasid_bits)
>> + list_for_each_entry(master, _domain->devices,
>> domain_head) {
>> + if (cfg->pasid_bits > master->ssid_bits)
>> + goto out;
>> + }
>> + if (cfg->vendor_data.smmuv3.s1fmt ==
>> STRTAB_STE_0_S1FMT_64K_L2 &&
>> + !(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
>> goto out;
>> smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
>> + smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
>> + smmu_domain->s1_cfg.s1fmt = cfg->vendor_data.smmuv3.s1fmt;
>
> And what about the SIDSS field?
>
I added this patch upon Shameer's request, to be more vSVA friendly.
Hower this series does not really target multiple CD support. At the
moment the driver only supports STRTAB_STE_1_S1DSS_SSID0 (0x2) I think.
At this moment maybe I can only check the s1dss field is 0x2. Or simply
removes this patch?
Thoughts?
Eric
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v14 13/13] iommu/smmuv3: Accept configs with more than one context descriptor
Hi Eric,
On 2021/2/24 4:56, Eric Auger wrote:
In preparation for vSVA, let's accept userspace provided configs
with more than one CD. We check the max CD against the host iommu
capability and also the format (linear versus 2 level).
Signed-off-by: Eric Auger
Signed-off-by: Shameer Kolothum
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 332d31c0680f..ab74a0289893 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3038,14 +3038,17 @@ static int arm_smmu_attach_pasid_table(struct
iommu_domain *domain,
if (smmu_domain->s1_cfg.set)
goto out;
- /*
-* we currently support a single CD so s1fmt and s1dss
-* fields are also ignored
-*/
- if (cfg->pasid_bits)
+ list_for_each_entry(master, _domain->devices, domain_head)
{
+ if (cfg->pasid_bits > master->ssid_bits)
+ goto out;
+ }
+ if (cfg->vendor_data.smmuv3.s1fmt == STRTAB_STE_0_S1FMT_64K_L2
&&
+ !(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
goto out;
smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
+ smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
+ smmu_domain->s1_cfg.s1fmt = cfg->vendor_data.smmuv3.s1fmt;
And what about the SIDSS field?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v14 13/13] iommu/smmuv3: Accept configs with more than one context descriptor
In preparation for vSVA, let's accept userspace provided configs
with more than one CD. We check the max CD against the host iommu
capability and also the format (linear versus 2 level).
Signed-off-by: Eric Auger
Signed-off-by: Shameer Kolothum
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 332d31c0680f..ab74a0289893 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3038,14 +3038,17 @@ static int arm_smmu_attach_pasid_table(struct
iommu_domain *domain,
if (smmu_domain->s1_cfg.set)
goto out;
- /*
-* we currently support a single CD so s1fmt and s1dss
-* fields are also ignored
-*/
- if (cfg->pasid_bits)
+ list_for_each_entry(master, _domain->devices, domain_head)
{
+ if (cfg->pasid_bits > master->ssid_bits)
+ goto out;
+ }
+ if (cfg->vendor_data.smmuv3.s1fmt == STRTAB_STE_0_S1FMT_64K_L2
&&
+ !(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
goto out;
smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
+ smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
+ smmu_domain->s1_cfg.s1fmt = cfg->vendor_data.smmuv3.s1fmt;
smmu_domain->s1_cfg.set = true;
smmu_domain->abort = false;
break;
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
