* [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-25 21:50 ` Rob Herring 0 siblings, 0 replies; 18+ messages in thread From: Rob Herring @ 2019-09-25 21:50 UTC (permalink / raw) To: Oleksandr Andrushchenko, Boris Ostrovsky Cc: linux-kernel, Robin Murphy, Julien Grall, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel As the comment says, this isn't a DT based device. of_dma_configure() is going to stop allowing a NULL DT node, so this needs to be fixed. Not sure exactly what setup besides arch_setup_dma_ops is needed... Cc: Robin Murphy <robin.murphy@arm.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Juergen Gross <jgross@suse.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: xen-devel@lists.xenproject.org Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/xen/gntdev.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index a446a7221e13..59906f9a40e4 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -34,9 +34,6 @@ #include <linux/slab.h> #include <linux/highmem.h> #include <linux/refcount.h> -#ifdef CONFIG_XEN_GRANT_DMA_ALLOC -#include <linux/of_device.h> -#endif #include <xen/xen.h> #include <xen/grant_table.h> @@ -625,14 +622,6 @@ static int gntdev_open(struct inode *inode, struct file *flip) flip->private_data = priv; #ifdef CONFIG_XEN_GRANT_DMA_ALLOC priv->dma_dev = gntdev_miscdev.this_device; - - /* - * The device is not spawn from a device tree, so arch_setup_dma_ops - * is not called, thus leaving the device with dummy DMA ops. - * Fix this by calling of_dma_configure() with a NULL node to set - * default DMA ops. - */ - of_dma_configure(priv->dma_dev, NULL, true); #endif pr_debug("priv %p\n", priv); -- 2.20.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-25 21:50 ` Rob Herring 0 siblings, 0 replies; 18+ messages in thread From: Rob Herring @ 2019-09-25 21:50 UTC (permalink / raw) To: Oleksandr Andrushchenko, Boris Ostrovsky Cc: Juergen Gross, Stefano Stabellini, linux-kernel, Julien Grall, xen-devel, Robin Murphy, Nicolas Saenz Julienne As the comment says, this isn't a DT based device. of_dma_configure() is going to stop allowing a NULL DT node, so this needs to be fixed. Not sure exactly what setup besides arch_setup_dma_ops is needed... Cc: Robin Murphy <robin.murphy@arm.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Juergen Gross <jgross@suse.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: xen-devel@lists.xenproject.org Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/xen/gntdev.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index a446a7221e13..59906f9a40e4 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -34,9 +34,6 @@ #include <linux/slab.h> #include <linux/highmem.h> #include <linux/refcount.h> -#ifdef CONFIG_XEN_GRANT_DMA_ALLOC -#include <linux/of_device.h> -#endif #include <xen/xen.h> #include <xen/grant_table.h> @@ -625,14 +622,6 @@ static int gntdev_open(struct inode *inode, struct file *flip) flip->private_data = priv; #ifdef CONFIG_XEN_GRANT_DMA_ALLOC priv->dma_dev = gntdev_miscdev.this_device; - - /* - * The device is not spawn from a device tree, so arch_setup_dma_ops - * is not called, thus leaving the device with dummy DMA ops. - * Fix this by calling of_dma_configure() with a NULL node to set - * default DMA ops. - */ - of_dma_configure(priv->dma_dev, NULL, true); #endif pr_debug("priv %p\n", priv); -- 2.20.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-25 21:50 ` [Xen-devel] " Rob Herring @ 2019-09-26 9:06 ` Nicolas Saenz Julienne -1 siblings, 0 replies; 18+ messages in thread From: Nicolas Saenz Julienne @ 2019-09-26 9:06 UTC (permalink / raw) To: Rob Herring, Oleksandr Andrushchenko, Boris Ostrovsky Cc: linux-kernel, Robin Murphy, Julien Grall, Juergen Gross, Stefano Stabellini, xen-devel [-- Attachment #1: Type: text/plain, Size: 2055 bytes --] On Wed, 2019-09-25 at 16:50 -0500, Rob Herring wrote: > As the comment says, this isn't a DT based device. of_dma_configure() > is going to stop allowing a NULL DT node, so this needs to be fixed. > > Not sure exactly what setup besides arch_setup_dma_ops is needed... > > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: Juergen Gross <jgross@suse.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: xen-devel@lists.xenproject.org > Signed-off-by: Rob Herring <robh@kernel.org> Just so it isn't forgotten, the same applies here: diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index ba1828acd8c9..de316a891f39 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -11,7 +11,6 @@ #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/module.h> -#include <linux/of_device.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> @@ -718,19 +717,7 @@ static int xen_drv_probe(struct xenbus_device *xb_dev, struct device *dev = &xb_dev->dev; int ret; - /* - * The device is not spawn from a device tree, so arch_setup_dma_ops - * is not called, thus leaving the device with dummy DMA ops. - * This makes the device return error on PRIME buffer import, which - * is not correct: to fix this call of_dma_configure() with a NULL - * node to set default DMA ops. - */ dev->coherent_dma_mask = DMA_BIT_MASK(32); - ret = of_dma_configure(dev, NULL, true); - if (ret < 0) { - DRM_ERROR("Cannot setup DMA ops, ret %d", ret); - return ret; - } front_info = devm_kzalloc(&xb_dev->dev, sizeof(*front_info), GFP_KERNEL); [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-26 9:06 ` Nicolas Saenz Julienne 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Saenz Julienne @ 2019-09-26 9:06 UTC (permalink / raw) To: Rob Herring, Oleksandr Andrushchenko, Boris Ostrovsky Cc: Juergen Gross, Stefano Stabellini, linux-kernel, Julien Grall, xen-devel, Robin Murphy [-- Attachment #1.1: Type: text/plain, Size: 2055 bytes --] On Wed, 2019-09-25 at 16:50 -0500, Rob Herring wrote: > As the comment says, this isn't a DT based device. of_dma_configure() > is going to stop allowing a NULL DT node, so this needs to be fixed. > > Not sure exactly what setup besides arch_setup_dma_ops is needed... > > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: Juergen Gross <jgross@suse.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: xen-devel@lists.xenproject.org > Signed-off-by: Rob Herring <robh@kernel.org> Just so it isn't forgotten, the same applies here: diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index ba1828acd8c9..de316a891f39 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -11,7 +11,6 @@ #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/module.h> -#include <linux/of_device.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> @@ -718,19 +717,7 @@ static int xen_drv_probe(struct xenbus_device *xb_dev, struct device *dev = &xb_dev->dev; int ret; - /* - * The device is not spawn from a device tree, so arch_setup_dma_ops - * is not called, thus leaving the device with dummy DMA ops. - * This makes the device return error on PRIME buffer import, which - * is not correct: to fix this call of_dma_configure() with a NULL - * node to set default DMA ops. - */ dev->coherent_dma_mask = DMA_BIT_MASK(32); - ret = of_dma_configure(dev, NULL, true); - if (ret < 0) { - DRM_ERROR("Cannot setup DMA ops, ret %d", ret); - return ret; - } front_info = devm_kzalloc(&xb_dev->dev, sizeof(*front_info), GFP_KERNEL); [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-25 21:50 ` [Xen-devel] " Rob Herring @ 2019-09-26 9:49 ` Julien Grall -1 siblings, 0 replies; 18+ messages in thread From: Julien Grall @ 2019-09-26 9:49 UTC (permalink / raw) To: Rob Herring, Oleksandr Andrushchenko, Boris Ostrovsky Cc: linux-kernel, Robin Murphy, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel Hi Rob, On 9/25/19 10:50 PM, Rob Herring wrote: > As the comment says, this isn't a DT based device. of_dma_configure() > is going to stop allowing a NULL DT node, so this needs to be fixed. And this can't work on arch not selecting CONFIG_OF and can select CONFIG_XEN_GRANT_DMA_ALLOC. We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just a nop. > > Not sure exactly what setup besides arch_setup_dma_ops is needed... We probably want to update dma_mask, coherent_dma_mask and dma_pfn_offset. Also, while look at of_configure_dma, I noticed that we consider the DMA will not be coherent for the grant-table. Oleksandr, do you know why they can't be coherent? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-26 9:49 ` Julien Grall 0 siblings, 0 replies; 18+ messages in thread From: Julien Grall @ 2019-09-26 9:49 UTC (permalink / raw) To: Rob Herring, Oleksandr Andrushchenko, Boris Ostrovsky Cc: Juergen Gross, Stefano Stabellini, linux-kernel, xen-devel, Robin Murphy, Nicolas Saenz Julienne Hi Rob, On 9/25/19 10:50 PM, Rob Herring wrote: > As the comment says, this isn't a DT based device. of_dma_configure() > is going to stop allowing a NULL DT node, so this needs to be fixed. And this can't work on arch not selecting CONFIG_OF and can select CONFIG_XEN_GRANT_DMA_ALLOC. We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just a nop. > > Not sure exactly what setup besides arch_setup_dma_ops is needed... We probably want to update dma_mask, coherent_dma_mask and dma_pfn_offset. Also, while look at of_configure_dma, I noticed that we consider the DMA will not be coherent for the grant-table. Oleksandr, do you know why they can't be coherent? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-26 9:49 ` [Xen-devel] " Julien Grall @ 2019-09-26 10:17 ` Oleksandr Andrushchenko -1 siblings, 0 replies; 18+ messages in thread From: Oleksandr Andrushchenko @ 2019-09-26 10:17 UTC (permalink / raw) To: Julien Grall, Rob Herring, Boris Ostrovsky Cc: linux-kernel, Robin Murphy, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel, Oleksandr Andrushchenko On 9/26/19 12:49 PM, Julien Grall wrote: > Hi Rob, > > > On 9/25/19 10:50 PM, Rob Herring wrote: >> As the comment says, this isn't a DT based device. of_dma_configure() >> is going to stop allowing a NULL DT node, so this needs to be fixed. > > And this can't work on arch not selecting CONFIG_OF and can select > CONFIG_XEN_GRANT_DMA_ALLOC. > > We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > a nop. > No luck is needed as [1] does nothing for those platforms not using CONFIG_OF >> >> Not sure exactly what setup besides arch_setup_dma_ops is needed... > > We probably want to update dma_mask, coherent_dma_mask and > dma_pfn_offset. > > Also, while look at of_configure_dma, I noticed that we consider the > DMA will not be coherent for the grant-table. Oleksandr, do you know > why they can't be coherent? The main and the only reason to use of_configure_dma is that if we don't then we are about to stay with dma_dummy_ops [2]. It effectively means that operations on dma-bufs will end up returning errors, like [3], [4], thus not making it possible for Xen PV DRM and DMA part of gntdev driver to do what we need (dma-bufs in our use-cases allow zero-copying while using graphics buffers and many more). I didn't find any better way of achieving that, but of_configure_dma... If there is any better solution which will not break the existing functionality then I will definitely change the drivers so we do not abuse DT ) Before that, please keep in mind that merging this RFC will break Xen PV DRM + DMA buf support in gntdev... Hope we can work out some acceptable solution, so everyone is happy > > Cheers, > Thank you, Oleksandr [1] https://elixir.bootlin.com/linux/v5.3.1/source/include/linux/of_device.h#L109 [2] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L33 [3] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L11 [4] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L18 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-26 10:17 ` Oleksandr Andrushchenko 0 siblings, 0 replies; 18+ messages in thread From: Oleksandr Andrushchenko @ 2019-09-26 10:17 UTC (permalink / raw) To: Julien Grall, Rob Herring, Boris Ostrovsky Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko, linux-kernel, xen-devel, Robin Murphy, Nicolas Saenz Julienne On 9/26/19 12:49 PM, Julien Grall wrote: > Hi Rob, > > > On 9/25/19 10:50 PM, Rob Herring wrote: >> As the comment says, this isn't a DT based device. of_dma_configure() >> is going to stop allowing a NULL DT node, so this needs to be fixed. > > And this can't work on arch not selecting CONFIG_OF and can select > CONFIG_XEN_GRANT_DMA_ALLOC. > > We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > a nop. > No luck is needed as [1] does nothing for those platforms not using CONFIG_OF >> >> Not sure exactly what setup besides arch_setup_dma_ops is needed... > > We probably want to update dma_mask, coherent_dma_mask and > dma_pfn_offset. > > Also, while look at of_configure_dma, I noticed that we consider the > DMA will not be coherent for the grant-table. Oleksandr, do you know > why they can't be coherent? The main and the only reason to use of_configure_dma is that if we don't then we are about to stay with dma_dummy_ops [2]. It effectively means that operations on dma-bufs will end up returning errors, like [3], [4], thus not making it possible for Xen PV DRM and DMA part of gntdev driver to do what we need (dma-bufs in our use-cases allow zero-copying while using graphics buffers and many more). I didn't find any better way of achieving that, but of_configure_dma... If there is any better solution which will not break the existing functionality then I will definitely change the drivers so we do not abuse DT ) Before that, please keep in mind that merging this RFC will break Xen PV DRM + DMA buf support in gntdev... Hope we can work out some acceptable solution, so everyone is happy > > Cheers, > Thank you, Oleksandr [1] https://elixir.bootlin.com/linux/v5.3.1/source/include/linux/of_device.h#L109 [2] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L33 [3] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L11 [4] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L18 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-26 10:17 ` [Xen-devel] " Oleksandr Andrushchenko @ 2019-09-26 10:46 ` Robin Murphy -1 siblings, 0 replies; 18+ messages in thread From: Robin Murphy @ 2019-09-26 10:46 UTC (permalink / raw) To: Oleksandr Andrushchenko, Julien Grall, Rob Herring, Boris Ostrovsky Cc: linux-kernel, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel, Oleksandr Andrushchenko On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: > > On 9/26/19 12:49 PM, Julien Grall wrote: >> Hi Rob, >> >> >> On 9/25/19 10:50 PM, Rob Herring wrote: >>> As the comment says, this isn't a DT based device. of_dma_configure() >>> is going to stop allowing a NULL DT node, so this needs to be fixed. >> >> And this can't work on arch not selecting CONFIG_OF and can select >> CONFIG_XEN_GRANT_DMA_ALLOC. >> >> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >> a nop. >> > No luck is needed as [1] does nothing for those platforms not using > CONFIG_OF >>> >>> Not sure exactly what setup besides arch_setup_dma_ops is needed... >> >> We probably want to update dma_mask, coherent_dma_mask and >> dma_pfn_offset. >> >> Also, while look at of_configure_dma, I noticed that we consider the >> DMA will not be coherent for the grant-table. Oleksandr, do you know >> why they can't be coherent? > The main and the only reason to use of_configure_dma is that if we don't > then we > are about to stay with dma_dummy_ops [2]. It effectively means that > operations on dma-bufs > will end up returning errors, like [3], [4], thus not making it possible > for Xen PV DRM and DMA > part of gntdev driver to do what we need (dma-bufs in our use-cases > allow zero-copying > while using graphics buffers and many more). > > I didn't find any better way of achieving that, but of_configure_dma... > If there is any better solution which will not break the existing > functionality then > I will definitely change the drivers so we do not abuse DT ) > Before that, please keep in mind that merging this RFC will break Xen PV > DRM + > DMA buf support in gntdev... > Hope we can work out some acceptable solution, so everyone is happy As I mentioned elsewhere, the recent dma-direct rework means that dma_dummy_ops are now only explicitly installed for the ACPI error case, so - much as I may dislike it - you should get regular (direct/SWIOTLB) ops by default again. Coherency is trickier - if the guest is allocating buffers for the PV device, which may be shared directly with hardware by the host driver, then the coherency of the PV device should really reflect that of the underlying hardware to avoid potential problems. There are some cases where the stage 2 attributes alone wouldn't be enough to correct a mismatch. Robin. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-26 10:46 ` Robin Murphy 0 siblings, 0 replies; 18+ messages in thread From: Robin Murphy @ 2019-09-26 10:46 UTC (permalink / raw) To: Oleksandr Andrushchenko, Julien Grall, Rob Herring, Boris Ostrovsky Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko, linux-kernel, xen-devel, Nicolas Saenz Julienne On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: > > On 9/26/19 12:49 PM, Julien Grall wrote: >> Hi Rob, >> >> >> On 9/25/19 10:50 PM, Rob Herring wrote: >>> As the comment says, this isn't a DT based device. of_dma_configure() >>> is going to stop allowing a NULL DT node, so this needs to be fixed. >> >> And this can't work on arch not selecting CONFIG_OF and can select >> CONFIG_XEN_GRANT_DMA_ALLOC. >> >> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >> a nop. >> > No luck is needed as [1] does nothing for those platforms not using > CONFIG_OF >>> >>> Not sure exactly what setup besides arch_setup_dma_ops is needed... >> >> We probably want to update dma_mask, coherent_dma_mask and >> dma_pfn_offset. >> >> Also, while look at of_configure_dma, I noticed that we consider the >> DMA will not be coherent for the grant-table. Oleksandr, do you know >> why they can't be coherent? > The main and the only reason to use of_configure_dma is that if we don't > then we > are about to stay with dma_dummy_ops [2]. It effectively means that > operations on dma-bufs > will end up returning errors, like [3], [4], thus not making it possible > for Xen PV DRM and DMA > part of gntdev driver to do what we need (dma-bufs in our use-cases > allow zero-copying > while using graphics buffers and many more). > > I didn't find any better way of achieving that, but of_configure_dma... > If there is any better solution which will not break the existing > functionality then > I will definitely change the drivers so we do not abuse DT ) > Before that, please keep in mind that merging this RFC will break Xen PV > DRM + > DMA buf support in gntdev... > Hope we can work out some acceptable solution, so everyone is happy As I mentioned elsewhere, the recent dma-direct rework means that dma_dummy_ops are now only explicitly installed for the ACPI error case, so - much as I may dislike it - you should get regular (direct/SWIOTLB) ops by default again. Coherency is trickier - if the guest is allocating buffers for the PV device, which may be shared directly with hardware by the host driver, then the coherency of the PV device should really reflect that of the underlying hardware to avoid potential problems. There are some cases where the stage 2 attributes alone wouldn't be enough to correct a mismatch. Robin. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-26 10:46 ` [Xen-devel] " Robin Murphy @ 2019-09-26 11:16 ` Oleksandr Andrushchenko -1 siblings, 0 replies; 18+ messages in thread From: Oleksandr Andrushchenko @ 2019-09-26 11:16 UTC (permalink / raw) To: Robin Murphy, Julien Grall, Rob Herring, Boris Ostrovsky Cc: linux-kernel, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel, Oleksandr Andrushchenko On 9/26/19 1:46 PM, Robin Murphy wrote: > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: >> >> On 9/26/19 12:49 PM, Julien Grall wrote: >>> Hi Rob, >>> >>> >>> On 9/25/19 10:50 PM, Rob Herring wrote: >>>> As the comment says, this isn't a DT based device. of_dma_configure() >>>> is going to stop allowing a NULL DT node, so this needs to be fixed. >>> >>> And this can't work on arch not selecting CONFIG_OF and can select >>> CONFIG_XEN_GRANT_DMA_ALLOC. >>> >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >>> a nop. >>> >> No luck is needed as [1] does nothing for those platforms not using >> CONFIG_OF >>>> >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... >>> >>> We probably want to update dma_mask, coherent_dma_mask and >>> dma_pfn_offset. >>> >>> Also, while look at of_configure_dma, I noticed that we consider the >>> DMA will not be coherent for the grant-table. Oleksandr, do you know >>> why they can't be coherent? >> The main and the only reason to use of_configure_dma is that if we don't >> then we >> are about to stay with dma_dummy_ops [2]. It effectively means that >> operations on dma-bufs >> will end up returning errors, like [3], [4], thus not making it possible >> for Xen PV DRM and DMA >> part of gntdev driver to do what we need (dma-bufs in our use-cases >> allow zero-copying >> while using graphics buffers and many more). >> >> I didn't find any better way of achieving that, but of_configure_dma... >> If there is any better solution which will not break the existing >> functionality then >> I will definitely change the drivers so we do not abuse DT ) >> Before that, please keep in mind that merging this RFC will break Xen PV >> DRM + >> DMA buf support in gntdev... >> Hope we can work out some acceptable solution, so everyone is happy > > As I mentioned elsewhere, the recent dma-direct rework means that > dma_dummy_ops are now only explicitly installed for the ACPI error > case, so - much as I may dislike it - you should get regular > (direct/SWIOTLB) ops by default again. Ah, my bad, I missed that change. So, if no dummy dma ops are to be used then I believe we can apply both changes, e.g. remove of_dma_configure from both of the drivers. > > Coherency is trickier - if the guest is allocating buffers for the PV > device, which may be shared directly with hardware by the host driver, > then the coherency of the PV device should really reflect that of the > underlying hardware to avoid potential problems. There are some cases > where the stage 2 attributes alone wouldn't be enough to correct a > mismatch. > > Robin. Thank you, Oleksandr ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-26 11:16 ` Oleksandr Andrushchenko 0 siblings, 0 replies; 18+ messages in thread From: Oleksandr Andrushchenko @ 2019-09-26 11:16 UTC (permalink / raw) To: Robin Murphy, Julien Grall, Rob Herring, Boris Ostrovsky Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko, linux-kernel, xen-devel, Nicolas Saenz Julienne On 9/26/19 1:46 PM, Robin Murphy wrote: > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: >> >> On 9/26/19 12:49 PM, Julien Grall wrote: >>> Hi Rob, >>> >>> >>> On 9/25/19 10:50 PM, Rob Herring wrote: >>>> As the comment says, this isn't a DT based device. of_dma_configure() >>>> is going to stop allowing a NULL DT node, so this needs to be fixed. >>> >>> And this can't work on arch not selecting CONFIG_OF and can select >>> CONFIG_XEN_GRANT_DMA_ALLOC. >>> >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >>> a nop. >>> >> No luck is needed as [1] does nothing for those platforms not using >> CONFIG_OF >>>> >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... >>> >>> We probably want to update dma_mask, coherent_dma_mask and >>> dma_pfn_offset. >>> >>> Also, while look at of_configure_dma, I noticed that we consider the >>> DMA will not be coherent for the grant-table. Oleksandr, do you know >>> why they can't be coherent? >> The main and the only reason to use of_configure_dma is that if we don't >> then we >> are about to stay with dma_dummy_ops [2]. It effectively means that >> operations on dma-bufs >> will end up returning errors, like [3], [4], thus not making it possible >> for Xen PV DRM and DMA >> part of gntdev driver to do what we need (dma-bufs in our use-cases >> allow zero-copying >> while using graphics buffers and many more). >> >> I didn't find any better way of achieving that, but of_configure_dma... >> If there is any better solution which will not break the existing >> functionality then >> I will definitely change the drivers so we do not abuse DT ) >> Before that, please keep in mind that merging this RFC will break Xen PV >> DRM + >> DMA buf support in gntdev... >> Hope we can work out some acceptable solution, so everyone is happy > > As I mentioned elsewhere, the recent dma-direct rework means that > dma_dummy_ops are now only explicitly installed for the ACPI error > case, so - much as I may dislike it - you should get regular > (direct/SWIOTLB) ops by default again. Ah, my bad, I missed that change. So, if no dummy dma ops are to be used then I believe we can apply both changes, e.g. remove of_dma_configure from both of the drivers. > > Coherency is trickier - if the guest is allocating buffers for the PV > device, which may be shared directly with hardware by the host driver, > then the coherency of the PV device should really reflect that of the > underlying hardware to avoid potential problems. There are some cases > where the stage 2 attributes alone wouldn't be enough to correct a > mismatch. > > Robin. Thank you, Oleksandr _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-26 11:16 ` [Xen-devel] " Oleksandr Andrushchenko @ 2019-09-26 19:27 ` Rob Herring -1 siblings, 0 replies; 18+ messages in thread From: Rob Herring @ 2019-09-26 19:27 UTC (permalink / raw) To: Oleksandr Andrushchenko Cc: Robin Murphy, Julien Grall, Boris Ostrovsky, linux-kernel, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel, Oleksandr Andrushchenko On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> wrote: > > On 9/26/19 1:46 PM, Robin Murphy wrote: > > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: > >> > >> On 9/26/19 12:49 PM, Julien Grall wrote: > >>> Hi Rob, > >>> > >>> > >>> On 9/25/19 10:50 PM, Rob Herring wrote: > >>>> As the comment says, this isn't a DT based device. of_dma_configure() > >>>> is going to stop allowing a NULL DT node, so this needs to be fixed. > >>> > >>> And this can't work on arch not selecting CONFIG_OF and can select > >>> CONFIG_XEN_GRANT_DMA_ALLOC. > >>> > >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > >>> a nop. > >>> > >> No luck is needed as [1] does nothing for those platforms not using > >> CONFIG_OF > >>>> > >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... > >>> > >>> We probably want to update dma_mask, coherent_dma_mask and > >>> dma_pfn_offset. > >>> > >>> Also, while look at of_configure_dma, I noticed that we consider the > >>> DMA will not be coherent for the grant-table. Oleksandr, do you know > >>> why they can't be coherent? > >> The main and the only reason to use of_configure_dma is that if we don't > >> then we > >> are about to stay with dma_dummy_ops [2]. It effectively means that > >> operations on dma-bufs > >> will end up returning errors, like [3], [4], thus not making it possible > >> for Xen PV DRM and DMA > >> part of gntdev driver to do what we need (dma-bufs in our use-cases > >> allow zero-copying > >> while using graphics buffers and many more). > >> > >> I didn't find any better way of achieving that, but of_configure_dma... > >> If there is any better solution which will not break the existing > >> functionality then > >> I will definitely change the drivers so we do not abuse DT ) > >> Before that, please keep in mind that merging this RFC will break Xen PV > >> DRM + > >> DMA buf support in gntdev... > >> Hope we can work out some acceptable solution, so everyone is happy > > > > As I mentioned elsewhere, the recent dma-direct rework means that > > dma_dummy_ops are now only explicitly installed for the ACPI error > > case, so - much as I may dislike it - you should get regular > > (direct/SWIOTLB) ops by default again. > Ah, my bad, I missed that change. So, if no dummy dma ops are to be used > then > I believe we can apply both changes, e.g. remove of_dma_configure from > both of the drivers. What about the dma masks? I think there's a default setup, but it is considered a driver bug to not set its mask. xen_drm_front sets the coherent_dma_mask (why only 32-bits though?), but not the dma_mask. gntdev is doing neither. I could copy out what of_dma_configure does but better for the Xen folks to decide what is needed or not and test the change. I'm not setup to test any of this. Rob ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-09-26 19:27 ` Rob Herring 0 siblings, 0 replies; 18+ messages in thread From: Rob Herring @ 2019-09-26 19:27 UTC (permalink / raw) To: Oleksandr Andrushchenko Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko, linux-kernel, Julien Grall, xen-devel, Boris Ostrovsky, Robin Murphy, Nicolas Saenz Julienne On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com> wrote: > > On 9/26/19 1:46 PM, Robin Murphy wrote: > > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: > >> > >> On 9/26/19 12:49 PM, Julien Grall wrote: > >>> Hi Rob, > >>> > >>> > >>> On 9/25/19 10:50 PM, Rob Herring wrote: > >>>> As the comment says, this isn't a DT based device. of_dma_configure() > >>>> is going to stop allowing a NULL DT node, so this needs to be fixed. > >>> > >>> And this can't work on arch not selecting CONFIG_OF and can select > >>> CONFIG_XEN_GRANT_DMA_ALLOC. > >>> > >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > >>> a nop. > >>> > >> No luck is needed as [1] does nothing for those platforms not using > >> CONFIG_OF > >>>> > >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... > >>> > >>> We probably want to update dma_mask, coherent_dma_mask and > >>> dma_pfn_offset. > >>> > >>> Also, while look at of_configure_dma, I noticed that we consider the > >>> DMA will not be coherent for the grant-table. Oleksandr, do you know > >>> why they can't be coherent? > >> The main and the only reason to use of_configure_dma is that if we don't > >> then we > >> are about to stay with dma_dummy_ops [2]. It effectively means that > >> operations on dma-bufs > >> will end up returning errors, like [3], [4], thus not making it possible > >> for Xen PV DRM and DMA > >> part of gntdev driver to do what we need (dma-bufs in our use-cases > >> allow zero-copying > >> while using graphics buffers and many more). > >> > >> I didn't find any better way of achieving that, but of_configure_dma... > >> If there is any better solution which will not break the existing > >> functionality then > >> I will definitely change the drivers so we do not abuse DT ) > >> Before that, please keep in mind that merging this RFC will break Xen PV > >> DRM + > >> DMA buf support in gntdev... > >> Hope we can work out some acceptable solution, so everyone is happy > > > > As I mentioned elsewhere, the recent dma-direct rework means that > > dma_dummy_ops are now only explicitly installed for the ACPI error > > case, so - much as I may dislike it - you should get regular > > (direct/SWIOTLB) ops by default again. > Ah, my bad, I missed that change. So, if no dummy dma ops are to be used > then > I believe we can apply both changes, e.g. remove of_dma_configure from > both of the drivers. What about the dma masks? I think there's a default setup, but it is considered a driver bug to not set its mask. xen_drm_front sets the coherent_dma_mask (why only 32-bits though?), but not the dma_mask. gntdev is doing neither. I could copy out what of_dma_configure does but better for the Xen folks to decide what is needed or not and test the change. I'm not setup to test any of this. Rob _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-09-26 19:27 ` [Xen-devel] " Rob Herring @ 2019-10-01 18:23 ` Stefano Stabellini -1 siblings, 0 replies; 18+ messages in thread From: Stefano Stabellini @ 2019-10-01 18:23 UTC (permalink / raw) To: Rob Herring Cc: Oleksandr Andrushchenko, Robin Murphy, Julien Grall, Boris Ostrovsky, linux-kernel, Nicolas Saenz Julienne, Juergen Gross, Stefano Stabellini, xen-devel, Oleksandr Andrushchenko On Thu, 26 Sep 2019, Rob Herring wrote: > On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko > <Oleksandr_Andrushchenko@epam.com> wrote: > > > > On 9/26/19 1:46 PM, Robin Murphy wrote: > > > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: > > >> > > >> On 9/26/19 12:49 PM, Julien Grall wrote: > > >>> Hi Rob, > > >>> > > >>> > > >>> On 9/25/19 10:50 PM, Rob Herring wrote: > > >>>> As the comment says, this isn't a DT based device. of_dma_configure() > > >>>> is going to stop allowing a NULL DT node, so this needs to be fixed. > > >>> > > >>> And this can't work on arch not selecting CONFIG_OF and can select > > >>> CONFIG_XEN_GRANT_DMA_ALLOC. > > >>> > > >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > > >>> a nop. > > >>> > > >> No luck is needed as [1] does nothing for those platforms not using > > >> CONFIG_OF > > >>>> > > >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... > > >>> > > >>> We probably want to update dma_mask, coherent_dma_mask and > > >>> dma_pfn_offset. > > >>> > > >>> Also, while look at of_configure_dma, I noticed that we consider the > > >>> DMA will not be coherent for the grant-table. Oleksandr, do you know > > >>> why they can't be coherent? > > >> The main and the only reason to use of_configure_dma is that if we don't > > >> then we > > >> are about to stay with dma_dummy_ops [2]. It effectively means that > > >> operations on dma-bufs > > >> will end up returning errors, like [3], [4], thus not making it possible > > >> for Xen PV DRM and DMA > > >> part of gntdev driver to do what we need (dma-bufs in our use-cases > > >> allow zero-copying > > >> while using graphics buffers and many more). > > >> > > >> I didn't find any better way of achieving that, but of_configure_dma... > > >> If there is any better solution which will not break the existing > > >> functionality then > > >> I will definitely change the drivers so we do not abuse DT ) > > >> Before that, please keep in mind that merging this RFC will break Xen PV > > >> DRM + > > >> DMA buf support in gntdev... > > >> Hope we can work out some acceptable solution, so everyone is happy > > > > > > As I mentioned elsewhere, the recent dma-direct rework means that > > > dma_dummy_ops are now only explicitly installed for the ACPI error > > > case, so - much as I may dislike it - you should get regular > > > (direct/SWIOTLB) ops by default again. > > Ah, my bad, I missed that change. So, if no dummy dma ops are to be used > > then > > I believe we can apply both changes, e.g. remove of_dma_configure from > > both of the drivers. > > What about the dma masks? I think there's a default setup, but it is > considered a driver bug to not set its mask. xen_drm_front sets the > coherent_dma_mask (why only 32-bits though?), but not the dma_mask. > gntdev is doing neither. I could copy out what of_dma_configure does > but better for the Xen folks to decide what is needed or not and test > the change. I'm not setup to test any of this. FYI I have seen the issue Oleksandr is talking about too. I confirm that the only reason for the of_configure_dma call is to get away from the dummy_dma_ops and use the default dma_ops instead. I think this should be mentioned in the commit message so that if one day the behavior regarding dummy_dma_ops changes one more time, hopefully we'll be able to figure out the issue more easily with bisection. In regards to the coherent_dma_mask and dma_mask, I can't see why gntdev would have any dma addressing limitations, so we should be able to set both to 64 bits. I also can't see why xen_drm_front would limit it to 32 bits, after all this is just the frontend, if anything it would be the backend that has a limitation. So, we should be able to set both dma_mask and coherent_dma_mask in xen_drm_front to 64 bits. Oleksandr, can you confirm? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-10-01 18:23 ` Stefano Stabellini 0 siblings, 0 replies; 18+ messages in thread From: Stefano Stabellini @ 2019-10-01 18:23 UTC (permalink / raw) To: Rob Herring Cc: Juergen Gross, Stefano Stabellini, Oleksandr Andrushchenko, Oleksandr Andrushchenko, linux-kernel, Julien Grall, xen-devel, Boris Ostrovsky, Robin Murphy, Nicolas Saenz Julienne On Thu, 26 Sep 2019, Rob Herring wrote: > On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko > <Oleksandr_Andrushchenko@epam.com> wrote: > > > > On 9/26/19 1:46 PM, Robin Murphy wrote: > > > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: > > >> > > >> On 9/26/19 12:49 PM, Julien Grall wrote: > > >>> Hi Rob, > > >>> > > >>> > > >>> On 9/25/19 10:50 PM, Rob Herring wrote: > > >>>> As the comment says, this isn't a DT based device. of_dma_configure() > > >>>> is going to stop allowing a NULL DT node, so this needs to be fixed. > > >>> > > >>> And this can't work on arch not selecting CONFIG_OF and can select > > >>> CONFIG_XEN_GRANT_DMA_ALLOC. > > >>> > > >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > > >>> a nop. > > >>> > > >> No luck is needed as [1] does nothing for those platforms not using > > >> CONFIG_OF > > >>>> > > >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... > > >>> > > >>> We probably want to update dma_mask, coherent_dma_mask and > > >>> dma_pfn_offset. > > >>> > > >>> Also, while look at of_configure_dma, I noticed that we consider the > > >>> DMA will not be coherent for the grant-table. Oleksandr, do you know > > >>> why they can't be coherent? > > >> The main and the only reason to use of_configure_dma is that if we don't > > >> then we > > >> are about to stay with dma_dummy_ops [2]. It effectively means that > > >> operations on dma-bufs > > >> will end up returning errors, like [3], [4], thus not making it possible > > >> for Xen PV DRM and DMA > > >> part of gntdev driver to do what we need (dma-bufs in our use-cases > > >> allow zero-copying > > >> while using graphics buffers and many more). > > >> > > >> I didn't find any better way of achieving that, but of_configure_dma... > > >> If there is any better solution which will not break the existing > > >> functionality then > > >> I will definitely change the drivers so we do not abuse DT ) > > >> Before that, please keep in mind that merging this RFC will break Xen PV > > >> DRM + > > >> DMA buf support in gntdev... > > >> Hope we can work out some acceptable solution, so everyone is happy > > > > > > As I mentioned elsewhere, the recent dma-direct rework means that > > > dma_dummy_ops are now only explicitly installed for the ACPI error > > > case, so - much as I may dislike it - you should get regular > > > (direct/SWIOTLB) ops by default again. > > Ah, my bad, I missed that change. So, if no dummy dma ops are to be used > > then > > I believe we can apply both changes, e.g. remove of_dma_configure from > > both of the drivers. > > What about the dma masks? I think there's a default setup, but it is > considered a driver bug to not set its mask. xen_drm_front sets the > coherent_dma_mask (why only 32-bits though?), but not the dma_mask. > gntdev is doing neither. I could copy out what of_dma_configure does > but better for the Xen folks to decide what is needed or not and test > the change. I'm not setup to test any of this. FYI I have seen the issue Oleksandr is talking about too. I confirm that the only reason for the of_configure_dma call is to get away from the dummy_dma_ops and use the default dma_ops instead. I think this should be mentioned in the commit message so that if one day the behavior regarding dummy_dma_ops changes one more time, hopefully we'll be able to figure out the issue more easily with bisection. In regards to the coherent_dma_mask and dma_mask, I can't see why gntdev would have any dma addressing limitations, so we should be able to set both to 64 bits. I also can't see why xen_drm_front would limit it to 32 bits, after all this is just the frontend, if anything it would be the backend that has a limitation. So, we should be able to set both dma_mask and coherent_dma_mask in xen_drm_front to 64 bits. Oleksandr, can you confirm? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API 2019-10-01 18:23 ` [Xen-devel] " Stefano Stabellini @ 2019-10-02 4:42 ` Oleksandr Andrushchenko -1 siblings, 0 replies; 18+ messages in thread From: Oleksandr Andrushchenko @ 2019-10-02 4:42 UTC (permalink / raw) To: Stefano Stabellini, Rob Herring Cc: Robin Murphy, Julien Grall, Boris Ostrovsky, linux-kernel, Nicolas Saenz Julienne, Juergen Gross, xen-devel, Oleksandr Andrushchenko On 10/1/19 9:23 PM, Stefano Stabellini wrote: > On Thu, 26 Sep 2019, Rob Herring wrote: >> On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko >> <Oleksandr_Andrushchenko@epam.com> wrote: >>> On 9/26/19 1:46 PM, Robin Murphy wrote: >>>> On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: >>>>> On 9/26/19 12:49 PM, Julien Grall wrote: >>>>>> Hi Rob, >>>>>> >>>>>> >>>>>> On 9/25/19 10:50 PM, Rob Herring wrote: >>>>>>> As the comment says, this isn't a DT based device. of_dma_configure() >>>>>>> is going to stop allowing a NULL DT node, so this needs to be fixed. >>>>>> And this can't work on arch not selecting CONFIG_OF and can select >>>>>> CONFIG_XEN_GRANT_DMA_ALLOC. >>>>>> >>>>>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >>>>>> a nop. >>>>>> >>>>> No luck is needed as [1] does nothing for those platforms not using >>>>> CONFIG_OF >>>>>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... >>>>>> We probably want to update dma_mask, coherent_dma_mask and >>>>>> dma_pfn_offset. >>>>>> >>>>>> Also, while look at of_configure_dma, I noticed that we consider the >>>>>> DMA will not be coherent for the grant-table. Oleksandr, do you know >>>>>> why they can't be coherent? >>>>> The main and the only reason to use of_configure_dma is that if we don't >>>>> then we >>>>> are about to stay with dma_dummy_ops [2]. It effectively means that >>>>> operations on dma-bufs >>>>> will end up returning errors, like [3], [4], thus not making it possible >>>>> for Xen PV DRM and DMA >>>>> part of gntdev driver to do what we need (dma-bufs in our use-cases >>>>> allow zero-copying >>>>> while using graphics buffers and many more). >>>>> >>>>> I didn't find any better way of achieving that, but of_configure_dma... >>>>> If there is any better solution which will not break the existing >>>>> functionality then >>>>> I will definitely change the drivers so we do not abuse DT ) >>>>> Before that, please keep in mind that merging this RFC will break Xen PV >>>>> DRM + >>>>> DMA buf support in gntdev... >>>>> Hope we can work out some acceptable solution, so everyone is happy >>>> As I mentioned elsewhere, the recent dma-direct rework means that >>>> dma_dummy_ops are now only explicitly installed for the ACPI error >>>> case, so - much as I may dislike it - you should get regular >>>> (direct/SWIOTLB) ops by default again. >>> Ah, my bad, I missed that change. So, if no dummy dma ops are to be used >>> then >>> I believe we can apply both changes, e.g. remove of_dma_configure from >>> both of the drivers. >> What about the dma masks? I think there's a default setup, but it is >> considered a driver bug to not set its mask. xen_drm_front sets the >> coherent_dma_mask (why only 32-bits though?), but not the dma_mask. >> gntdev is doing neither. I could copy out what of_dma_configure does >> but better for the Xen folks to decide what is needed or not and test >> the change. I'm not setup to test any of this. > FYI I have seen the issue Oleksandr is talking about too. I confirm that > the only reason for the of_configure_dma call is to get away from the > dummy_dma_ops and use the default dma_ops instead. I think this should > be mentioned in the commit message so that if one day the behavior > regarding dummy_dma_ops changes one more time, hopefully we'll be able > to figure out the issue more easily with bisection. > > In regards to the coherent_dma_mask and dma_mask, I can't see why gntdev > would have any dma addressing limitations, so we should be able to set > both to 64 bits. I also can't see why xen_drm_front would limit it to > 32 bits, after all this is just the frontend, if anything it would be > the backend that has a limitation. So, we should be able to set both > dma_mask and coherent_dma_mask in xen_drm_front to 64 bits. Oleksandr, > can you confirm? I am totally fine with 64-bits in both cases and agree with what Stefano says. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API @ 2019-10-02 4:42 ` Oleksandr Andrushchenko 0 siblings, 0 replies; 18+ messages in thread From: Oleksandr Andrushchenko @ 2019-10-02 4:42 UTC (permalink / raw) To: Stefano Stabellini, Rob Herring Cc: Juergen Gross, Oleksandr Andrushchenko, linux-kernel, Julien Grall, xen-devel, Boris Ostrovsky, Robin Murphy, Nicolas Saenz Julienne On 10/1/19 9:23 PM, Stefano Stabellini wrote: > On Thu, 26 Sep 2019, Rob Herring wrote: >> On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko >> <Oleksandr_Andrushchenko@epam.com> wrote: >>> On 9/26/19 1:46 PM, Robin Murphy wrote: >>>> On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: >>>>> On 9/26/19 12:49 PM, Julien Grall wrote: >>>>>> Hi Rob, >>>>>> >>>>>> >>>>>> On 9/25/19 10:50 PM, Rob Herring wrote: >>>>>>> As the comment says, this isn't a DT based device. of_dma_configure() >>>>>>> is going to stop allowing a NULL DT node, so this needs to be fixed. >>>>>> And this can't work on arch not selecting CONFIG_OF and can select >>>>>> CONFIG_XEN_GRANT_DMA_ALLOC. >>>>>> >>>>>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >>>>>> a nop. >>>>>> >>>>> No luck is needed as [1] does nothing for those platforms not using >>>>> CONFIG_OF >>>>>>> Not sure exactly what setup besides arch_setup_dma_ops is needed... >>>>>> We probably want to update dma_mask, coherent_dma_mask and >>>>>> dma_pfn_offset. >>>>>> >>>>>> Also, while look at of_configure_dma, I noticed that we consider the >>>>>> DMA will not be coherent for the grant-table. Oleksandr, do you know >>>>>> why they can't be coherent? >>>>> The main and the only reason to use of_configure_dma is that if we don't >>>>> then we >>>>> are about to stay with dma_dummy_ops [2]. It effectively means that >>>>> operations on dma-bufs >>>>> will end up returning errors, like [3], [4], thus not making it possible >>>>> for Xen PV DRM and DMA >>>>> part of gntdev driver to do what we need (dma-bufs in our use-cases >>>>> allow zero-copying >>>>> while using graphics buffers and many more). >>>>> >>>>> I didn't find any better way of achieving that, but of_configure_dma... >>>>> If there is any better solution which will not break the existing >>>>> functionality then >>>>> I will definitely change the drivers so we do not abuse DT ) >>>>> Before that, please keep in mind that merging this RFC will break Xen PV >>>>> DRM + >>>>> DMA buf support in gntdev... >>>>> Hope we can work out some acceptable solution, so everyone is happy >>>> As I mentioned elsewhere, the recent dma-direct rework means that >>>> dma_dummy_ops are now only explicitly installed for the ACPI error >>>> case, so - much as I may dislike it - you should get regular >>>> (direct/SWIOTLB) ops by default again. >>> Ah, my bad, I missed that change. So, if no dummy dma ops are to be used >>> then >>> I believe we can apply both changes, e.g. remove of_dma_configure from >>> both of the drivers. >> What about the dma masks? I think there's a default setup, but it is >> considered a driver bug to not set its mask. xen_drm_front sets the >> coherent_dma_mask (why only 32-bits though?), but not the dma_mask. >> gntdev is doing neither. I could copy out what of_dma_configure does >> but better for the Xen folks to decide what is needed or not and test >> the change. I'm not setup to test any of this. > FYI I have seen the issue Oleksandr is talking about too. I confirm that > the only reason for the of_configure_dma call is to get away from the > dummy_dma_ops and use the default dma_ops instead. I think this should > be mentioned in the commit message so that if one day the behavior > regarding dummy_dma_ops changes one more time, hopefully we'll be able > to figure out the issue more easily with bisection. > > In regards to the coherent_dma_mask and dma_mask, I can't see why gntdev > would have any dma addressing limitations, so we should be able to set > both to 64 bits. I also can't see why xen_drm_front would limit it to > 32 bits, after all this is just the frontend, if anything it would be > the backend that has a limitation. So, we should be able to set both > dma_mask and coherent_dma_mask in xen_drm_front to 64 bits. Oleksandr, > can you confirm? I am totally fine with 64-bits in both cases and agree with what Stefano says. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-10-02 4:43 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-25 21:50 [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API Rob Herring 2019-09-25 21:50 ` [Xen-devel] " Rob Herring 2019-09-26 9:06 ` Nicolas Saenz Julienne 2019-09-26 9:06 ` [Xen-devel] " Nicolas Saenz Julienne 2019-09-26 9:49 ` Julien Grall 2019-09-26 9:49 ` [Xen-devel] " Julien Grall 2019-09-26 10:17 ` Oleksandr Andrushchenko 2019-09-26 10:17 ` [Xen-devel] " Oleksandr Andrushchenko 2019-09-26 10:46 ` Robin Murphy 2019-09-26 10:46 ` [Xen-devel] " Robin Murphy 2019-09-26 11:16 ` Oleksandr Andrushchenko 2019-09-26 11:16 ` [Xen-devel] " Oleksandr Andrushchenko 2019-09-26 19:27 ` Rob Herring 2019-09-26 19:27 ` [Xen-devel] " Rob Herring 2019-10-01 18:23 ` Stefano Stabellini 2019-10-01 18:23 ` [Xen-devel] " Stefano Stabellini 2019-10-02 4:42 ` Oleksandr Andrushchenko 2019-10-02 4:42 ` [Xen-devel] " Oleksandr Andrushchenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.