Re: [PATCH] iommu/mediatek: Fix protect memory setting

[Yong Wu]

Hi Joerg,

On Tue, 2018-03-20 at 13:57 -0500, Joerg Roedel wrote:
> On Sun, Mar 18, 2018 at 09:52:54AM +0800, Yong Wu wrote:
> > To avoid adding this complex macro or a new function, I put
> > it in the code and backup its value in the suspend registers.
> 
> Missing Signed-off-by.

Signed-off-by and Reported-by have been added above.


Reported-by: Honghui Zhang 
Signed-off-by: Yong Wu 


> 
> > ---
> >  drivers/iommu/mtk_iommu.c | 15 ++-
> >  drivers/iommu/mtk_iommu.h |  1 +
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> 
> Joerg


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

RE: [PATCH v5 2/7] vfio/type1: Check reserve region conflict and update iova list

[Tian, Kevin]

> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Wednesday, March 21, 2018 6:38 AM
> 
> On Mon, 19 Mar 2018 07:51:58 +
> "Tian, Kevin"  wrote:
> 
> > > From: Shameer Kolothum
> > > Sent: Friday, March 16, 2018 12:35 AM
> > >
> > > This retrieves the reserved regions associated with dev group and
> > > checks for conflicts with any existing dma mappings. Also update
> > > the iova list excluding the reserved regions.
> > >
> > > Signed-off-by: Shameer Kolothum
> > > 
> > > ---
> > >  drivers/vfio/vfio_iommu_type1.c | 90
> > > +
> > >  1 file changed, 90 insertions(+)
> > >
> > > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > > b/drivers/vfio/vfio_iommu_type1.c
> > > index 1123c74..cfe2bb2 100644
> > > --- a/drivers/vfio/vfio_iommu_type1.c
> > > +++ b/drivers/vfio/vfio_iommu_type1.c
> > > @@ -1313,6 +1313,82 @@ static int vfio_iommu_aper_resize(struct
> > > list_head *iova,
> > >   return 0;
> > >  }
> > >
> > > +/*
> > > + * Check reserved region conflicts with existing dma mappings
> > > + */
> > > +static bool vfio_iommu_resv_conflict(struct vfio_iommu *iommu,
> > > + struct list_head *resv_regions)
> > > +{
> > > + struct iommu_resv_region *region;
> > > +
> > > + /* Check for conflict with existing dma mappings */
> > > + list_for_each_entry(region, resv_regions, list) {
> > > + if (vfio_find_dma(iommu, region->start, region->length))
> > > + return true;
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > +/*
> > > + * Check iova region overlap with  reserved regions and
> > > + * exclude them from the iommu iova range
> > > + */
> > > +static int vfio_iommu_resv_exclude(struct list_head *iova,
> > > + struct list_head *resv_regions)
> > > +{
> > > + struct iommu_resv_region *resv;
> > > + struct vfio_iova *n, *next;
> > > +
> > > + list_for_each_entry(resv, resv_regions, list) {
> > > + phys_addr_t start, end;
> > > +
> > > + start = resv->start;
> > > + end = resv->start + resv->length - 1;
> > > +
> > > + list_for_each_entry_safe(n, next, iova, list) {
> > > + int ret = 0;
> > > +
> > > + /* No overlap */
> > > + if ((start > n->end) || (end < n->start))
> > > + continue;
> > > + /*
> > > +  * Insert a new node if current node overlaps with
> > > the
> > > +  * reserve region to exlude that from valid iova
> > > range.
> > > +  * Note that, new node is inserted before the
> > > current
> > > +  * node and finally the current node is deleted
> > > keeping
> > > +  * the list updated and sorted.
> > > +  */
> > > + if (start > n->start)
> > > + ret = vfio_iommu_iova_insert(>list,
> > > + n->start, start - 1);
> > > + if (!ret && end < n->end)
> > > + ret = vfio_iommu_iova_insert(>list,
> > > + end + 1, n->end);
> > > + if (ret)
> > > + return ret;
> >
> > Is it safer to delete the 1st node here in case of failure of the 2nd node?
> > There is no problem with current logic since upon error iova_copy will
> > be released anyway. However this function alone doesn't assume the
> > fact of a temporary list, thus it's better to keep the list clean w/o 
> > garbage
> > left from any error handling.
> 
> I don't think the proposal makes the list notably more sane on failure
> than we have here.  If the function returns an error and the list is
> modified in any way, how can the caller recover?  We're operating on a
> principle of modify a copy and throw it away on error, the only
> function level solution to the problem you're noting is to make each
> function generate a working copy, which is clearly inefficient.  This
> is a static function, not intended for general use, so I think a
> sufficient approach to address your concern is to simply note the error
> behavior in the comment above the function, the list is in an
> unknown/inconsistent state on error.  Thanks,
> 

'static' doesn't mean it cannot be used for general purpose in the same
file. Clarifying the expected behavior in comment is OK to me.

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

RE: [PATCH v5 0/7] vfio/type1: Add support for valid iova list management

[Tian, Kevin]

> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Wednesday, March 21, 2018 6:55 AM
> 
> On Mon, 19 Mar 2018 08:28:32 +
> "Tian, Kevin"  wrote:
> 
> > > From: Shameer Kolothum
> > > Sent: Friday, March 16, 2018 12:35 AM
> > >
> > > This series introduces an iova list associated with a vfio
> > > iommu. The list is kept updated taking care of iommu apertures,
> > > and reserved regions. Also this series adds checks for any conflict
> > > with existing dma mappings whenever a new device group is attached
> to
> > > the domain.
> > >
> > > User-space can retrieve valid iova ranges using
> VFIO_IOMMU_GET_INFO
> > > ioctl capability chains. Any dma map request outside the valid iova
> > > range will be rejected.
> >
> > GET_INFO is done at initialization time which is good for cold attached
> > devices. If a hotplugged device may cause change of valid iova ranges
> > at run-time, then there could be potential problem (which however is
> > difficult for user space or orchestration stack to figure out in advance)
> > Can we do some extension like below to make hotplug case cleaner?
> 
> Let's be clear what we mean by hotplug here, as I see it, the only
> relevant hotplug would be a physical device, hot added to the host,
> which becomes a member of an existing, in-use IOMMU group.  If, on the
> other hand, we're talking about hotplug related to the user process,
> there's nothing asynchronous about that.  For instance in the QEMU
> case, QEMU must add the group to the container, at which point it can
> evaluate the new iova list and remove the group from the container if
> it doesn't like the result.  So what would be a case of the available
> iova list for a group changing as a result of adding a device?

My original thought was about the latter case. At that moment 
I was not sure whether the window between adding/removing 
the group may cause some issue if there are right some IOVA 
allocations happening in parallel. But looks Qemu can anyway
handle it well as long as such scenario is considered.

> 
> > - An interface allowing user space to request VFIO rejecting further
> > attach_group if doing so may cause iova range change. e.g. Qemu can
> > do such request once completing initial GET_INFO;
> 
> For the latter case above, it seems unnecessary, QEMU can revert the
> attach, we're only promising that the attach won't interfere with
> existing mappings.  For the host hotplug case, the user has no control,
> the new device is a member of the iommu group and therefore necessarily
> becomes a part of container.  I imagine there are plenty of other holes
> in this scenario already.
> 
> > - or an event notification to user space upon change of valid iova
> > ranges when attaching a new device at run-time. It goes one step
> > further - even attach may cause iova range change, it may still
> > succeed as long as Qemu hasn't allocated any iova in impacted
> > range
> 
> Same as above, in the QEMU hotplug case, the user is orchestrating the
> adding of the group to the container, they can then check the iommu
> info on their own and determine what, if any, changes are relevant to
> them, knowing that the addition would not succeed if any current
> mappings where affected.  In the host case, a notification would be
> required, but we'd first need to identify exactly how the iova list can
> change asynchronous to the user doing anything.  Thanks,

for host hotplug, possibly notification could be an opt-in model.
meaning if user space doesn't explicitly request receiving notification 
on such event, the device is just left in unused state (vfio-pci still
claims the device, assuming it assigned to the container owner, but
the owner doesn't use it)

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

Re: [PATCH v5 2/7] vfio/type1: Check reserve region conflict and update iova list

[Alex Williamson]

On Mon, 19 Mar 2018 07:51:58 +
"Tian, Kevin"  wrote:

> > From: Shameer Kolothum
> > Sent: Friday, March 16, 2018 12:35 AM
> > 
> > This retrieves the reserved regions associated with dev group and
> > checks for conflicts with any existing dma mappings. Also update
> > the iova list excluding the reserved regions.
> > 
> > Signed-off-by: Shameer Kolothum
> > 
> > ---
> >  drivers/vfio/vfio_iommu_type1.c | 90
> > +
> >  1 file changed, 90 insertions(+)
> > 
> > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > b/drivers/vfio/vfio_iommu_type1.c
> > index 1123c74..cfe2bb2 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -1313,6 +1313,82 @@ static int vfio_iommu_aper_resize(struct
> > list_head *iova,
> > return 0;
> >  }
> > 
> > +/*
> > + * Check reserved region conflicts with existing dma mappings
> > + */
> > +static bool vfio_iommu_resv_conflict(struct vfio_iommu *iommu,
> > +   struct list_head *resv_regions)
> > +{
> > +   struct iommu_resv_region *region;
> > +
> > +   /* Check for conflict with existing dma mappings */
> > +   list_for_each_entry(region, resv_regions, list) {
> > +   if (vfio_find_dma(iommu, region->start, region->length))
> > +   return true;
> > +   }
> > +
> > +   return false;
> > +}
> > +
> > +/*
> > + * Check iova region overlap with  reserved regions and
> > + * exclude them from the iommu iova range
> > + */
> > +static int vfio_iommu_resv_exclude(struct list_head *iova,
> > +   struct list_head *resv_regions)
> > +{
> > +   struct iommu_resv_region *resv;
> > +   struct vfio_iova *n, *next;
> > +
> > +   list_for_each_entry(resv, resv_regions, list) {
> > +   phys_addr_t start, end;
> > +
> > +   start = resv->start;
> > +   end = resv->start + resv->length - 1;
> > +
> > +   list_for_each_entry_safe(n, next, iova, list) {
> > +   int ret = 0;
> > +
> > +   /* No overlap */
> > +   if ((start > n->end) || (end < n->start))
> > +   continue;
> > +   /*
> > +* Insert a new node if current node overlaps with
> > the
> > +* reserve region to exlude that from valid iova
> > range.
> > +* Note that, new node is inserted before the
> > current
> > +* node and finally the current node is deleted
> > keeping
> > +* the list updated and sorted.
> > +*/
> > +   if (start > n->start)
> > +   ret = vfio_iommu_iova_insert(>list,
> > +   n->start, start - 1);
> > +   if (!ret && end < n->end)
> > +   ret = vfio_iommu_iova_insert(>list,
> > +   end + 1, n->end);
> > +   if (ret)
> > +   return ret;  
> 
> Is it safer to delete the 1st node here in case of failure of the 2nd node?
> There is no problem with current logic since upon error iova_copy will
> be released anyway. However this function alone doesn't assume the
> fact of a temporary list, thus it's better to keep the list clean w/o garbage
> left from any error handling.

I don't think the proposal makes the list notably more sane on failure
than we have here.  If the function returns an error and the list is
modified in any way, how can the caller recover?  We're operating on a
principle of modify a copy and throw it away on error, the only
function level solution to the problem you're noting is to make each
function generate a working copy, which is clearly inefficient.  This
is a static function, not intended for general use, so I think a
sufficient approach to address your concern is to simply note the error
behavior in the comment above the function, the list is in an
unknown/inconsistent state on error.  Thanks,

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

Re: [PATCHv3] iommu/intel: Ratelimit each dmar fault printing

[Dmitry Safonov via iommu]

On Thu, 2018-03-15 at 16:28 +0100, Joerg Roedel wrote:
> On Thu, Mar 15, 2018 at 02:42:00PM +, Dmitry Safonov wrote:
> > But even with loop-limit we will need ratelimit each printk()
> > *also*.
> > Otherwise loop-limit will be based on time spent printing, not on
> > anything else..
> > The patch makes sense even with loop-limit in my opinion.
> 
> Looks like I mis-read your patch, somehow it looked to me as if you
> replace all 'ratelimited' usages with a call to __ratelimit(), but
> you
> just move 'ratelimited' into the loop, which actually makes sense.

So, is it worth to apply the patch?

> But still, this alone is no proper fix for the soft-lockups you are
> seeing.

Hmm, but this fixes my softlockup issue, because it's about time spent
in printk() inside irq-disabled section, rather about exiting the dmar-
clearing loop.
And on my hw doesn't make any difference to limit loop or not because
clearing a fault is much faster than hw could generate a new fault.
ITOW, it fixes the softlockup for me and the loop-related lockup can't
happen on hw I have (so it's the other issue, [possible?] on other hw).

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

Re: [PATCH] iommu/mediatek: Fix protect memory setting

[Joerg Roedel]

On Sun, Mar 18, 2018 at 09:52:54AM +0800, Yong Wu wrote:
> To avoid adding this complex macro or a new function, I put
> it in the code and backup its value in the suspend registers.

Missing Signed-off-by.

> ---
>  drivers/iommu/mtk_iommu.c | 15 ++-
>  drivers/iommu/mtk_iommu.h |  1 +
>  2 files changed, 11 insertions(+), 5 deletions(-)


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

Re: [PATCH 1/1] iommu/vt-d: Use real PASID for flush in caching mode

[Joerg Roedel]

On Fri, Mar 16, 2018 at 12:31:36PM +0800, Lu Baolu wrote:
> If caching mode is supported, the hardware will cache
> none-present or erroneous translation entries. Hence,
> software should explicitly invalidate the PASID cache
> after a PASID table entry becomes present. We should
> issue such invalidation with the PASID value that we
> have changed. PASID 0 is not reserved for this case.
> 
> Cc: Jacob Pan 
> Cc: Kevin Tian 
> Cc: Sankaran Rajesh 
> Suggested-by: Ashok Raj 
> Signed-off-by: Liu Yi L 
> Signed-off-by: Lu Baolu 
> ---
>  drivers/iommu/intel-svm.c | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)

Applied, thanks.

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

Re: [PATCH 1/3] dt-bindings: iommu: ipmmu-vmsa: Add device tree support for r8a774[35]

[Joerg Roedel]

On Wed, Jan 24, 2018 at 03:42:00PM +, Biju Das wrote:
> Document r8a774[35] specific compatible strings. The Renesas RZ/G1[ME]
> (r8a774[35]) IPMMU are identical to the R-Car Gen2 family.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Chris Paterson 
> ---
>  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied, thanks.

Patches 2 and 3 seems already to be applied to another tree.

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

Re: [PATCH v2 06/21] fpga: Remove depends on HAS_DMA in case of platform dependency

[Alan Tull]

On Tue, Mar 20, 2018 at 5:04 AM, Geert Uytterhoeven
 wrote:

Hi Geert,

> Hi Alan,
>
> On Mon, Mar 19, 2018 at 5:06 PM, Alan Tull  wrote:
>> On Fri, Mar 16, 2018 at 8:51 AM, Geert Uytterhoeven
>>  wrote:
>> This essentially removes this commit
>>
>> commit 1c8cb409491403036919dd1c6b45013dc8835a44
>> Author: Sudip Mukherjee 
>> Date:   Wed Aug 3 13:45:46 2016 -0700
>>
>> drivers/fpga/Kconfig: fix build failure
>>
>> While building m32r allmodconfig the build is failing with the error:
>>
>>   ERROR: "bad_dma_ops" [drivers/fpga/zynq-fpga.ko] undefined!
>>
>> Xilinx Zynq FPGA is using DMA but there was no dependency while
>> building.
>>
>> Link: 
>> http://lkml.kernel.org/r/1464346526-13913-1-git-send-email-sudipm.mukher...@gmail.com
>> Signed-off-by: Sudip Mukherjee 
>> Acked-by: Moritz Fischer 
>> Cc: Alan Tull 
>> Signed-off-by: Andrew Morton 
>> Signed-off-by: Linus Torvalds 
>
> Yes it does. The major change is that the first (core) series introduces
> all needed dummies to do successful compile-testing on NO_DMA=y platforms.

OK yes, I looked at the first patch that does the fix.  Looks good.
Thanks for doing this.

>
>>> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
>>> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
>>> In most cases this other symbol is an architecture or platform specific
>>> symbol, or PCI.
>>>
>>> Generic symbols and drivers without platform dependencies keep their
>>> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
>>> cannot work anyway.
>>>
>>> This simplifies the dependencies, and allows to improve compile-testing.
>>>
>>> Signed-off-by: Geert Uytterhoeven 
>>> Reviewed-by: Mark Brown 
>>> Acked-by: Robin Murphy 
Acked-by: Alan Tull 

Regards,
Alan

>
> 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 12/14] dma-direct: handle the memory encryption bit in common code

[Catalin Marinas]

On Mon, Mar 19, 2018 at 08:49:30PM +0100, Christoph Hellwig wrote:
> On Mon, Mar 19, 2018 at 06:01:41PM +, Catalin Marinas wrote:
> > I don't particularly like maintaining an arm64-specific dma-direct.h
> > either but arm64 seems to be the only architecture that needs to
> > potentially force a bounce when cache_line_size() > ARCH_DMA_MINALIGN
> > and the device is non-coherent.
> 
> mips is another likely candidate, see all the recent drama about
> dma_get_alignmet().  And I'm also having major discussion about even
> exposing the cache line size architecturally for RISC-V, so changes
> are high it'll have to deal with this mess sooner or later as they
> probably can't agree on a specific cache line size.

On Arm, the cache line size varies between 32 and 128 on publicly
available hardware (and I wouldn't exclude higher numbers at some
point). In addition, the cache line size has a different meaning in the
DMA context, we call it "cache writeback granule" on Arm which is
greater than or equal the minimum cache line size.

So the aim is to have L1_CACHE_BYTES small enough for acceptable
performance numbers and ARCH_DMA_MINALIGN the maximum from a correctness
perspective (the latter is defined by some larger cache lines in L2/L3).

To make things worse, there is no clear definition in the generic kernel
on what cache_line_size() means and the default definition returns
L1_CACHE_BYTES. On arm64, we define it to the hardware's cache
writeback granule (CWG), if available, with a fallback on
ARCH_DMA_MINALIGN. The network layer, OTOH, seems to assume that
SMP_CACHE_BYTES is sufficient for DMA alignment (L1_CACHE_BYTES in
arm64's case).

> > As I said above, adding a check in swiotlb.c for
> > !is_device_dma_coherent(dev) && (ARCH_DMA_MINALIGN < cache_line_size())
> > feels too architecture specific.
> 
> And what exactly is architecture specific about that?  It is a totally
> generic concept, which at this point also seems entirely theoretical
> based on the previous mail in this thread.

The concept may be generic but the kernel macros/functions used here
aren't. is_device_dma_coherent() is only defined on arm and arm64. The
relation between ARCH_DMA_MINALIGN, L1_CACHE_BYTES and cache_line_size()
seems to be pretty ad-hoc. ARCH_DMA_MINALIGN is also only defined for
some architectures and, while there is dma_get_cache_alignment() which
returns this constant, it doesn't seem to be used much.

I'm all for fixing this in a generic way but I think we first need
swiotlb.c to become aware of non-cache-coherent DMA devices.

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

Re: [PATCH] iommu/ipmmu-vmsa: Document R-Car M3-N IPMMU DT bindings

[Geert Uytterhoeven]

On Tue, Mar 20, 2018 at 7:48 AM, Magnus Damm  wrote:
> From: Magnus Damm 
>
> Update the IPMMU DT binding documentation to include the r8a77965 compat
> string for the IPMMU devices included in the R-Car M3-N SoC.
>
> Signed-off-by: Magnus Damm 

(as already supplied to Jacopo's submission)
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

Re: use generic dma-direct and swiotlb code for x86 V3

[Konrad Rzeszutek Wilk]

On Tue, Mar 20, 2018, 5:03 AM Ingo Molnar  wrote:

>
> * Christoph Hellwig  wrote:
>
> > On Tue, Mar 20, 2018 at 09:37:51AM +0100, Ingo Molnar wrote:
> > > > git://git.infradead.org/users/hch/misc.git dma-direct-x86
> > >
> > > Btw., what's the upstreaming route for these patches?
> > >
> > > While it's a multi-arch series it's all pretty x86-heavy as well so we
> can host it
> > > in -tip (in tip:core/dma or such), but feel free to handle it yourself
> as well:
> > >
> > >   Reviewed-by: Ingo Molnar 
> >
> > Either way is fine with me.  The dma-mapping tree is pretty light this
> > cycles, so I don't expect any conflicts.  If you want it feel free to
> grab
> > it, otherwise I'll queue it up.
>
> Ok, I picked your series up into tip:core/dma:
>
>  - added the newly arrived Tested-by's and Reviewed-by's
>  - some minor edits to titles/changelogs, no functional changes
>
> will push it all out after testing.
>


Are you testing with swiotlb=force?

Thanks

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

Re: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of platform dependency

[Geert Uytterhoeven]

Hi Madalin-cristian,

On Mon, Mar 19, 2018 at 6:27 AM, Madalin-cristian Bucur
 wrote:
>> -Original Message-
>> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
>> On Behalf Of Geert Uytterhoeven
>> Remove dependencies on HAS_DMA where a Kconfig symbol depends on
>> another
>> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
>> In most cases this other symbol is an architecture or platform specific
>> symbol, or PCI.
>>
>> Generic symbols and drivers without platform dependencies keep their
>> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
>> cannot work anyway.
>>
>> This simplifies the dependencies, and allows to improve compile-testing.
>>
>> Notes:
>>   - FSL_FMAN keeps its dependency on HAS_DMA, as it calls set_dma_ops(),
>> which does not exist if HAS_DMA=n (Do we need a dummy? The use of
>> set_dma_ops() in this driver is questionable),
>
> Hi,
>
> The set_dma_ops() is no longer required in the fsl/fman, I'll send a patch to 
> remove it.

Thank you, looking forward to it!

>>   - SND_SOC_LPASS_IPQ806X and SND_SOC_LPASS_PLATFORM loose their
>> dependency on HAS_DMA, as they are selected from
>> SND_SOC_APQ8016_SBC.
>>
>> Signed-off-by: Geert Uytterhoeven 
>> Reviewed-by: Mark Brown 
>> Acked-by: Robin Murphy 

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 v2 00/21] Allow compile-testing NO_DMA (drivers)

[Wolfram Sang]

> To play it safe, you want to postpone the subsystem patches until the core
> part has landed upstream. I will rebase and resubmit after v4.17-rc1.

Thanks for the heads up. I'll wait for the rebased patch then and apply
it after rc1 for 4.17.



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

Re: [PATCH v2 14/21] mtd: Remove depends on HAS_DMA in case of platform dependency

[Geert Uytterhoeven]

Hi Boris,

On Sun, Mar 18, 2018 at 11:04 PM, Boris Brezillon
 wrote:
> On Fri, 16 Mar 2018 14:51:47 +0100
> Geert Uytterhoeven  wrote:
>
>> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
>> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
>> In most cases this other symbol is an architecture or platform specific
>> symbol, or PCI.
>>
>> Generic symbols and drivers without platform dependencies keep their
>> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
>> cannot work anyway.
>>
>> This simplifies the dependencies, and allows to improve compile-testing.
>>
>
> Don't know which release you're targeting but it's likely to conflict
> with the change I have in my nand/next branch. Is this a problem if I
> take this patch through the mtd tree after [1] has reached Linus' tree?

No problem, I will rebase and resubmit after v4.17-rc1.

> [1]https://lkml.org/lkml/2018/3/16/435
>
>> Signed-off-by: Geert Uytterhoeven 
>> Reviewed-by: Mark Brown 
>> Acked-by: Robin Murphy 

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 v2 06/21] fpga: Remove depends on HAS_DMA in case of platform dependency

[Geert Uytterhoeven]

Hi Alan,

On Mon, Mar 19, 2018 at 5:06 PM, Alan Tull  wrote:
> On Fri, Mar 16, 2018 at 8:51 AM, Geert Uytterhoeven
>  wrote:
> This essentially removes this commit
>
> commit 1c8cb409491403036919dd1c6b45013dc8835a44
> Author: Sudip Mukherjee 
> Date:   Wed Aug 3 13:45:46 2016 -0700
>
> drivers/fpga/Kconfig: fix build failure
>
> While building m32r allmodconfig the build is failing with the error:
>
>   ERROR: "bad_dma_ops" [drivers/fpga/zynq-fpga.ko] undefined!
>
> Xilinx Zynq FPGA is using DMA but there was no dependency while
> building.
>
> Link: 
> http://lkml.kernel.org/r/1464346526-13913-1-git-send-email-sudipm.mukher...@gmail.com
> Signed-off-by: Sudip Mukherjee 
> Acked-by: Moritz Fischer 
> Cc: Alan Tull 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 

Yes it does. The major change is that the first (core) series introduces
all needed dummies to do successful compile-testing on NO_DMA=y platforms.

>> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
>> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
>> In most cases this other symbol is an architecture or platform specific
>> symbol, or PCI.
>>
>> Generic symbols and drivers without platform dependencies keep their
>> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
>> cannot work anyway.
>>
>> This simplifies the dependencies, and allows to improve compile-testing.
>>
>> Signed-off-by: Geert Uytterhoeven 
>> Reviewed-by: Mark Brown 
>> Acked-by: Robin Murphy 

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 v2 00/21] Allow compile-testing NO_DMA (drivers)

[Geert Uytterhoeven]

Hi Wolfram,

On Fri, Mar 16, 2018 at 10:23 PM, Wolfram Sang  wrote:
>> To avoid allmodconfig/allyesconfig regressions on NO_DMA=y platforms,
>> this (drivers) series should be applied after the previous (core)
>> series (but not many people may notice/care ;-)
>
> I still don't get if there is a dependency on the core patches. I.e.
> shall I apply the subsystem patch now by myself or do you want to push
> the series after the core patch and need my ack here?

Yes, there is a dependency. Without the core patches, enabling COMPILE_TEST,
and compile-testing drivers irrelevant on obscure NO_DMA=y platforms will
cause build failures.

To play it safe, you want to postpone the subsystem patches until the core
part has landed upstream. I will rebase and resubmit after v4.17-rc1.

Thanks, and sorry for being unclear.

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 v9 3/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device

[Vivek Gautam]

Hi Robin,


On 3/14/2018 11:16 PM, Robin Murphy wrote:

On 13/03/18 08:55, Vivek Gautam wrote:

From: Sricharan R 

The smmu device probe/remove and add/remove master device callbacks
gets called when the smmu is not linked to its master, that is without
the context of the master device. So calling runtime apis in those 
places

separately.

Signed-off-by: Sricharan R 
[vivek: Cleanup pm runtime calls]
Signed-off-by: Vivek Gautam 
Reviewed-by: Tomasz Figa 
---
  drivers/iommu/arm-smmu.c | 95 


  1 file changed, 87 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index d5873d545024..56a04ae80bf3 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -268,6 +268,20 @@ static struct arm_smmu_option_prop 
arm_smmu_options[] = {

  { 0, NULL},
  };
  +static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
+{
+    if (pm_runtime_enabled(smmu->dev))
+    return pm_runtime_get_sync(smmu->dev);
+
+    return 0;
+}
+
+static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
+{
+    if (pm_runtime_enabled(smmu->dev))
+    pm_runtime_put(smmu->dev);
+}
+
  static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain 
*dom)

  {
  return container_of(dom, struct arm_smmu_domain, domain);
@@ -913,11 +927,15 @@ static void 
arm_smmu_destroy_domain_context(struct iommu_domain *domain)

  struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
  struct arm_smmu_device *smmu = smmu_domain->smmu;
  struct arm_smmu_cfg *cfg = _domain->cfg;
-    int irq;
+    int ret, irq;
    if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
  return;
  +    ret = arm_smmu_rpm_get(smmu);
+    if (ret < 0)
+    return;
+
  /*
   * Disable the context bank and free the page tables before 
freeing

   * it.
@@ -932,6 +950,8 @@ static void 
arm_smmu_destroy_domain_context(struct iommu_domain *domain)

    free_io_pgtable_ops(smmu_domain->pgtbl_ops);
  __arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
+
+    arm_smmu_rpm_put(smmu);
  }
    static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
@@ -1213,10 +1233,15 @@ static int arm_smmu_attach_dev(struct 
iommu_domain *domain, struct device *dev)

  return -ENODEV;
    smmu = fwspec_smmu(fwspec);
+
+    ret = arm_smmu_rpm_get(smmu);
+    if (ret < 0)
+    return ret;
+
  /* Ensure that the domain is finalised */
  ret = arm_smmu_init_domain_context(domain, smmu);
  if (ret < 0)
-    return ret;
+    goto rpm_put;
    /*
   * Sanity check the domain. We don't support domains across
@@ -1230,29 +1255,47 @@ static int arm_smmu_attach_dev(struct 
iommu_domain *domain, struct device *dev)

  }
    /* Looks ok, so add the device to the domain */
-    return arm_smmu_domain_add_master(smmu_domain, fwspec);
+    ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
+
+rpm_put:
+    arm_smmu_rpm_put(smmu);
+    return ret;
  }
    static int arm_smmu_map(struct iommu_domain *domain, unsigned 
long iova,

  phys_addr_t paddr, size_t size, int prot)
  {
  struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+    struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+    struct arm_smmu_device *smmu = smmu_domain->smmu;


Nit: please use arm_smmu_domain for ops as well (as it was before 
523d7423e21b), or consistently elide it for smmu - the mixture of both 
methods is just a horrible mess (here and in unmap).


Sure, will make it consistent for arm_smmu_device (in both places - 
map/unmap)


 struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;




+    int ret;
    if (!ops)
  return -ENODEV;
  -    return ops->map(ops, iova, paddr, size, prot);
+    arm_smmu_rpm_get(smmu);
+    ret = ops->map(ops, iova, paddr, size, prot);
+    arm_smmu_rpm_put(smmu);
+
+    return ret;
  }
    static size_t arm_smmu_unmap(struct iommu_domain *domain, 
unsigned long iova,

   size_t size)
  {
  struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+    struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+    struct arm_smmu_device *smmu = smmu_domain->smmu;
+    size_t ret;
    if (!ops)
  return 0;
  -    return ops->unmap(ops, iova, size);
+    arm_smmu_rpm_get(smmu);
+    ret = ops->unmap(ops, iova, size);
+    arm_smmu_rpm_put(smmu);
+
+    return ret;
  }
    static void arm_smmu_iotlb_sync(struct iommu_domain *domain)
@@ -1407,14 +1450,22 @@ static int arm_smmu_add_device(struct device 
*dev)

  while (i--)
  cfg->smendx[i] = INVALID_SMENDX;
  +    ret = arm_smmu_rpm_get(smmu);
+    if (ret < 0)
+    goto out_cfg_free;
+
  ret = arm_smmu_master_alloc_smes(dev);


Nit: it would be easier to just do the rpm_put here; 

Re: use generic dma-direct and swiotlb code for x86 V3

[Ingo Molnar]

* Christoph Hellwig  wrote:

> On Tue, Mar 20, 2018 at 09:37:51AM +0100, Ingo Molnar wrote:
> > > git://git.infradead.org/users/hch/misc.git dma-direct-x86
> > 
> > Btw., what's the upstreaming route for these patches?
> > 
> > While it's a multi-arch series it's all pretty x86-heavy as well so we can 
> > host it 
> > in -tip (in tip:core/dma or such), but feel free to handle it yourself as 
> > well:
> > 
> >   Reviewed-by: Ingo Molnar 
> 
> Either way is fine with me.  The dma-mapping tree is pretty light this
> cycles, so I don't expect any conflicts.  If you want it feel free to grab
> it, otherwise I'll queue it up.

Ok, I picked your series up into tip:core/dma:

 - added the newly arrived Tested-by's and Reviewed-by's
 - some minor edits to titles/changelogs, no functional changes

will push it all out after testing.

Thanks,

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

Re: use generic dma-direct and swiotlb code for x86 V3

[Christoph Hellwig]

On Tue, Mar 20, 2018 at 09:37:51AM +0100, Ingo Molnar wrote:
> > git://git.infradead.org/users/hch/misc.git dma-direct-x86
> 
> Btw., what's the upstreaming route for these patches?
> 
> While it's a multi-arch series it's all pretty x86-heavy as well so we can 
> host it 
> in -tip (in tip:core/dma or such), but feel free to handle it yourself as 
> well:
> 
>   Reviewed-by: Ingo Molnar 

Either way is fine with me.  The dma-mapping tree is pretty light this
cycles, so I don't expect any conflicts.  If you want it feel free to grab
it, otherwise I'll queue it up.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: use generic dma-direct and swiotlb code for x86 V3

[Ingo Molnar]

* Christoph Hellwig  wrote:

> On Mon, Mar 19, 2018 at 11:27:37AM -0400, Konrad Rzeszutek Wilk wrote:
> > On Mon, Mar 19, 2018 at 11:38:12AM +0100, Christoph Hellwig wrote:
> > > Hi all,
> > > 
> > > this series switches the x86 code the the dma-direct implementation
> > > for direct (non-iommu) dma and the generic swiotlb ops.  This includes
> > > getting rid of the special ops for the AMD memory encryption case and
> > > the STA2x11 SOC.  The generic implementations are based on the x86
> > > code, so they provide the same functionality.
> > 
> > I need to test this on my baremertal and Xen setup - and I lost your
> > git repo URL - any chance you mention point out to me so I can
> > kick of a build?
> 
> git://git.infradead.org/users/hch/misc.git dma-direct-x86

Btw., what's the upstreaming route for these patches?

While it's a multi-arch series it's all pretty x86-heavy as well so we can host 
it 
in -tip (in tip:core/dma or such), but feel free to handle it yourself as well:

  Reviewed-by: Ingo Molnar 

Thanks,

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

Re: [PATCH] iommu/ipmmu-vmsa: Document R-Car M3-N IPMMU DT bindings

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Tuesday, 20 March 2018 08:48:11 EET Magnus Damm wrote:
> From: Magnus Damm 
> 
> Update the IPMMU DT binding documentation to include the r8a77965 compat
> string for the IPMMU devices included in the R-Car M3-N SoC.
> 
> Signed-off-by: Magnus Damm 

Reviewed-by: Laurent Pinchart 

> ---
> 
>  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1 +
>  1 file changed, 1 insertion(+)
> 
> --- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> +++
> work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt   
2018-03
> -20 15:30:50.640607110 +0900 @@ -17,6 +17,7 @@ Required Properties:
>  - "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-r8a77995" for the R8A77995 (R-Car D3) IPMMU.
>  - "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v9 1/5] driver core: Find an existing link between two devices

[Vivek Gautam]

Hi Lukasz,


On 3/14/2018 5:57 PM, Lukas Wunner wrote:

On Wed, Mar 14, 2018 at 12:14:15PM +, Robin Murphy wrote:

On Wed, Mar 14, 2018 at 8:12 PM, Rafael J. Wysocki  wrote:

On Tuesday, March 13, 2018 12:23:34 PM CET Tomasz Figa wrote:

On Tue, Mar 13, 2018 at 7:34 PM, Vivek Gautam  
wrote:

On Tue, Mar 13, 2018 at 3:45 PM, Tomasz Figa  wrote:

On Tue, Mar 13, 2018 at 5:55 PM, Vivek Gautam  
wrote:

The lists managing the device-links can be traversed to
find the link between two devices. The device_link_add() APIs
does traverse these lists to check if there's already a link
setup between the two devices.
So, add a new APIs, device_link_find(), to find an existing
device link between two devices - suppliers and consumers.

I'm wondering if this API would be useful for anything else that the
problem we're trying to solve with deleting links without storing them
anywhere. Perhaps a device_link_del_dev(consumer, supplier) would be a
better alternative?

Yea, that sounds simpler i think. Will add this API instead of
find_link(). Thanks.

Perhaps let's wait for a moment to see if there are other opinions. :)

Rafael, Lucas, any thoughts?

It is not clear to me what the device_link_del_dev(consumer, supplier)
would do.

Not quite - the issue here is that we have one supplier with an arbitrarily
large number of consumers, and would prefer that supplier not to have to
spend a whole bunch of memory to store all the struct device_link pointers
for the sole reason of having something to give to device_link_del() at the
end, given that the device links code is already keeping track of everything
internally anyway.

Makes sense to me.  How about an additional flag which autoremoves the
link on provider unbind?


If I understand this correctly, if we create the device link with 
DL_FLAG_AUTOREMOVE, the link is deleted after a consumer unbind. During 
a supplier unbind all we get is a WARN_ON with DL_FLAG_AUTOREMOVE. I 
guess that's an intended behavior?


If this is the case, then the consumer/supplier drivers just don't have 
to take care of deleting the device link explicitly.

Is my understanding correct?

regards
Vivek



Thanks,

Lukas


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

[PATCH] iommu/ipmmu-vmsa: Document R-Car M3-N IPMMU DT bindings

[Magnus Damm]

From: Magnus Damm 

Update the IPMMU DT binding documentation to include the r8a77965 compat
string for the IPMMU devices included in the R-Car M3-N SoC.

Signed-off-by: Magnus Damm 
---

 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1 +
 1 file changed, 1 insertion(+)

--- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 
2018-03-20 15:30:50.640607110 +0900
@@ -17,6 +17,7 @@ Required Properties:
 - "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-r8a77995" for the R8A77995 (R-Car D3) IPMMU.
 - "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu