[PATCH 1/3] driver core: initialize a default DMA mask for platform device

2018-08-28 Thread Christoph Hellwig
We still treat devices without a DMA mask as defaulting to 32-bits for
both mask, but a few releases ago we've started warning about such
cases, as they require special cases to work around this sloppyness.
Add a dma_mask field to struct platform_object so that we can initialize
the dma_mask pointer in struct device and initialize both masks to
32-bits by default.  Architectures can still override this in
arch_setup_pdev_archdata if needed.

Note that the code looks a little odd with the various conditionals
because we have to support platform_device structures that are
statically allocated.

Reported-by: Guenter Roeck 
Signed-off-by: Christoph Hellwig 
---
 drivers/base/platform.c | 15 +--
 include/linux/platform_device.h |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index dff82a3c2caa..baf4b06cf2d9 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -225,6 +225,17 @@ struct platform_object {
char name[];
 };
 
+static void setup_pdev_archdata(struct platform_device *pdev)
+{
+   if (!pdev->dev.coherent_dma_mask)
+   pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   if (!pdev->dma_mask)
+   pdev->dma_mask = DMA_BIT_MASK(32);
+   if (!pdev->dev.dma_mask)
+   pdev->dev.dma_mask = &pdev->dma_mask;
+   arch_setup_pdev_archdata(pdev);
+};
+
 /**
  * platform_device_put - destroy a platform device
  * @pdev: platform device to free
@@ -271,7 +282,7 @@ struct platform_device *platform_device_alloc(const char 
*name, int id)
pa->pdev.id = id;
device_initialize(&pa->pdev.dev);
pa->pdev.dev.release = platform_device_release;
-   arch_setup_pdev_archdata(&pa->pdev);
+   setup_pdev_archdata(&pa->pdev);
}
 
return pa ? &pa->pdev : NULL;
@@ -472,7 +483,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
 int platform_device_register(struct platform_device *pdev)
 {
device_initialize(&pdev->dev);
-   arch_setup_pdev_archdata(pdev);
+   setup_pdev_archdata(pdev);
return platform_device_add(pdev);
 }
 EXPORT_SYMBOL_GPL(platform_device_register);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 1a9f38f27f65..d9dc4883d5fb 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -25,6 +25,7 @@ struct platform_device {
int id;
boolid_auto;
struct device   dev;
+   u64 dma_mask;
u32 num_resources;
struct resource *resource;
 
-- 
2.18.0

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


various dma_mask fixups

2018-08-28 Thread Christoph Hellwig
Fix warnings and regressions from requiring a dma mask.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 3/3] of/platform: initialise AMBA default DMA masks

2018-08-28 Thread Christoph Hellwig
From: Linus Walleij 

commit a5516219b102 ("of/platform: Initialise default DMA masks") sets up
the coherent_dma_mask of platform devices created from the device tree,
but fails to do the same for AMBA (PrimeCell) devices.

This leads to a regression in kernel v4.19-rc1 triggering the
WARN_ON_ONCE(dev && !dev->coherent_dma_mask) in dma_alloc_attrs().

This regresses the PL111 DRM driver in drivers/gpu/drm/pl111 that uses
the AMBA PrimeCell to instantiate the frame buffer device, as it cannot
allocate a chunk of coherent memory anymore due to the missing mask.

Fixes: a5516219b102 ("of/platform: Initialise default DMA masks")
Signed-off-by: Linus Walleij 
[hch: added a comment, and droped a conditional that can't be true]
Signed-off-by: Christoph Hellwig 
---
 drivers/of/platform.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7ba90c290a42..6c59673933e9 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -241,6 +241,10 @@ static struct amba_device *of_amba_device_create(struct 
device_node *node,
if (!dev)
goto err_clear_flag;
 
+   /* AMBA devices only support a single DMA mask */
+   dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
+
/* setup generic device info */
dev->dev.of_node = of_node_get(node);
dev->dev.fwnode = &node->fwnode;
-- 
2.18.0

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


[PATCH 2/3] sparc: set a default 32-bit dma mask for OF devices

2018-08-28 Thread Christoph Hellwig
This keeps the historic default behavior for devices without a DMA mask,
but removes the warning about a lacking DMA mask for doing DMA without
a mask.

Reported-by: Meelis Roos 
Tested-by: Meelis Roos 
Signed-off-by: Christoph Hellwig 
---
 arch/sparc/kernel/of_device_32.c | 5 +
 arch/sparc/kernel/of_device_64.c | 4 
 2 files changed, 9 insertions(+)

diff --git a/arch/sparc/kernel/of_device_32.c b/arch/sparc/kernel/of_device_32.c
index 3641a294ed54..7f3dec7e1e78 100644
--- a/arch/sparc/kernel/of_device_32.c
+++ b/arch/sparc/kernel/of_device_32.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -381,6 +382,10 @@ static struct platform_device * __init 
scan_one_device(struct device_node *dp,
else
dev_set_name(&op->dev, "%08x", dp->phandle);
 
+   op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   op->dev.dma_mask = &op->dma_mask;
+   op->dma_mask = DMA_BIT_MASK(32);
+
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
   dp->full_name);
diff --git a/arch/sparc/kernel/of_device_64.c b/arch/sparc/kernel/of_device_64.c
index 44e4d4435bed..d94c31822da1 100644
--- a/arch/sparc/kernel/of_device_64.c
+++ b/arch/sparc/kernel/of_device_64.c
@@ -2,6 +2,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -675,6 +676,9 @@ static struct platform_device * __init 
scan_one_device(struct device_node *dp,
dev_set_name(&op->dev, "root");
else
dev_set_name(&op->dev, "%08x", dp->phandle);
+   op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   op->dev.dma_mask = &op->dma_mask;
+   op->dma_mask = DMA_BIT_MASK(32);
 
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
-- 
2.18.0

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


Re: [PATCH] of/platform: Initialise AMBA default DMA masks

2018-08-28 Thread Christoph Hellwig
On Tue, Aug 28, 2018 at 03:14:14PM +0100, Russell King - ARM Linux wrote:
> But yes, the fundamental fact is that AMBA devices don't have any
> care about the differences between coherent and streaming DMA.  The
> distinction that we make in the kernel is purely a software one when
> it comes to these devices.
> 
> Most AMBA devices themselves are not DMA capable, as they are only
> connected to the APB (Amba peripheral bus) and they rely on a
> separate DMA engine for their DMA.  APB devices should not have DMA
> masks - their DMA capabilities are entirely down to the DMA controller.
> So, the majority of AMBA devices should not have any DMA masks.
> 
> Only those connected to a bus that they can master on (eg AXI) should
> have DMA masks - things like the PL08x DMA controllers, PL11x LCD
> controllers, etc.  As I've said above, there is no difference between
> streaming and coherent DMA for these devices.

So for now I plan to apply the patch from Linus to just set a dma
mask, as that gets back the previous behavior where dma did just
work (as it did without a mask).

But if Linus, you or someone else familiar with amba would like to
add an explicit opt-in into dma support eventually that would be
even better.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] of/platform: Initialise AMBA default DMA masks

2018-08-28 Thread Christoph Hellwig
On Tue, Aug 28, 2018 at 03:25:55PM +0200, Linus Walleij wrote:
> Do you think we can proceed with this patch or do you want me to
> revert the split back?

I'll apply this patch (probably with a little common in the source
explaining the situation), based on the feedback from you and Russell.

> 
> FWIW the platform devices have the same problem, but I know
> I know, two wrongs does not make one right :/

I have a patch pending for that..
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: new SBus related DMA warnings in 4.18+git

2018-08-28 Thread Christoph Hellwig
On Wed, Aug 29, 2018 at 08:47:04AM +0300, Meelis Roos wrote:
> > Updated version including that has survived contact with a sparc cross
> > compiler below.  Note that this is on top of the previous patch adding
> > a dma_mask to struct platform_device, which I've included as well.
> 
> Works completely fine on both Ultra 1 and Ultra 2 - nothing DMA-relaated 
> in dmesg.

Thanks for testing!  I'll submit the patch for 4.19 in a bit.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: new SBus related DMA warnings in 4.18+git

2018-08-28 Thread Meelis Roos
> Updated version including that has survived contact with a sparc cross
> compiler below.  Note that this is on top of the previous patch adding
> a dma_mask to struct platform_device, which I've included as well.

Works completely fine on both Ultra 1 and Ultra 2 - nothing DMA-relaated 
in dmesg.

-- 
Meelis Roos (mr...@linux.ee)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [Patch v15 4/5] dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2

2018-08-28 Thread Rob Herring
On Mon, Aug 27, 2018 at 04:25:50PM +0530, Vivek Gautam wrote:
> Add bindings doc for Qcom's smmu-v2 implementation.
> 
> Signed-off-by: Vivek Gautam 
> Reviewed-by: Tomasz Figa 
> Tested-by: Srinivas Kandagatla 
> ---
> 
> Changes since v14:
>  - This is a new patch added in v15 after noticing the new
>checkpatch warning for separate dt-bindings doc.
>  - This patch also addresses comments given by Rob and Robin to add
>a list of valid values of '' in "qcom,-smmu-v2"
>compatible string.
> 
>  .../devicetree/bindings/iommu/arm,smmu.txt | 47 
> ++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt 
> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 8a6ffce12af5..52198a539606 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -17,10 +17,24 @@ conditions.
>  "arm,mmu-401"
>  "arm,mmu-500"
>  "cavium,smmu-v2"
> +"qcom,-smmu-v2", "qcom,smmu-v2"

The v2 in the compatible string is kind of redundant unless the SoC has 
other SMMU types.

>  
>depending on the particular implementation and/or the
>version of the architecture implemented.
>  
> +  A number of Qcom SoCs use qcom,smmu-v2 version of the IP.
> +  "qcom,-smmu-v2" represents a soc specific compatible
> +  string that should be present along with the "qcom,smmu-v2"
> +  to facilitate SoC specific clocks/power connections and to
> +  address specific bug fixes.
> +  '' string in "qcom,-smmu-v2" should be one of the
> +  following:
> +  msm8996 - for msm8996 Qcom SoC.
> +  sdm845 - for sdm845 Qcom Soc.

Rather than all this prose, it would be simpler to just add 2 lines with 
the full compatibles rather than . The  thing is not going to 
work when/if we move bindings to json-schema also.

> +
> +  An example string would be -
> +  "qcom,msm8996-smmu-v2", "qcom,smmu-v2".
> +
>  - reg   : Base address and size of the SMMU.
>  
>  - #global-interrupts : The number of global interrupts exposed by the
> @@ -71,6 +85,22 @@ conditions.
>or using stream matching with #iommu-cells = <2>, and
>may be ignored if present in such cases.
>  
> +- clock-names:List of the names of clocks input to the device. The
> +  required list depends on particular implementation and
> +  is as follows:
> +  - for "qcom,smmu-v2":
> +- "bus": clock required for downstream bus access and
> + for the smmu ptw,
> +- "iface": clock required to access smmu's registers
> +   through the TCU's programming interface.
> +  - unspecified for other implementations.
> +
> +- clocks: Specifiers for all clocks listed in the clock-names 
> property,
> +  as per generic clock bindings.
> +
> +- power-domains:  Specifiers for power domains required to be powered on for
> +  the SMMU to operate, as per generic power domain bindings.
> +
>  ** Deprecated properties:
>  
>  - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -137,3 +167,20 @@ conditions.
>  iommu-map = <0 &smmu3 0 0x400>;
>  ...
>  };
> +
> + /* Qcom's arm,smmu-v2 implementation */
> + smmu4: iommu {

Needs a unit-address.

> + compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
> + reg = <0xd0 0x1>;
> +
> + #global-interrupts = <1>;
> + interrupts = ,
> +  ,
> +  ;
> + #iommu-cells = <1>;
> + power-domains = <&mmcc MDSS_GDSC>;
> +
> + clocks = <&mmcc SMMU_MDP_AXI_CLK>,
> +  <&mmcc SMMU_MDP_AHB_CLK>;
> + clock-names = "bus", "iface";
> + };
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 01/23] iommu: introduce bind_pasid_table API function

2018-08-28 Thread Jacob Pan
On Fri, 24 Aug 2018 15:20:08 +0200
Auger Eric  wrote:

> Hi Yi Liu,
> 
> On 08/24/2018 02:47 PM, Liu, Yi L wrote:
> > Hi Eric,
> >   
> >> From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
> >> boun...@lists.linux-foundation.org] On Behalf Of Auger Eric
> >> Sent: Friday, August 24, 2018 12:35 AM
> >>
> >> Hi Jacob,
> >>
> >> On 05/11/2018 10:53 PM, Jacob Pan wrote:  
> >>> Virtual IOMMU was proposed to support Shared Virtual Memory (SVM)
> >>> use in the guest:
> >>> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
> >>>
> >>> As part of the proposed architecture, when an SVM capable PCI
> >>> device is assigned to a guest, nested mode is turned on. Guest
> >>> owns the first level page tables (request with PASID) which
> >>> performs GVA->GPA translation. Second level page tables are owned
> >>> by the host for GPA->HPA translation for both request with and
> >>> without PASID.
> >>>
> >>> A new IOMMU driver interface is therefore needed to perform tasks
> >>> as follows:
> >>> * Enable nested translation and appropriate translation type
> >>> * Assign guest PASID table pointer (in GPA) and size to host IOMMU
> >>>
> >>> This patch introduces new API functions to perform bind/unbind
> >>> guest PASID tables. Based on common data, model specific IOMMU
> >>> drivers can be extended to perform the specific steps for binding
> >>> pasid table of assigned devices.
> >>>
> >>> Signed-off-by: Jean-Philippe Brucker
> >>>  Signed-off-by: Liu, Yi L
> >>>  Signed-off-by: Ashok Raj
> >>>  Signed-off-by: Jacob Pan
> >>>  ---  
> > 
> > [...]
> >   
> >>> +#ifndef _UAPI_IOMMU_H
> >>> +#define _UAPI_IOMMU_H
> >>> +
> >>> +#include 
> >>> +
> >>> +/**
> >>> + * PASID table data used to bind guest PASID table to the host
> >>> IOMMU. This will
> >>> + * enable guest managed first level page tables.
> >>> + * @version: for future extensions and identification of the
> >>> data format
> >>> + * @bytes: size of this structure
> >>> + * @base_ptr:PASID table pointer
> >>> + * @pasid_bits:  number of bits supported in the guest
> >>> PASID table, must be  
> >> less  
> >>> + *   or equal than the host supported PASID size.
> >>> + */
> >>> +struct pasid_table_config {
> >>> + __u32 version;
> >>> +#define PASID_TABLE_CFG_VERSION_1 1
> >>> + __u32 bytes;
> >>> + __u64 base_ptr;
> >>> + __u8 pasid_bits;  
> >>
> >> As reported in "[RFC 00/13] SMMUv3 Nested Stage Setup" thread,
> >> this API could be used for ARM SMMUv3 nested stage enablement
> >> without many changes. Assuming SMMUv3 nested stage is confirmed to
> >> be interesting for vendors and maintainers, we could try to unify
> >> the APIs.  
> > 
> > Just a quick question on nested stage on SMMUv3. If virtualizer
> > wants to enable nested stage on SMMUv3, does it link the whole
> > guest CD table to host or do it in other manner?  
> Yes that's correct. On ARM SMMUv3 you have Stream Table Entries (STEs,
> indexed by ReqID=streamid). If stage 1 is used, the STE points to 1 or
> more contiguous Context Descriptors (CDs).
> So STE looks like the VTD Context-Entry and CD table looks like the
> VTD PASID table as far as I understand.
> >   
> >> As far as I understand the VTD PASID table is equivalent to the ARM
> >> SMMUv3 context descriptor table (CD). This corresponds to the
> >> stage 1 context table with one or more entries, each corresponding
> >> to one PASID.  
> > 
> > PASID table is index by PASID, and have multiple entries. A PASID
> > table would have 2^PASID_BITS entries.  
> On ARM SMMUv3 the  number of CDs is 2 ^STE.S1CDMax.
> >   
> >> maybe using the s1ctx_table_config terminology instead of
> >> pasid_table_config would be more generic, the pasid table being
> >> Intel naming.
> >>
> >> on top of pasid_bits, I think an "asid_bits" field may be needed
> >> too. The guest IOMMU might support a different number of asid bits
> >> from the host one.  
> > 
> > Maybe needed for SMMUv3. I've noticed you've placed it in
> > struct iommu_smmu_s1_config.
> >   
> >>
> >> Although without having skimmed through the whole series yet, I
> >> wonder how you handle the case where stage1 is bypassed or
> >> disabled? The guest may define the S1 context entries but bypass
> >> or abort stage 1 translations globally. Looks something missing to
> >> me at first sight.  
> > 
> > Sorry, I didn't quite follow here. What usage is case such for?
> > like stage 1 is bypassed or disabled. IOVA or SVA?  
> Each STE entry has a config field which tells how S1 and S2 behave
> 
> Options are no traffic at all or any combination of the following:
> 
> S1S2
> bypassbypass
> translbypass
> bypasstransl
> transltransl
> 
> host manages S2 info. guest sets S1 related fields.
> 
> To me the guest SET.Config should be passed to the host so that this
> latter writes the correct global Config field value in the STE,
> including S1 + S2 info.
> 
Global config ( VT-d global command reg) is IOMMU wide, we cannot let
guest con

Re: [PATCH v5 01/23] iommu: introduce bind_pasid_table API function

2018-08-28 Thread Jacob Pan
On Tue, 28 Aug 2018 10:34:19 +0200
Auger Eric  wrote:

> > Anyway, for the new VT-d 3.0 spec. we no longer need this API. In
> > stead, I will introduce bind_guest_pasid() API, where per device
> > PASID table is allocated by the host.  
> 
> So what is the exact state of this series? Is it outdated as you don't
> target VT-d 2.5 anymore? Will you keep the rest of the API?

Hi Eric,

I am not targeting VT-d 2.5 for SVA related work. I am working on the
rest of the APIs for supporting VT-d v3, which includes guest PASID
bind, fault reporting, and invalidation passdown from the guest. These
are based on some recent patches from Baolu.
https://lkml.org/lkml/2018/7/16/62

So I feel it is better for you to take over bind_pasid_table() API in
your series. I will drop it from my next version.

Thanks,

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


Re: [PATCH] of/platform: Initialise AMBA default DMA masks

2018-08-28 Thread Russell King - ARM Linux
On Tue, Aug 28, 2018 at 03:25:55PM +0200, Linus Walleij wrote:
> On Tue, Aug 28, 2018 at 11:21 AM Christoph Hellwig  wrote:
> 
> > > + dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> > > + if (!dev->dev.dma_mask)
> > > + dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
> >
> > We should never set dma_mask to point to the coherent_dma_mask,
> > as that will cause problems with devices that have different
> > mask for the two.
> >
> > How about something like this?
> (...)
> > +   u64 dma_mask;
> 
> We did that before, so this becomes effectively a revert of:
> commit 446b2a9380b64b9d7410d86ee8226031e03645cf
> 
> DMA-API: amba: get rid of separate dma_mask
> 
> AMBA Primecell devices always treat streaming and coherent DMA exactly
> the same, so there's no point in having the masks separated.
> 
> So there is some history around this.
> 
> There is also some code in drivers/amba/bus.c affected by that
> and I need to look over all static amba device allocations if we
> reintroduce this.

I don't have the rest of this thread to read...

But yes, the fundamental fact is that AMBA devices don't have any
care about the differences between coherent and streaming DMA.  The
distinction that we make in the kernel is purely a software one when
it comes to these devices.

Most AMBA devices themselves are not DMA capable, as they are only
connected to the APB (Amba peripheral bus) and they rely on a
separate DMA engine for their DMA.  APB devices should not have DMA
masks - their DMA capabilities are entirely down to the DMA controller.
So, the majority of AMBA devices should not have any DMA masks.

Only those connected to a bus that they can master on (eg AXI) should
have DMA masks - things like the PL08x DMA controllers, PL11x LCD
controllers, etc.  As I've said above, there is no difference between
streaming and coherent DMA for these devices.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] of/platform: Initialise AMBA default DMA masks

2018-08-28 Thread Linus Walleij
On Tue, Aug 28, 2018 at 11:21 AM Christoph Hellwig  wrote:

> > + dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> > + if (!dev->dev.dma_mask)
> > + dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
>
> We should never set dma_mask to point to the coherent_dma_mask,
> as that will cause problems with devices that have different
> mask for the two.
>
> How about something like this?
(...)
> +   u64 dma_mask;

We did that before, so this becomes effectively a revert of:
commit 446b2a9380b64b9d7410d86ee8226031e03645cf

DMA-API: amba: get rid of separate dma_mask

AMBA Primecell devices always treat streaming and coherent DMA exactly
the same, so there's no point in having the masks separated.

So there is some history around this.

There is also some code in drivers/amba/bus.c affected by that
and I need to look over all static amba device allocations if we
reintroduce this.

That said, the remaining static allocations (a.k.a. boardfiles) appear
to be very few and very limited, almost all systems use device tree
or ACPI or iterative dynamic allocation (from amba/bus.c functions)
of the amba devices now.

Do you think we can proceed with this patch or do you want me to
revert the split back?

FWIW the platform devices have the same problem, but I know
I know, two wrongs does not make one right :/

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


Re: [PATCH v3 06/19] dt-bindings: memory: tegra: Squash tegra20-gart into tegra20-mc

2018-08-28 Thread Dmitry Osipenko
On 28.08.2018 13:47, Thierry Reding wrote:
> On Mon, Aug 20, 2018 at 10:35:54PM +0300, Dmitry Osipenko wrote:
>> On 20.08.2018 22:27, Dmitry Osipenko wrote:
>>> On 20.08.2018 22:12, Rob Herring wrote:
 On Sat, Aug 18, 2018 at 06:54:17PM +0300, Dmitry Osipenko wrote:
> Splitting GART and Memory Controller wasn't a good decision that was made
> back in the day. Given that the GART driver hasn't ever been used by
> anything in the kernel, we decided that it will be better to correct the
> mistakes of the past and merge two bindings into a single one. In a result

 As a result...

> there is a DT ABI change for the Memory Controller that allows not to
> break newer kernels using older DT by introducing a new required property,
> the memory clock. Adding the new clock property also puts the tegra20-mc
> binding in line with the bindings of the later Tegra generations.

 I don't understand this part. It looks to me like you are breaking 
 compatibility. The driver failing to probe with an old DT is okay?
>>>
>>> Yes, DT compatibility is broken. New driver won't probe/load with the old 
>>> DT,
>>> that's what we want.
>>>
 OS's like OpenSUSE use new DTs with older kernel versions, so you should 
 consider how to not break them as well. I guess if all this is optional 
 or has been unused, then there shouldn't be a problem.
>>>
>>> That's interesting.. Memory Controller isn't optional, I guess we could 
>>> change
>>> compatible to "nvidia,tegra20-mc-gart".bled in kernels config by
>> default and driver is functional, but it's okay 
>>
>> * I meant it's not optional in a sense that it's enaif MC driver will stop 
>> to probe
>> with older kernels as it is used only for reporting memory errors.
> 
> Yeah, we don't really regress at runtime. The errors reported by the
> current driver are very rare, and even if you encounter them, they're
> pretty cryptic, so I think this is one of the exceptional cases where
> breaking the ABI "for the greater good" is acceptable.

It's now became apparent that factoring out EMC from MC isn't a good idea too
because MC need to interact with EMC and probably vice versa. Looks like we
should consider restructuring MC for all Tegra's.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: new SBus related DMA warnings in 4.18+git

2018-08-28 Thread Christoph Hellwig
Updated version including that has survived contact with a sparc cross
compiler below.  Note that this is on top of the previous patch adding
a dma_mask to struct platform_device, which I've included as well.
>From e22d27b9bf48c0e3d6eb106f596972c9357ed24d Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Mon, 27 Aug 2018 17:23:24 +0200
Subject: driver core: initialize a default DMA mask for platform device

We still treat devices without a DMA mask as defaulting to 32-bits for
both mask, but a few releases ago we've started warning about such
cases, as they require special cases to work around this sloppyness.
Add a dma_mask field to struct platform_object so that we can initialize
the dma_mask pointer in struct device and initialize both masks to
32-bits by default.  Architectures can still override this in
arch_setup_pdev_archdata if needed.

Note that the code looks a little odd with the various conditionals
because we have to support platform_device structures that are
statically allocated.

Signed-off-by: Christoph Hellwig 
---
 drivers/base/platform.c | 15 +--
 include/linux/platform_device.h |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index dff82a3c2caa..baf4b06cf2d9 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -225,6 +225,17 @@ struct platform_object {
 	char name[];
 };
 
+static void setup_pdev_archdata(struct platform_device *pdev)
+{
+	if (!pdev->dev.coherent_dma_mask)
+		pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+	if (!pdev->dma_mask)
+		pdev->dma_mask = DMA_BIT_MASK(32);
+	if (!pdev->dev.dma_mask)
+		pdev->dev.dma_mask = &pdev->dma_mask;
+	arch_setup_pdev_archdata(pdev);
+};
+
 /**
  * platform_device_put - destroy a platform device
  * @pdev: platform device to free
@@ -271,7 +282,7 @@ struct platform_device *platform_device_alloc(const char *name, int id)
 		pa->pdev.id = id;
 		device_initialize(&pa->pdev.dev);
 		pa->pdev.dev.release = platform_device_release;
-		arch_setup_pdev_archdata(&pa->pdev);
+		setup_pdev_archdata(&pa->pdev);
 	}
 
 	return pa ? &pa->pdev : NULL;
@@ -472,7 +483,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
 int platform_device_register(struct platform_device *pdev)
 {
 	device_initialize(&pdev->dev);
-	arch_setup_pdev_archdata(pdev);
+	setup_pdev_archdata(pdev);
 	return platform_device_add(pdev);
 }
 EXPORT_SYMBOL_GPL(platform_device_register);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 1a9f38f27f65..d9dc4883d5fb 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -25,6 +25,7 @@ struct platform_device {
 	int		id;
 	bool		id_auto;
 	struct device	dev;
+	u64		dma_mask;
 	u32		num_resources;
 	struct resource	*resource;
 
-- 
2.18.0

>From b4c6e6779559a1bcda41fad0f2e8b713fcf96446 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Tue, 28 Aug 2018 11:25:51 +0200
Subject: sparc: set a default 32-bit dma mask for OF devices

This keeps the historic default behavior for devices without a DMA mask,
but removes the warning about a lacking DMA mask for doing DMA without
a mask.

Signed-off-by: Christoph Hellwig 
---
 arch/sparc/kernel/of_device_32.c | 5 +
 arch/sparc/kernel/of_device_64.c | 4 
 2 files changed, 9 insertions(+)

diff --git a/arch/sparc/kernel/of_device_32.c b/arch/sparc/kernel/of_device_32.c
index 3641a294ed54..7f3dec7e1e78 100644
--- a/arch/sparc/kernel/of_device_32.c
+++ b/arch/sparc/kernel/of_device_32.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -381,6 +382,10 @@ static struct platform_device * __init scan_one_device(struct device_node *dp,
 	else
 		dev_set_name(&op->dev, "%08x", dp->phandle);
 
+	op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+	op->dev.dma_mask = &op->dma_mask;
+	op->dma_mask = DMA_BIT_MASK(32);
+
 	if (of_device_register(op)) {
 		printk("%s: Could not register of device.\n",
 		   dp->full_name);
diff --git a/arch/sparc/kernel/of_device_64.c b/arch/sparc/kernel/of_device_64.c
index 44e4d4435bed..d94c31822da1 100644
--- a/arch/sparc/kernel/of_device_64.c
+++ b/arch/sparc/kernel/of_device_64.c
@@ -2,6 +2,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -675,6 +676,9 @@ static struct platform_device * __init scan_one_device(struct device_node *dp,
 		dev_set_name(&op->dev, "root");
 	else
 		dev_set_name(&op->dev, "%08x", dp->phandle);
+	op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+	op->dev.dma_mask = &op->dma_mask;
+	op->dma_mask = DMA_BIT_MASK(32);
 
 	if (of_device_register(op)) {
 		printk("%s: Could not register of device.\n",
-- 
2.18.0

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

Re: [PATCH v3 06/19] dt-bindings: memory: tegra: Squash tegra20-gart into tegra20-mc

2018-08-28 Thread Thierry Reding
On Mon, Aug 20, 2018 at 10:35:54PM +0300, Dmitry Osipenko wrote:
> On 20.08.2018 22:27, Dmitry Osipenko wrote:
> > On 20.08.2018 22:12, Rob Herring wrote:
> >> On Sat, Aug 18, 2018 at 06:54:17PM +0300, Dmitry Osipenko wrote:
> >>> Splitting GART and Memory Controller wasn't a good decision that was made
> >>> back in the day. Given that the GART driver hasn't ever been used by
> >>> anything in the kernel, we decided that it will be better to correct the
> >>> mistakes of the past and merge two bindings into a single one. In a result
> >>
> >> As a result...
> >>
> >>> there is a DT ABI change for the Memory Controller that allows not to
> >>> break newer kernels using older DT by introducing a new required property,
> >>> the memory clock. Adding the new clock property also puts the tegra20-mc
> >>> binding in line with the bindings of the later Tegra generations.
> >>
> >> I don't understand this part. It looks to me like you are breaking 
> >> compatibility. The driver failing to probe with an old DT is okay?
> > 
> > Yes, DT compatibility is broken. New driver won't probe/load with the old 
> > DT,
> > that's what we want.
> > 
> >> OS's like OpenSUSE use new DTs with older kernel versions, so you should 
> >> consider how to not break them as well. I guess if all this is optional 
> >> or has been unused, then there shouldn't be a problem.
> > 
> > That's interesting.. Memory Controller isn't optional, I guess we could 
> > change
> > compatible to "nvidia,tegra20-mc-gart".
> 
> * I meant it's not optional in a sense that it's enabled in kernels config by
> default and driver is functional, but it's okay if MC driver will stop to 
> probe
> with older kernels as it is used only for reporting memory errors.

Yeah, we don't really regress at runtime. The errors reported by the
current driver are very rare, and even if you encounter them, they're
pretty cryptic, so I think this is one of the exceptional cases where
breaking the ABI "for the greater good" is acceptable.

Thierry


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

Re: new SBus related DMA warnings in 4.18+git

2018-08-28 Thread Meelis Roos
> Based on the other thread here is a new attempt:

  CC  arch/sparc/kernel/of_device_64.o
arch/sparc/kernel/of_device_64.c: In function ‘scan_one_device’:
arch/sparc/kernel/of_device_64.c:678:2: error: implicit declaration of function 
‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration]
  op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
  ^
arch/sparc/kernel/of_device_64.c:679:24: error: ‘struct platform_device’ has no 
member named ‘dma_mask’
  op->dev.dma_mask = &op->dma_mask;
^
arch/sparc/kernel/of_device_64.c:680:4: error: ‘struct platform_device’ has no 
member named ‘dma_mask’
  op->dma_mask = DMA_BIT_MASK(32);
^
cc1: all warnings being treated as errors
scripts/Makefile.build:307: recipe for target 
'arch/sparc/kernel/of_device_64.o' failed


-- 
Meelis Roos (mr...@linux.ee)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: new SBus related DMA warnings in 4.18+git

2018-08-28 Thread Christoph Hellwig
On Mon, Aug 27, 2018 at 10:04:58PM +0300, Meelis Roos wrote:
> > On Sun, Aug 26, 2018 at 10:48:44AM +0300, Meelis Roos wrote:
> > > Tried yesterdays git 4.18.0-12789-gaa5b105 on a Sun Ultra 1 with SBus 
> > > and several SBus connected devicess give DMA mapping related warnings:
> > 
> > This should have been around since the warning was added.
> > 
> > The patch below should fix it:
> > 
> > ---
> > >From 6294e0e330851ee06e66ab85b348f1d92d375d7a Mon Sep 17 00:00:00 2001
> > From: Christoph Hellwig 
> > Date: Mon, 27 Aug 2018 17:23:24 +0200
> > Subject: driver core: initialize a default DMA mask for platform device
> 
> No, it does not fix it. Dmesg from another SBus machine that conmpiled 
> it faster (Sun Ultra 2):

Based on the other thread here is a new attempt:

---
diff --git a/arch/sparc/kernel/of_device_32.c b/arch/sparc/kernel/of_device_32.c
index 3641a294ed54..1738f7f12a9f 100644
--- a/arch/sparc/kernel/of_device_32.c
+++ b/arch/sparc/kernel/of_device_32.c
@@ -381,6 +381,10 @@ static struct platform_device * __init 
scan_one_device(struct device_node *dp,
else
dev_set_name(&op->dev, "%08x", dp->phandle);
 
+   op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   op->dev.dma_mask = &op->dma_mask;
+   op->dma_mask = DMA_BIT_MASK(32);
+
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
   dp->full_name);
diff --git a/arch/sparc/kernel/of_device_64.c b/arch/sparc/kernel/of_device_64.c
index 44e4d4435bed..60ebb1f976ab 100644
--- a/arch/sparc/kernel/of_device_64.c
+++ b/arch/sparc/kernel/of_device_64.c
@@ -675,6 +675,9 @@ static struct platform_device * __init 
scan_one_device(struct device_node *dp,
dev_set_name(&op->dev, "root");
else
dev_set_name(&op->dev, "%08x", dp->phandle);
+   op->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   op->dev.dma_mask = &op->dma_mask;
+   op->dma_mask = DMA_BIT_MASK(32);
 
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: new SBus related DMA warnings in 4.18+git

2018-08-28 Thread Christoph Hellwig
On Mon, Aug 27, 2018 at 01:41:36PM -0700, David Miller wrote:
> From: Sam Ravnborg 
> Date: Mon, 27 Aug 2018 21:15:18 +0200
> 
> > Why not fix the drivers rather than papering over the fact
> > that they are missing the DMA mask?
> > 
> > We are in the lucky situation the Meelis can test any patches.
> 
> SBUS drivers never set the mask because they never needed to and could
> use the default, since SBUS IOMMUs were quite universal in nature.

In addition to that the historical expectation is that you can do
dma to platform_devices without setup, and for some reason sbus
devices all show up as platform_devices.

> I think it's more error prone to add the mask setting to every single
> SBUS driver than to just have the core setup the default properly.

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


Re: [PATCH] of/platform: Initialise AMBA default DMA masks

2018-08-28 Thread Christoph Hellwig
>   /* setup generic device info */
> + dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> + if (!dev->dev.dma_mask)
> + dev->dev.dma_mask = &dev->dev.coherent_dma_mask;

We should never set dma_mask to point to the coherent_dma_mask,
as that will cause problems with devices that have different
mask for the two.

How about something like this?

---
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7ba90c290a42..c04ed124305c 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -242,6 +242,9 @@ static struct amba_device *of_amba_device_create(struct 
device_node *node,
goto err_clear_flag;
 
/* setup generic device info */
+   dev->dma_mask = DMA_BIT_MASK(32);
+   dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   dev->dev.dma_mask = &dev->dma_mask;
dev->dev.of_node = of_node_get(node);
dev->dev.fwnode = &node->fwnode;
dev->dev.parent = parent ? : &platform_bus;
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index d143c13bed26..fbc7adf3ca54 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -34,6 +34,7 @@ struct amba_device {
unsigned intperiphid;
unsigned intirq[AMBA_NR_IRQS];
char*driver_override;
+   u64 dma_mask;
 };
 
 struct amba_driver {
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] of/platform: Initialise AMBA default DMA masks

2018-08-28 Thread Linus Walleij
commit a5516219b102 ("of/platform: Initialise default DMA masks")
sets up the coherent_dma_mask of platform devices created
from the device tree, but fails to do the same for AMBA
(PrimeCell) devices.

This leads to a regression in kernel v4.19-rc1 triggering the
WARN_ONCE() in kernel/dma/coherent.c, dma_alloc_attrs()
WARN_ON_ONCE(dev && !dev->coherent_dma_mask):

[ cut here ]
WARNING: CPU: 0 PID: 1 at ../include/linux/dma-mapping.h:522 
drm_gem_cma_create+0x1dc/0x21c
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.19.0-rc1+ #15
Hardware name: ARM-Versatile (Device Tree Support)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (__warn+0xcc/0xf4)
[] (__warn) from [] (warn_slowpath_null+0x3c/0x48)
[] (warn_slowpath_null) from [] 
(drm_gem_cma_create+0x1dc/0x21c)
[] (drm_gem_cma_create) from [] 
(drm_gem_cma_dumb_create+0x44/0x98)
[] (drm_gem_cma_dumb_create) from [] 
(drm_client_framebuffer_create+0x80/0x204)
[] (drm_client_framebuffer_create) from [] 
(drm_fb_helper_generic_probe+0x4c/0x200)
[] (drm_fb_helper_generic_probe) from [] 
(__drm_fb_helper_initial_config_and_unlock+0x1cc/0x454)
[] (__drm_fb_helper_initial_config_and_unlock) from [] 
(drm_fb_helper_fbdev_setup+0x104/0x218)
[] (drm_fb_helper_fbdev_setup) from [] 
(drm_fbdev_cma_init+0x7c/0xac)
[] (drm_fbdev_cma_init) from [] 
(drm_fb_cma_fbdev_init+0x8/0x14)
[] (drm_fb_cma_fbdev_init) from [] 
(pl111_amba_probe+0x3c8/0x4a4)
[] (pl111_amba_probe) from [] (amba_probe+0xd8/0x154)
[] (amba_probe) from [] (really_probe+0x200/0x2ac)
[] (really_probe) from [] (driver_probe_device+0x5c/0x168)
[] (driver_probe_device) from [] (__driver_attach+0xd0/0xd4)
[] (__driver_attach) from [] (bus_for_each_dev+0x70/0xb4)
[] (bus_for_each_dev) from [] (bus_add_driver+0x170/0x204)
[] (bus_add_driver) from [] (driver_register+0x74/0x108)
[] (driver_register) from [] (do_one_initcall+0x48/0x1a0)
[] (do_one_initcall) from [] 
(kernel_init_freeable+0x104/0x1c4)
[] (kernel_init_freeable) from [] (kernel_init+0x8/0xf0)
[] (kernel_init) from [] (ret_from_fork+0x14/0x34)
Exception stack(0xc781ffb0 to 0xc781fff8)
ffa0:    
ffc0:        
ffe0:     0013 
---[ end trace 2dc47eb796bde006 ]---

This regresses the PL111 DRM driver in drivers/gpu/drm/pl111
that uses the AMBA PrimeCell to instantiate the frame buffer
device, as it cannot allocate a chunk of coherent memory
anymore due to the missing mask.

Fixes: a5516219b102 ("of/platform: Initialise default DMA masks")
Cc: Robin Murphy 
Cc: Rob Herring 
Cc: Christoph Hellwig 
Cc: Eric Anholt 
Cc: Noralf Trønnes 
Signed-off-by: Linus Walleij 
---
I don't know which tree Robins patch came in from, but I assume
Christoph's, so can you carry this patch as well?
---
 drivers/of/platform.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7ba90c290a42..7435c79ca56d 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -242,6 +242,9 @@ static struct amba_device *of_amba_device_create(struct 
device_node *node,
goto err_clear_flag;
 
/* setup generic device info */
+   dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+   if (!dev->dev.dma_mask)
+   dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
dev->dev.of_node = of_node_get(node);
dev->dev.fwnode = &node->fwnode;
dev->dev.parent = parent ? : &platform_bus;
-- 
2.17.1

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

Re: [PATCH v5 01/23] iommu: introduce bind_pasid_table API function

2018-08-28 Thread Auger Eric
Hi Jacob,

On 08/28/2018 07:14 AM, Jacob Pan wrote:
> On Fri, 24 Aug 2018 17:00:51 +0200
> Auger Eric  wrote:
> 
>> Hi Jacob,
>>
>> On 05/11/2018 10:53 PM, Jacob Pan wrote:
>>> Virtual IOMMU was proposed to support Shared Virtual Memory (SVM)
>>> use in the guest:
>>> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
>>>
>>> As part of the proposed architecture, when an SVM capable PCI
>>> device is assigned to a guest, nested mode is turned on. Guest owns
>>> the first level page tables (request with PASID) which performs
>>> GVA->GPA translation. Second level page tables are owned by the
>>> host for GPA->HPA translation for both request with and without
>>> PASID.
>>>
>>> A new IOMMU driver interface is therefore needed to perform tasks as
>>> follows:
>>> * Enable nested translation and appropriate translation type
>>> * Assign guest PASID table pointer (in GPA) and size to host IOMMU
>>>
>>> This patch introduces new API functions to perform bind/unbind
>>> guest PASID tables. Based on common data, model specific IOMMU
>>> drivers can be extended to perform the specific steps for binding
>>> pasid table of assigned devices.
>>>
>>> Signed-off-by: Jean-Philippe Brucker 
>>> Signed-off-by: Liu, Yi L 
>>> Signed-off-by: Ashok Raj 
>>> Signed-off-by: Jacob Pan 
>>> ---
>>>  drivers/iommu/iommu.c  | 19 +++
>>>  include/linux/iommu.h  | 24 
>>>  include/uapi/linux/iommu.h | 33 +
>>>  3 files changed, 76 insertions(+)
>>>  create mode 100644 include/uapi/linux/iommu.h
>>>
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index d2aa2320..3a69620 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -1325,6 +1325,25 @@ int iommu_attach_device(struct iommu_domain
>>> *domain, struct device *dev) }
>>>  EXPORT_SYMBOL_GPL(iommu_attach_device);
>>>  
>>> +int iommu_bind_pasid_table(struct iommu_domain *domain, struct
>>> device *dev,
>>> +   struct pasid_table_config *pasidt_binfo)  
>> As Jean-Philippe, I must confessed i am very confused by having both
>> the iommu_domain and dev passed as argument.
>>
>> I know this was discussed when the RFC was submitted and maybe I
>> missed the main justification behind that choice. I understand that
>> at the HW level we want to change the context entry or ARM CD in my
>> case for a specific device. But on other hand, at the logical level,
>> I understand the iommu_domain is representing a set of translation
>> config & page tables shared by all the devices within the domain
>> (hope this is fundamentally correct ?!). So to me we can't change the
>> device translation setup without changing the whole iommu_device setup
>> otherwise this would mean this device has a translation configuration
>> that is not consistent anymore with the other devices in the same
>> domain. Is that correct? So can't we only keep the iommu_domain arg?
>>
> I agree with you on your understanding of HW and logical level. I think
> there is a new twist to the definition of domain introduced by having
> PASID and vSVA. Up until now, domain only means 2nd level mapping. In
> that sense, bind guest PASID table does not alter domain. For VT-d 2.5
> spec. implementation of the bind_pasid_table(), we needed some per
> device data, also flags such as indication for IO page fault handling.
> 
> Anyway, for the new VT-d 3.0 spec. we no longer need this API. In
> stead, I will introduce bind_guest_pasid() API, where per device PASID
> table is allocated by the host.

So what is the exact state of this series? Is it outdated as you don't
target VT-d 2.5 anymore? Will you keep the rest of the API?

Thanks

Eric
> 
>>> +{
>>> +   if (unlikely(!domain->ops->bind_pasid_table))
>>> +   return -ENODEV;
>>> +
>>> +   return domain->ops->bind_pasid_table(domain, dev,
>>> pasidt_binfo); +}
>>> +EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
>>> +
>>> +void iommu_unbind_pasid_table(struct iommu_domain *domain, struct
>>> device *dev) +{
>>> +   if (unlikely(!domain->ops->unbind_pasid_table))
>>> +   return;
>>> +
>>> +   domain->ops->unbind_pasid_table(domain, dev);
>>> +}
>>> +EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
>>> +
>>>  static void __iommu_detach_device(struct iommu_domain *domain,
>>>   struct device *dev)
>>>  {
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 19938ee..5199ca4 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -25,6 +25,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  
>>>  #define IOMMU_READ (1 << 0)
>>>  #define IOMMU_WRITE(1 << 1)
>>> @@ -187,6 +188,8 @@ struct iommu_resv_region {
>>>   * @domain_get_windows: Return the number of windows for a domain
>>>   * @of_xlate: add OF master IDs to iommu grouping
>>>   * @pgsize_bitmap: bitmap of all possible supported page sizes
>>> + * @bind_pasid_table: bind pasid table 

[PATCH 1/1] swiotlb: fix comment on swiotlb_bounce()

2018-08-28 Thread Dongli Zhang
Fix the comment as swiotlb_bounce() is used to copy from original dma
location to swiotlb buffer during swiotlb_tbl_map_single(), while to
copy from swiotlb buffer to original dma location during
swiotlb_tbl_unmap_single().

Signed-off-by: Dongli Zhang 
---
 kernel/dma/swiotlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 4f8a6db..8fd4cfe 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -435,7 +435,7 @@ int is_swiotlb_buffer(phys_addr_t paddr)
 }
 
 /*
- * Bounce: copy the swiotlb buffer back to the original dma location
+ * Bounce: copy the swiotlb buffer from or back to the original dma location
  */
 static void swiotlb_bounce(phys_addr_t orig_addr, phys_addr_t tlb_addr,
   size_t size, enum dma_data_direction dir)
-- 
2.7.4

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


Re: [PATCH 4/5] iommu/arm-smmu: Make way to add Qcom's smmu-500 errata handling

2018-08-28 Thread Vivek Gautam

Hi Robin,


On 8/14/2018 10:29 PM, Robin Murphy wrote:

On 14/08/18 11:55, Vivek Gautam wrote:

Cleanup to re-use some of the stuff

Signed-off-by: Vivek Gautam 
---
  drivers/iommu/arm-smmu.c | 32 +---
  1 file changed, 25 insertions(+), 7 deletions(-)


I think the overall diffstat would be an awful lot smaller if the 
erratum workaround just has its own readl_poll_timeout() as it does in 
the vendor kernel. The burst-polling loop is for minimising latency in 
high-throughput situations, and if you're in a workaround which has to 
lock *every* register write and issue two firmware calls around each 
sync I think you're already well out of that game.


Sorry for the delayed response. I was on vacation.
I will fix this in my next version by adding the separate 
read_poll_timeout() for the erratum WA.





diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 32e86df80428..75c146751c87 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -391,21 +391,31 @@ static void __arm_smmu_free_bitmap(unsigned 
long *map, int idx)

  clear_bit(idx, map);
  }
  -/* Wait for any pending TLB invalidations to complete */
-static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu,
-    void __iomem *sync, void __iomem *status)
+static int __arm_smmu_tlb_sync_wait(void __iomem *status)
  {
  unsigned int spin_cnt, delay;
  -    writel_relaxed(0, sync);
  for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) {
  for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) {
  if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE))
-    return;
+    return 0;
  cpu_relax();
  }
  udelay(delay);
  }
+
+    return -EBUSY;
+}
+
+/* Wait for any pending TLB invalidations to complete */
+static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu,
+    void __iomem *sync, void __iomem *status)
+{
+    writel_relaxed(0, sync);
+
+    if (!__arm_smmu_tlb_sync_wait(status))
+    return;
+
  dev_err_ratelimited(smmu->dev,
  "TLB sync timed out -- SMMU may be deadlocked\n");
  }
@@ -461,8 +471,9 @@ static void arm_smmu_tlb_inv_context_s2(void 
*cookie)

  arm_smmu_tlb_sync_global(smmu);
  }
  -static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, 
size_t size,

-  size_t granule, bool leaf, void *cookie)
+static void __arm_smmu_tlb_inv_range_nosync(unsigned long iova, 
size_t size,

+    size_t granule, bool leaf,
+    void *cookie)
  {
  struct arm_smmu_domain *smmu_domain = cookie;
  struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
@@ -498,6 +509,13 @@ static void 
arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,

  }
  }
  +static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, 
size_t size,

+  size_t granule, bool leaf,
+  void *cookie)
+{
+    __arm_smmu_tlb_inv_range_nosync(iova, size, granule, leaf, cookie);
+}
+


AFAICS even after patch #5 this does absolutely nothing except make 
the code needlessly harder to read :(


Sure, I will rather call arm_smmu_tlb_inv_range_nosync() from
qcom_errata_tlb_inv_range_nosync() then make this change.
Thanks for the review.

Best regards
Vivek



Robin.


  /*
   * On MMU-401 at least, the cost of firing off multiple TLBIVMIDs 
appears
   * almost negligible, but the benefit of getting the first one in 
as far ahead




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