Re: [PATCH] staging: android: ion: Skip sync if not mapped
On Tue, Apr 14, 2020 at 04:18:47PM +0200, Ørjan Eide wrote:
> @@ -238,6 +242,10 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment
> *attachment,
> struct sg_table *table,
> enum dma_data_direction direction)
> {
> + struct ion_dma_buf_attachment *a = attachment->priv;
> +
> + a->mapped = false;
Possibly a stupid question but here we're not holding a lock. Is
concurrency an issue?
> +
> dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
> }
>
> @@ -297,6 +305,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
> *dmabuf,
>
> mutex_lock(&buffer->lock);
> list_for_each_entry(a, &buffer->attachments, list) {
> + if (!a->mapped)
> + continue;
> dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
> direction);
> }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Skip sync if not mapped
On Tue, Apr 14, 2020 at 09:41:31PM -0700, John Stultz wrote:
> On Tue, Apr 14, 2020 at 7:28 AM Greg Kroah-Hartman
> wrote:
> >
> > On Tue, Apr 14, 2020 at 04:18:47PM +0200, Ørjan Eide wrote:
> > > Only sync the sg-list of an Ion dma-buf attachment when the attachment
> > > is actually mapped on the device.
> > >
> > > dma-bufs may be synced at any time. It can be reached from user space
> > > via DMA_BUF_IOCTL_SYNC, so there are no guarantees from callers on when
> > > syncs may be attempted, and dma_buf_end_cpu_access() and
> > > dma_buf_begin_cpu_access() may not be paired.
> > >
> > > Since the sg_list's dma_address isn't set up until the buffer is used
> > > on the device, and dma_map_sg() is called on it, the dma_address will be
> > > NULL if sync is attempted on the dma-buf before it's mapped on a device.
> > >
> > > Before v5.0 (commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops
> > > into the dma_direct code")) this was a problem as the dma-api (at least
> > > the swiotlb_dma_ops on arm64) would use the potentially invalid
> > > dma_address. How that failed depended on how the device handled physical
> > > address 0. If 0 was a valid address to physical ram, that page would get
> > > flushed a lot, while the actual pages in the buffer would not get synced
> > > correctly. While if 0 is an invalid physical address it may cause a
> > > fault and trigger a crash.
> > >
> > > In v5.0 this was incidentally fixed by commit 55897af63091 ("dma-direct:
> > > merge swiotlb_dma_ops into the dma_direct code"), as this moved the
> > > dma-api to use the page pointer in the sg_list, and (for Ion buffers at
> > > least) this will always be valid if the sg_list exists at all.
> > >
> > > But, this issue is re-introduced in v5.3 with
> > > commit 449fa54d6815 ("dma-direct: correct the physical addr in
> > > dma_direct_sync_sg_for_cpu/device") moves the dma-api back to the old
> > > behaviour and picks the dma_address that may be invalid.
> > >
> > > dma-buf core doesn't ensure that the buffer is mapped on the device, and
> > > thus have a valid sg_list, before calling the exporter's
> > > begin_cpu_access.
> > >
> > > Signed-off-by: Ørjan Eide
> > > ---
> > > drivers/staging/android/ion/ion.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > Resubmit without disclaimer, sorry about that.
> > >
> > > This seems to be part of a bigger issue where dma-buf exporters assume
> > > that their dma-buf begin_cpu_access and end_cpu_access callbacks have a
> > > certain guaranteed behavior, which isn't ensured by dma-buf core.
> > >
> > > This patch fixes this in ion only, but it also needs to be fixed for
> > > other exporters, either handled like this in each exporter, or in
> > > dma-buf core before calling into the exporters.
> > >
> > > diff --git a/drivers/staging/android/ion/ion.c
> > > b/drivers/staging/android/ion/ion.c
> > > index 38b51eace4f9..7b752ba0cb6d 100644
> > > --- a/drivers/staging/android/ion/ion.c
> > > +++ b/drivers/staging/android/ion/ion.c
> >
> > Now that we have the dma-buff stuff in the tree, do we even need the
> > ion code in the kernel anymore? Can't we delete it now?
> >
>
> I agree that we shouldn't be taking further (non-security/cleanup)
> patches to the ION code.
>
> I'd like to give developers a little bit of a transition period (I was
> thinking a year, but really just one LTS release that has both would
> do) where they can move their ION heaps over to dmabuf heaps and test
> both against the same tree.
>
> But I do think we can mark it as deprecated and let folks know that
> around the end of the year it will be deleted.
No one ever notices "depreciated" things, they only notice if the code
is no longer there :)
So I'm all for just deleting it and seeing who even notices...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: Comedi: Drivers: das08: Fixed some coding style issues
A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Mon, Apr 13, 2020 at 04:55:08PM +0200, Carlos Guerrero Álvarez wrote: > What do you mean with the From line? Look at the patch you sent, the "From:" line on your email does not have your name, only your email address. Please fix that up when you resend any future patches. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
CURSOS BONIFICABLES DESDE CASA (Empleados activos y en ERTE)
Buenos días Se encuentra abierto el plazo de inscripción de Cursos Bonificables para empleados en activo y en situación de ERTE. Todos los cursos son totalmente Bonificables con cargo al Crédito de Formación 2020 que dispone las empresa. Se realizan desde casa en modalidad individual E-learning a través de la plataforma web y con total flexibilidad horaria. Deseáis que os mandemos la información? Saludos cordiales. Alex Pons Director departamento formación. FOESCO Formación Estatal Continua. Entidad Organizadora: B171823AP www.foesco.com e-mail: cur...@foesco.net Tel: 910 323 794 (Horario de 9h a 15h y de 17h a 20h de Lunes a Viernes) FOESCO ofrece formación a empresas y trabajadores en activo a través de cursos bonificados por la Fundación Estatal para la Formación en el Empleo (antiguo FORCEM) que gestiona las acciones formativas de FORMACIÓN CONTINUA para trabajadores y se rige por la ley 30/2015 de 9 de Septiembre. Si no desea recibir mas información de FOESCO responda a este correo con la palabra BAJA en el asunto. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/20] staging: wfx: align semantic of beacon filter with other filters
On Wed, Apr 15, 2020 at 06:11:39PM +0200, Jerome Pouiller wrote: > From: Jérôme Pouiller > > Filters provided by HIF API are sometime inclusive, sometime exclusive. > > This patch align the behavior and name of the beacon filter with the > other filters. Also avoid double negation: "disable filter" Hooray! I have been wanting to suggest this every time I see the ->disable_beacon_filter name, especially for patch 7/20. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] Add new uio device for PCI with dynamic memory allocation
This device combines the uio_pci_generic driver and the uio_dmem_genirq driver since PCI uses a slightly different API for interrupts. A fixed number of DMA capable memory regions can be defined using the module parameter "dmem_sizes". The memory is not allocated until the uio device file is opened for the first time. When the device file is closed, the allocated memory block is freed. Physical (DMA) addresses for the dynamic regions are provided to the userspace via /sys/class/uio/uioX/maps/mapY/addr When no processes are holding the device file open, the address returned to userspace is DMA_ERROR_CODE. Signed-off-by: Manuel Stahl --- MAINTAINERS | 6 + drivers/uio/Kconfig | 9 + drivers/uio/Makefile | 1 + drivers/uio/uio_pci_dmem_genirq.c | 351 ++ 4 files changed, 367 insertions(+) create mode 100644 drivers/uio/uio_pci_dmem_genirq.c diff --git a/MAINTAINERS b/MAINTAINERS index e64e5db31497..446931530dbc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7149,6 +7149,12 @@ L: k...@vger.kernel.org S: Supported F: drivers/uio/uio_pci_generic.c +GENERIC UIO DRIVER FOR PCI DEVICES WITH DMA +M: "Manuel Stahl" +L: k...@vger.kernel.org +S: Supported +F: drivers/uio/uio_pci_dmem_genirq.c + GENERIC VDSO LIBRARY M: Andy Lutomirski M: Thomas Gleixner diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig index 202ee81cfc2b..0d3f8a01ec74 100644 --- a/drivers/uio/Kconfig +++ b/drivers/uio/Kconfig @@ -94,6 +94,15 @@ config UIO_PCI_GENERIC primarily, for virtualization scenarios. If you compile this as a module, it will be called uio_pci_generic. +config UIO_PCI_DMEM_GENIRQ + tristate "Generic driver for PCI 2.3 and PCI Express cards with DMA" + depends on PCI + help + Generic driver that you can bind, dynamically, to any + PCI 2.3 compliant and PCI Express card. It is useful + for FPGAs with DMA capability connected via PCI. + If you compile this as a module, it will be called uio_pci_dmem_genirq. + config UIO_NETX tristate "Hilscher NetX Card driver" depends on PCI diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile index c285dd2a4539..202d6bfdd5aa 100644 --- a/drivers/uio/Makefile +++ b/drivers/uio/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_UIO_DMEM_GENIRQ) += uio_dmem_genirq.o obj-$(CONFIG_UIO_AEC) += uio_aec.o obj-$(CONFIG_UIO_SERCOS3) += uio_sercos3.o obj-$(CONFIG_UIO_PCI_GENERIC) += uio_pci_generic.o +obj-$(CONFIG_UIO_PCI_DMEM_GENIRQ) += uio_pci_dmem_genirq.o obj-$(CONFIG_UIO_NETX) += uio_netx.o obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o obj-$(CONFIG_UIO_MF624) += uio_mf624.o diff --git a/drivers/uio/uio_pci_dmem_genirq.c b/drivers/uio/uio_pci_dmem_genirq.c new file mode 100644 index ..be1bdcc552fe --- /dev/null +++ b/drivers/uio/uio_pci_dmem_genirq.c @@ -0,0 +1,351 @@ +// SPDX-License-Identifier: GPL-2.0 +/* uio_pci_generic - generic UIO driver for PCI 2.3 devices with DMA memory + * + * Copyright (C) 2016 Fraunhofer IIS + * Author: Manuel Stahl + * + * Based on uio_pci_generic.c by Michael S. Tsirkin + * and uio_dmem_genirq.c by Damian Hobson-Garcia. + * + * Since the driver does not declare any device ids, you must allocate + * id and bind the device to the driver yourself. For example: + * + * # echo "8086 10f5" > /sys/bus/pci/drivers/uio_pci_dmem_genirq/new_id + * # echo -n :00:19.0 > /sys/bus/pci/drivers/e1000e/unbind + * # echo -n :00:19.0 > /sys/bus/pci/drivers/uio_pci_dmem_genirq/bind + * # ls -l /sys/bus/pci/devices/:00:19.0/driver + * .../:00:19.0/driver -> ../../../bus/pci/drivers/uio_pci_dmem_genirq + * + * Or use a modprobe alias: + * # alias pci:v10EEd1000sv*sd*sc*i* uio_pci_dmem_genirq + * + * Driver won't bind to devices which do not support the Interrupt Disable Bit + * in the command register. All devices compliant to PCI 2.3 (circa 2002) and + * all compliant PCI Express devices should support this bit. + * + * The DMA mask bits and sizes of dynamic regions are derived from module + * parameters. + * + * The format for specifying dynamic region sizes in module parameters + * is as follows: + * + * uio_pci_dmem_genirq.dmem_sizes := [;] + *:= :[,] + *:= : + * := standard linux memsize + * + * Examples: + * + * 1) UIO dmem device with 3 dynamic regions: + * uio_pci_dmem_genirq.dmem_sizes=8086:10f5:4K,16K,4M + * + * 2) Two UIO dmem devices with different number of dynamic regions: + * uio_pci_dmem_genirq.dmem_sizes=8086:10f5:4K,16K,4M;1234:0001:8K + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_VERSION "0.01.0" +#define DRIVER_AUTHOR "Manuel Stahl " +#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices with DMA memory" +#define DRIVER_NAME "uio_pci_dmem_genirq"
Re: [PATCH] staging: android: ion: Skip sync if not mapped
Great! Thanks! regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Skip sync if not mapped
On Thu, Apr 16, 2020 at 12:49:56PM +0300, Dan Carpenter wrote:
> On Tue, Apr 14, 2020 at 04:18:47PM +0200, Ørjan Eide wrote:
> > @@ -238,6 +242,10 @@ static void ion_unmap_dma_buf(struct
> > dma_buf_attachment *attachment,
> > struct sg_table *table,
> > enum dma_data_direction direction)
> > {
> > + struct ion_dma_buf_attachment *a = attachment->priv;
> > +
> > + a->mapped = false;
>
> Possibly a stupid question but here we're not holding a lock. Is
> concurrency an issue?
Thanks for taking a look.
Here and in ion_map_dma_buf(), where mapped is set, this should be safe.
The callers must synchronize calls to map/unmap, and ensure that they
are called in pairs.
I think there may be a potential issue between where mapped is set here,
and where it's read in ion_dma_buf_{begin,end}_cpu_access(). Coherency
issues the mapped bool won't blow up, at worst the contents of the
buffer may be invalid as cache synchronization may have been skipped.
This is still an improvement as before it would either crash or sync the
wrong page repeatedly.
The mapped state is tied to the dma_map_sg() call, so we would need take
the buffer->lock around both dma_map_sg and setting mapped to be sure
that the buffer will always have been synced.
> > +
> > dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
> > }
> >
> > @@ -297,6 +305,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
> > *dmabuf,
> >
> > mutex_lock(&buffer->lock);
> > list_for_each_entry(a, &buffer->attachments, list) {
> > + if (!a->mapped)
> > + continue;
> > dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
> > direction);
> > }
--
Ørjan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
