Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1724005 > unrolled thread

[RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Yisheng Xie <xieyisheng1@huawei.com> - 2017-08-31 10:40 +0200
    Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> - 2017-09-05 15:00 +0200
      Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Bob Liu <liubo95@huawei.com> - 2017-09-06 03:10 +0200
        Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> - 2017-09-06 12:00 +0200
          Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Bob Liu <liubo95@huawei.com> - 2017-09-07 03:50 +0200
            Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> - 2017-09-07 18:30 +0200
      Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Yisheng Xie <xieyisheng1@huawei.com> - 2017-09-06 03:20 +0200
        Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> - 2017-09-06 12:00 +0200
          Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Bob Liu <liubo95@huawei.com> - 2017-09-07 04:00 +0200
            Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> - 2017-09-07 18:30 +0200
    Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3 Hanjun Guo <hanjun.guo@linaro.org> - 2017-09-06 03:30 +0200

#1724005 — [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
https://www.spinics.net/lists/arm-kernel/msg565155.html

But for some platform devices(aka on-chip integrated devices), there is also
SVM requirement, which works based on the SMMU stall mode.
Jean-Philippe has prepared a prototype patchset to support it:
git://linux-arm.org/linux-jpb.git svm/stall

We tested this patchset with some fixes on a on-chip integrated device. The
basic function is ok, so I just send them out for review, although this
patchset heavily depends on the former patchset (PCIe SVM support for ARM
SMMUv3), which is still under discussion.

Patch Overview:
*1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
*4 is to realise the SVM function for platform device
*5 is fix a bug when test SVM function while SMMU donnot support this feature
*6 avoid ILLEGAL setting of STE and CD entry about stall

Acctually here, I also have a question about SVM on SMMUv3:

1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
   it will register a mmu_notify. Therefore, when a page range is invalid, we can
   send TLBI or ATC invalid without BTM?

2. According to ACPI IORT spec, named component specific data has a node flags field
   whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
   Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
   a single platform device which should be enough, because SMMU only support 20 bit pasid

3. Presently, the pasid is allocate for a task but not for a context, if a task is trying
   to bind to 2 device A and B:
     a) A support 5 pasid bits
     b) B support 2 pasid bits
     c) when the task bind to device A, it allocate pasid = 16
     d) then it must be fail when trying to bind to task B, for its highest pasid is 4.
   So it should allocate a single pasid for a context to avoid this?


Jean-Philippe Brucker (3):
  dt-bindings: document stall and PASID properties for IOMMU masters
  iommu/of: Add stall and pasid properties to iommu_fwspec
  iommu/arm-smmu-v3: Add SVM support for platform devices

Yisheng Xie (3):
  ACPI: IORT: Add stall and pasid properties to iommu_fwspec
  iommu/arm-smmu-v3: fix panic when handle stall mode irq
  iommu/arm-smmu-v3: Avoid ILLEGAL setting of STE.S1STALLD and CD.S

 Documentation/devicetree/bindings/iommu/iommu.txt |  13 ++
 drivers/acpi/arm64/iort.c                         |  20 ++
 drivers/iommu/arm-smmu-v3.c                       | 230 ++++++++++++++++++----
 drivers/iommu/of_iommu.c                          |  11 +
 include/acpi/actbl2.h                             |   5 +
 include/linux/iommu.h                             |   2 +
 6 files changed, 244 insertions(+), 37 deletions(-)

--
1.7.12.4

[toc] | [next] | [standalone]

#1726703

On 31/08/17 09:20, Yisheng Xie wrote:
> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
> https://www.spinics.net/lists/arm-kernel/msg565155.html
> 
> But for some platform devices(aka on-chip integrated devices), there is also
> SVM requirement, which works based on the SMMU stall mode.
> Jean-Philippe has prepared a prototype patchset to support it:
> git://linux-arm.org/linux-jpb.git svm/stall

Only meant for testing at that point, and unfit even for an RFC.

> We tested this patchset with some fixes on a on-chip integrated device. The
> basic function is ok, so I just send them out for review, although this
> patchset heavily depends on the former patchset (PCIe SVM support for ARM
> SMMUv3), which is still under discussion.
> 
> Patch Overview:
> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
> *4 is to realise the SVM function for platform device
> *5 is fix a bug when test SVM function while SMMU donnot support this feature
> *6 avoid ILLEGAL setting of STE and CD entry about stall
> 
> Acctually here, I also have a question about SVM on SMMUv3:
> 
> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>    send TLBI or ATC invalid without BTM?

We could, but the end goal for SVM is to perfectly mirror the CPU page
tables. So for platform SVM we would like to get rid of MMU notifiers
entirely.

> 2. According to ACPI IORT spec, named component specific data has a node flags field
>    whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
>    Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
>    a single platform device which should be enough, because SMMU only support 20 bit pasid
> 
> 3. Presently, the pasid is allocate for a task but not for a context, if a task is trying
>    to bind to 2 device A and B:
>      a) A support 5 pasid bits
>      b) B support 2 pasid bits
>      c) when the task bind to device A, it allocate pasid = 16
>      d) then it must be fail when trying to bind to task B, for its highest pasid is 4.
>    So it should allocate a single pasid for a context to avoid this?

Ideally yes, but the model chosen for the IOMMU API was one PASID per
task, so I implemented this model (the PASID allocator will be common to
IOMMU core in the future).

Therefore the PASID allocation will fail in your example, and there is no
way around it. If you do (d) then (c), the task will have PASID 4.

Thanks,
Jean

[toc] | [prev] | [next] | [standalone]

#1727099

On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
> On 31/08/17 09:20, Yisheng Xie wrote:
>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>
>> But for some platform devices(aka on-chip integrated devices), there is also
>> SVM requirement, which works based on the SMMU stall mode.
>> Jean-Philippe has prepared a prototype patchset to support it:
>> git://linux-arm.org/linux-jpb.git svm/stall
> 
> Only meant for testing at that point, and unfit even for an RFC.
> 

Sorry for the misunderstanding.
The PRI mode patches is in RFC even no hardware for testing, so I thought it's fine for "Stall mode" patches sent as RFC.
We have tested the Stall mode on our platform.
Anyway, I should confirm with you in advance.

Btw, Would you consider the "stall mode" upstream at first? Since there is no hardware for testing the PRI mode.
(We can provide you the hardware which support SMMU stall mode if necessary.)

>> We tested this patchset with some fixes on a on-chip integrated device. The
>> basic function is ok, so I just send them out for review, although this
>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>> SMMUv3), which is still under discussion.
>>
>> Patch Overview:
>> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
>> *4 is to realise the SVM function for platform device
>> *5 is fix a bug when test SVM function while SMMU donnot support this feature
>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>
>> Acctually here, I also have a question about SVM on SMMUv3:
>>
>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>>    send TLBI or ATC invalid without BTM?
> 
> We could, but the end goal for SVM is to perfectly mirror the CPU page
> tables. So for platform SVM we would like to get rid of MMU notifiers
> entirely.
> 
>> 2. According to ACPI IORT spec, named component specific data has a node flags field
>>    whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
>>    Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
>>    a single platform device which should be enough, because SMMU only support 20 bit pasid
>>

Any comment on this?
The ACPI IORT spec may need be updated?

Regards,
Liubo

>> 3. Presently, the pasid is allocate for a task but not for a context, if a task is trying
>>    to bind to 2 device A and B:
>>      a) A support 5 pasid bits
>>      b) B support 2 pasid bits
>>      c) when the task bind to device A, it allocate pasid = 16
>>      d) then it must be fail when trying to bind to task B, for its highest pasid is 4.
>>    So it should allocate a single pasid for a context to avoid this?
> 
> Ideally yes, but the model chosen for the IOMMU API was one PASID per
> task, so I implemented this model (the PASID allocator will be common to
> IOMMU core in the future).
> 
> Therefore the PASID allocation will fail in your example, and there is no
> way around it. If you do (d) then (c), the task will have PASID 4.
> 
> Thanks,
> Jean
> 
> .
> 

[toc] | [prev] | [next] | [standalone]

#1727309

On 06/09/17 02:02, Bob Liu wrote:
> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>> On 31/08/17 09:20, Yisheng Xie wrote:
>>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>>
>>> But for some platform devices(aka on-chip integrated devices), there is also
>>> SVM requirement, which works based on the SMMU stall mode.
>>> Jean-Philippe has prepared a prototype patchset to support it:
>>> git://linux-arm.org/linux-jpb.git svm/stall
>>
>> Only meant for testing at that point, and unfit even for an RFC.
>>
> 
> Sorry for the misunderstanding.
> The PRI mode patches is in RFC even no hardware for testing, so I thought it's fine for "Stall mode" patches sent as RFC.
> We have tested the Stall mode on our platform.
> Anyway, I should confirm with you in advance.
> 
> Btw, Would you consider the "stall mode" upstream at first? Since there is no hardware for testing the PRI mode.
> (We can provide you the hardware which support SMMU stall mode if necessary.)

Yes. What's blocking the ATS, PRI and PASID patches at the moment is the
lack of endpoints for testing. There has been lots of discussion on the
API side since my first RFC and I'd like to resubmit the API changes soon.
It is the same API for ATS+PRI+PASID and SSID+Stall, so the backend
doesn't matter.

I'm considering upstreaming SSID+Stall first if it can be tested on
hardware (having direct access to it would certainly speed things up).
This would require some work in moving the PCI bits at the end of the
series. I can reserve some time in the coming months to do it, but I need
to know what to focus on. Are you able to test SSID as well?

>>> We tested this patchset with some fixes on a on-chip integrated device. The
>>> basic function is ok, so I just send them out for review, although this
>>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>>> SMMUv3), which is still under discussion.
>>>
>>> Patch Overview:
>>> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
>>> *4 is to realise the SVM function for platform device
>>> *5 is fix a bug when test SVM function while SMMU donnot support this feature
>>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>>
>>> Acctually here, I also have a question about SVM on SMMUv3:
>>>
>>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>>>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>>>    send TLBI or ATC invalid without BTM?
>>
>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>> tables. So for platform SVM we would like to get rid of MMU notifiers
>> entirely.
>>
>>> 2. According to ACPI IORT spec, named component specific data has a node flags field
>>>    whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
>>>    Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
>>>    a single platform device which should be enough, because SMMU only support 20 bit pasid
>>>
> 
> Any comment on this?
> The ACPI IORT spec may need be updated?

I suppose that the Named Component Node could be used for SSID and stall
capability bits. Can't the ACPI namespace entries be extended to host
these capabilities in a more generic way? Platforms with different IOMMUs
might also need this information some day.

Thanks,
Jean

[toc] | [prev] | [next] | [standalone]

#1727866

On 2017/9/6 17:57, Jean-Philippe Brucker wrote:
> On 06/09/17 02:02, Bob Liu wrote:
>> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>>> On 31/08/17 09:20, Yisheng Xie wrote:
>>>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>>>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>>>
>>>> But for some platform devices(aka on-chip integrated devices), there is also
>>>> SVM requirement, which works based on the SMMU stall mode.
>>>> Jean-Philippe has prepared a prototype patchset to support it:
>>>> git://linux-arm.org/linux-jpb.git svm/stall
>>>
>>> Only meant for testing at that point, and unfit even for an RFC.
>>>
>>
>> Sorry for the misunderstanding.
>> The PRI mode patches is in RFC even no hardware for testing, so I thought it's fine for "Stall mode" patches sent as RFC.
>> We have tested the Stall mode on our platform.
>> Anyway, I should confirm with you in advance.
>>
>> Btw, Would you consider the "stall mode" upstream at first? Since there is no hardware for testing the PRI mode.
>> (We can provide you the hardware which support SMMU stall mode if necessary.)
> 
> Yes. What's blocking the ATS, PRI and PASID patches at the moment is the
> lack of endpoints for testing. There has been lots of discussion on the
> API side since my first RFC and I'd like to resubmit the API changes soon.
> It is the same API for ATS+PRI+PASID and SSID+Stall, so the backend
> doesn't matter.
> 

Indeed!

> I'm considering upstreaming SSID+Stall first if it can be tested on
> hardware (having direct access to it would certainly speed things up).

Glad to hear that.

> This would require some work in moving the PCI bits at the end of the
> series. I can reserve some time in the coming months to do it, but I need
> to know what to focus on. Are you able to test SSID as well?
> 

Yes, but the difficulty is our devices are on-chip integrated hardware accelerators which requires complicate driver.
You may need much time to understand the driver.
That's the same case as intel/amd SVM, the current user is their GPU :-(

Btw, what kind of device/method do you think is ideal for testing arm-SVM?

>>>> We tested this patchset with some fixes on a on-chip integrated device. The
>>>> basic function is ok, so I just send them out for review, although this
>>>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>>>> SMMUv3), which is still under discussion.
>>>>
>>>> Patch Overview:
>>>> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
>>>> *4 is to realise the SVM function for platform device
>>>> *5 is fix a bug when test SVM function while SMMU donnot support this feature
>>>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>>>
>>>> Acctually here, I also have a question about SVM on SMMUv3:
>>>>
>>>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>>>>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>>>>    send TLBI or ATC invalid without BTM?
>>>
>>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>>> tables. So for platform SVM we would like to get rid of MMU notifiers
>>> entirely.
>>>
>>>> 2. According to ACPI IORT spec, named component specific data has a node flags field
>>>>    whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
>>>>    Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
>>>>    a single platform device which should be enough, because SMMU only support 20 bit pasid
>>>>
>>
>> Any comment on this?
>> The ACPI IORT spec may need be updated?
> 
> I suppose that the Named Component Node could be used for SSID and stall
> capability bits. Can't the ACPI namespace entries be extended to host
> these capabilities in a more generic way? Platforms with different IOMMUs
> might also need this information some day.
> 

Hmm, that would be better.
But in anyway, it depends on the ACPI IORT Spec would be extended in next version.

--
Thanks,
Bob Liu

[toc] | [prev] | [next] | [standalone]

#1728299

On 07/09/17 02:41, Bob Liu wrote:
>> This would require some work in moving the PCI bits at the end of the
>> series. I can reserve some time in the coming months to do it, but I need
>> to know what to focus on. Are you able to test SSID as well?
>>
> 
> Yes, but the difficulty is our devices are on-chip integrated hardware accelerators which requires complicate driver.
> You may need much time to understand the driver.
> That's the same case as intel/amd SVM, the current user is their GPU :-(
> 
> Btw, what kind of device/method do you think is ideal for testing arm-SVM?

A simple, bare DMA engine would be ideal. Something just capable of
performing memcpy with parameters (PASID, input IOVA, output IOVA, size)
can be used for validating SVM and virtualization. You could easily create
reproducible unit tests and userspace drivers. If it supports isolated
channels (as in SR-IOV), even better.

As you said, having a useful device like a full GPU/accelerator as opposed
to a dummy validation engine makes it difficult to fully test the SMMU.
However it can be helpful for evaluating driver performances and is still
good enough for confirming that the IOMMU works.

Thanks,
Jean

[toc] | [prev] | [next] | [standalone]

#1727104

Hi Jean-Philippe,

On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
> On 31/08/17 09:20, Yisheng Xie wrote:
>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>
>> But for some platform devices(aka on-chip integrated devices), there is also
>> SVM requirement, which works based on the SMMU stall mode.
>> Jean-Philippe has prepared a prototype patchset to support it:
>> git://linux-arm.org/linux-jpb.git svm/stall
> 
> Only meant for testing at that point, and unfit even for an RFC.

Sorry about that, I should ask you before send it out. It's my mistake. For I also
have some question about this patchset.

We have related device, and would like to do some help about it. Do you have
any plan about upstream ?

> 
>> We tested this patchset with some fixes on a on-chip integrated device. The
>> basic function is ok, so I just send them out for review, although this
>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>> SMMUv3), which is still under discussion.
>>
>> Patch Overview:
>> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
>> *4 is to realise the SVM function for platform device
>> *5 is fix a bug when test SVM function while SMMU donnot support this feature
>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>
>> Acctually here, I also have some questions about SVM on SMMUv3:
>>
>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>>    send TLBI or ATC invalid without BTM?
> 
> We could, but the end goal for SVM is to perfectly mirror the CPU page
> tables. So for platform SVM we would like to get rid of MMU notifiers
> entirely.

I see, but for some SMMU which do not support BTM, it cannot benefit from SVM.

Meanwhile, do you mean even with BTM feature, the PCI-e device also need to send a
ATC invalid by MMU notify? It seems not fair, why not hardware do the entirely work
in this case? It may costly for send ATC invalid and sync.

> 
>> 2. According to ACPI IORT spec, named component specific data has a node flags field
>>    whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
>>    Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
>>    a single platform device which should be enough, because SMMU only support 20 bit pasid
>>
>> 3. Presently, the pasid is allocate for a task but not for a context, if a task is trying
>>    to bind to 2 device A and B:
>>      a) A support 5 pasid bits
>>      b) B support 2 pasid bits
>>      c) when the task bind to device A, it allocate pasid = 16
>>      d) then it must be fail when trying to bind to task B, for its highest pasid is 4.
>>    So it should allocate a single pasid for a context to avoid this?
> 
> Ideally yes, but the model chosen for the IOMMU API was one PASID per
> task, so I implemented this model (the PASID allocator will be common to
> IOMMU core in the future).
It is fair, for each IOMMU need PASID allocator to support SVM.

Thanks
Yisheng Xie

> 
> Therefore the PASID allocation will fail in your example, and there is no
> way around it. If you do (d) then (c), the task will have PASID 4.
> 
> Thanks,
> Jean
> 
> .
> 

[toc] | [prev] | [next] | [standalone]

#1727308

On 06/09/17 02:16, Yisheng Xie wrote:
> Hi Jean-Philippe,
> 
> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>> On 31/08/17 09:20, Yisheng Xie wrote:
>>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>>
>>> But for some platform devices(aka on-chip integrated devices), there is also
>>> SVM requirement, which works based on the SMMU stall mode.
>>> Jean-Philippe has prepared a prototype patchset to support it:
>>> git://linux-arm.org/linux-jpb.git svm/stall
>>
>> Only meant for testing at that point, and unfit even for an RFC.
> 
> Sorry about that, I should ask you before send it out. It's my mistake. For I also
> have some question about this patchset.
> 
> We have related device, and would like to do some help about it. Do you have
> any plan about upstream ?
> 
>>
>>> We tested this patchset with some fixes on a on-chip integrated device. The
>>> basic function is ok, so I just send them out for review, although this
>>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>>> SMMUv3), which is still under discussion.
>>>
>>> Patch Overview:
>>> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
>>> *4 is to realise the SVM function for platform device
>>> *5 is fix a bug when test SVM function while SMMU donnot support this feature
>>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>>
>>> Acctually here, I also have some questions about SVM on SMMUv3:
>>>
>>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>>>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>>>    send TLBI or ATC invalid without BTM?
>>
>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>> tables. So for platform SVM we would like to get rid of MMU notifiers
>> entirely.
> 
> I see, but for some SMMU which do not support BTM, it cannot benefit from SVM.
> 
> Meanwhile, do you mean even with BTM feature, the PCI-e device also need to send a
> ATC invalid by MMU notify? It seems not fair, why not hardware do the entirely work
> in this case? It may costly for send ATC invalid and sync.

It will certainly be costly. But there are major problems with
transforming broadcast TLB maintenance into ATC invalidations in HW:

* VMID:ASID to SID:SSID conversion. TLBIs use VMID:ASID, while ATCIs use
SID:SSID.

* Most importantly, ATC invalidations accounting. Each endpoint has a
limited number of in-flight ATC invalidate requests. The conversion module
would have to buffer incoming invalidations and wait for in-flight ATC
invalidation to complete before sending the next ones. In case of
overflow, either we lose invalidation (which opens security holes) or we
somehow put back-pressure on the interconnect (no idea how feasible this
is, I suspect really hard).

Solving the last one is also quite difficult in software, but at least we
can still invalidate a range. In hardware we would invalidate the ATC
page-by-page and quickly jam the bus.

Thanks,
Jean

[toc] | [prev] | [next] | [standalone]

#1727868

On 2017/9/6 17:59, Jean-Philippe Brucker wrote:
> On 06/09/17 02:16, Yisheng Xie wrote:
>> Hi Jean-Philippe,
>>
>> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>>> On 31/08/17 09:20, Yisheng Xie wrote:
>>>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>>>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>>>
>>>> But for some platform devices(aka on-chip integrated devices), there is also
>>>> SVM requirement, which works based on the SMMU stall mode.
>>>> Jean-Philippe has prepared a prototype patchset to support it:
>>>> git://linux-arm.org/linux-jpb.git svm/stall
>>>
>>> Only meant for testing at that point, and unfit even for an RFC.
>>
>> Sorry about that, I should ask you before send it out. It's my mistake. For I also
>> have some question about this patchset.
>>
>> We have related device, and would like to do some help about it. Do you have
>> any plan about upstream ?
>>
>>>
>>>> We tested this patchset with some fixes on a on-chip integrated device. The
>>>> basic function is ok, so I just send them out for review, although this
>>>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>>>> SMMUv3), which is still under discussion.
>>>>
>>>> Patch Overview:
>>>> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
>>>> *4 is to realise the SVM function for platform device
>>>> *5 is fix a bug when test SVM function while SMMU donnot support this feature
>>>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>>>
>>>> Acctually here, I also have some questions about SVM on SMMUv3:
>>>>
>>>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>>>>    it will register a mmu_notify. Therefore, when a page range is invalid, we can
>>>>    send TLBI or ATC invalid without BTM?
>>>
>>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>>> tables. So for platform SVM we would like to get rid of MMU notifiers
>>> entirely.
>>
>> I see, but for some SMMU which do not support BTM, it cannot benefit from SVM.
>>
>> Meanwhile, do you mean even with BTM feature, the PCI-e device also need to send a
>> ATC invalid by MMU notify? It seems not fair, why not hardware do the entirely work
>> in this case? It may costly for send ATC invalid and sync.
> 
> It will certainly be costly. But there are major problems with
> transforming broadcast TLB maintenance into ATC invalidations in HW:
> 
> * VMID:ASID to SID:SSID conversion. TLBIs use VMID:ASID, while ATCIs use
> SID:SSID.
> 
> * Most importantly, ATC invalidations accounting. Each endpoint has a
> limited number of in-flight ATC invalidate requests. The conversion module
> would have to buffer incoming invalidations and wait for in-flight ATC
> invalidation to complete before sending the next ones. In case of
> overflow, either we lose invalidation (which opens security holes) or we
> somehow put back-pressure on the interconnect (no idea how feasible this
> is, I suspect really hard).
> 
> Solving the last one is also quite difficult in software, but at least we
> can still invalidate a range. In hardware we would invalidate the ATC
> page-by-page and quickly jam the bus.
> 

Speak to the invalidation, I have one more question.

There is a time window between 1) modify page table;  2) tlb invalidate;

ARM-CPU                           Device

1. modify page table

                             ^^^^^
                              Can still write data through smmu tlb even page table was already modified.
                              (At this point, the same virtual addr may not point to the same thing for CPU and device!!!
                               I'm afraid there may be some data-loss or other potential problems if this situation happens.)

2. tlb invalidate range

--
Thanks,
Bob

[toc] | [prev] | [next] | [standalone]

#1728298

On 07/09/17 02:55, Bob Liu wrote:
> Speak to the invalidation, I have one more question.
> 
> There is a time window between 1) modify page table;  2) tlb invalidate;
> 
> ARM-CPU                           Device
> 
> 1. modify page table
> 
>                              ^^^^^
>                               Can still write data through smmu tlb even page table was already modified.
>                               (At this point, the same virtual addr may not point to the same thing for CPU and device!!!
>                                I'm afraid there may be some data-loss or other potential problems if this situation happens.)
> 
> 2. tlb invalidate range

The mm code serializes map/unmap operations with mm->mmap_sem, and at a
lower level I think the pte lock is used to prevent more subtle races.
Don't take my word for it though, mm/ is still very obscure to me. So the
kernel shouldn't be able to reuse the VA for something else before the tlb
invalidation completes. Even if you're using the CMDQ to invalidate
instead of TLBI instructions, you're still called by a notifier from the
mm code so there is no problem.

Thanks,
Jean

[toc] | [prev] | [next] | [standalone]

#1727108

On 2017/8/31 16:20, Yisheng Xie wrote:
> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
> https://www.spinics.net/lists/arm-kernel/msg565155.html
> 
> But for some platform devices(aka on-chip integrated devices), there is also
> SVM requirement, which works based on the SMMU stall mode.
> Jean-Philippe has prepared a prototype patchset to support it:
> git://linux-arm.org/linux-jpb.git svm/stall
> 
> We tested this patchset with some fixes on a on-chip integrated device. The
> basic function is ok, so I just send them out for review, although this
> patchset heavily depends on the former patchset (PCIe SVM support for ARM
> SMMUv3), which is still under discussion.
> 
> Patch Overview:
> *1 to 3 prepare for device tree or acpi get the device stall ability and pasid bits
> *4 is to realise the SVM function for platform device
> *5 is fix a bug when test SVM function while SMMU donnot support this feature
> *6 avoid ILLEGAL setting of STE and CD entry about stall
> 
> Acctually here, I also have a question about SVM on SMMUv3:
> 
> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task to device,
>     it will register a mmu_notify. Therefore, when a page range is invalid, we can
>     send TLBI or ATC invalid without BTM?
> 
> 2. According to ACPI IORT spec, named component specific data has a node flags field
>     whoes bit0 is for Stall support. However, it do not have any field for pasid bit.
>     Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 pasid bit for
>     a single platform device which should be enough, because SMMU only support 20 bit pasid

I think we can propose something similar, it's a missing function in
IORT.

Thanks
Hanjun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web