linux-kernel - Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <de1a6b52-5e4f-1c0a-af3d-f6adb4b01daf@huawei.com>
Date:   Wed, 6 Sep 2017 09:16:38 +0800
From:   Yisheng Xie <xieyisheng1@...wei.com>
To:     Jean-Philippe Brucker <jean-philippe.brucker@....com>
CC:     <joro@...tes.org>, <robh+dt@...nel.org>, <mark.rutland@....com>,
        <lorenzo.pieralisi@....com>, <hanjun.guo@...aro.org>,
        <sudeep.holla@....com>, <rjw@...ysocki.net>, <lenb@...nel.org>,
        <will.deacon@....com>, <robin.murphy@....com>,
        <robert.moore@...el.com>, <lv.zheng@...el.com>,
        <iommu@...ts.linux-foundation.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-acpi@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devel@...ica.org>,
        <liubo95@...wei.com>, <chenjiankang1@...wei.com>,
        <xieyisheng@...wei.com>
Subject: Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

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
> 
> .
>