linux-kernel - Re: [PATCH v1 0/4] vfio/iommu: Flag to allow userspace to set DMA buffers system cacheable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+M8utNVshFNJyrw1PcrVSJbFbDqPRkM_PjpF8CGy7Nf8pJRjQ@mail.gmail.com>
Date: Tue, 27 Aug 2024 19:55:15 -0700
From: Manoj Vishwanathan <manojvishy@...gle.com>
To: Robin Murphy <robin.murphy@....com>
Cc: Jason Gunthorpe <jgg@...dia.com>, Alex Williamson <alex.williamson@...hat.com>, 
	Will Deacon <will@...nel.org>, Joerg Roedel <joro@...tes.org>, linux-arm-kernel@...ts.infradead.org, 
	kvm@...r.kernel.org, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	David Dillow <dillow@...gle.com>, David Decotigny <decot@...gle.com>
Subject: Re: [PATCH v1 0/4] vfio/iommu: Flag to allow userspace to set DMA
 buffers system cacheable

On Tue, Aug 27, 2024 at 10:31 AM Robin Murphy <robin.murphy@....com> wrote:
>
> On 27/08/2024 12:17 am, Jason Gunthorpe wrote:
> > On Mon, Aug 26, 2024 at 11:04:47AM -0600, Alex Williamson wrote:
> >> On Mon, 26 Aug 2024 07:16:37 +0000
> >> Manoj Vishwanathan <manojvishy@...gle.com> wrote:
> >>
> >>> Hi maintainers,
> >>>
> >>> This RFC patch introduces the ability for userspace to control whether
> >>> device (DMA) buffers are marked as cacheable, enabling them to utilize
> >>> the system-level cache.
> >>>
> >>> The specific changes made in this patch are:
> >>>
> >>> * Introduce a new flag in `include/linux/iommu.h`:
> >>>      * `IOMMU_SYS_CACHE` -  Indicates if the associated page should be cached in the system's cache hierarchy.
> >>> * Add `VFIO_DMA_MAP_FLAG_SYS_CACHE` to `include/uapi/linux/vfio.h`:
> >
> > You'll need a much better description of what this is supposed to do
> > when you resend it.
> >
Thanks Jason, I will add more information before re-sending the patch.
> > IOMMU_CACHE already largely means that pages should be cached.
> >
> > So I don't know what ARM's "INC_OCACHE" actually is doing. Causing
> > writes to land in a cache somewhere in hierarchy? Something platform
> > specific?
>
> Kinda both - the Inner Non-Cacheable attribute means it's still
> fundamentally non-snooping and non-coherent with CPU caches, but the
> Outer Write-back Write-allocate attribute can still control allocation
> in a system cache downstream of the point of coherency if the platform
> is built with such a thing (it's not overly common).
>
> However, as you point out, this is in direct conflict with the Inner
> Write-back Write-allocate attribute implied by the IOMMU_CACHE which
> VFIO adds in further down in vfio_iommu_map(). Plus the way it's
> actually implemented in patch #2, IOMMU_CACHE still takes precedence and
> would lead to this new value being completely ignored, so there's a lot
> which smells suspicious here...
>
Thanks for your quick feedback.
I tested this with a 5.15-based kernel and applied the patch to get
early results.
I see the issue with IOMMU_CACHE overriding IOMMU_SYS_CACHE in patch #2.
This would likely be a problem on 5.15 as well, and I need to
investigate further
to understand how we observed better performance in our tests
while trying to exercise the system cache.
Let me do some more testing and get back.
Thanks,
Manoj
> Thanks,
> Robin.
>
> > I have no idea. By your description it sounds similar to the
> > x86 data placement stuff, whatever that was called, and the more
> > modern TPH approach.
> >
> > Jason