RE: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support

2020-04-17 Thread Tian, Kevin
> From: Jacob Pan 
> Sent: Friday, April 17, 2020 11:29 PM
> 
> On Fri, 17 Apr 2020 09:46:55 +0200
> Auger Eric  wrote:
> 
> > Hi Kevin,
> > On 4/17/20 4:45 AM, Tian, Kevin wrote:
> > >> From: Auger Eric
> > >> Sent: Thursday, April 16, 2020 6:43 PM
> > >>
> > > [...]
> > > + if (svm) {
> > > + /*
> > > +  * If we found svm for the PASID, there must
> > > be at
> > > +  * least one device bond, otherwise svm should
> > > be freed.
> > > +  */
> > > + if (WARN_ON(list_empty(&svm->devs))) {
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> > > +
> > > + for_each_svm_dev(sdev, svm, dev) {
> > > + /* In case of multiple sub-devices of
> > > the same pdev
> > > +  * assigned, we should allow multiple
> > > bind calls with
> > > +  * the same PASID and pdev.
> > > +  */
> > > + sdev->users++;
> >  What if this is not an mdev device. Is it also allowed?
> > >>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
> > >>> example of normal use case. You can bind the same PCI device (PF
> > >>> or SRIOV VF) more than once to the same PASID. Just need to
> > >>> unbind also.
> > >>
> > >> I don't get the point of binding a non mdev device several times
> > >> with the same PASID. Do you intend to allow that at userspace
> > >> level or prevent this from happening in VFIO?
> > >
> > > I feel it's better to prevent this from happening, otherwise VFIO
> > > also needs to track the bind count and do multiple unbinds at
> > > mm_exit. But it's not necessary to prevent it in VFIO. We can check
> > > here upon whether aux_domain is valid, and if not return -EBUSY.
> > Ah OK. So if we can detect the case here it is even better
> >
> I don't understand why VFIO cannot track, since it is mdev aware. if we
> don;t refcount the users, one mdev unbind will result unbind for all
> mdev under the same pdev. That may not be the right thing to do.
> 

The open here is not for mdev, which refcount is still required. Eric's
point is for non-mdev endpoints. It's meaningless and not intuitive 
to allow binding a PASID multiple-times to the same device. 

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


Re: [patch 0/7] unencrypted atomic DMA pools with dynamic expansion

2020-04-17 Thread David Rientjes via iommu
On Fri, 17 Apr 2020, Christoph Hellwig wrote:

> So modulo a few comments that I can fix up myself this looks good.  Unless
> you want to resend for some reason I'm ready to pick this up once I open
> the dma-mapping tree after -rc2.
> 

Yes, please do, and thanks to both you and Thomas for the guidance and 
code reviews.

Once these patches take on their final form in your branch, how supportive 
would you be of stable backports going back to 4.19 LTS?

There have been several changes to this area over time, so there are 
varying levels of rework that need to be done for each stable kernel back 
to 4.19.  But I'd be happy to do that work if you are receptive to it.

For rationale, without these fixes, all SEV enabled guests warn of 
blocking in rcu read side critical sections when using drivers that 
allocate atomically though the DMA API that calls set_memory_decrypted().  
Users can see warnings such as these in the guest:

BUG: sleeping function called from invalid context at mm/vmalloc.c:1710
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 3383, name: fio
2 locks held by fio/3383:
 #0: 93b6a8568348 (&sb->s_type->i_mutex_key#16){+.+.}, at: 
ext4_file_write_iter+0xa2/0x5d0
 #1: a52a61a0 (rcu_read_lock){}, at: hctx_lock+0x1a/0xe0
CPU: 0 PID: 3383 Comm: fio Tainted: GW 5.5.10 #14
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
01/01/2011
Call Trace:
 dump_stack+0x98/0xd5
 ___might_sleep+0x175/0x260
 __might_sleep+0x4a/0x80
 _vm_unmap_aliases+0x45/0x250
 vm_unmap_aliases+0x19/0x20
 __set_memory_enc_dec+0xa4/0x130
 set_memory_decrypted+0x10/0x20
 dma_direct_alloc_pages+0x148/0x150
 dma_direct_alloc+0xe/0x10
 dma_alloc_attrs+0x86/0xc0
 dma_pool_alloc+0x16f/0x2b0
 nvme_queue_rq+0x878/0xc30 [nvme]
 __blk_mq_try_issue_directly+0x135/0x200
 blk_mq_request_issue_directly+0x4f/0x80
 blk_mq_try_issue_list_directly+0x46/0xb0
 blk_mq_sched_insert_requests+0x19b/0x2b0
 blk_mq_flush_plug_list+0x22f/0x3b0
 blk_flush_plug_list+0xd1/0x100
 blk_finish_plug+0x2c/0x40
 iomap_dio_rw+0x427/0x490
 ext4_file_write_iter+0x181/0x5d0
 aio_write+0x109/0x1b0
 io_submit_one+0x7d0/0xfa0
 __x64_sys_io_submit+0xa2/0x280
 do_syscall_64+0x5f/0x250
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v11 07/10] iommu/vt-d: Add svm/sva invalidate function

2020-04-17 Thread Jacob Pan
On Thu, 16 Apr 2020 12:59:14 +0200
Auger Eric  wrote:

> Hi Jacob,
> On 4/10/20 11:56 PM, Jacob Pan wrote:
> > On Thu, 9 Apr 2020 10:50:34 +0200
> > Auger Eric  wrote:
> >   
> >> Hi Jacob,
> >>
> >> On 4/3/20 8:42 PM, Jacob Pan wrote:  
> >>> When Shared Virtual Address (SVA) is enabled for a guest OS via
> >>> vIOMMU, we need to provide invalidation support at IOMMU API and
> >>> driver level. This patch adds Intel VT-d specific function to
> >>> implement iommu passdown invalidate API for shared virtual
> >>> address.
> >>>
> >>> The use case is for supporting caching structure invalidation
> >>> of assigned SVM capable devices. Emulated IOMMU exposes queue
> >>> invalidation capability and passes down all descriptors from the
> >>> guest to the physical IOMMU.
> >>>
> >>> The assumption is that guest to host device ID mapping should be
> >>> resolved prior to calling IOMMU driver. Based on the device
> >>> handle, host IOMMU driver can replace certain fields before
> >>> submit to the invalidation queue.
> >>>
> >>> ---
> >>> v11 - Removed 2D map array, use -EINVAL in granularity lookup
> >>> array. Fixed devTLB invalidation granularity mapping. Disregard
> >>> G=1 case and use address selective invalidation only.
> >>>
> >>> v7 review fixed in v10
> >>> ---
> >>>
> >>> Signed-off-by: Jacob Pan 
> >>> Signed-off-by: Ashok Raj 
> >>> Signed-off-by: Liu, Yi L 
> >>> ---
> >>>  drivers/iommu/intel-iommu.c | 158
> >>>  1 file changed, 158
> >>> insertions(+)
> >>>
> >>> diff --git a/drivers/iommu/intel-iommu.c
> >>> b/drivers/iommu/intel-iommu.c index 94c7993dac6a..045c5c08d71d
> >>> 100644 --- a/drivers/iommu/intel-iommu.c
> >>> +++ b/drivers/iommu/intel-iommu.c
> >>> @@ -5594,6 +5594,163 @@ static void
> >>> intel_iommu_aux_detach_device(struct iommu_domain *domain,
> >>> aux_domain_remove_dev(to_dmar_domain(domain), dev); }
> >>>  
> >>> +/*
> >>> + * 2D array for converting and sanitizing IOMMU generic TLB
> >>> granularity to
> >>> + * VT-d granularity. Invalidation is typically included in the
> >>> unmap operation
> >>> + * as a result of DMA or VFIO unmap. However, for assigned
> >>> devices guest
> >>> + * owns the first level page tables. Invalidations of translation
> >>> caches in the
> >>> + * guest are trapped and passed down to the host.
> >>> + *
> >>> + * vIOMMU in the guest will only expose first level page tables,
> >>> therefore
> >>> + * we do not support IOTLB granularity for request without PASID
> >>> (second level).
> >>> + *
> >>> + * For example, to find the VT-d granularity encoding for IOTLB
> >>> + * type and page selective granularity within PASID:
> >>> + * X: indexed by iommu cache type
> >>> + * Y: indexed by enum iommu_inv_granularity
> >>> + * [IOMMU_CACHE_INV_TYPE_IOTLB][IOMMU_INV_GRANU_ADDR]
> >>> + */
> >>> +
> >>> +const static int
> >>> inv_type_granu_table[IOMMU_CACHE_INV_TYPE_NR][IOMMU_INV_GRANU_NR]
> >>> = {
> >>> + /*
> >>> +  * PASID based IOTLB invalidation: PASID selective (per
> >>> PASID),
> >>> +  * page selective (address granularity)
> >>> +  */
> >>> + {-EINVAL, QI_GRAN_NONG_PASID, QI_GRAN_PSI_PASID},
> >>> + /* PASID based dev TLBs */
> >>> + {-EINVAL, -EINVAL, QI_DEV_IOTLB_GRAN_PASID_SEL},
> >>> + /* PASID cache */
> >>> + {-EINVAL, -EINVAL, -EINVAL}
> >>> +};
> >>> +
> >>> +static inline int to_vtd_granularity(int type, int granu)
> >>> +{
> >>> + return inv_type_granu_table[type][granu];
> >>> +}
> >>> +
> >>> +static inline u64 to_vtd_size(u64 granu_size, u64 nr_granules)
> >>> +{
> >>> + u64 nr_pages = (granu_size * nr_granules) >>
> >>> VTD_PAGE_SHIFT; +
> >>> + /* VT-d size is encoded as 2^size of 4K pages, 0 for 4k,
> >>> 9 for 2MB, etc.
> >>> +  * IOMMU cache invalidate API passes granu_size in bytes,
> >>> and number of
> >>> +  * granu size in contiguous memory.
> >>> +  */
> >>> + return order_base_2(nr_pages);
> >>> +}
> >>> +
> >>> +#ifdef CONFIG_INTEL_IOMMU_SVM
> >>> +static int intel_iommu_sva_invalidate(struct iommu_domain
> >>> *domain,
> >>> + struct device *dev, struct
> >>> iommu_cache_invalidate_info *inv_info) +{
> >>> + struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> >>> + struct device_domain_info *info;
> >>> + struct intel_iommu *iommu;
> >>> + unsigned long flags;
> >>> + int cache_type;
> >>> + u8 bus, devfn;
> >>> + u16 did, sid;
> >>> + int ret = 0;
> >>> + u64 size = 0;
> >>> +
> >>> + if (!inv_info || !dmar_domain ||
> >>> + inv_info->version !=
> >>> IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
> >>> + return -EINVAL;
> >>> +
> >>> + if (!dev || !dev_is_pci(dev))
> >>> + return -ENODEV;
> >>
> >> Check (domain->flags & DOMAIN_FLAG_NESTING_MODE)?  
> > Good point
> >   
> >>> +
> >>> + iommu = device_to_iommu(dev, &bus, &devfn);
> >>> + if (!iommu)
> >>> + return -ENODEV;
> >>> +
> >>> + spin_lock_irqsave(&device_domain_lock, flags);
> >>> + spin_lock(&iommu->lock);
> >>> + info = iommu_support_dev_iotlb(dmar_domain, iommu, bus

Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support

2020-04-17 Thread Jacob Pan
On Fri, 17 Apr 2020 09:46:55 +0200
Auger Eric  wrote:

> Hi Kevin,
> On 4/17/20 4:45 AM, Tian, Kevin wrote:
> >> From: Auger Eric
> >> Sent: Thursday, April 16, 2020 6:43 PM
> >>  
> > [...]  
> > +   if (svm) {
> > +   /*
> > +* If we found svm for the PASID, there must
> > be at
> > +* least one device bond, otherwise svm should
> > be freed.
> > +*/
> > +   if (WARN_ON(list_empty(&svm->devs))) {
> > +   ret = -EINVAL;
> > +   goto out;
> > +   }
> > +
> > +   for_each_svm_dev(sdev, svm, dev) {
> > +   /* In case of multiple sub-devices of
> > the same pdev
> > +* assigned, we should allow multiple
> > bind calls with
> > +* the same PASID and pdev.
> > +*/
> > +   sdev->users++;  
>  What if this is not an mdev device. Is it also allowed?  
> >>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
> >>> example of normal use case. You can bind the same PCI device (PF
> >>> or SRIOV VF) more than once to the same PASID. Just need to
> >>> unbind also.  
> >>
> >> I don't get the point of binding a non mdev device several times
> >> with the same PASID. Do you intend to allow that at userspace
> >> level or prevent this from happening in VFIO?  
> > 
> > I feel it's better to prevent this from happening, otherwise VFIO
> > also needs to track the bind count and do multiple unbinds at
> > mm_exit. But it's not necessary to prevent it in VFIO. We can check
> > here upon whether aux_domain is valid, and if not return -EBUSY.  
> Ah OK. So if we can detect the case here it is even better
> 
I don't understand why VFIO cannot track, since it is mdev aware. if we
don;t refcount the users, one mdev unbind will result unbind for all
mdev under the same pdev. That may not be the right thing to do.

> Thanks
> 
> Eric
> >   
> >>
> >> Besides, the comment is a bit misleading as it gives the
> >> impression it is only true for mdev and there is no associated
> >> check.  
> > 
> > Thanks
> > Kevin
> >   
> 

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


Re: [PATCH v3 1/3] iommu/vt-d: Allow 32bit devices to uses DMA domain

2020-04-17 Thread Lu Baolu

Hi Christoph,

On 2020/4/17 14:50, Christoph Hellwig wrote:

On Thu, Apr 16, 2020 at 03:40:38PM +0800, Lu Baolu wrote:

description.  I'd need to look at the final code, but it seems like
this will still cause bounce buffering instead of using dynamic
mapping, which still seems like an awful idea.


Yes. If the user chooses to use identity domain by default through
kernel command, identity domain will be applied for all devices. For
those devices with limited addressing capability, bounce buffering will
be used when they try to access the memory beyond their address
capability. This won't cause any kernel regression as far as I can see.

Switching domain during runtime with drivers loaded will cause real
problems as I said in the commit message. That's the reason why I am
proposing to remove it. If we want to keep it, we have to make sure that
switching domain for one device should not impact other devices which
share the same domain with it. Furthermore, it's better to implement it
in the generic layer to keep device driver behavior consistent on all
architectures.


I don't disagree with the technical points.  What I pointed out is that

  a) the actual technical change is not in the commit log, which it
 should be


Sorry! I should make the commit message more comprehensive.


  b) that I still think taking away the ability to dynamically map
 devices in the identify domain after all the time we allowed for
 that is going to cause nasty regressions.



This change just asks Intel IOMMU driver to use the default domain
specified by the generic default domain framework, just like what other
vendor iommu drivers do. I understand that some users wants to use DMA
domain for some specific devices when the default domain type is
identity, and vice versa, use identity domain for some devices while
default one is DMA. I think Sai's patch series posted at

https://www.spinics.net/lists/iommu/msg41680.html

is a good start. It changes the default domain with all device drivers
unbound in the generic layer, hence every vendor iommu driver could
benefit from it.

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


Re: [PATCH v4] dt-bindings: iommu: renesas, ipmmu-vmsa: convert to json-schema

2020-04-17 Thread Geert Uytterhoeven
Hi Shimoda-san,

(not related to the json-schema conversion)

On Fri, Apr 17, 2020 at 9:03 AM Yoshihiro Shimoda
 wrote:
> +  renesas,ipmmu-main:
> +$ref: /schemas/types.yaml#/definitions/phandle-array
> +description:
> +  Reference to the main IPMMU instance in two cells. The first cell is
> +  a phandle to the main IPMMU and the second cell is the interrupt bit
> +  number associated with the particular cache IPMMU device. The interrupt
> +  bit number needs to match the main IPMMU IMSSTR register. Only used by
> +  cache IPMMU instances.

Note: Apparently the master IPMMU instance has an interrupt bit number
in the MMU Interrupt Status Register, too.  This is currently not
described in DT, as no renesas,ipmmu-main property is present for the
master instance.  Currently this doesn't matter, as the driver doesn't
use the interrupt bits at all.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] dma-mapping: move the remaining DMA API calls out of line

2020-04-17 Thread Christoph Hellwig
On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
> And the fact they were exported leaves possibility that there is a
> driver somewhere relying on these symbols or distro kernel won't build
> because the symbol disappeared from exports (I do not know what KABI
> guarantees or if mainline kernel cares).

We absolutely do not care.  In fact for abuses of APIs that drivers
should not use we almost care to make them private and break people
abusing them.

> I do not care in particular but
> some might, a line separated with empty lines in the commit log would do.

I'll add a blurb for the next version.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support

2020-04-17 Thread Auger Eric
Hi Kevin,
On 4/17/20 4:45 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Thursday, April 16, 2020 6:43 PM
>>
> [...]
> + if (svm) {
> + /*
> +  * If we found svm for the PASID, there must be at
> +  * least one device bond, otherwise svm should be
> freed.
> +  */
> + if (WARN_ON(list_empty(&svm->devs))) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + for_each_svm_dev(sdev, svm, dev) {
> + /* In case of multiple sub-devices of the
> same pdev
> +  * assigned, we should allow multiple bind
> calls with
> +  * the same PASID and pdev.
> +  */
> + sdev->users++;
 What if this is not an mdev device. Is it also allowed?
>>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
>>> example of normal use case. You can bind the same PCI device (PF or
>>> SRIOV VF) more than once to the same PASID. Just need to unbind also.
>>
>> I don't get the point of binding a non mdev device several times with
>> the same PASID. Do you intend to allow that at userspace level or
>> prevent this from happening in VFIO?
> 
> I feel it's better to prevent this from happening, otherwise VFIO also
> needs to track the bind count and do multiple unbinds at mm_exit.
> But it's not necessary to prevent it in VFIO. We can check here
> upon whether aux_domain is valid, and if not return -EBUSY.
Ah OK. So if we can detect the case here it is even better

Thanks

Eric
> 
>>
>> Besides, the comment is a bit misleading as it gives the impression it
>> is only true for mdev and there is no associated check.
> 
> Thanks
> Kevin
> 

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


Re: [PATCH v4] dt-bindings: iommu: renesas, ipmmu-vmsa: convert to json-schema

2020-04-17 Thread Geert Uytterhoeven
On Fri, Apr 17, 2020 at 9:03 AM Yoshihiro Shimoda
 wrote:
> Convert Renesas VMSA-Compatible IOMMU bindings documentation
> to json-schema.
>
> Note that original documentation doesn't mention renesas,ipmmu-vmsa
> for R-Mobile APE6. But, R-Mobile APE6 is similar to the R-Car
> Gen2. So, renesas,ipmmu-r8a73a4 belongs the renesas,ipmmu-vmsa
> section.
>
> Signed-off-by: Yoshihiro Shimoda 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] dma-debug: make __dma_entry_alloc_check_leak() static

2020-04-17 Thread Jason Yan
Fix the following sparse warning:

kernel/dma/debug.c:659:6: warning: symbol '__dma_entry_alloc_check_leak'
was not declared. Should it be static?

Reported-by: Hulk Robot 
Signed-off-by: Jason Yan 
---
 kernel/dma/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 9e1777c81f55..36c962a86bf2 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -656,7 +656,7 @@ static struct dma_debug_entry *__dma_entry_alloc(void)
return entry;
 }
 
-void __dma_entry_alloc_check_leak(void)
+static void __dma_entry_alloc_check_leak(void)
 {
u32 tmp = nr_total_entries % nr_prealloc_entries;
 
-- 
2.21.1

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


Re: [PATCH] dma-debug: make __dma_entry_alloc_check_leak() static

2020-04-17 Thread Christoph Hellwig
On Fri, Apr 17, 2020 at 03:36:05PM +0800, Jason Yan wrote:
> Fix the following sparse warning:
> 
> kernel/dma/debug.c:659:6: warning: symbol '__dma_entry_alloc_check_leak'
> was not declared. Should it be static?
> 
> Reported-by: Hulk Robot 
> Signed-off-by: Jason Yan 

Looks good, I'll queue this up for 5.8.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [patch 0/7] unencrypted atomic DMA pools with dynamic expansion

2020-04-17 Thread Christoph Hellwig
So modulo a few comments that I can fix up myself this looks good.  Unless
you want to resend for some reason I'm ready to pick this up once I open
the dma-mapping tree after -rc2.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [patch 5/7] dma-pool: add pool sizes to debugfs

2020-04-17 Thread Christoph Hellwig
On Wed, Apr 15, 2020 at 08:45:08AM -0500, Tom Lendacky wrote:
>
>
> On 4/14/20 7:04 PM, David Rientjes wrote:
>> The atomic DMA pools can dynamically expand based on non-blocking
>> allocations that need to use it.
>>
>> Export the sizes of each of these pools, in bytes, through debugfs for
>> measurement.
>>
>> Suggested-by: Christoph Hellwig 
>> Signed-off-by: David Rientjes 
>> ---
>>   kernel/dma/pool.c | 41 +
>>   1 file changed, 41 insertions(+)
>>
>> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
>> index cf052314d9e4..3e22022c933b 100644
>> --- a/kernel/dma/pool.c
>> +++ b/kernel/dma/pool.c
>> @@ -2,6 +2,7 @@
>>   /*
>>* Copyright (C) 2020 Google LLC
>>*/
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -15,6 +16,11 @@
>>   static struct gen_pool *atomic_pool_dma __ro_after_init;
>>   static struct gen_pool *atomic_pool_dma32 __ro_after_init;
>>   static struct gen_pool *atomic_pool_kernel __ro_after_init;
>> +#ifdef CONFIG_DEBUG_FS
>
> I don't think you need the #ifdef any more unless you just want to save 
> space. All of the debugfs routines have versions for whether 
> CONFIG_DEBUG_FS is defined or not.

Agreed.  I can fix this up.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [patch 4/7] dma-direct: atomic allocations must come from atomic coherent pools

2020-04-17 Thread Christoph Hellwig


The subject should say something like "atomic unencrypted allocations.."
as many other atomic allocations are fine.  Which brings up that with
the codebase in this patch we can't really support architectures that
require both an atomic pool for uncached remapping for just some devices
and unencrypted for others.  We don't have such an archicture right now,
and I hope we don't grow one, but we probably need a little safeguard
with a BUILD_BUG_ON if both options are set.  I can send an incremental
patch for that if that is ok with you.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [patch 1/7] dma-remap: separate DMA atomic pools from direct remap code

2020-04-17 Thread Christoph Hellwig
On Tue, Apr 14, 2020 at 05:04:52PM -0700, David Rientjes wrote:
> DMA atomic pools will be needed beyond only CONFIG_DMA_DIRECT_REMAP so
> separate them out into their own file.
> 
> This also adds a new Kconfig option that can be subsequently used for
> options, such as CONFIG_AMD_MEM_ENCRYPT, that will utilize the coherent
> pools but do not have a dependency on direct remapping.
> 
> For this patch alone, there is no functional change introduced.
> 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: David Rientjes 
> ---
>  kernel/dma/Kconfig  |   6 ++-
>  kernel/dma/Makefile |   1 +
>  kernel/dma/pool.c   | 123 
>  kernel/dma/remap.c  | 114 
>  4 files changed, 129 insertions(+), 115 deletions(-)
>  create mode 100644 kernel/dma/pool.c
> 
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 4c103a24e380..d006668c0027 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -79,10 +79,14 @@ config DMA_REMAP
>   select DMA_NONCOHERENT_MMAP
>   bool
>  
> -config DMA_DIRECT_REMAP
> +config DMA_COHERENT_POOL
>   bool
>   select DMA_REMAP
>  
> +config DMA_DIRECT_REMAP
> + bool
> + select DMA_COHERENT_POOL
> +
>  config DMA_CMA
>   bool "DMA Contiguous Memory Allocator"
>   depends on HAVE_DMA_CONTIGUOUS && CMA
> diff --git a/kernel/dma/Makefile b/kernel/dma/Makefile
> index d237cf3dc181..370f63344e9c 100644
> --- a/kernel/dma/Makefile
> +++ b/kernel/dma/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_DMA_DECLARE_COHERENT)+= coherent.o
>  obj-$(CONFIG_DMA_VIRT_OPS)   += virt.o
>  obj-$(CONFIG_DMA_API_DEBUG)  += debug.o
>  obj-$(CONFIG_SWIOTLB)+= swiotlb.o
> +obj-$(CONFIG_DMA_COHERENT_POOL)  += pool.o
>  obj-$(CONFIG_DMA_REMAP)  += remap.o
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> new file mode 100644
> index ..6612c2d51d3c
> --- /dev/null
> +++ b/kernel/dma/pool.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Google LLC

This now also lost the ARM copyright in addition to the Linuxfoundation
one, but I can fix that up.  Otherwise it looks good to me.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4] dt-bindings: iommu: renesas, ipmmu-vmsa: convert to json-schema

2020-04-17 Thread Yoshihiro Shimoda
Convert Renesas VMSA-Compatible IOMMU bindings documentation
to json-schema.

Note that original documentation doesn't mention renesas,ipmmu-vmsa
for R-Mobile APE6. But, R-Mobile APE6 is similar to the R-Car
Gen2. So, renesas,ipmmu-r8a73a4 belongs the renesas,ipmmu-vmsa
section.

Signed-off-by: Yoshihiro Shimoda 
---
 Changes from v3:
 - Fix renesas,ipmmu-r8a7795's section
 https://patchwork.kernel.org/patch/11494079/

 Changes from v2:
 - Add a description for R-Mobile APE6 on the commit log.
 - Change renesas,ipmmu-r8a73a4 section on the compatible.
 - Add items on the interrupts.
 - Add power-domains to required.
 - Add oneOf for interrupts and renesas,ipmmu-main
 https://patchwork.kernel.org/patch/11490581/

 Changes from v1:
 - Fix typo in the subject.
 - Add a description on #iommu-cells.
 https://patchwork.kernel.org/patch/11485415/

 .../bindings/iommu/renesas,ipmmu-vmsa.txt  |  73 ---
 .../bindings/iommu/renesas,ipmmu-vmsa.yaml | 101 +
 2 files changed, 101 insertions(+), 73 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
 create mode 100644 
Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml

diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 
b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
deleted file mode 100644
index 020d6f2..
--- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-* Renesas VMSA-Compatible IOMMU
-
-The IPMMU is an IOMMU implementation compatible with the ARM VMSA page tables.
-It provides address translation for bus masters outside of the CPU, each
-connected to the IPMMU through a port called micro-TLB.
-
-
-Required Properties:
-
-  - compatible: Must contain SoC-specific and generic entry below in case
-the device is compatible with the R-Car Gen2 VMSA-compatible IPMMU.
-
-- "renesas,ipmmu-r8a73a4" for the R8A73A4 (R-Mobile APE6) IPMMU.
-- "renesas,ipmmu-r8a7743" for the R8A7743 (RZ/G1M) IPMMU.
-- "renesas,ipmmu-r8a7744" for the R8A7744 (RZ/G1N) IPMMU.
-- "renesas,ipmmu-r8a7745" for the R8A7745 (RZ/G1E) IPMMU.
-- "renesas,ipmmu-r8a774a1" for the R8A774A1 (RZ/G2M) IPMMU.
-- "renesas,ipmmu-r8a774b1" for the R8A774B1 (RZ/G2N) IPMMU.
-- "renesas,ipmmu-r8a774c0" for the R8A774C0 (RZ/G2E) IPMMU.
-- "renesas,ipmmu-r8a7790" for the R8A7790 (R-Car H2) IPMMU.
-- "renesas,ipmmu-r8a7791" for the R8A7791 (R-Car M2-W) IPMMU.
-- "renesas,ipmmu-r8a7793" for the R8A7793 (R-Car M2-N) IPMMU.
-- "renesas,ipmmu-r8a7794" for the R8A7794 (R-Car E2) IPMMU.
-- "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
-- "renesas,ipmmu-r8a7796" for the R8A7796 (R-Car M3-W) IPMMU.
-- "renesas,ipmmu-r8a77965" for the R8A77965 (R-Car M3-N) IPMMU.
-- "renesas,ipmmu-r8a77970" for the R8A77970 (R-Car V3M) IPMMU.
-- "renesas,ipmmu-r8a77980" for the R8A77980 (R-Car V3H) IPMMU.
-- "renesas,ipmmu-r8a77990" for the R8A77990 (R-Car E3) IPMMU.
-- "renesas,ipmmu-r8a77995" for the R8A77995 (R-Car D3) IPMMU.
-- "renesas,ipmmu-vmsa" for generic R-Car Gen2 or RZ/G1 VMSA-compatible
-  IPMMU.
-
-  - reg: Base address and size of the IPMMU registers.
-  - interrupts: Specifiers for the MMU fault interrupts. For instances that
-support secure mode two interrupts must be specified, for non-secure and
-secure mode, in that order. For instances that don't support secure mode a
-single interrupt must be specified. Not required for cache IPMMUs.
-
-  - #iommu-cells: Must be 1.
-
-Optional properties:
-
-  - renesas,ipmmu-main: reference to the main IPMMU instance in two cells.
-The first cell is a phandle to the main IPMMU and the second cell is
-the interrupt bit number associated with the particular cache IPMMU device.
-The interrupt bit number needs to match the main IPMMU IMSSTR register.
-Only used by cache IPMMU instances.
-
-
-Each bus master connected to an IPMMU must reference the IPMMU in its device
-node with the following property:
-
-  - iommus: A reference to the IPMMU in two cells. The first cell is a phandle
-to the IPMMU and the second cell the number of the micro-TLB that the
-device is connected to.
-
-
-Example: R8A7791 IPMMU-MX and VSP1-D0 bus master
-
-   ipmmu_mx: mmu@fe951000 {
-   compatible = "renasas,ipmmu-r8a7791", "renasas,ipmmu-vmsa";
-   reg = <0 0xfe951000 0 0x1000>;
-   interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>,
-<0 221 IRQ_TYPE_LEVEL_HIGH>;
-   #iommu-cells = <1>;
-   };
-
-   vsp@fe928000 {
-   ...
-   iommus = <&ipmmu_mx 13>;
-   ...
-   };
diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml 
b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
new file mode 100644
index ..d103897