[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ecbd75e7c9a96ac38846a17917339d2cae6fe74c.camel@linux.ibm.com>
Date: Thu, 20 Jun 2024 14:06:29 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: Alex Williamson <alex.williamson@...hat.com>,
Gerald Schaefer
<gerald.schaefer@...ux.ibm.com>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily
Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle
<svens@...ux.ibm.com>,
Gerd Bayer <gbayer@...ux.ibm.com>,
Matthew Rosato
<mjrosato@...ux.ibm.com>,
Jason Gunthorpe <jgg@...pe.ca>, linux-s390@...r.kernel.org,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v3 2/3] vfio/pci: Tolerate oversized BARs by disallowing
mmap
On Wed, 2024-06-19 at 21:09 -0700, Christoph Hellwig wrote:
> On Wed, Jun 19, 2024 at 12:56:47PM +0200, Niklas Schnelle wrote:
> > In short, the ISM BAR 0 is stupidly large but this is intentional. It
> > not fitting in the VMAP is simply the least crazy filter I could come
> > up with to keep the ISM device from causing trouble for use of vfio-pci
> > mmap() for other, normal, PCI devices.
>
> Then maybe add a PCI quirk to prevent mapping it. This would also
> affect the sysfs resource0 file unless I'm missing something.
>
The resource<N> files are globally disabled on s390x due to lack of
HAVE_PCI_MMAP/ARCH_GENERIC_PCI_MMAP_RESOURCE at the moment. I might
change that in the future with a analogous argument as for
VFIO_PCI_MMAP but for its not there. Once we add it we of course need
the same special ISM treatment there too.
Looking at the existing PCI quirks I don't see anything that would fit
so I'm guessing you would want to add a new quirk? As I understand it
we would then have to export something like a is_pci_mmap_broken(pdev)
function while currently the only quirk function that seems to be
exported is pxi_fixup_device(). But then what happens if
CONFIG_PCI_QUIRKS=n? Also the header comment in drivers/pci/quirks.c
says that platform specific devices shouldn't go in there and ISM is
platform specific.
Instead of exporting IOREMAP_START/IOREMAP_END maybe there is another
reasonable maximum resource length? Or maybe we could create a size_t
ioremap_area_size() helper similar to is_ioremap_addr() but not
inlined. The latter already uses IOREMAP_START/IOREMAP_END so not sure
how that works when IOREMAP_END is not exported?
Thanks,
Niklas
Powered by blists - more mailing lists