Re: [PATCH next] iommu: intel: make DMAR_TABLE select IOMMU_API

2020-10-12 Thread Lu Baolu

Hi,

On 10/12/20 8:31 PM, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

Since commit c40c1018 ("iommu/vt-d: Gracefully handle DMAR units
with no supported address widths") dmar.c needs struct iommu_device to
be defined. We need to unconditionally select IOMMU_API when DMAR_TABLE
is selected. This fixes the following build error when IOMMU_API is not
selected:

drivers/iommu/intel/dmar.c: In function ‘free_iommu’:
drivers/iommu/intel/dmar.c:1139:41: error: ‘struct iommu_device’ has no member 
named ‘ops’
  1139 |  if (intel_iommu_enabled && iommu->iommu.ops) {


Thanks!

How about making it symmetric with the registration code?

if (intel_iommu_enabled && !iommu->drhd->ignored)

Best regards,
baolu


 ^

Fixes: c40c1018 ("iommu/vt-d: Gracefully handle DMAR units with no supported 
address widths")
Signed-off-by: Bartosz Golaszewski 
---
  drivers/iommu/intel/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
index 5337ee1584b0..f814b7585ba8 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -1,13 +1,13 @@
  # SPDX-License-Identifier: GPL-2.0-only
  # Intel IOMMU support
  config DMAR_TABLE
+   select IOMMU_API
bool
  
  config INTEL_IOMMU

bool "Support for Intel IOMMU using DMA Remapping Devices"
depends on PCI_MSI && ACPI && (X86 || IA64)
select DMA_OPS
-   select IOMMU_API
select IOMMU_IOVA
select NEED_DMA_MAP_STATE
select DMAR_TABLE


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

Re: [PATCH v4 0/7] Convert the intel iommu driver to the dma-iommu api

2020-10-12 Thread Lu Baolu

Hi Tvrtko,

On 10/12/20 4:44 PM, Tvrtko Ursulin wrote:


On 29/09/2020 01:11, Lu Baolu wrote:

Hi Tvrtko,

On 9/28/20 5:44 PM, Tvrtko Ursulin wrote:


On 27/09/2020 07:34, Lu Baolu wrote:

Hi,

The previous post of this series could be found here.

https://lore.kernel.org/linux-iommu/20200912032200.11489-1-baolu...@linux.intel.com/ 



This version introduce a new patch [4/7] to fix an issue reported here.

https://lore.kernel.org/linux-iommu/51a1baec-48d1-c0ac-181b-1fba92aa4...@linux.intel.com/ 



There aren't any other changes.

Please help to test and review.

Best regards,
baolu

Lu Baolu (3):
   iommu: Add quirk for Intel graphic devices in map_sg


Since I do have patches to fix i915 to handle this, do we want to 
co-ordinate the two and avoid having to add this quirk and then later 
remove it? Or you want to go the staged approach?


I have no preference. It depends on which patch goes first. Let the
maintainers help here.


FYI we have merged the required i915 patches to out tree last week or 
so. I *think* this means they will go into 5.11. So the i915 specific 
workaround patch will not be needed in Intel IOMMU.


Thanks for letting us know this. I will drop the workaround patch and
test the whole series after the next rc1.

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

Re: [PATCH v3 01/24] dt-bindings: iommu: mediatek: Convert IOMMU to DT schema

2020-10-12 Thread Krzysztof Kozlowski
On Tue, 6 Oct 2020 at 06:27, Yong Wu  wrote:
>
> On Fri, 2020-10-02 at 13:07 +0200, Krzysztof Kozlowski wrote:
> > On Wed, Sep 30, 2020 at 03:06:24PM +0800, Yong Wu wrote:
> > > Convert MediaTek IOMMU to DT schema.
> > >
> > > Signed-off-by: Yong Wu 
> > > ---
> > >  .../bindings/iommu/mediatek,iommu.txt | 103 
> > >  .../bindings/iommu/mediatek,iommu.yaml| 154 ++
> > >  2 files changed, 154 insertions(+), 103 deletions(-)
> > >  delete mode 100644 
> > > Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > >
>
> [...]
>
> > > +properties:
> > > +  compatible:
> > > +oneOf:
> > > +  - enum:
> > > +  - mediatek,mt2701-m4u # mt2701 generation one HW
> > > +  - mediatek,mt2712-m4u # mt2712 generation two HW
> > > +  - mediatek,mt6779-m4u # mt6779 generation two HW
> > > +  - mediatek,mt8173-m4u # mt8173 generation two HW
> > > +  - mediatek,mt8183-m4u # mt8183 generation two HW
> > > +
> > > +  - description: mt7623 generation one HW
> > > +items:
> > > +  - const: mediatek,mt7623-m4u
> > > +  - const: mediatek,mt2701-m4u
> > > +
> > > +  reg:
> > > +maxItems: 1
> > > +
> > > +  interrupts:
> > > +maxItems: 1
> > > +
> > > +  clocks:
> > > +description: |
> > > +  bclk is optional. here is the list which require this bclk:
> > > +  mt2701, mt2712, mt7623 and mt8173.
> >
> > Similarly to my comment in other patch, this should be part of schema
> > within 'if-then'.
>
> Thanks for the review.
>
> I will change like this:
>
> =
>   clocks:
> items:
>   - description: bclk is the block clock.
>
>   clock-names:
> items:
>   - const: bclk
>
> required:
>   - compatible
>   - reg
>   - interrupts
>   - mediatek,larbs
>   - '#iommu-cells'
> if:
>   properties:
> compatible:
>   contains:
> enum:
>   - mediatek,mt2701-m4u
>   - mediatek,mt2712-m4u
>   - mediatek,mt8173-m4u
>
> then:
>  required:
>- clocks
> ==
>
> If this is not right, please tell me.
> (dt_binding_check is ok.)

Looks fine, except "if" should be part of some "allOf" block.

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


Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

2020-10-12 Thread Rob Herring
On Sat, Oct 10, 2020 at 10:12 AM Nicolas Saenz Julienne
 wrote:
>
> The function provides the CPU physical address addressable by the most
> constrained bus in the system. It might be useful in order to
> dynamically set up memory zones during boot.
>
> Signed-off-by: Nicolas Saenz Julienne 
> ---
>  drivers/of/address.c | 34 ++
>  include/linux/of.h   |  7 +++
>  2 files changed, 41 insertions(+)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index eb9ab4f1e80b..755e97b65096 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const 
> struct bus_dma_region **map)
>  }
>  #endif /* CONFIG_HAS_DMA */
>
> +/**
> + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> + *
> + * Gets the CPU physical address limit for safe DMA addressing system wide by
> + * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
> + */
> +u64 __init of_dma_safe_phys_limit(void)
> +{
> +   struct device_node *np = NULL;
> +   struct of_range_parser parser;
> +   const __be32 *ranges = NULL;
> +   u64 phys_dma_limit = ~0ULL;
> +   struct of_range range;
> +   int len;
> +
> +   for_each_of_allnodes(np) {
> +   dma_addr_t cpu_end = 0;
> +
> +   ranges = of_get_property(np, "dma-ranges", );
> +   if (!ranges || !len)
> +   continue;
> +
> +   of_dma_range_parser_init(, np);
> +   for_each_of_range(, )
> +   if (range.cpu_addr + range.size > cpu_end)
> +   cpu_end = range.cpu_addr + range.size;

This doesn't work if you have more than one level of dma-ranges. The
address has to be translated first. It should be okay to do that on
the start or end address (if not, your DT is broken).

Please add/extend a unittest for this.

> +
> +   if (phys_dma_limit > cpu_end)
> +   phys_dma_limit = cpu_end;
> +   }
> +
> +   return phys_dma_limit;
> +}
> +
>  /**
>   * of_dma_is_coherent - Check if device is coherent
>   * @np:device node
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 481ec0467285..958c64cffa92 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
>const char *map_name, const char *map_mask_name,
>struct device_node **target, u32 *id_out);
>
> +u64 of_dma_safe_phys_limit(void);
> +
>  #else /* CONFIG_OF */
>
>  static inline void of_core_init(void)
> @@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 
> id,
> return -EINVAL;
>  }
>
> +static inline u64 of_dma_safe_phys_limit(void)
> +{
> +   return ~0ULL;
> +}
> +
>  #define of_match_ptr(_ptr) NULL
>  #define of_match_node(_matches, _node) NULL
>  #endif /* CONFIG_OF */
> --
> 2.28.0
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()

2020-10-12 Thread Ard Biesheuvel
On Mon, 12 Oct 2020 at 13:37, Catalin Marinas  wrote:
>
> On Sat, Oct 10, 2020 at 05:12:31PM +0200, Nicolas Saenz Julienne wrote:
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index f6902a2b4ea6..0eca5865dcb1 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -196,14 +196,16 @@ static void __init zone_sizes_init(unsigned long min, 
> > unsigned long max)
> >   unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
> >
> >  #ifdef CONFIG_ZONE_DMA
> > + zone_dma_bits = ARM64_ZONE_DMA_BITS;
> > +
> >   if (IS_ENABLED(CONFIG_ACPI)) {
> >   extern unsigned int acpi_iort_get_zone_dma_size(void);
> >
> >   zone_dma_bits = min(zone_dma_bits,
> >   acpi_iort_get_zone_dma_size());
> > - arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
> >   }
> >
> > + arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
> >   max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> >  #endif
> >  #ifdef CONFIG_ZONE_DMA32
> > @@ -394,11 +396,6 @@ void __init arm64_memblock_init(void)
> >
> >   early_init_fdt_scan_reserved_mem();
> >
> > - if (IS_ENABLED(CONFIG_ZONE_DMA)) {
> > - zone_dma_bits = ARM64_ZONE_DMA_BITS;
> > - arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
> > - }
>
> arm64_dma_phys_limit is used by memblock_alloc_low() (via
> ARCH_LOW_ADDRESS_LIMIT). I think it's too late to leave its
> initialisation to zone_sizes_init().
>

The only generic caller of memblock_alloc_low() is swiotlb_init(),
which is called much later. So at that point, we definitely need
ARCH_LOW_ADDRESS_LIMIT to be set correctly, but that means doing it in
zone_sizes_init() is early enough.

So the only problematic reference seems to be crashkernel_reserve() afaict.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH next] iommu: intel: make DMAR_TABLE select IOMMU_API

2020-10-12 Thread Bartosz Golaszewski
From: Bartosz Golaszewski 

Since commit c40c1018 ("iommu/vt-d: Gracefully handle DMAR units
with no supported address widths") dmar.c needs struct iommu_device to
be defined. We need to unconditionally select IOMMU_API when DMAR_TABLE
is selected. This fixes the following build error when IOMMU_API is not
selected:

drivers/iommu/intel/dmar.c: In function ‘free_iommu’:
drivers/iommu/intel/dmar.c:1139:41: error: ‘struct iommu_device’ has no member 
named ‘ops’
 1139 |  if (intel_iommu_enabled && iommu->iommu.ops) {
^

Fixes: c40c1018 ("iommu/vt-d: Gracefully handle DMAR units with no 
supported address widths")
Signed-off-by: Bartosz Golaszewski 
---
 drivers/iommu/intel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
index 5337ee1584b0..f814b7585ba8 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -1,13 +1,13 @@
 # SPDX-License-Identifier: GPL-2.0-only
 # Intel IOMMU support
 config DMAR_TABLE
+   select IOMMU_API
bool
 
 config INTEL_IOMMU
bool "Support for Intel IOMMU using DMA Remapping Devices"
depends on PCI_MSI && ACPI && (X86 || IA64)
select DMA_OPS
-   select IOMMU_API
select IOMMU_IOVA
select NEED_DMA_MAP_STATE
select DMAR_TABLE
-- 
2.28.0

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

Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema

2020-10-12 Thread Krzysztof Kozlowski
On Mon, 12 Oct 2020 at 14:02, Yong Wu  wrote:
>
> On Mon, 2020-10-12 at 09:18 +0200, Krzysztof Kozlowski wrote:
> > On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> > > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > > > On Tue, 6 Oct 2020 at 06:27, Yong Wu  wrote:
> > > > >
> > > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > > > Convert MediaTek SMI to DT schema.
> > > > > > >
> > > > > > > Signed-off-by: Yong Wu 
> > > > > > > ---
> > > > > > >  .../mediatek,smi-common.txt   |  49 -
> > > > > > >  .../mediatek,smi-common.yaml  | 100 
> > > > > > > ++
> > > > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 -
> > > > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 
> > > > > > > 
> > > > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > > > >  delete mode 100644 
> > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > > > >  create mode 100644 
> > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > > > >  delete mode 100644 
> > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > > > >  create mode 100644 
> > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > > > ...
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +oneOf:
> > > > > > > +  - enum:
> > > > > > > +  - mediatek,mt2701-smi-common
> > > > > > > +  - mediatek,mt2712-smi-common
> > > > > > > +  - mediatek,mt6779-smi-common
> > > > > > > +  - mediatek,mt8173-smi-common
> > > > > > > +  - mediatek,mt8183-smi-common
> > > > > > > +
> > > > > > > +  - description: for mt7623
> > > > > > > +items:
> > > > > > > +  - const: mediatek,mt7623-smi-common
> > > > > > > +  - const: mediatek,mt2701-smi-common
> > > > > > > +
> > > > > > > +  reg:
> > > > > > > +maxItems: 1
> > > > > > > +
> > > > > > > +  clocks:
> > > > > > > +description: |
> > > > > > > +  apb and smi are mandatory. the async is only for 
> > > > > > > generation 1 smi HW.
> > > > > > > +  gals(global async local sync) also is optional, here is 
> > > > > > > the list which
> > > > > > > +  require gals: mt6779 and mt8183.
> > > > > > > +minItems: 2
> > > > > > > +maxItems: 4
> > > > > > > +items:
> > > > > > > +  - description: apb is Advanced Peripheral Bus clock, It's 
> > > > > > > the clock for
> > > > > > > +  setting the register.
> > > > > > > +  - description: smi is the clock for transfer data and 
> > > > > > > command.
> > > > > > > +  - description: async is asynchronous clock, it help 
> > > > > > > transform the smi clock
> > > > > > > +  into the emi clock domain.
> > > > > > > +  - description: gals0 is the path0 clock of gals.
> > > > > > > +  - description: gals1 is the path1 clock of gals.
> > > > > > > +
> > > > > > > +  clock-names:
> > > > > > > +oneOf:
> > > > > > > +  - items:
> > > > > > > +  - const: apb
> > > > > > > +  - const: smi
> > > > > > > +  - items:
> > > > > > > +  - const: apb
> > > > > > > +  - const: smi
> > > > > > > +  - const: async
> > > > > > > +  - items:
> > > > > > > +  - const: apb
> > > > > > > +  - const: smi
> > > > > > > +  - const: gals0
> > > > > > > +  - const: gals1
> > > > > >
> > > > > > Similarly to my comment to other properties, this requirement per
> > > > > > compatible should be part of the schema within 'if-then'.
> > > > >
> > > > > I'm not so familiar with this format. Do this has "if-then-'else
> > > > > if'-then-else"?
> > > >
> > > > These are mutually exclusive conditions, so you can skip else:
> > > >  - if-then
> > > >  - if-then
> > > >  - if-then
> > > > It will be more readable then stacking 'if' under 'else'
> > >
> > > Thanks. I will use something like this:
> > >
> > >  anyOf:
> >
> > Then it should be oneOf as only one condition can be valid.
>
> I did do this at the beginning. But I get a warning log when
> dt_binding_check.

Mhmm, right, since "if-else" matches in either of arms, then oneOf
will complain as it expects only one of items to match.  Then just go
with allOf. anyOf might match zero of items, so it would not catch
actual errors, I think.

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


Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema

2020-10-12 Thread Yong Wu
On Mon, 2020-10-12 at 09:18 +0200, Krzysztof Kozlowski wrote:
> On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > > On Tue, 6 Oct 2020 at 06:27, Yong Wu  wrote:
> > > >
> > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > > Convert MediaTek SMI to DT schema.
> > > > > >
> > > > > > Signed-off-by: Yong Wu 
> > > > > > ---
> > > > > >  .../mediatek,smi-common.txt   |  49 -
> > > > > >  .../mediatek,smi-common.yaml  | 100 
> > > > > > ++
> > > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 -
> > > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 
> > > > > > 
> > > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > > >  delete mode 100644 
> > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > > >  create mode 100644 
> > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > > >  delete mode 100644 
> > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > > >  create mode 100644 
> > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > > ...
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +oneOf:
> > > > > > +  - enum:
> > > > > > +  - mediatek,mt2701-smi-common
> > > > > > +  - mediatek,mt2712-smi-common
> > > > > > +  - mediatek,mt6779-smi-common
> > > > > > +  - mediatek,mt8173-smi-common
> > > > > > +  - mediatek,mt8183-smi-common
> > > > > > +
> > > > > > +  - description: for mt7623
> > > > > > +items:
> > > > > > +  - const: mediatek,mt7623-smi-common
> > > > > > +  - const: mediatek,mt2701-smi-common
> > > > > > +
> > > > > > +  reg:
> > > > > > +maxItems: 1
> > > > > > +
> > > > > > +  clocks:
> > > > > > +description: |
> > > > > > +  apb and smi are mandatory. the async is only for generation 
> > > > > > 1 smi HW.
> > > > > > +  gals(global async local sync) also is optional, here is the 
> > > > > > list which
> > > > > > +  require gals: mt6779 and mt8183.
> > > > > > +minItems: 2
> > > > > > +maxItems: 4
> > > > > > +items:
> > > > > > +  - description: apb is Advanced Peripheral Bus clock, It's 
> > > > > > the clock for
> > > > > > +  setting the register.
> > > > > > +  - description: smi is the clock for transfer data and 
> > > > > > command.
> > > > > > +  - description: async is asynchronous clock, it help 
> > > > > > transform the smi clock
> > > > > > +  into the emi clock domain.
> > > > > > +  - description: gals0 is the path0 clock of gals.
> > > > > > +  - description: gals1 is the path1 clock of gals.
> > > > > > +
> > > > > > +  clock-names:
> > > > > > +oneOf:
> > > > > > +  - items:
> > > > > > +  - const: apb
> > > > > > +  - const: smi
> > > > > > +  - items:
> > > > > > +  - const: apb
> > > > > > +  - const: smi
> > > > > > +  - const: async
> > > > > > +  - items:
> > > > > > +  - const: apb
> > > > > > +  - const: smi
> > > > > > +  - const: gals0
> > > > > > +  - const: gals1
> > > > >
> > > > > Similarly to my comment to other properties, this requirement per
> > > > > compatible should be part of the schema within 'if-then'.
> > > >
> > > > I'm not so familiar with this format. Do this has "if-then-'else
> > > > if'-then-else"?
> > > 
> > > These are mutually exclusive conditions, so you can skip else:
> > >  - if-then
> > >  - if-then
> > >  - if-then
> > > It will be more readable then stacking 'if' under 'else'
> > 
> > Thanks. I will use something like this:
> > 
> >  anyOf:
> 
> Then it should be oneOf as only one condition can be valid.

I did do this at the beginning. But I get a warning log when
dt_binding_check.


Below is my schema and the detailed warning log:
//===
  clocks:
description: |
  x
minItems: 2
maxItems: 4
items:
  - description: apb is the clock for setting the register.
  - description: smi is the clock for transfer data and command.
  - description: async is asynchronous clock.
  - description: gals0 is the path0 clock of gals.
  - description: gals1 is the path1 clock of gals.

  clock-names:
minItems: 2
maxItems: 4 

required:
  - compatible
  - reg
  - power-domains
  - clocks
  - clock-names

oneOf:
  - if: #only for gen1 HW
  properties:
compatible:
  contains:
enum:
  - mediatek,mt2701-smi-common
then:
   properties:
 clock:
   items:
 minItems: 3
 maxItems: 3
 

Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()

2020-10-12 Thread Catalin Marinas
On Sat, Oct 10, 2020 at 05:12:31PM +0200, Nicolas Saenz Julienne wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index f6902a2b4ea6..0eca5865dcb1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -196,14 +196,16 @@ static void __init zone_sizes_init(unsigned long min, 
> unsigned long max)
>   unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
>  
>  #ifdef CONFIG_ZONE_DMA
> + zone_dma_bits = ARM64_ZONE_DMA_BITS;
> +
>   if (IS_ENABLED(CONFIG_ACPI)) {
>   extern unsigned int acpi_iort_get_zone_dma_size(void);
>  
>   zone_dma_bits = min(zone_dma_bits,
>   acpi_iort_get_zone_dma_size());
> - arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
>   }
>  
> + arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
>   max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
>  #endif
>  #ifdef CONFIG_ZONE_DMA32
> @@ -394,11 +396,6 @@ void __init arm64_memblock_init(void)
>  
>   early_init_fdt_scan_reserved_mem();
>  
> - if (IS_ENABLED(CONFIG_ZONE_DMA)) {
> - zone_dma_bits = ARM64_ZONE_DMA_BITS;
> - arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
> - }

arm64_dma_phys_limit is used by memblock_alloc_low() (via
ARCH_LOW_ADDRESS_LIMIT). I think it's too late to leave its
initialisation to zone_sizes_init().

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


Re: [PATCH 1/4] of/fdt: Update zone_dma_bits when running in bcm2711

2020-10-12 Thread Catalin Marinas
On Mon, Oct 12, 2020 at 08:47:15AM +0200, Christoph Hellwig wrote:
> On Fri, Oct 09, 2020 at 06:10:52PM +0100, Catalin Marinas wrote:
> > kdump wants DMA-able memory and,
> 
> DMAable by whom?  The only way to guranteed DMAable memory is to use
> the DMA memory allocator(s) and pass a specific device to them.  Everyting
> else is just fundamentally broken.  Note that even when device is not
> DMAable we can still use swiotlb to access it.

What I meant is that the new kexec'ed kernel needs some memory in the
ZONE_DMA range, currently set to the bottom 30-bit even for platforms
that can cope with the whole 32-bit range (anything other than RPi4).
The memory range available to the kdump kernels is limited to what
reserve_crashkernel() allocated, which may not fit in the lower 1GB.

There are two ongoing threads (complementary):

1. Change the arm64 reserve_crashkernel() similar to x86 which allocates
   memory above 4G with a small block in the ZONE_DMA range.

2. Allow zone_dma_bits to be 32 for arm64 platforms other than RPi4.

The second point also fixes some regressions with CMA reservations that
could no longer fit in the lower 1GB.

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


Re: [PATCH v4 0/7] Convert the intel iommu driver to the dma-iommu api

2020-10-12 Thread Tvrtko Ursulin


On 29/09/2020 01:11, Lu Baolu wrote:

Hi Tvrtko,

On 9/28/20 5:44 PM, Tvrtko Ursulin wrote:


On 27/09/2020 07:34, Lu Baolu wrote:

Hi,

The previous post of this series could be found here.

https://lore.kernel.org/linux-iommu/20200912032200.11489-1-baolu...@linux.intel.com/ 



This version introduce a new patch [4/7] to fix an issue reported here.

https://lore.kernel.org/linux-iommu/51a1baec-48d1-c0ac-181b-1fba92aa4...@linux.intel.com/ 



There aren't any other changes.

Please help to test and review.

Best regards,
baolu

Lu Baolu (3):
   iommu: Add quirk for Intel graphic devices in map_sg


Since I do have patches to fix i915 to handle this, do we want to 
co-ordinate the two and avoid having to add this quirk and then later 
remove it? Or you want to go the staged approach?


I have no preference. It depends on which patch goes first. Let the
maintainers help here.


FYI we have merged the required i915 patches to out tree last week or 
so. I *think* this means they will go into 5.11. So the i915 specific 
workaround patch will not be needed in Intel IOMMU.


Regards,

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

(proposal) RE: [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs

2020-10-12 Thread Tian, Kevin
> From: Jason Wang 
> Sent: Monday, September 14, 2020 12:20 PM
>
[...]
 > If it's possible, I would suggest a generic uAPI instead of a VFIO
> specific one.
> 
> Jason suggest something like /dev/sva. There will be a lot of other
> subsystems that could benefit from this (e.g vDPA).
> 
> Have you ever considered this approach?
> 

Hi, Jason,

We did some study on this approach and below is the output. It's a
long writing but I didn't find a way to further abstract w/o losing 
necessary context. Sorry about that.

Overall the real purpose of this series is to enable IOMMU nested
translation capability with vSVA as one major usage, through
below new uAPIs:
1) Report/enable IOMMU nested translation capability;
2) Allocate/free PASID;
3) Bind/unbind guest page table;
4) Invalidate IOMMU cache;
5) Handle IOMMU page request/response (not in this series);
1/3/4) is the minimal set for using IOMMU nested translation, with 
the other two optional. For example, the guest may enable vSVA on 
a device without using PASID. Or, it may bind its gIOVA page table 
which doesn't require page fault support. Finally, all operations can 
be applied to either physical device or subdevice.

Then we evaluated each uAPI whether generalizing it is a good thing 
both in concept and regarding to complexity.

First, unlike other uAPIs which are all backed by iommu_ops, PASID 
allocation/free is through the IOASID sub-system. From this angle
we feel generalizing PASID management does make some sense. 
First, PASID is just a number and not related to any device before 
it's bound to a page table and IOMMU domain. Second, PASID is a 
global resource (at least on Intel VT-d), while having separate VFIO/
VDPA allocation interfaces may easily cause confusion in userspace,
e.g. which interface to be used if both VFIO/VDPA devices exist. 
Moreover, an unified interface allows centralized control over how 
many PASIDs are allowed per process.

One unclear part with this generalization is about the permission.
Do we open this interface to any process or only to those which
have assigned devices? If the latter, what would be the mechanism
to coordinate between this new interface and specific passthrough 
frameworks? A more tricky case, vSVA support on ARM (Eric/Jean
please correct me) plans to do per-device PASID namespace which
is built on a bind_pasid_table iommu callback to allow guest fully 
manage its PASIDs on a given passthrough device. I'm not sure 
how such requirement can be unified w/o involving passthrough
frameworks, or whether ARM could also switch to global PASID 
style...

Second, IOMMU nested translation is a per IOMMU domain
capability. Since IOMMU domains are managed by VFIO/VDPA
 (alloc/free domain, attach/detach device, set/get domain attribute,
etc.), reporting/enabling the nesting capability is an natural 
extension to the domain uAPI of existing passthrough frameworks. 
Actually, VFIO already includes a nesting enable interface even 
before this series. So it doesn't make sense to generalize this uAPI 
out.

Then the tricky part comes with the remaining operations (3/4/5),
which are all backed by iommu_ops thus effective only within an 
IOMMU domain. To generalize them, the first thing is to find a way 
to associate the sva_FD (opened through generic /dev/sva) with an 
IOMMU domain that is created by VFIO/VDPA. The second thing is 
to replicate {domain<->device/subdevice} association in /dev/sva 
path because some operations (e.g. page fault) is triggered/handled 
per device/subdevice. Therefore, /dev/sva must provide both per-
domain and per-device uAPIs similar to what VFIO/VDPA already 
does. Moreover, mapping page fault to subdevice requires pre-
registering subdevice fault data to IOMMU layer when binding 
guest page table, while such fault data can be only retrieved from 
parent driver through VFIO/VDPA. 

However, we failed to find a good way even at the 1st step about
domain association. The iommu domains are not exposed to the
userspace, and there is no 1:1 mapping between domain and device.
In VFIO, all devices within the same VFIO container share the address
space but they may be organized in multiple IOMMU domains based
on their bus type. How (should we let) the userspace know the
domain information and open an sva_FD for each domain is the main
problem here.

In the end we just realized that doing such generalization doesn't
really lead to a clear design and instead requires tight coordination 
between /dev/sva and VFIO/VDPA for almost every new uAPI 
(especially about synchronization when the domain/device 
association is changed or when the device/subdevice is being reset/
drained). Finally it may become a usability burden to the userspace
on proper use of the two interfaces on the assigned device.
 
Based on above analysis we feel that just generalizing PASID mgmt.
might be a good thing to look at while the remaining operations are 
better being VFIO/VDPA specific 

Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings

2020-10-12 Thread Bjorn Andersson
On Mon 21 Sep 23:08 CEST 2020, Will Deacon wrote:

> On Sat, Sep 12, 2020 at 10:25:59PM -0500, Bjorn Andersson wrote:
> > On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:
> > > On 2020-09-04 16:55, Bjorn Andersson wrote:
> > > > Add a new operation to allow platform implementations to inherit any
> > > > stream mappings from the boot loader.
> > > 
> > > Is there a reason we need an explicit step for this? The aim of the
> > > cfg_probe hook is that the SMMU software state should all be set up by 
> > > then,
> > > and you can mess about with it however you like before arm_smmu_reset()
> > > actually commits anything to hardware. I would have thought you could
> > > permanently steal a context bank, configure it as your bypass hole, read 
> > > out
> > > the previous SME configuration and tweak smmu->smrs and smmu->s2crs
> > > appropriately all together "invisibly" at that point.
> > 
> > I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
> > configuration impl hook before consuming features") we no longer have
> > setup pgsize_bitmap as we hit cfg_probe, which means that I need to
> > replicate this logic to set up the iommu_domain.
> > 
> > If I avoid setting up an iommu_domain for the identity context, as you
> > request in patch 8, this shouldn't be needed anymore.
> > 
> > > If that can't work, I'm very curious as to what I've overlooked.
> > > 
> > 
> > I believe that will work, I will rework the patches and try it out.
> 
> Did you get a chance to rework this?
> 

Finally got a chance to dig through this properly.

Initial results where positive and with an implementation of cfg_probe
in qcom_smmu_impl I'm able to probe the arm-smmu driver just fine - and
display (e.g. efifb) stays alive.

Unfortunately as the display driver (drivers/gpu/drm/msm) is about to
probe a new iommu domain is created, which due to its match against
qcom_smmu_client_of_match[] becomes of type IOMMU_DOMAIN_IDENTITY.
This results in a S2CR of BYPASS type, which the firmware intercepts and
turns the stream into a type FAULT.

So while the cfg_probe looks very reasonable we're still in need of a
mechanism to use the fake identity context for the iommu domain
associated with the display controller.


The workings of the display driver is that it gets the iommu domain
setup for byass and then after that creates a translation context for
this same stream where it maps the framebuffer.

For testing purposes I made def_domain_type always return 0 in the qcom
impl and the result is that we get a few page faults while probing the
display driver, but these are handled somewhat gracefully and the
initialization did proceed and the system comes up nicely (but in the
case that the display driver would probe defer this leads to an storm of
faults as the screen continues to be refreshed).

TL;DR I think we still need to have a way to get the arm-smmu driver to
allow the qcom implementation to configure identity domains to use
translation - but we can make the setup of the identity context a detail
of the qcom driver.

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


Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema

2020-10-12 Thread Krzysztof Kozlowski
On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > On Tue, 6 Oct 2020 at 06:27, Yong Wu  wrote:
> > >
> > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > Convert MediaTek SMI to DT schema.
> > > > >
> > > > > Signed-off-by: Yong Wu 
> > > > > ---
> > > > >  .../mediatek,smi-common.txt   |  49 -
> > > > >  .../mediatek,smi-common.yaml  | 100 
> > > > > ++
> > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 -
> > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 
> > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > >  delete mode 100644 
> > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > >  create mode 100644 
> > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > >  delete mode 100644 
> > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > >  create mode 100644 
> > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > ...
> > > > > +properties:
> > > > > +  compatible:
> > > > > +oneOf:
> > > > > +  - enum:
> > > > > +  - mediatek,mt2701-smi-common
> > > > > +  - mediatek,mt2712-smi-common
> > > > > +  - mediatek,mt6779-smi-common
> > > > > +  - mediatek,mt8173-smi-common
> > > > > +  - mediatek,mt8183-smi-common
> > > > > +
> > > > > +  - description: for mt7623
> > > > > +items:
> > > > > +  - const: mediatek,mt7623-smi-common
> > > > > +  - const: mediatek,mt2701-smi-common
> > > > > +
> > > > > +  reg:
> > > > > +maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +description: |
> > > > > +  apb and smi are mandatory. the async is only for generation 1 
> > > > > smi HW.
> > > > > +  gals(global async local sync) also is optional, here is the 
> > > > > list which
> > > > > +  require gals: mt6779 and mt8183.
> > > > > +minItems: 2
> > > > > +maxItems: 4
> > > > > +items:
> > > > > +  - description: apb is Advanced Peripheral Bus clock, It's the 
> > > > > clock for
> > > > > +  setting the register.
> > > > > +  - description: smi is the clock for transfer data and command.
> > > > > +  - description: async is asynchronous clock, it help transform 
> > > > > the smi clock
> > > > > +  into the emi clock domain.
> > > > > +  - description: gals0 is the path0 clock of gals.
> > > > > +  - description: gals1 is the path1 clock of gals.
> > > > > +
> > > > > +  clock-names:
> > > > > +oneOf:
> > > > > +  - items:
> > > > > +  - const: apb
> > > > > +  - const: smi
> > > > > +  - items:
> > > > > +  - const: apb
> > > > > +  - const: smi
> > > > > +  - const: async
> > > > > +  - items:
> > > > > +  - const: apb
> > > > > +  - const: smi
> > > > > +  - const: gals0
> > > > > +  - const: gals1
> > > >
> > > > Similarly to my comment to other properties, this requirement per
> > > > compatible should be part of the schema within 'if-then'.
> > >
> > > I'm not so familiar with this format. Do this has "if-then-'else
> > > if'-then-else"?
> > 
> > These are mutually exclusive conditions, so you can skip else:
> >  - if-then
> >  - if-then
> >  - if-then
> > It will be more readable then stacking 'if' under 'else'
> 
> Thanks. I will use something like this:
> 
>  anyOf:

Then it should be oneOf as only one condition can be valid.

Best regards,
Krzysztof

>- if: #gen1 hw
>  then:
>use apb/smi/async clocks
> 
>- if: #gen2 hw that has gals.
>  then:
>use apb/smi/gals0/gals1 clocks
>  else: # gen2 hw that doesn't have gals.
>use apb/smi clocks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 1/1] iommu/amd: Remove unnecessary assignment

2020-10-12 Thread Adrian Huang
From: Adrian Huang 

The values of local variables are assigned after local variables
are declared, so no need to assign the initial value during the
variable declaration.

And, no need to assign NULL for the local variable 'ivrs_base' 
after invoking acpi_put_table().

Signed-off-by: Adrian Huang 
---
 drivers/iommu/amd/init.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 1ba6b4cc56e8..f171078f7ea0 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1858,7 +1858,7 @@ static void print_iommu_info(void)
 static int __init amd_iommu_init_pci(void)
 {
struct amd_iommu *iommu;
-   int ret = 0;
+   int ret;
 
for_each_iommu(iommu) {
ret = iommu_init_pci(iommu);
@@ -2494,8 +2494,8 @@ static void __init free_dma_resources(void)
 static int __init early_amd_iommu_init(void)
 {
struct acpi_table_header *ivrs_base;
+   int i, remap_cache_sz, ret;
acpi_status status;
-   int i, remap_cache_sz, ret = 0;
u32 pci_id;
 
if (!amd_iommu_detected)
@@ -2637,7 +2637,6 @@ static int __init early_amd_iommu_init(void)
 out:
/* Don't leak any ACPI memory */
acpi_put_table(ivrs_base);
-   ivrs_base = NULL;
 
return ret;
 }
-- 
2.17.1

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


Re: [PATCH 1/4] of/fdt: Update zone_dma_bits when running in bcm2711

2020-10-12 Thread Christoph Hellwig
On Fri, Oct 09, 2020 at 06:10:52PM +0100, Catalin Marinas wrote:
> kdump wants DMA-able memory and,

DMAable by whom?  The only way to guranteed DMAable memory is to use
the DMA memory allocator(s) and pass a specific device to them.  Everyting
else is just fundamentally broken.  Note that even when device is not
DMAable we can still use swiotlb to access it.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu