Re: [PATCH v2 0/8] MT2712 IOMMU SUPPORT

[Yong Wu]

On Tue, 2017-08-22 at 22:38 +0800, Joerg Roedel wrote:
> On Mon, Aug 21, 2017 at 07:00:13PM +0800, Yong Wu wrote:
> > Yong Wu (8):
> >   dt-bindings: mediatek: Add binding for mt2712 IOMMU and SMI
> >   iommu/mediatek: Move MTK_M4U_TO_LARB/PORT into mtk_iommu.c
> >   iommu/mediatek: Add mt2712 IOMMU support
> >   iommu/mediatek: Merge 2 M4U HWs into one iommu domain
> >   iommu/mediatek: Move pgtable allocation into domain_alloc
> >   iommu/mediatek: Disable iommu clock when system suspend
> >   iommu/mediatek: Enlarge the validate PA range for 4GB mode
> >   memory: mtk-smi: Degrade SMI init to module_init
> 
> Applied patches 2-7. Patch 1 is something for Matthias.

Hi Matthias,

   Could you help review the patch[1/8]? If it's ok for you too,Could
you help apply that one via your tree.
   Thanks.


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

Re: [RFC PATCH 6/6] iommu/arm-smmu-v3: Avoid ILLEGAL setting of STE.S1STALLD and CD.S

[Yisheng Xie]

Hi Jean-Philippe,

On 2017/9/5 20:54, Jean-Philippe Brucker wrote:
> On 31/08/17 09:20, Yisheng Xie wrote:
>> It is ILLEGAL to set STE.S1STALLD if STALL_MODEL is not 0b00, which
>> means we should not disable stall mode if stall/terminate mode is not
>> configuable.
>>
>> Meanwhile, it is also ILLEGAL when STALL_MODEL==0b10 && CD.S==0 which
>> means if stall mode is force we should always set CD.S.
>>
>> This patch add ARM_SMMU_FEAT_TERMINATE feature bit for smmu, and use
>> TERMINATE feature checking to ensue above ILLEGAL cases from happening.
>>
>> Signed-off-by: Yisheng Xie 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 22 --
>>  1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index dbda2eb..0745522 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -55,6 +55,7 @@
>>  #define IDR0_STALL_MODEL_SHIFT  24
>>  #define IDR0_STALL_MODEL_MASK   0x3
>>  #define IDR0_STALL_MODEL_STALL  (0 << IDR0_STALL_MODEL_SHIFT)
>> +#define IDR0_STALL_MODEL_NS (1 << IDR0_STALL_MODEL_SHIFT)
>>  #define IDR0_STALL_MODEL_FORCE  (2 << IDR0_STALL_MODEL_SHIFT)
>>  #define IDR0_TTENDIAN_SHIFT 21
>>  #define IDR0_TTENDIAN_MASK  0x3
>> @@ -766,6 +767,7 @@ struct arm_smmu_device {
>>  #define ARM_SMMU_FEAT_SVM   (1 << 15)
>>  #define ARM_SMMU_FEAT_HA(1 << 16)
>>  #define ARM_SMMU_FEAT_HD(1 << 17)
>> +#define ARM_SMMU_FEAT_TERMINATE (1 << 18)
> 
> I'd rather introduce something like "ARM_SMMU_FEAT_STALL_FORCE" instead.
> Terminate model has another meaning, and is defined by a different bit in
> IDR0.

Ok, sound more reasonable.

Thanks
Yisheng Xie

> 
> Thanks,
> Jean
> 
> .
> 

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

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

[Hanjun Guo]

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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH 4/6] iommu/arm-smmu-v3: Add SVM support for platform devices

[Yisheng Xie]

Hi Jean-Philippe,

On 2017/9/6 8:51, Bob Liu wrote:
> On 2017/9/5 20:53, Jean-Philippe Brucker wrote:
>> On 31/08/17 09:20, Yisheng Xie wrote:
>>> From: Jean-Philippe Brucker 
>>>
>>> Platform device can realise SVM function by using the stall mode. That
>>> is to say, when device access a memory via iova which is not populated,
>>> it will stalled and when SMMU try to translate this iova, it also will
>>> stall and meanwhile send an event to CPU via MSI.
>>>
>>> After SMMU driver handle the event and populated the iova, it will send
>>> a RESUME command to SMMU to exit the stall mode, therefore the platform
>>> device can contiue access the memory.
>>>
>>> Signed-off-by: Jean-Philippe Brucker 
>>
>> No. Please don't forge a signed-off-by under a commit message you wrote,

Sorry about that, it is my mistake.

> 
> Really sorry for that.
> We sent out the wrong version, I should take more careful review.
> 
> Regards,
> Liubo
> 
>> it's rude. I didn't sign it, didn't consider it fit for mainline or even
>> as an RFC, and wanted to have another read before sending. My mistake,
>> I'll think twice before sharing prototypes in the future.
>>
>> Thanks,
>> Jean
>>
> 
> 
> 
> 
> .
> 

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

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

[Yisheng Xie]

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

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

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

[Bob Liu]

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


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

Re: [RFC PATCH 4/6] iommu/arm-smmu-v3: Add SVM support for platform devices

[Bob Liu]

On 2017/9/5 20:53, Jean-Philippe Brucker wrote:
> On 31/08/17 09:20, Yisheng Xie wrote:
>> From: Jean-Philippe Brucker 
>>
>> Platform device can realise SVM function by using the stall mode. That
>> is to say, when device access a memory via iova which is not populated,
>> it will stalled and when SMMU try to translate this iova, it also will
>> stall and meanwhile send an event to CPU via MSI.
>>
>> After SMMU driver handle the event and populated the iova, it will send
>> a RESUME command to SMMU to exit the stall mode, therefore the platform
>> device can contiue access the memory.
>>
>> Signed-off-by: Jean-Philippe Brucker 
> 
> No. Please don't forge a signed-off-by under a commit message you wrote,

Really sorry for that.
We sent out the wrong version, I should take more careful review.

Regards,
Liubo

> it's rude. I didn't sign it, didn't consider it fit for mainline or even
> as an RFC, and wanted to have another read before sending. My mistake,
> I'll think twice before sharing prototypes in the future.
> 
> Thanks,
> Jean
> 



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

[PATCH v2 0/2] Dual MMU support for TI DRA7xx DSPs

[Suman Anna via iommu]

Hi Joerg,

The following is v2 of the patch series that enhances the OMAP IOMMU driver
to support the mirror-programming of the two MMUs present within the DSP
subsystems on TI DRA7xx/AM57xx family of SoCs.

The main changes are to address the review comments on the registration of
the MMU devices with the IOMMU core. I have added logic in omap_iommu_probe
function to not register the second MMU device with the IOMMU core. There is
also only one iommu_group now and the iommu sysfs entries do not show the second
core anymore. Example output log given below.

I have chosen to make the modifications in the probe rather than the .add_device
callback as this is much simpler. It eliminates the need for additional logic 
for
refcounting (need the iommu_group allocation only for the first client) and 
moving
the iommu_group allocation out of probe. This also ensures the IOMMU device and
group registration is done during probe itself irrespective of whether there are
client nodes defined or not in the DTS (for the .add_device callbacks to be
effective).

Patches still based on your 4.13 arm/omap branch. DT representation and usage
details are the same as described in v1 [1].

regards
Suman

[1] https://marc.info/?l=linux-omap=150418532431811=2

Following is the output of sysfs with these MMUs exercised
with additional patches now:

root@am57xx-evm:~# dmesg | grep mmu
[0.520328] omap-iommu 40d01000.mmu: 40d01000.mmu registered
[0.520676] omap-iommu 40d02000.mmu: 40d02000.mmu registered
[0.521148] omap-iommu 58882000.mmu: 58882000.mmu registered
[0.521638] omap-iommu 55082000.mmu: 55082000.mmu registered
[0.56] omap-iommu 41501000.mmu: 41501000.mmu registered
[0.522575] omap-iommu 41502000.mmu: 41502000.mmu registered
[0.523040] iommu: Adding device 5882.ipu to group 1
[0.523192] iommu: Adding device 5502.ipu to group 2
[0.523444] iommu: Adding device 4080.dsp to group 0
[0.523841] iommu: Adding device 4100.dsp to group 3
root@am57xx-evm:~# ls -l /sys/class/iommu/
lrwxrwxrwx1 root root 0 Aug  8 03:59 40d01000.mmu -> 
../../devices/platform/4400.ocp/40d01000.mmu/iommu/40d01000.mmu
lrwxrwxrwx1 root root 0 Aug  8 03:59 41501000.mmu -> 
../../devices/platform/4400.ocp/41501000.mmu/iommu/41501000.mmu
lrwxrwxrwx1 root root 0 Aug  8 03:59 55082000.mmu -> 
../../devices/platform/4400.ocp/55082000.mmu/iommu/55082000.mmu
lrwxrwxrwx1 root root 0 Aug  8 03:59 58882000.mmu -> 
../../devices/platform/4400.ocp/58882000.mmu/iommu/58882000.mmu
root@am57xx-evm:~# ls -l /sys/kernel/iommu_groups/
drwxr-xr-x3 root root 0 Aug  8 03:59 0
drwxr-xr-x3 root root 0 Aug  8 03:59 1
drwxr-xr-x3 root root 0 Aug  8 03:59 2
drwxr-xr-x3 root root 0 Aug  8 03:59 3
root@am57xx-evm:~# ls -l /sys/kernel/iommu_groups/*/devices
/sys/kernel/iommu_groups/0/devices:
lrwxrwxrwx1 root root 0 Aug  8 03:59 4080.dsp -> 
../../../../devices/platform/4400.ocp/4080.dsp

/sys/kernel/iommu_groups/1/devices:
lrwxrwxrwx1 root root 0 Aug  8 03:59 5882.ipu -> 
../../../../devices/platform/4400.ocp/5882.ipu

/sys/kernel/iommu_groups/2/devices:
lrwxrwxrwx1 root root 0 Aug  8 03:59 5502.ipu -> 
../../../../devices/platform/4400.ocp/5502.ipu

/sys/kernel/iommu_groups/3/devices:
lrwxrwxrwx1 root root 0 Aug  8 03:59 4100.dsp -> 
../../../../devices/platform/4400.ocp/4100.dsp
root@am57xx-evm:~# ls -l /sys/kernel/iommu_groups/*/devices/*/  


/sys/kernel/iommu_groups/0/devices/4080.dsp/:
lrwxrwxrwx1 root root 0 Aug  8 04:04 driver -> 
../../../../bus/platform/drivers/omap-rproc
-rw-r--r--1 root root  4096 Aug  8 03:59 driver_override
lrwxrwxrwx1 root root 0 Aug  8 03:59 iommu -> 
../40d01000.mmu/iommu/40d01000.mmu
lrwxrwxrwx1 root root 0 Aug  8 03:59 iommu_group -> 
../../../../kernel/iommu_groups/0
-r--r--r--1 root root  4096 Aug  8 03:59 modalias
lrwxrwxrwx1 root root 0 Aug  8 03:59 of_node -> 
../../../../firmware/devicetree/base/ocp/dsp@4080
drwxr-xr-x2 root root 0 Aug  8 03:59 power
drwxr-xr-x3 root root 0 Aug  8 04:04 remoteproc
lrwxrwxrwx1 root root 0 Aug  8 03:59 subsystem -> 
../../../../bus/platform
-rw-r--r--1 root root  4096 Aug  8 03:59 uevent

/sys/kernel/iommu_groups/1/devices/5882.ipu/:
lrwxrwxrwx1 root root 0 Aug  8 04:04 driver -> 
../../../../bus/platform/drivers/omap-rproc
-rw-r--r--1 root root  4096 Aug  8 03:59 driver_override
lrwxrwxrwx1 

[PATCH v2 1/2] iommu/omap: Change the attach detection logic

[Suman Anna via iommu]

The OMAP IOMMU driver allows only a single device (eg: a rproc
device) to be attached per domain. The current attach detection
logic relies on a check for an attached iommu for the respective
client device. Change this logic to use the client device pointer
instead in preparation for supporting multiple iommu devices to be
bound to a single iommu domain, and thereby to a client device.

Signed-off-by: Suman Anna 
---
v2: No changes

 drivers/iommu/omap-iommu.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bd67e1b2c64e..81ef729994ce 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -805,7 +805,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
struct iommu_domain *domain = obj->domain;
struct omap_iommu_domain *omap_domain = to_omap_domain(domain);
 
-   if (!omap_domain->iommu_dev)
+   if (!omap_domain->dev)
return IRQ_NONE;
 
errs = iommu_report_fault(obj, );
@@ -1118,8 +1118,8 @@ omap_iommu_attach_dev(struct iommu_domain *domain, struct 
device *dev)
 
spin_lock(_domain->lock);
 
-   /* only a single device is supported per domain for now */
-   if (omap_domain->iommu_dev) {
+   /* only a single client device can be attached to a domain */
+   if (omap_domain->dev) {
dev_err(dev, "iommu domain is already attached\n");
ret = -EBUSY;
goto out;
@@ -1148,9 +1148,14 @@ static void _omap_iommu_detach_dev(struct 
omap_iommu_domain *omap_domain,
 {
struct omap_iommu *oiommu = dev_to_omap_iommu(dev);
 
+   if (!omap_domain->dev) {
+   dev_err(dev, "domain has no attached device\n");
+   return;
+   }
+
/* only a single device is supported per domain for now */
-   if (omap_domain->iommu_dev != oiommu) {
-   dev_err(dev, "invalid iommu device\n");
+   if (omap_domain->dev != dev) {
+   dev_err(dev, "invalid attached device\n");
return;
}
 
@@ -1219,7 +1224,7 @@ static void omap_iommu_domain_free(struct iommu_domain 
*domain)
 * An iommu device is still attached
 * (currently, only one device can be attached) ?
 */
-   if (omap_domain->iommu_dev)
+   if (omap_domain->dev)
_omap_iommu_detach_dev(omap_domain, omap_domain->dev);
 
kfree(omap_domain->pgtable);
-- 
2.13.1

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

[PATCH v2 2/2] iommu/omap: Add support to program multiple iommus

[Suman Anna via iommu]

A client user instantiates and attaches to an iommu_domain to
program the OMAP IOMMU associated with the domain. The iommus
programmed by a client user are bound with the iommu_domain
through the user's device archdata. The OMAP IOMMU driver
currently supports only one IOMMU per IOMMU domain per user.

The OMAP IOMMU driver has been enhanced to support allowing
multiple IOMMUs to be programmed by a single client user. This
support is being added mainly to handle the DSP subsystems on
the DRA7xx SoCs, which have two MMUs within the same subsystem.
These MMUs provide translations for a processor core port and
an internal EDMA port. This support allows both the MMUs to
be programmed together, but with each one retaining it's own
internal state objects. The internal EDMA block is managed by
the software running on the DSPs, and this design provides
on-par functionality with previous generation OMAP DSPs where
the EDMA and the DSP core shared the same MMU.

The multiple iommus are expected to be provided through a
sentinel terminated array of omap_iommu_arch_data objects
through the client user's device archdata. The OMAP driver
core is enhanced to loop through the array of attached
iommus and program them for all common operations. The
sentinel-terminated logic is used so as to not change the
omap_iommu_arch_data structure.

NOTE:
1. The IOMMU group and IOMMU core registration is done only for
   the DSP processor core MMU even though both MMUs are represented
   by their own platform device and are probed individually. The
   IOMMU device linking uses this registered MMU device. The struct
   iommu_device for the second MMU is not used even though memory
   for it is allocated.
2. The OMAP IOMMU debugfs code still continues to operate on
   individual IOMMU objects.

Signed-off-by: Suman Anna 
[t-kri...@ti.com: ported support to 4.13 based kernel]
Signed-off-by: Tero Kristo 
---
v2:
 - Added a new helper omap_iommu_can_register() function which is
   used in probe to selectively register the probed IOMMU platform
   devices with the IOMMU core.
 - The driver remove function is also adjusted accordingly for the
   iommu_group removal and unregistration
 - Revised the patch description with NOTE #1 rewritten.
 - All other changes are same as v1
v1: https://patchwork.kernel.org/patch/9932177/

 drivers/iommu/omap-iommu.c | 358 ++---
 drivers/iommu/omap-iommu.h |  30 ++--
 2 files changed, 285 insertions(+), 103 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 81ef729994ce..e135ab830ebf 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -2,6 +2,7 @@
  * omap iommu: tlb and pagetable primitives
  *
  * Copyright (C) 2008-2010 Nokia Corporation
+ * Copyright (C) 2013-2017 Texas Instruments Incorporated - http://www.ti.com/
  *
  * Written by Hiroshi DOYU ,
  * Paul Mundt and Toshihiro Kobayashi
@@ -71,13 +72,23 @@ static struct omap_iommu_domain *to_omap_domain(struct 
iommu_domain *dom)
  **/
 void omap_iommu_save_ctx(struct device *dev)
 {
-   struct omap_iommu *obj = dev_to_omap_iommu(dev);
-   u32 *p = obj->ctx;
+   struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
+   struct omap_iommu *obj;
+   u32 *p;
int i;
 
-   for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
-   p[i] = iommu_read_reg(obj, i * sizeof(u32));
-   dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]);
+   if (!arch_data)
+   return;
+
+   while (arch_data->iommu_dev) {
+   obj = arch_data->iommu_dev;
+   p = obj->ctx;
+   for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
+   p[i] = iommu_read_reg(obj, i * sizeof(u32));
+   dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i,
+   p[i]);
+   }
+   arch_data++;
}
 }
 EXPORT_SYMBOL_GPL(omap_iommu_save_ctx);
@@ -88,13 +99,23 @@ EXPORT_SYMBOL_GPL(omap_iommu_save_ctx);
  **/
 void omap_iommu_restore_ctx(struct device *dev)
 {
-   struct omap_iommu *obj = dev_to_omap_iommu(dev);
-   u32 *p = obj->ctx;
+   struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
+   struct omap_iommu *obj;
+   u32 *p;
int i;
 
-   for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
-   iommu_write_reg(obj, p[i], i * sizeof(u32));
-   dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]);
+   if (!arch_data)
+   return;
+
+   while (arch_data->iommu_dev) {
+   obj = arch_data->iommu_dev;
+   p = obj->ctx;
+   for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
+   iommu_write_reg(obj, p[i], i * sizeof(u32));
+   dev_dbg(obj->dev, "%s\t[%02d] %08x\n", 

RE: [PATCH] iommu/dma: limit the IOVA allocated to dma-ranges region

[Krishna Reddy]

Robin,
>>Why? IOVA allocation is already constrained as much as it should be - if the 
>>device's DMA mask is wrong that's another problem, and this isn't the right 
>>place to fix it.

This is because of following reasons.
1. Many of the HW modules in Tegra114 and Tegra30 can't access IOVA that 
overlap with MMIO. Even though these HW modules support 32-bit addressing, They 
can't access IOVA overlapping with range 0xff00: to 0x:f, which is 
MMIO region for high vectors. These modules can only see 0x8000: to 
0xff00: as IOVA. If dma mask is set to use 31-bit, the IOVA range reduces 
from ~2GB to 1GB.  The patch is to get most of IOVA range usable by using 
dma-ranges property. As we already use dma-ranges start as bottom on IOVA 
range, The patch is to restrict the IOVA top as well using dma-ranges range. 
2. Most of  the drivers in Linux Kernel are setting mask as either 32-bit or 
64-bit.  Especially, I am referring to driver/mmc/host/sdhci.c 
(sdhci_set_dma_mask()) here.  In Tegra124/210/186, HW modules support 34-bit 
addressing.  But, sdhci only support setting mask as either 32-bit or 64-bit.  
As you pointed, This can be fixed by changing mask in sdhci common code.  Or 
This patch can help here as well without changing the driver code. 1 is the 
main driving factor for this patch.

>>dma_32bit_pfn means nothing more than an internal detail of IOVA allocator 
>>caching, which is subject to change[1]. As-is, on some platforms this patch 
>>will effectively force all allocations to fail already.

I see that your patches are removing specifying end of IOVA during 
init_iova_domain() and use mask as the end of IOVA range.  Do you have any 
suggestion on how to handle 1 in a way to use most of IOVA range supported by 
HW?  Can IOVA code look for dma-ranges on its own and limit the iova top to 
lowest of mask and dma-ranges, if it is present? or any other ways you can 
think of?

-KR

-Original Message-
From: Robin Murphy [mailto:robin.mur...@arm.com] 
Sent: Friday, September 1, 2017 2:43 AM
To: Joerg Roedel ; Krishna Reddy 
Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] iommu/dma: limit the IOVA allocated to dma-ranges region

On 01/09/17 10:26, Joerg Roedel wrote:
> Adding Robin for review.
> 
> On Thu, Aug 31, 2017 at 03:08:21PM -0700, Krishna Reddy wrote:
>> Limit the IOVA allocated to dma-ranges specified for the device.
>> This is necessary to ensure that IOVA allocated is addressable by 
>> device.

Why? IOVA allocation is already constrained as much as it should be - if the 
device's DMA mask is wrong that's another problem, and this isn't the right 
place to fix it.

dma_32bit_pfn means nothing more than an internal detail of IOVA allocator 
caching, which is subject to change[1]. As-is, on some platforms this patch 
will effectively force all allocations to fail already.

Robin.

[1]:https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg19694.html
>> Signed-off-by: Krishna Reddy 
>> ---
>>  drivers/iommu/dma-iommu.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c 
>> index 9d1cebe7f6cb..e8a8320b571b 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -364,6 +364,7 @@ static dma_addr_t iommu_dma_alloc_iova(struct 
>> iommu_domain *domain,
>>  struct iommu_dma_cookie *cookie = domain->iova_cookie;
>>  struct iova_domain *iovad = >iovad;
>>  unsigned long shift, iova_len, iova = 0;
>> +dma_addr_t dma_end_addr;
>>  
>>  if (cookie->type == IOMMU_DMA_MSI_COOKIE) {
>>  cookie->msi_iova += size;
>> @@ -381,6 +382,10 @@ static dma_addr_t iommu_dma_alloc_iova(struct 
>> iommu_domain *domain,
>>  if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1)))
>>  iova_len = roundup_pow_of_two(iova_len);
>>  
>> +/* Limit IOVA allocated to device addressable dma-ranges region. */
>> +dma_end_addr = (dma_addr_t)iovad->dma_32bit_pfn << shift;
>> +dma_limit = dma_limit > dma_end_addr ? dma_end_addr : dma_limit;
> 
> This looks like a good use-case for min().
> 
>> +
>>  if (domain->geometry.force_aperture)
>>  dma_limit = min(dma_limit, domain->geometry.aperture_end);
>>  
>> --
>> 2.1.4

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

Re: [v4,0/3] iommu/ipmmu-vmsa: r8a7796 support V4

[Oleksandr]

Hi, Magnus, maintainers, all.

On 19.06.17 14:04, Magnus Damm wrote:

iommu/ipmmu-vmsa: r8a7796 support V4

[PATCH v4 1/3] iommu/ipmmu-vmsa: Add r8a7796 DT binding
[PATCH v4 2/3] iommu/ipmmu-vmsa: Increase maximum micro-TLBS to 48
[PATCH v4 3/3] iommu/ipmmu-vmsa: Hook up r8a7796 DT matching code

This series adds r8a7796 support to the IPMMU driver. The DT binding
gets updated, maximum number of micro-TLBs are increased and the
driver is adjusted to match on the new DT binding.

I am interested in adding IPMMU-VMSA support to Xen hypervisor.

I did some preparations for making IPMMU-VMSA to feel comfortable [1] 
inside Xen
followed by direct porting Linux IPMMU-VMSA driver and ARM LPAE 
page-table allocator [2] to it.


I decided to base on the "BSP" driver [3] because it had more complete 
support than the "mainline" one [4].


During review I got a feedback that "BSP" driver wasn't the best choice 
to be ported.
Xen ARM maintainers worry about "BSP" driver which haven't had a 
thorough review by the Linux community and as the result might have bugs 
which will never be fixed, etc.


So, for the IPMMU-VMSA support to be accepted by Xen community I should 
either write our own driver based on BSP/mainline/whatever which 
contains only relevant to Xen things or
direct port from "mainline" driver. As the second option relies on the 
required support [5] which isn't in mainline yet, it is not clear when 
this support gets merged and how it will be modified/reworked before,
we preliminarily decided to follow the first option. But, I would like 
to consider second option again. Despite the complexity of second 
option, it has one significant benefit.


I see that Linux driver is being developed quite actively and looking 
over all related patch series I got a feeling that required support was 
about to reach upstream.


Could you, please, clarify some questions which, I hope, help us to make 
a decision:

1. Do you have approximate time-frame for getting this support in?
2. Are fundamental/significant changes planned for this support?

Also, may I ask for a link to github branch which contains current (and 
likely r8a7795 and 32-bit ARM update) patch series?


Thank you in advance!

And sorry for the most likely incorrect format of this email.

[1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg115901.html
[2] https://lists.xen.org/archives/html/xen-devel/2017-07/msg02679.html
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/tree/drivers/iommu/ipmmu-vmsa.c?h=v4.9/rcar-3.5.3
[4] 
http://elixir.free-electrons.com/linux/latest/source/drivers/iommu/ipmmu-vmsa.c

[5] https://lists.linuxfoundation.org/pipermail/iommu/2017-June/022567.html
https://lists.linuxfoundation.org/pipermail/iommu/2017-June/022577.html
https://lkml.org/lkml/2017/7/17/393


Changes since V3:
  - Rebased on top of [PATCH v4 00/09] iommu/ipmmu-vmsa: r8a7795 support V4
  - Patch 3/3 updated with Reviewed-by - thanks Geert!

Changes since V2:
  - Patch 2/3 updated with an outer set of () - thanks Ramesh!
  - Patch 2/3 updated with Reviewed-by - thanks Geert!
  - Patch 3/3 updated to include white list support

Changes since V1:
  - Patch 1/3 updated with more Acked-by tags
  - Patch 2/3 updated with high I/O register range support

Patch 1/3 is ready for upstream merge and includes the following tags:
Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
Acked-by: Rob Herring 
Acked-by: Simon Horman 
Acked-by: Geert Uytterhoeven 

Patch 2/3 and 3/3 are quite trivial but have no acked-by so far.

Signed-off-by: Magnus Damm 
---

  Developed on top of next-20170614 with the following series applied
  [PATCH 00/04] iommu/ipmmu-vmsa: 32-bit ARM update
  [PATCH v4 00/09] iommu/ipmmu-vmsa: r8a7795 support V4

  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1
  drivers/iommu/ipmmu-vmsa.c |   24 
+++---
  2 files changed, 18 insertions(+), 7 deletions(-)


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

Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module

[Alex Williamson]

On Mon, 4 Sep 2017 15:20:11 +0530
Anup Patel  wrote:

> Sorry for delayed response...
> 
> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
>  wrote:
> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:  
> >> This patch adds Broadcom FlexRM low-level reset for
> >> VFIO platform.
> >>  
> >
> > Is there an document that explains and /or details the various
> > registers?  
> 
> Yes, there is a document but its not publicly accessible.
> 
> >> It will do the following:
> >> 1. Disable/Deactivate each FlexRM ring
> >> 2. Flush each FlexRM ring
> >>
> >> The cleanup sequence for FlexRM rings is adapted from
> >> Broadcom FlexRM mailbox driver.
> >>
> >> Signed-off-by: Anup Patel 
> >> Reviewed-by: Oza Oza 
> >> Reviewed-by: Scott Branden 
> >> Reviewed-by: Eric Auger 
> >> ---
> >>  drivers/vfio/platform/reset/Kconfig|   9 ++
> >>  drivers/vfio/platform/reset/Makefile   |   1 +
> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 
> >> +
> >>  3 files changed, 110 insertions(+)
> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >>
> >> diff --git a/drivers/vfio/platform/reset/Kconfig 
> >> b/drivers/vfio/platform/reset/Kconfig
> >> index 705..392e3c0 100644
> >> --- a/drivers/vfio/platform/reset/Kconfig
> >> +++ b/drivers/vfio/platform/reset/Kconfig
> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
> >> Enables the VFIO platform driver to handle reset for AMD XGBE
> >>
> >> If you don't know what to do here, say N.
> >> +
> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> >> + tristate "VFIO support for Broadcom FlexRM reset"
> >> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> >> + default ARCH_BCM_IPROC
> >> + help
> >> +   Enables the VFIO platform driver to handle reset for Broadcom 
> >> FlexRM
> >> +
> >> +   If you don't know what to do here, say N.
> >> diff --git a/drivers/vfio/platform/reset/Makefile 
> >> b/drivers/vfio/platform/reset/Makefile
> >> index 93f4e23..8d9874b 100644
> >> --- a/drivers/vfio/platform/reset/Makefile
> >> +++ b/drivers/vfio/platform/reset/Makefile
> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
> >>
> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += 
> >> vfio-platform-calxedaxgmac.o
> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c 
> >> b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >> new file mode 100644
> >> index 000..966a813
> >> --- /dev/null
> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >> @@ -0,0 +1,100 @@
> >> +/*
> >> + * Copyright (C) 2017 Broadcom
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program.  If not, see .
> >> + */
> >> +
> >> +/*
> >> + * This driver provides reset support for Broadcom FlexRM ring manager
> >> + * to VFIO platform.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#include "vfio_platform_private.h"
> >> +
> >> +/* FlexRM configuration */
> >> +#define RING_REGS_SIZE   0x1
> >> +#define RING_VER_MAGIC   0x76303031
> >> +
> >> +/* Per-Ring register offsets */
> >> +#define RING_VER 0x000
> >> +#define RING_CONTROL 0x034
> >> +#define RING_FLUSH_DONE  0x038
> >> +
> >> +/* Register RING_CONTROL fields */
> >> +#define CONTROL_FLUSH_SHIFT  5
> >> +
> >> +/* Register RING_FLUSH_DONE fields */
> >> +#define FLUSH_DONE_MASK  0x1
> >> +
> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> >> +{
> >> + unsigned int timeout;
> >> +
> >> + /* Disable/inactivate ring */
> >> + writel_relaxed(0x0, ring + RING_CONTROL);
> >> +
> >> + /* Flush ring with timeout of 1s */
> >> + timeout = 1000;  
> >
> > Perhaps a #define for this value?  
> 
> This magic value 1000 makes it explicit that we try till 1 

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

[Jean-Philippe Brucker]

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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH 6/6] iommu/arm-smmu-v3: Avoid ILLEGAL setting of STE.S1STALLD and CD.S

[Jean-Philippe Brucker]

On 31/08/17 09:20, Yisheng Xie wrote:
> It is ILLEGAL to set STE.S1STALLD if STALL_MODEL is not 0b00, which
> means we should not disable stall mode if stall/terminate mode is not
> configuable.
> 
> Meanwhile, it is also ILLEGAL when STALL_MODEL==0b10 && CD.S==0 which
> means if stall mode is force we should always set CD.S.
> 
> This patch add ARM_SMMU_FEAT_TERMINATE feature bit for smmu, and use
> TERMINATE feature checking to ensue above ILLEGAL cases from happening.
> 
> Signed-off-by: Yisheng Xie 
> ---
>  drivers/iommu/arm-smmu-v3.c | 22 --
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index dbda2eb..0745522 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -55,6 +55,7 @@
>  #define IDR0_STALL_MODEL_SHIFT   24
>  #define IDR0_STALL_MODEL_MASK0x3
>  #define IDR0_STALL_MODEL_STALL   (0 << IDR0_STALL_MODEL_SHIFT)
> +#define IDR0_STALL_MODEL_NS  (1 << IDR0_STALL_MODEL_SHIFT)
>  #define IDR0_STALL_MODEL_FORCE   (2 << IDR0_STALL_MODEL_SHIFT)
>  #define IDR0_TTENDIAN_SHIFT  21
>  #define IDR0_TTENDIAN_MASK   0x3
> @@ -766,6 +767,7 @@ struct arm_smmu_device {
>  #define ARM_SMMU_FEAT_SVM(1 << 15)
>  #define ARM_SMMU_FEAT_HA (1 << 16)
>  #define ARM_SMMU_FEAT_HD (1 << 17)
> +#define ARM_SMMU_FEAT_TERMINATE  (1 << 18)

I'd rather introduce something like "ARM_SMMU_FEAT_STALL_FORCE" instead.
Terminate model has another meaning, and is defined by a different bit in
IDR0.

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

Re: [RFC PATCH 5/6] iommu/arm-smmu-v3: fix panic when handle stall mode irq

[Jean-Philippe Brucker]

On 31/08/17 09:20, Yisheng Xie wrote:
> When SMMU do not support SVM feature, however the master support SVM,
> which means matser can stall and with mult-pasid number, then the user
> can bind a task to device using API like iommu_bind_task(). however,
> when device trigger a stall mode fault i will cause panic:
> 
> [  106.996087] Unable to handle kernel NULL pointer dereference at virtual 
> address 0100
> [  106.996122] user pgtable: 4k pages, 48-bit VAs, pgd = 80003e023000
> [  106.996150] [0100] *pgd=3e04a003, 
> *pud=3e04b003, *pmd=
> [  106.996201] Internal error: Oops: 9606 [#1] PREEMPT SM
> [  106.996224] Modules linked in:
> [  106.996256] CPU: 0 PID: 916 Comm: irq/14-arm-smmu Not tainted 
> 4.13.0-rc5-00035-g1235ddd-dirty #67
> [  106.996288] Hardware name: Hisilicon PhosphorHi1383 ESL (DT)
> [  106.996317] task: 80003adc1c00 task.stack: 80003a9f8000
> [  106.996347] PC is at __queue_work+0x30/0x3a8
> [  106.996374] LR is at queue_work_on+0x60/0x78
> [  106.996401] pc : [] lr : [] pstate: 
> 40c001c9
> [  106.996430] sp : 80003a9fbc20
> [  106.996451] x29: 80003a9fbc20 x28: 80003adc1c00
> [  106.996488] x27: 08d05080 x26: 80003ab0e028
> [  106.996526] x25: 80003a9900ac x24: 0001
> [  106.996562] x23: 0040 x22: 
> [  106.996598] x21:  x20: 0140
> [  106.996634] x19: 80003ab0e028 x18: 0010
> [  106.996670] x17: a52a5040 x16: 0820f260
> [  106.996708] x15: 0018e97629e0 x14: 80003fb89468
> [  106.996744] x13:  x12: 80003abb0600
> [  106.996781] x11:  x10: 01010100
> [  106.996817] x9 : 85de5010 x8 : e4830001
> [  106.996854] x7 : 80003a9fbcf8 x6 : 000fffe0
> [  106.996890] x5 :  x4 : 0001
> [  106.996926] x3 :  x2 : 80003ab0e028
> [  106.996962] x1 :  x0 : 01c0
> [  106.997002] Process irq/14-arm-smmu (pid: 916, stack limit 
> =0x80003a9f8000)
> [  106.997035] Stack: (0x80003a9fbc20 to 0x80003a9fc000)
> [...]
> [  106.998366] Call trace:
> [  106.998842] [] __queue_work+0x30/0x3a8
> [  106.998874] [] queue_work_on+0x60/0x78
> [  106.998912] [] arm_smmu_handle_stall+0x104/0x138
> [  106.998952] [] arm_smmu_evtq_thread+0xc0/0x158
> [  106.998989] [] irq_thread_fn+0x28/0x68
> [  106.999025] [] irq_thread+0x128/0x1d0
> [  106.999060] [] kthread+0xfc/0x128
> [  106.999093] [] ret_from_fork+0x10/0x50
> [  106.999130] Code: a90153f3 a90573fb d53b4220 363814c0 (b94102a0)
> [  106.999159] ---[ end trace 7e5c9f0cb1f2fecd ]---
> 
> And the resean is we donot init fault_queue while the fault handle need
> to use it. 
>
> Fix by return -EINVAL in arm_smmu_bind_task() when smmu do not
> support the feature of SVM.
> 
> Signed-off-by: Yisheng Xie 
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d44256a..dbda2eb 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2922,6 +2922,8 @@ static int arm_smmu_bind_task(struct device *dev, 
> struct task_struct *task,
>   return -EINVAL;
>  
>   smmu = master->smmu;
> + if (!(smmu->features & ARM_SMMU_FEAT_SVM))
> + return -EINVAL;

FEAT_SVM is set when the SMMU supports the same page table format as the
MMU, it doesn't say anything about PRI/stall ability. To fix the above
splat we should either instantiate fault_queue even when !FEAT_SVM, or
avoid enabling master->can_fault and can_stall if !FEAT_SVM. I prefer the
latter.

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

Re: [RFC PATCH 4/6] iommu/arm-smmu-v3: Add SVM support for platform devices

[Jean-Philippe Brucker]

On 31/08/17 09:20, Yisheng Xie wrote:
> From: Jean-Philippe Brucker 
> 
> Platform device can realise SVM function by using the stall mode. That
> is to say, when device access a memory via iova which is not populated,
> it will stalled and when SMMU try to translate this iova, it also will
> stall and meanwhile send an event to CPU via MSI.
> 
> After SMMU driver handle the event and populated the iova, it will send
> a RESUME command to SMMU to exit the stall mode, therefore the platform
> device can contiue access the memory.
> 
> Signed-off-by: Jean-Philippe Brucker 

No. Please don't forge a signed-off-by under a commit message you wrote,
it's rude. I didn't sign it, didn't consider it fit for mainline or even
as an RFC, and wanted to have another read before sending. My mistake,
I'll think twice before sharing prototypes in the future.

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

Re: [RFC PATCH 2/6] iommu/of: Add stall and pasid properties to iommu_fwspec

[Jean-Philippe Brucker]

On 31/08/17 09:20, Yisheng Xie wrote:
> From: Jean-Philippe Brucker 
> 
> Add stall and pasid properties to iommu_fwspec.
> 
> Signed-off-by: Jean-Philippe Brucker 

No. This is a draft, I didn't sign it off.

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

Re: [RFC PATCH 1/6] dt-bindings: document stall and PASID properties for IOMMU masters

[Jean-Philippe Brucker]

On 31/08/17 09:20, Yisheng Xie wrote:
> From: Jean-Philippe Brucker 
> 
> Document the bindings for stall and PASID capable platform devices.
> 
> Signed-off-by: Jean-Philippe Brucker 

Huh? No, I don't think I did. Please leave the patch as you found it, and
ask me for a version I consider clean next time.

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

Re: [PATCH] dma-coherent: fix dma_declare_coherent_memory() logic error

[Christoph Hellwig]

Thanks Arnd,

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

Re: [PATCH v6 3/3] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[John Garry]

Hi Will, Lorenzo, Robin,

I have created the patch to add DT support for this erratum.
However, currently I have only added support for pci-based devices.
I'm a bit stumped on how to add platform device support, or if we
should also add support at all. And I would rather ask before
sending the patches.

The specific issue is that I know of no platform device with an ITS
msi-parent which we would want reserve, i.e. do not translate msi.
And, if there were, how to do it.

The only platform devices I know off with msi ITS parents are SMMUv3
and mbigen. For mbigen, the msi are not translated. Actually, as I
see under IORT solution, for mbigen we don't reserve the hw msi
region as the SMMUv3 does not have an ID mapping. And we have no
equivalent ID mapping property for DT SMMUv3, so cannot create an
equivalent check.

So here is how the DT iommu get reserved region function with only
pci device support looks:

/**
 * of_iommu_its_get_resv_regions - Reserved region driver helper
 * @dev: Device from iommu_get_resv_regions()
 * @list: Reserved region list from iommu_get_resv_regions()
 *
 * Returns: Number of reserved regions on success(0 if no associated ITS),
 *  appropriate error value otherwise.
 */
int of_iommu_its_get_resv_regions(struct device *dev, struct
list_head *head)
{
struct device_node *of_node = NULL;
struct device *parent_dev;
const __be32 *msi_map;
int num_mappings, i, err, len, resv = 0;

/* walk up to find the parent with a msi-map property */
for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
if (!parent_dev->of_node)
continue;

/*
 * Current logic to reserve ITS regions for only PCI root
 * complex.
 */
msi_map = of_get_property(parent_dev->of_node, "msi-map", );
if (msi_map) {
of_node = parent_dev->of_node;
break;
}
}

if (!of_node)
return 0;

num_mappings = of_count_phandle_with_args(of_node, "msi-map",
NULL) / 4;

for (i = 0; i < num_mappings; i++) {
int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
struct iommu_resv_region *region;
struct device_node *msi_node;
struct resource resource;
u32 phandle;

phandle = be32_to_cpup(msi_map + 1);
msi_node = of_find_node_by_phandle(phandle);
if (!msi_node)
return -ENODEV;

/*
 * There may be different types of msi-controller, so check
 * for ITS.
 */
if (!of_device_is_compatible(msi_node, "arm,gic-v3-its")) {
of_node_put(msi_node);
continue;
}

err = of_address_to_resource(msi_node, 0, );

of_node_put(msi_node);
if (err)
return err;

region = iommu_alloc_resv_region(resource.start, SZ_128K, prot,
 IOMMU_RESV_MSI);
if (region) {
list_add_tail(>list, head);
resv++;
}
}

return (resv == num_mappings) ? resv : -ENODEV;
}

Any feedback is appreciated.


Hi John,

I appreciate it is not trivial to make the code generic, part of the
snippet above can be shared between ACPI/IORT and OF, the only sticking
point is probably the "compatible" string that has to be parameterized
since having this code in generic IOMMU layer is a bit horrible to
have it ITS specific (and it is a problem that was existing already
in the original patch with the hardcoded ITS node type or function
name).


Hi Lorenzo,

Agreed, checking the ITS compatible string in IOMMU code is not nice. 
However the function is just trying to replicate our ACPI version, which 
calls IORT code directly, and this is ITS specific. Anyway, we can make 
it generic.




To sum it up the hook:

- has to be called from SMMU driver in a firmware agnostic way


ok


- somehow it has to ascertain which interrupt controller "compatible"
  (which in IORT is a node type) to match against so the hook has to
  take an id of sorts


OK

I will mention 2 more points after discussion on OF solution with Shameer:
- for platform device, we can add suppport by checking for the devices 
msi-parent property
- for both pci and platform device, we should check device rid for 
msi-controller also, like IORT solution


BTW, even though merge window is open, we would like to send some 
solution to the lists this week, so any outstanding topics can 
potentially be discussed at LPC next week. Let me know if you're unhappy 
about this.


All the best,
John




I need to go back and have a look at the original patch to see how
we can accommodate both OF/ACPI - certainly the region reservations
code can and should be shared.

Lorenzo

.




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

[PATCH] dma-coherent: fix dma_declare_coherent_memory() logic error

[Arnd Bergmann]

A recent change interprets the return code of dma_init_coherent_memory
as an error value, but it is instead a boolean, where 'true' indicates
success. This leads causes the caller to always do the wrong thing,
and also triggers a compile-time warning about it:

drivers/base/dma-coherent.c: In function 'dma_declare_coherent_memory':
drivers/base/dma-coherent.c:99:15: error: 'mem' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]

I ended up changing the code a little more, to give use the usual
error handling, as this seemed the best way to fix up the warning
and make the code look reasonable at the same time.

Fixes: 2436bdcda53f ("dma-coherent: remove the DMA_MEMORY_MAP and DMA_MEMORY_IO 
flags")
Signed-off-by: Arnd Bergmann 
---
 drivers/base/dma-coherent.c | 38 +-
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index f82a504583d4..a39b2166b145 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -37,7 +37,7 @@ static inline dma_addr_t dma_get_device_base(struct device 
*dev,
return mem->device_base;
 }
 
-static bool dma_init_coherent_memory(
+static int dma_init_coherent_memory(
phys_addr_t phys_addr, dma_addr_t device_addr, size_t size, int flags,
struct dma_coherent_mem **mem)
 {
@@ -45,20 +45,28 @@ static bool dma_init_coherent_memory(
void __iomem *mem_base = NULL;
int pages = size >> PAGE_SHIFT;
int bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
+   int ret;
 
-   if (!size)
+   if (!size) {
+   ret = -EINVAL;
goto out;
+   }
 
mem_base = memremap(phys_addr, size, MEMREMAP_WC);
-   if (!mem_base)
+   if (!mem_base) {
+   ret = -EINVAL;
goto out;
-
+   }
dma_mem = kzalloc(sizeof(struct dma_coherent_mem), GFP_KERNEL);
-   if (!dma_mem)
+   if (!dma_mem) {
+   ret = -ENOMEM;
goto out;
+   }
dma_mem->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
-   if (!dma_mem->bitmap)
+   if (!dma_mem->bitmap) {
+   ret = -ENOMEM;
goto out;
+   }
 
dma_mem->virt_base = mem_base;
dma_mem->device_base = device_addr;
@@ -68,13 +76,13 @@ static bool dma_init_coherent_memory(
spin_lock_init(_mem->spinlock);
 
*mem = dma_mem;
-   return true;
+   return 0;
 
 out:
kfree(dma_mem);
if (mem_base)
memunmap(mem_base);
-   return false;
+   return ret;
 }
 
 static void dma_release_coherent_memory(struct dma_coherent_mem *mem)
@@ -338,14 +346,18 @@ static struct reserved_mem *dma_reserved_default_memory 
__initdata;
 static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
 {
struct dma_coherent_mem *mem = rmem->priv;
+   int ret;
+
+   if (!mem)
+   return -ENODEV;
 
-   if (!mem &&
-   !dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
- DMA_MEMORY_EXCLUSIVE,
- )) {
+   ret = dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
+  DMA_MEMORY_EXCLUSIVE, );
+
+   if (ret) {
pr_err("Reserved memory: failed to init DMA memory pool at %pa, 
size %ld MiB\n",
>base, (unsigned long)rmem->size / SZ_1M);
-   return -ENODEV;
+   return ret;
}
mem->use_dev_dma_pfn_offset = true;
rmem->priv = mem;
-- 
2.9.0

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