Hi Jean-Philippe,

On 2017/9/5 20:56, Jean-Philippe Brucker wrote:

On 31/08/17 09:20, Yisheng Xie wrote:

quoted

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 ?

quoted

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.

quoted

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

.