Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, 18 Apr 2023 22:10:03 -0700, Christoph Hellwig wrote: > On Tue, Apr 18, 2023 at 02:18:52PM +0800, Xuan Zhuo wrote: > > Sorry, rethink about this, I think we maybe misunderstand something. > > > > First of all, let me give you a brief introduce of virtio device and pci > > device. > > If I make mistake, please point out. > > > > > > First, when one virtio pci device is probed, then the virtio pci driver > > will be > > called. Then we got one pci_device. > > Yes. > > > Then virtio_pci_probe will alloc one new device, and register it to virtio > > bus > > by register_virtio_device(). > > > > > > So here we have two device: pci-device and virtio-device. > > Yes. > > > If we call DMA API inside virtio, we use the pci-device. The virtio-device > > is > > not used for DMA API. > > Exactly. > > > Now we want to use the virtio-device to do direct dma. The virtio-device > > is created by virtio_pci_probe() of virtio pci driver. And register to > > virtio > > bus. So no firmware and not iommu and the bus is virtio bus, why we can not > > change the dma_ops of virtio-device? > > Because firmware doesn't know about your virtio-device. It is just a > made up Linux concept, and the IOMMU and firmware tables for it don't > know about it. DMA must only ever be done on actual physical > (including "physical" devices emulated by a hypervisor) devices, not > on devices made up by Linux. It's clear for me. Thanks. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 18, 2023 at 02:18:52PM +0800, Xuan Zhuo wrote: > Sorry, rethink about this, I think we maybe misunderstand something. > > First of all, let me give you a brief introduce of virtio device and pci > device. > If I make mistake, please point out. > > > First, when one virtio pci device is probed, then the virtio pci driver will > be > called. Then we got one pci_device. Yes. > Then virtio_pci_probe will alloc one new device, and register it to virtio bus > by register_virtio_device(). > > > So here we have two device: pci-device and virtio-device. Yes. > If we call DMA API inside virtio, we use the pci-device. The virtio-device is > not used for DMA API. Exactly. > Now we want to use the virtio-device to do direct dma. The virtio-device > is created by virtio_pci_probe() of virtio pci driver. And register to virtio > bus. So no firmware and not iommu and the bus is virtio bus, why we can not > change the dma_ops of virtio-device? Because firmware doesn't know about your virtio-device. It is just a made up Linux concept, and the IOMMU and firmware tables for it don't know about it. DMA must only ever be done on actual physical (including "physical" devices emulated by a hypervisor) devices, not on devices made up by Linux. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, 11 Apr 2023 05:16:19 -0700, Christoph Hellwig wrote: > On Tue, Apr 11, 2023 at 03:23:43PM +0800, Xuan Zhuo wrote: > > > If a direct map or not is used is a decision done by the platform code, > > > often based on firmware tables. You can't just override that. > > > > > > Can Virtio Device set its own dma_ops? It is a device on the virtual bus. It > > sets its own DMA_OPS. I think it is reasonable. > > No, it can't. virtio devices are backed by PCI, platform or other > bus devices, and the (often virtual) firmware controls how DMA mapping > is to be performed for them, at least for the platform_access case. Sorry, rethink about this, I think we maybe misunderstand something. First of all, let me give you a brief introduce of virtio device and pci device. If I make mistake, please point out. First, when one virtio pci device is probed, then the virtio pci driver will be called. Then we got one pci_device. Then virtio_pci_probe will alloc one new device, and register it to virtio bus by register_virtio_device(). So here we have two device: pci-device and virtio-device. If we call DMA API inside virtio, we use the pci-device. The virtio-device is not used for DMA API. Now we want to use the virtio-device to do direct dma. The virtio-device is created by virtio_pci_probe() of virtio pci driver. And register to virtio bus. So no firmware and not iommu and the bus is virtio bus, why we can not change the dma_ops of virtio-device? Thanks. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Wed, Apr 12, 2023 at 10:03:46AM +0800, Xuan Zhuo wrote: > We discusses this question last at [1]. We planned to pass one device to xsk. > Then xsk can use the DMA API on this device. The device can be one hw > device(such as PCI, mmio) or virtio device. If it is one hw device, no > problem. What do you mean with one here? A virtio device should never be baken by more than one hardware device. > If it is one virtio device, then we expect that DMA API on that will return > direct dma map, because the dma_ops of the virtio device is NULL. But on some > platform such as sparc64 or some case of x86, the arch has own dma_ops. So we > wrong. No, you can't expect a device to use any particular dma ops. Most platforms support either the direct mapping or some form of IOMMU. But for virtio thinks are even more complicated - unless VIRTIO_F_ACCESS_PLATFORM is set (which really must be set for all real hardware devices for them to work), the DMA API isn't even used at all. That means the virtual platform (i.e.g qemu) does DMA based on physical addresses and virtio ignores all the plaform DMA setup. > And as the discuss of this thread, we cannot get direct dma address from > virtio device by DMA API. So, we need xsk to expose the DMA ops to the > virtio-net driver when virtio core can not use the DMA API. > > All the processing of dma inside xsk: > 1. map/unmap at enable/disable > 2. dma sync For the VIRTIO_F_ACCESS_PLATFORM you can just use the DMA API like any other device, but this really should be done inside virtio instead of an upper layer. For the !VIRTIO_F_ACCESS_PLATFORM case there is no need to sync, and the dma mapping is a simple virt_to_phys or page_to_phys, so there's no real need to premap to start with. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
Here, this has cc the maintainers of AF_XDP. For on the same page, I summarize it. We discusses this question last at [1]. We planned to pass one device to xsk. Then xsk can use the DMA API on this device. The device can be one hw device(such as PCI, mmio) or virtio device. If it is one hw device, no problem. If it is one virtio device, then we expect that DMA API on that will return direct dma map, because the dma_ops of the virtio device is NULL. But on some platform such as sparc64 or some case of x86, the arch has own dma_ops. So we wrong. And as the discuss of this thread, we cannot get direct dma address from virtio device by DMA API. So, we need xsk to expose the DMA ops to the virtio-net driver when virtio core can not use the DMA API. All the processing of dma inside xsk: 1. map/unmap at enable/disable 2. dma sync Solution: 1. xsk uses phy memory and passes the dma sync when the dev is null. 2. xsk uses the callbacks passed by driver to do the dma. If I miss something, please point out. Thanks. [1] https://lore.kernel.org/virtualization/1677727282.6062915-2-xuanz...@linux.alibaba.com/ ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 03:12:13PM +0800, Xuan Zhuo wrote: > > Hi Jason > > Can we force virtio core to use dma api without VIRTIO_F_ACCESS_PLATFORM? So in your last mail you asked for it to be the other way around? But, no you can't do this either unless you control the platform as in the Xen PV case. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 03:23:43PM +0800, Xuan Zhuo wrote: > > If a direct map or not is used is a decision done by the platform code, > > often based on firmware tables. You can't just override that. > > > Can Virtio Device set its own dma_ops? It is a device on the virtual bus. It > sets its own DMA_OPS. I think it is reasonable. No, it can't. virtio devices are backed by PCI, platform or other bus devices, and the (often virtual) firmware controls how DMA mapping is to be performed for them, at least for the platform_access case. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 3:21 PM Xuan Zhuo wrote: > > > Hi Jason > > Can we force virtio core to use dma api without VIRTIO_F_ACCESS_PLATFORM? > > I don't think so. Without ACCESS_PLATFORM it's not DMA, so DMA API should be avoided. And we have several software devices for virtio now. I'd suggest involving AF_XDP maintainers to join the discussion to seek a solution. I tend to agree with Christoph that everything will be simplified if DMA is done at the driver level. Thanks > Thanks. > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 12:54:46PM +0800, Jason Wang wrote: > On Tue, Apr 11, 2023 at 12:10 PM Christoph Hellwig wrote: > > > > On Tue, Apr 11, 2023 at 11:56:47AM +0800, Jason Wang wrote: > > > > The DMA device for virtio_pci is the underlying PCI device, always. > > > > !VIRTIO_F_ACCESS_PLATFORM means there is no dma device at all. Because > > > > of all these things you can't just expose a pointer to the dma_device > > > > as that is just a completely wrong way of thinking about the problem. > > > > > > Ok, so if there's no DMA at all we should avoid using the DMA API > > > completely. This means we should check dma_dev against NULL in > > > virtio_has_dma_quirk(). > > > > No nee to add a check to virtio_has_dma_quirk. > > Ok, just to clarify, I meant there could be a case where the virtqueue > is emulated by software, in this case we need check whether the > virtqueue has a dma device or not in vring_use_dma_api(). If not we > need return false. > > > But looking at virtio_has_dma_quirk shows that virtio-gpu is > > pretty messed up already as well. Yes. For gem objects allocated in guest ram virtio-gpu effectively passes a scatter list to the host. It doesn't use vrings for that though, so it has to re-implement some of the logic the virtio core already has for the vrings. > > It can't really poke into the DMA details. We'll need core virtio > > helpers for allocating and syncing a sg_table instead and make > > virtio_has_dma_quirk private to the core. That should work. take care, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 23:45:33 -0700, Christoph Hellwig wrote: > On Tue, Apr 11, 2023 at 02:33:29PM +0800, Xuan Zhuo wrote: > > Do you mean my idea is wrong, right? > > > > Can you explain it? Why can't we support a device that uses only dirct map? > > If a direct map or not is used is a decision done by the platform code, > often based on firmware tables. You can't just override that. Can Virtio Device set its own dma_ops? It is a device on the virtual bus. It sets its own DMA_OPS. I think it is reasonable. Thanks. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
Hi Jason Can we force virtio core to use dma api without VIRTIO_F_ACCESS_PLATFORM? Thanks. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, 11 Apr 2023 14:51:29 +0800, Xuan Zhuo wrote: > On Mon, 10 Apr 2023 23:46:56 -0700, Christoph Hellwig > wrote: > > On Tue, Apr 11, 2023 at 02:28:00PM +0800, Xuan Zhuo wrote: > > > > That is just completely broken, virtio or not. Highlevel code like > > > > sockets must never do dma mappings themselves. > > > > > > AF_XDP provides some API for net driver. This API will do dma map or dma > > > sync. > > > > > > cc AF_XDP maintainers. > > > > So in that case AF_XDP doesn't actually require a dma device as you > > claimed early and gets the layering right after all. Just don't use that > > API from a network driver that doesn't need to do dma mappings like > > virtio for the !platform_access case then. > > > Yes For Virtio-Net support AF_XDP, if we cannot provide a direct DMA map device, then we have two solutions now, but we need to cooperate with AF_XDP. 1. pass NULL as dev, then AF_XDP use phy memory as dma address, and skip sync 2. do dma/sync by callback from virtio-net Thanks. > > ___ > Virtualization mailing list > Virtualization@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 23:46:56 -0700, Christoph Hellwig wrote: > On Tue, Apr 11, 2023 at 02:28:00PM +0800, Xuan Zhuo wrote: > > > That is just completely broken, virtio or not. Highlevel code like > > > sockets must never do dma mappings themselves. > > > > AF_XDP provides some API for net driver. This API will do dma map or dma > > sync. > > > > cc AF_XDP maintainers. > > So in that case AF_XDP doesn't actually require a dma device as you > claimed early and gets the layering right after all. Just don't use that > API from a network driver that doesn't need to do dma mappings like > virtio for the !platform_access case then. Yes ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 02:28:00PM +0800, Xuan Zhuo wrote: > > That is just completely broken, virtio or not. Highlevel code like > > sockets must never do dma mappings themselves. > > AF_XDP provides some API for net driver. This API will do dma map or dma sync. > > cc AF_XDP maintainers. So in that case AF_XDP doesn't actually require a dma device as you claimed early and gets the layering right after all. Just don't use that API from a network driver that doesn't need to do dma mappings like virtio for the !platform_access case then. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 02:33:29PM +0800, Xuan Zhuo wrote: > Do you mean my idea is wrong, right? > > Can you explain it? Why can't we support a device that uses only dirct map? If a direct map or not is used is a decision done by the platform code, often based on firmware tables. You can't just override that. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 23:30:29 -0700, Christoph Hellwig wrote: > > > Can you please explain what you're even trying to do? > > > > I want to force a device use direct dma map. > > You fundamentally can't do that. It can't work. Do you mean my idea is wrong, right? Can you explain it? Why can't we support a device that uses only dirct map? Thanks. > ___ > Virtualization mailing list > Virtualization@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 23:20:59 -0700, Christoph Hellwig wrote: > On Tue, Apr 11, 2023 at 02:11:17PM +0800, Xuan Zhuo wrote: > > NO, all dma maping is done inside xdp socket. That is done > > when setup. > > That is just completely broken, virtio or not. Highlevel code like > sockets must never do dma mappings themselves. AF_XDP provides some API for net driver. This API will do dma map or dma sync. cc AF_XDP maintainers. If we cannot set Virtio Deivce to a direct DMA device, then we can only make some modifications in AF_XDP. Thanks. > ___ > Virtualization mailing list > Virtualization@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
> > Can you please explain what you're even trying to do? > > I want to force a device use direct dma map. You fundamentally can't do that. It can't work. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 20:26:20 -0700, Christoph Hellwig wrote: > On Tue, Apr 11, 2023 at 09:56:19AM +0800, Xuan Zhuo wrote: > > Do you mean we should not change the dma_ops from the outside of the core > > kernel/dma/? > > Basically, yes. That plus probing for the IOMMU drivers. > > > How about adding one new "dma_direct" to struct devive? > > Why would we? > > Can you please explain what you're even trying to do? I want to force a device use direct dma map. Thanks. > But checking > a dma_direct flag for sure isn't the solution. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 02:11:17PM +0800, Xuan Zhuo wrote: > NO, all dma maping is done inside xdp socket. That is done > when setup. That is just completely broken, virtio or not. Highlevel code like sockets must never do dma mappings themselves. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, 11 Apr 2023 11:56:47 +0800, Jason Wang wrote: > On Tue, Apr 11, 2023 at 11:31 AM Christoph Hellwig wrote: > > > > On Tue, Apr 11, 2023 at 10:09:40AM +0800, Jason Wang wrote: > > > We want to support AF_XDP for virtio-net. It means AF_XDP needs to > > > know the dma device to perform DMA mapping. So we introduce a helper > > > to expose the dma dev of the virtio device. > > > > The whole virtio architecture is based around the core code doing > > the DMA mapping. I can't see how you can just export a helper to > > expose the dma device. You'd have to complete rework the layering > > of the virtio code if you want to do it in the upper level drivers. > > And why would you want to do this? The low-level code is the only > > piece that can actually know if you need to do a dma mapping. All > > the kernel subsystems that don't do it inside the low-level drivers > > or helpers closely associtated are a giant and hard to fix map > > (see usb for the prime exhibit). > > > > So the first question is: why do you want this for XF_ADP, > > Xuan, is it possible to set up the DMA mapping inside the virtio > driver itself? I vaguely remember at least the RX buffer mapping is > done by the driver. If this is true, we can avoid exposing DMA details > to the upper layer. NO, all dma maping is done inside xdp socket. That is done when setup. When adding to RX Ring, xdp socket will call DMA SYNC. > > > and > > the next question will be how to do that without making a complete > > mess. > > > > > This works fine as long as VIRTIO_F_ACCESS_PLATFORM is negotiated. But > > > when it is not, the virtio driver needs to use a physical address so > > > we want to expose the virtio device without dma_ops in the hope that > > > it will go for direct mapping where the physical address is used. But > > > it may not work on some specific setups (arches that assume an IOMMU > > > or have arch dma ops). > > > > The DMA device for virtio_pci is the underlying PCI device, always. > > !VIRTIO_F_ACCESS_PLATFORM means there is no dma device at all. Because > > of all these things you can't just expose a pointer to the dma_device > > as that is just a completely wrong way of thinking about the problem. > > Ok, so if there's no DMA at all we should avoid using the DMA API > completely. This means we should check dma_dev against NULL in > virtio_has_dma_quirk(). > > Thanks > > > > > ___ > Virtualization mailing list > Virtualization@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 1:14 PM Christoph Hellwig wrote: > > On Tue, Apr 11, 2023 at 12:54:46PM +0800, Jason Wang wrote: > > Ok, just to clarify, I meant there could be a case where the virtqueue > > is emulated by software, in this case we need check whether the > > virtqueue has a dma device or not in vring_use_dma_api(). If not we > > need return false. > > Well, that's what vring_use_dma_api basically does. Such devics > just should never have VIRTIO_F_ACCESS_PLATFORM set. If there is > a really good reason for such a device to have VIRTIO_F_ACCESS_PLATFORM > set we'd need an extra quirk in vring_use_dma_api indeed. > This is useful for some vDPA devices where the control virtqueue is emulated by the vDPA parent (who will decode the control virtqueue commands to vendor specific control commands). Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 12:54:46PM +0800, Jason Wang wrote: > Ok, just to clarify, I meant there could be a case where the virtqueue > is emulated by software, in this case we need check whether the > virtqueue has a dma device or not in vring_use_dma_api(). If not we > need return false. Well, that's what vring_use_dma_api basically does. Such devics just should never have VIRTIO_F_ACCESS_PLATFORM set. If there is a really good reason for such a device to have VIRTIO_F_ACCESS_PLATFORM set we'd need an extra quirk in vring_use_dma_api indeed. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 12:10 PM Christoph Hellwig wrote: > > On Tue, Apr 11, 2023 at 11:56:47AM +0800, Jason Wang wrote: > > > The DMA device for virtio_pci is the underlying PCI device, always. > > > !VIRTIO_F_ACCESS_PLATFORM means there is no dma device at all. Because > > > of all these things you can't just expose a pointer to the dma_device > > > as that is just a completely wrong way of thinking about the problem. > > > > Ok, so if there's no DMA at all we should avoid using the DMA API > > completely. This means we should check dma_dev against NULL in > > virtio_has_dma_quirk(). > > No nee to add a check to virtio_has_dma_quirk. Ok, just to clarify, I meant there could be a case where the virtqueue is emulated by software, in this case we need check whether the virtqueue has a dma device or not in vring_use_dma_api(). If not we need return false. > > But looking at virtio_has_dma_quirk shows that virtio-gpu is > pretty messed up already as well. > > It can't really poke into the DMA details. We'll need core virtio > helpers for allocating and syncing a sg_table instead and make > virtio_has_dma_quirk private to the core. Adding Gerd. Thanks > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 11:56:47AM +0800, Jason Wang wrote: > > The DMA device for virtio_pci is the underlying PCI device, always. > > !VIRTIO_F_ACCESS_PLATFORM means there is no dma device at all. Because > > of all these things you can't just expose a pointer to the dma_device > > as that is just a completely wrong way of thinking about the problem. > > Ok, so if there's no DMA at all we should avoid using the DMA API > completely. This means we should check dma_dev against NULL in > virtio_has_dma_quirk(). No nee to add a check to virtio_has_dma_quirk. But looking at virtio_has_dma_quirk shows that virtio-gpu is pretty messed up already as well. It can't really poke into the DMA details. We'll need core virtio helpers for allocating and syncing a sg_table instead and make virtio_has_dma_quirk private to the core. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 11:31 AM Christoph Hellwig wrote: > > On Tue, Apr 11, 2023 at 10:09:40AM +0800, Jason Wang wrote: > > We want to support AF_XDP for virtio-net. It means AF_XDP needs to > > know the dma device to perform DMA mapping. So we introduce a helper > > to expose the dma dev of the virtio device. > > The whole virtio architecture is based around the core code doing > the DMA mapping. I can't see how you can just export a helper to > expose the dma device. You'd have to complete rework the layering > of the virtio code if you want to do it in the upper level drivers. > And why would you want to do this? The low-level code is the only > piece that can actually know if you need to do a dma mapping. All > the kernel subsystems that don't do it inside the low-level drivers > or helpers closely associtated are a giant and hard to fix map > (see usb for the prime exhibit). > > So the first question is: why do you want this for XF_ADP, Xuan, is it possible to set up the DMA mapping inside the virtio driver itself? I vaguely remember at least the RX buffer mapping is done by the driver. If this is true, we can avoid exposing DMA details to the upper layer. > and > the next question will be how to do that without making a complete > mess. > > > This works fine as long as VIRTIO_F_ACCESS_PLATFORM is negotiated. But > > when it is not, the virtio driver needs to use a physical address so > > we want to expose the virtio device without dma_ops in the hope that > > it will go for direct mapping where the physical address is used. But > > it may not work on some specific setups (arches that assume an IOMMU > > or have arch dma ops). > > The DMA device for virtio_pci is the underlying PCI device, always. > !VIRTIO_F_ACCESS_PLATFORM means there is no dma device at all. Because > of all these things you can't just expose a pointer to the dma_device > as that is just a completely wrong way of thinking about the problem. Ok, so if there's no DMA at all we should avoid using the DMA API completely. This means we should check dma_dev against NULL in virtio_has_dma_quirk(). Thanks > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 10:09:40AM +0800, Jason Wang wrote: > We want to support AF_XDP for virtio-net. It means AF_XDP needs to > know the dma device to perform DMA mapping. So we introduce a helper > to expose the dma dev of the virtio device. The whole virtio architecture is based around the core code doing the DMA mapping. I can't see how you can just export a helper to expose the dma device. You'd have to complete rework the layering of the virtio code if you want to do it in the upper level drivers. And why would you want to do this? The low-level code is the only piece that can actually know if you need to do a dma mapping. All the kernel subsystems that don't do it inside the low-level drivers or helpers closely associtated are a giant and hard to fix map (see usb for the prime exhibit). So the first question is: why do you want this for XF_ADP, and the next question will be how to do that without making a complete mess. > This works fine as long as VIRTIO_F_ACCESS_PLATFORM is negotiated. But > when it is not, the virtio driver needs to use a physical address so > we want to expose the virtio device without dma_ops in the hope that > it will go for direct mapping where the physical address is used. But > it may not work on some specific setups (arches that assume an IOMMU > or have arch dma ops). The DMA device for virtio_pci is the underlying PCI device, always. !VIRTIO_F_ACCESS_PLATFORM means there is no dma device at all. Because of all these things you can't just expose a pointer to the dma_device as that is just a completely wrong way of thinking about the problem. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 09:56:19AM +0800, Xuan Zhuo wrote: > Do you mean we should not change the dma_ops from the outside of the core > kernel/dma/? Basically, yes. That plus probing for the IOMMU drivers. > How about adding one new "dma_direct" to struct devive? Why would we? Can you please explain what you're even trying to do? But checking a dma_direct flag for sure isn't the solution. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Tue, Apr 11, 2023 at 10:01 AM Xuan Zhuo wrote:
>
> On Mon, 10 Apr 2023 08:27:14 -0700, Christoph Hellwig
> wrote:
> > On Mon, Apr 10, 2023 at 01:14:13PM +0800, Jason Wang wrote:
> > > > But rethink, this is unreliable. Some platforms have returned their own
> > > > ops,
> > > > including X86.
> > > >
> > > > Because the priority of dev->dma_ops is higher than get_arch_dma_ops(),
> > > > we
> > > > should not let dev->dma_ops be null. We should define a set of dma_ops
> > > > to
> > > > actively implement direct dMA.
> > >
> > > Then this will duplicate with existing DMA API helpers. Could we tweak
> > > the sparc or introduce a new flag for the device structure then the
> > > DMA API knows it needs to use direct mapping?
> > >
> > > Adding Christoph for more comments.
> >
> > Code outside of the core kernel/dma/ code has no business doing
> > anything with the dma_ops.
>
> Do you mean we should not change the dma_ops from the outside of the core
> kernel/dma/?
>
> How about adding one new "dma_direct" to struct devive?
>
> --- a/include/linux/dma-map-ops.h
> +++ b/include/linux/dma-map-ops.h
> @@ -88,6 +88,9 @@ struct dma_map_ops {
>
> static inline const struct dma_map_ops *get_dma_ops(struct device *dev)
> {
> + if (dev->dma_direct)
> + return NULL;
> +
> if (dev->dma_ops)
> return dev->dma_ops;
> return get_arch_dma_ops();
>
>
> Thanks.
>
>
>
> > WTF is going on?
We want to support AF_XDP for virtio-net. It means AF_XDP needs to
know the dma device to perform DMA mapping. So we introduce a helper
to expose the dma dev of the virtio device.
This works fine as long as VIRTIO_F_ACCESS_PLATFORM is negotiated. But
when it is not, the virtio driver needs to use a physical address so
we want to expose the virtio device without dma_ops in the hope that
it will go for direct mapping where the physical address is used. But
it may not work on some specific setups (arches that assume an IOMMU
or have arch dma ops).
Thanks
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 08:27:14 -0700, Christoph Hellwig
wrote:
> On Mon, Apr 10, 2023 at 01:14:13PM +0800, Jason Wang wrote:
> > > But rethink, this is unreliable. Some platforms have returned their own
> > > ops,
> > > including X86.
> > >
> > > Because the priority of dev->dma_ops is higher than get_arch_dma_ops(), we
> > > should not let dev->dma_ops be null. We should define a set of dma_ops to
> > > actively implement direct dMA.
> >
> > Then this will duplicate with existing DMA API helpers. Could we tweak
> > the sparc or introduce a new flag for the device structure then the
> > DMA API knows it needs to use direct mapping?
> >
> > Adding Christoph for more comments.
>
> Code outside of the core kernel/dma/ code has no business doing
> anything with the dma_ops.
Do you mean we should not change the dma_ops from the outside of the core
kernel/dma/?
How about adding one new "dma_direct" to struct devive?
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -88,6 +88,9 @@ struct dma_map_ops {
static inline const struct dma_map_ops *get_dma_ops(struct device *dev)
{
+ if (dev->dma_direct)
+ return NULL;
+
if (dev->dma_ops)
return dev->dma_ops;
return get_arch_dma_ops();
Thanks.
> WTF is going on?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, Apr 10, 2023 at 01:14:13PM +0800, Jason Wang wrote: > > But rethink, this is unreliable. Some platforms have returned their own ops, > > including X86. > > > > Because the priority of dev->dma_ops is higher than get_arch_dma_ops(), we > > should not let dev->dma_ops be null. We should define a set of dma_ops to > > actively implement direct dMA. > > Then this will duplicate with existing DMA API helpers. Could we tweak > the sparc or introduce a new flag for the device structure then the > DMA API knows it needs to use direct mapping? > > Adding Christoph for more comments. Code outside of the core kernel/dma/ code has no business doing anything with the dma_ops. WTF is going on? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 02:40:58 -0400, "Michael S. Tsirkin"
wrote:
> On Mon, Apr 10, 2023 at 02:03:37PM +0800, Xuan Zhuo wrote:
> > On Mon, 10 Apr 2023 13:14:13 +0800, Jason Wang wrote:
> > > On Mon, Apr 10, 2023 at 11:48 AM Xuan Zhuo
> > > wrote:
> > > >
> > > > On Fri, 7 Apr 2023 07:02:34 -0400, "Michael S. Tsirkin"
> > > > wrote:
> > > > > On Thu, Apr 06, 2023 at 10:20:09AM -0700, Guenter Roeck wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > > > > > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > > > > > caller can do dma operation in advance. The purpose is to keep
> > > > > > > memory
> > > > > > > mapped across multiple add/get buf operations.
> > > > > > >
> > > > > > > Signed-off-by: Xuan Zhuo
> > > > > > > Acked-by: Jason Wang
> > > > > >
> > > > > > On sparc64, this patch results in
> > > > > >
> > > > > > [4.364643] Unable to handle kernel NULL pointer dereference
> > > > > > [4.365157] tsk->{mm,active_mm}->context =
> > > > > > [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> > > > > > [4.365611] \|/ \|/
> > > > > > [4.365611] "@'/ .. \`@"
> > > > > > [4.365611] /_| \__/ |_\
> > > > > > [4.365611] \__U_/
> > > > > > [4.366165] swapper/0(1): Oops [#1]
> > > > > > [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G
> > > > > > N 6.3.0-rc5-next-20230405 #1
> > > > > > [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> > > > > > 004375c4 Y: 0002Tainted: G N
> > > > > > [4.367548] TPC:
> > > > > > [4.368111] g0: g1: 01a93a50 g2:
> > > > > > g3: 01a96170
> > > > > > [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6:
> > > > > > f8000417 g7: 0005
> > > > > > [4.368769] o0: o1: o2:
> > > > > > 0001 o3: f800040b78d8
> > > > > > [4.369095] o4: o5: sp:
> > > > > > f80004172d51 ret_pc: 004375ac
> > > > > > [4.369934] RPC:
> > > > > > [4.370160] l0: 0002 l1: 0002 l2:
> > > > > > l3:
> > > > > > [4.370493] l4: 0001 l5: 0119d2b0 l6:
> > > > > > f8000416e9a0 l7: 018df000
> > > > > > [4.370820] i0: 0001 i1: i2:
> > > > > > f80004657758 i3: 0125ac00
> > > > > > [4.371145] i4: 02362400 i5: i6:
> > > > > > f80004172e01 i7: 0050e288
> > > > > > [4.371473] I7:
> > > > > > [4.371683] Call Trace:
> > > > > > [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> > > > > > [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> > > > > > [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> > > > > > [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> > > > > > [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> > > > > > [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> > > > > > [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> > > > > > [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> > > > > > [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> > > > > > [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> > > > > > [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> > > > > > [4.374330] [<00f37264>] kernel_init+0x18/0x134
> > > > > > [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> > > > > > [4.374738] [<>] 0x0
> > > > > > [4.374981] Disabling lock debugging due to kernel taint
> > > > > > [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> > > > > > [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> > > > > > [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> > > > > > [4.375916] Caller[00ac7d64]:
> > > > > > driver_probe_device+0x24/0x120
> > > > > > [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> > > > > > [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> > > > > > [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> > > > > > [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> > > > > > [4.377025] Caller[01b2a690]:
> > > > > > virtio_net_driver_init+0x74/0xa8
> > > > > > [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> > > > > > [4.377480] Caller[01b02f50]:
> > > > > > kernel_init_freeable+0x1bc/0x228
> > > > > > [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> > > > > > [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> > >
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, Apr 10, 2023 at 02:03:37PM +0800, Xuan Zhuo wrote:
> On Mon, 10 Apr 2023 13:14:13 +0800, Jason Wang wrote:
> > On Mon, Apr 10, 2023 at 11:48 AM Xuan Zhuo
> > wrote:
> > >
> > > On Fri, 7 Apr 2023 07:02:34 -0400, "Michael S. Tsirkin"
> > > wrote:
> > > > On Thu, Apr 06, 2023 at 10:20:09AM -0700, Guenter Roeck wrote:
> > > > > Hi,
> > > > >
> > > > > On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > > > > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > > > > caller can do dma operation in advance. The purpose is to keep
> > > > > > memory
> > > > > > mapped across multiple add/get buf operations.
> > > > > >
> > > > > > Signed-off-by: Xuan Zhuo
> > > > > > Acked-by: Jason Wang
> > > > >
> > > > > On sparc64, this patch results in
> > > > >
> > > > > [4.364643] Unable to handle kernel NULL pointer dereference
> > > > > [4.365157] tsk->{mm,active_mm}->context =
> > > > > [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> > > > > [4.365611] \|/ \|/
> > > > > [4.365611] "@'/ .. \`@"
> > > > > [4.365611] /_| \__/ |_\
> > > > > [4.365611] \__U_/
> > > > > [4.366165] swapper/0(1): Oops [#1]
> > > > > [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G
> > > > > N 6.3.0-rc5-next-20230405 #1
> > > > > [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> > > > > 004375c4 Y: 0002Tainted: G N
> > > > > [4.367548] TPC:
> > > > > [4.368111] g0: g1: 01a93a50 g2:
> > > > > g3: 01a96170
> > > > > [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6:
> > > > > f8000417 g7: 0005
> > > > > [4.368769] o0: o1: o2:
> > > > > 0001 o3: f800040b78d8
> > > > > [4.369095] o4: o5: sp:
> > > > > f80004172d51 ret_pc: 004375ac
> > > > > [4.369934] RPC:
> > > > > [4.370160] l0: 0002 l1: 0002 l2:
> > > > > l3:
> > > > > [4.370493] l4: 0001 l5: 0119d2b0 l6:
> > > > > f8000416e9a0 l7: 018df000
> > > > > [4.370820] i0: 0001 i1: i2:
> > > > > f80004657758 i3: 0125ac00
> > > > > [4.371145] i4: 02362400 i5: i6:
> > > > > f80004172e01 i7: 0050e288
> > > > > [4.371473] I7:
> > > > > [4.371683] Call Trace:
> > > > > [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> > > > > [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> > > > > [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> > > > > [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> > > > > [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> > > > > [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> > > > > [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> > > > > [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> > > > > [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> > > > > [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> > > > > [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> > > > > [4.374330] [<00f37264>] kernel_init+0x18/0x134
> > > > > [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> > > > > [4.374738] [<>] 0x0
> > > > > [4.374981] Disabling lock debugging due to kernel taint
> > > > > [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> > > > > [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> > > > > [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> > > > > [4.375916] Caller[00ac7d64]:
> > > > > driver_probe_device+0x24/0x120
> > > > > [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> > > > > [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> > > > > [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> > > > > [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> > > > > [4.377025] Caller[01b2a690]:
> > > > > virtio_net_driver_init+0x74/0xa8
> > > > > [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> > > > > [4.377480] Caller[01b02f50]:
> > > > > kernel_init_freeable+0x1bc/0x228
> > > > > [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> > > > > [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> > > > > [4.378140] Caller[]: 0x0
> > > > > [4.378309] Instruction DUMP:
> > > > > [4.378339] 80a22000
> > > > > [4.378556] 1246
> > > > > [4.378654] b0102001
> > > > > [4.378746]
> > > > >
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, 10 Apr 2023 13:14:13 +0800, Jason Wang wrote:
> On Mon, Apr 10, 2023 at 11:48 AM Xuan Zhuo wrote:
> >
> > On Fri, 7 Apr 2023 07:02:34 -0400, "Michael S. Tsirkin"
> > wrote:
> > > On Thu, Apr 06, 2023 at 10:20:09AM -0700, Guenter Roeck wrote:
> > > > Hi,
> > > >
> > > > On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > > > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > > > caller can do dma operation in advance. The purpose is to keep memory
> > > > > mapped across multiple add/get buf operations.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo
> > > > > Acked-by: Jason Wang
> > > >
> > > > On sparc64, this patch results in
> > > >
> > > > [4.364643] Unable to handle kernel NULL pointer dereference
> > > > [4.365157] tsk->{mm,active_mm}->context =
> > > > [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> > > > [4.365611] \|/ \|/
> > > > [4.365611] "@'/ .. \`@"
> > > > [4.365611] /_| \__/ |_\
> > > > [4.365611] \__U_/
> > > > [4.366165] swapper/0(1): Oops [#1]
> > > > [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G
> > > > N 6.3.0-rc5-next-20230405 #1
> > > > [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> > > > 004375c4 Y: 0002Tainted: G N
> > > > [4.367548] TPC:
> > > > [4.368111] g0: g1: 01a93a50 g2:
> > > > g3: 01a96170
> > > > [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6:
> > > > f8000417 g7: 0005
> > > > [4.368769] o0: o1: o2:
> > > > 0001 o3: f800040b78d8
> > > > [4.369095] o4: o5: sp:
> > > > f80004172d51 ret_pc: 004375ac
> > > > [4.369934] RPC:
> > > > [4.370160] l0: 0002 l1: 0002 l2:
> > > > l3:
> > > > [4.370493] l4: 0001 l5: 0119d2b0 l6:
> > > > f8000416e9a0 l7: 018df000
> > > > [4.370820] i0: 0001 i1: i2:
> > > > f80004657758 i3: 0125ac00
> > > > [4.371145] i4: 02362400 i5: i6:
> > > > f80004172e01 i7: 0050e288
> > > > [4.371473] I7:
> > > > [4.371683] Call Trace:
> > > > [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> > > > [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> > > > [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> > > > [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> > > > [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> > > > [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> > > > [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> > > > [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> > > > [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> > > > [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> > > > [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> > > > [4.374330] [<00f37264>] kernel_init+0x18/0x134
> > > > [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> > > > [4.374738] [<>] 0x0
> > > > [4.374981] Disabling lock debugging due to kernel taint
> > > > [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> > > > [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> > > > [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> > > > [4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
> > > > [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> > > > [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> > > > [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> > > > [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> > > > [4.377025] Caller[01b2a690]:
> > > > virtio_net_driver_init+0x74/0xa8
> > > > [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> > > > [4.377480] Caller[01b02f50]:
> > > > kernel_init_freeable+0x1bc/0x228
> > > > [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> > > > [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> > > > [4.378140] Caller[]: 0x0
> > > > [4.378309] Instruction DUMP:
> > > > [4.378339] 80a22000
> > > > [4.378556] 1246
> > > > [4.378654] b0102001
> > > > [4.378746]
> > > > [4.378838] b0102000
> > > > [4.378930] 80a04019
> > > > [4.379022] b1653001
> > > > [4.379115] 81cfe008
> > > > [4.379507] 913a2000
> > > > [4.379617]
> > > > [4.380504] Kernel panic - not syncing: Attempted to kill init!
> > >
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Mon, Apr 10, 2023 at 11:48 AM Xuan Zhuo wrote:
>
> On Fri, 7 Apr 2023 07:02:34 -0400, "Michael S. Tsirkin"
> wrote:
> > On Thu, Apr 06, 2023 at 10:20:09AM -0700, Guenter Roeck wrote:
> > > Hi,
> > >
> > > On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > > caller can do dma operation in advance. The purpose is to keep memory
> > > > mapped across multiple add/get buf operations.
> > > >
> > > > Signed-off-by: Xuan Zhuo
> > > > Acked-by: Jason Wang
> > >
> > > On sparc64, this patch results in
> > >
> > > [4.364643] Unable to handle kernel NULL pointer dereference
> > > [4.365157] tsk->{mm,active_mm}->context =
> > > [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> > > [4.365611] \|/ \|/
> > > [4.365611] "@'/ .. \`@"
> > > [4.365611] /_| \__/ |_\
> > > [4.365611] \__U_/
> > > [4.366165] swapper/0(1): Oops [#1]
> > > [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G N
> > > 6.3.0-rc5-next-20230405 #1
> > > [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> > > 004375c4 Y: 0002Tainted: G N
> > > [4.367548] TPC:
> > > [4.368111] g0: g1: 01a93a50 g2:
> > > g3: 01a96170
> > > [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6:
> > > f8000417 g7: 0005
> > > [4.368769] o0: o1: o2:
> > > 0001 o3: f800040b78d8
> > > [4.369095] o4: o5: sp:
> > > f80004172d51 ret_pc: 004375ac
> > > [4.369934] RPC:
> > > [4.370160] l0: 0002 l1: 0002 l2:
> > > l3:
> > > [4.370493] l4: 0001 l5: 0119d2b0 l6:
> > > f8000416e9a0 l7: 018df000
> > > [4.370820] i0: 0001 i1: i2:
> > > f80004657758 i3: 0125ac00
> > > [4.371145] i4: 02362400 i5: i6:
> > > f80004172e01 i7: 0050e288
> > > [4.371473] I7:
> > > [4.371683] Call Trace:
> > > [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> > > [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> > > [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> > > [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> > > [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> > > [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> > > [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> > > [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> > > [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> > > [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> > > [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> > > [4.374330] [<00f37264>] kernel_init+0x18/0x134
> > > [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> > > [4.374738] [<>] 0x0
> > > [4.374981] Disabling lock debugging due to kernel taint
> > > [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> > > [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> > > [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> > > [4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
> > > [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> > > [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> > > [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> > > [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> > > [4.377025] Caller[01b2a690]: virtio_net_driver_init+0x74/0xa8
> > > [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> > > [4.377480] Caller[01b02f50]: kernel_init_freeable+0x1bc/0x228
> > > [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> > > [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> > > [4.378140] Caller[]: 0x0
> > > [4.378309] Instruction DUMP:
> > > [4.378339] 80a22000
> > > [4.378556] 1246
> > > [4.378654] b0102001
> > > [4.378746]
> > > [4.378838] b0102000
> > > [4.378930] 80a04019
> > > [4.379022] b1653001
> > > [4.379115] 81cfe008
> > > [4.379507] 913a2000
> > > [4.379617]
> > > [4.380504] Kernel panic - not syncing: Attempted to kill init!
> > > exitcode=0x0009
> > >
> > > Reverting it fixes the problem. Bisect log attached.
> > >
> > > The reason is that dma_supported() calls dma_4u_supported(), which
> > > expects that dev->archdata.iommu has been initialized. However,
> > > this is not
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Fri, 7 Apr 2023 07:02:34 -0400, "Michael S. Tsirkin" wrote:
> On Thu, Apr 06, 2023 at 10:20:09AM -0700, Guenter Roeck wrote:
> > Hi,
> >
> > On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > caller can do dma operation in advance. The purpose is to keep memory
> > > mapped across multiple add/get buf operations.
> > >
> > > Signed-off-by: Xuan Zhuo
> > > Acked-by: Jason Wang
> >
> > On sparc64, this patch results in
> >
> > [4.364643] Unable to handle kernel NULL pointer dereference
> > [4.365157] tsk->{mm,active_mm}->context =
> > [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> > [4.365611] \|/ \|/
> > [4.365611] "@'/ .. \`@"
> > [4.365611] /_| \__/ |_\
> > [4.365611] \__U_/
> > [4.366165] swapper/0(1): Oops [#1]
> > [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G N
> > 6.3.0-rc5-next-20230405 #1
> > [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> > 004375c4 Y: 0002Tainted: G N
> > [4.367548] TPC:
> > [4.368111] g0: g1: 01a93a50 g2:
> > g3: 01a96170
> > [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6:
> > f8000417 g7: 0005
> > [4.368769] o0: o1: o2:
> > 0001 o3: f800040b78d8
> > [4.369095] o4: o5: sp:
> > f80004172d51 ret_pc: 004375ac
> > [4.369934] RPC:
> > [4.370160] l0: 0002 l1: 0002 l2:
> > l3:
> > [4.370493] l4: 0001 l5: 0119d2b0 l6:
> > f8000416e9a0 l7: 018df000
> > [4.370820] i0: 0001 i1: i2:
> > f80004657758 i3: 0125ac00
> > [4.371145] i4: 02362400 i5: i6:
> > f80004172e01 i7: 0050e288
> > [4.371473] I7:
> > [4.371683] Call Trace:
> > [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> > [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> > [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> > [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> > [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> > [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> > [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> > [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> > [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> > [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> > [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> > [4.374330] [<00f37264>] kernel_init+0x18/0x134
> > [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> > [4.374738] [<>] 0x0
> > [4.374981] Disabling lock debugging due to kernel taint
> > [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> > [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> > [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> > [4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
> > [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> > [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> > [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> > [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> > [4.377025] Caller[01b2a690]: virtio_net_driver_init+0x74/0xa8
> > [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> > [4.377480] Caller[01b02f50]: kernel_init_freeable+0x1bc/0x228
> > [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> > [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> > [4.378140] Caller[]: 0x0
> > [4.378309] Instruction DUMP:
> > [4.378339] 80a22000
> > [4.378556] 1246
> > [4.378654] b0102001
> > [4.378746]
> > [4.378838] b0102000
> > [4.378930] 80a04019
> > [4.379022] b1653001
> > [4.379115] 81cfe008
> > [4.379507] 913a2000
> > [4.379617]
> > [4.380504] Kernel panic - not syncing: Attempted to kill init!
> > exitcode=0x0009
> >
> > Reverting it fixes the problem. Bisect log attached.
> >
> > The reason is that dma_supported() calls dma_4u_supported(), which
> > expects that dev->archdata.iommu has been initialized. However,
> > this is not the case for the virtio device. Result is a null pointer
> > dereference in dma_4u_supported().
> >
> > static int dma_4u_supported(struct device *dev, u64 device_mask)
> > {
> > struct iommu *iommu = dev->archdata.iommu;
> >
> >
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On 4/6/23 20:17, Xuan Zhuo wrote:
On Thu, 6 Apr 2023 10:20:09 -0700, Guenter Roeck wrote:
Hi,
On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
Added virtqueue_dma_dev() to get DMA device for virtio. Then the
caller can do dma operation in advance. The purpose is to keep memory
mapped across multiple add/get buf operations.
Signed-off-by: Xuan Zhuo
Acked-by: Jason Wang
On sparc64, this patch results in
[4.364643] Unable to handle kernel NULL pointer dereference
[4.365157] tsk->{mm,active_mm}->context =
[4.365394] tsk->{mm,active_mm}->pgd = f8402000
[4.365611] \|/ \|/
[4.365611] "@'/ .. \`@"
[4.365611] /_| \__/ |_\
[4.365611] \__U_/
[4.366165] swapper/0(1): Oops [#1]
[4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G N
6.3.0-rc5-next-20230405 #1
[4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
004375c4 Y: 0002Tainted: G N
[4.367548] TPC:
[4.368111] g0: g1: 01a93a50 g2:
g3: 01a96170
[4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6: f8000417
g7: 0005
[4.368769] o0: o1: o2: 0001
o3: f800040b78d8
[4.369095] o4: o5: sp: f80004172d51
ret_pc: 004375ac
[4.369934] RPC:
[4.370160] l0: 0002 l1: 0002 l2:
l3:
[4.370493] l4: 0001 l5: 0119d2b0 l6: f8000416e9a0
l7: 018df000
[4.370820] i0: 0001 i1: i2: f80004657758
i3: 0125ac00
[4.371145] i4: 02362400 i5: i6: f80004172e01
i7: 0050e288
[4.371473] I7:
[4.371683] Call Trace:
[4.371864] [<0050e288>] dma_set_mask+0x28/0x80
[4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
[4.372348] [<00ac7a18>] really_probe+0xb8/0x340
[4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
[4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
[4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
[4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
[4.373440] [<00ac8d94>] driver_register+0x74/0x120
[4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
[4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
[4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
[4.374330] [<00f37264>] kernel_init+0x18/0x134
[4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
[4.374738] [<>] 0x0
[4.374981] Disabling lock debugging due to kernel taint
[4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
[4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
[4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
[4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
[4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
[4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
[4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
[4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
[4.377025] Caller[01b2a690]: virtio_net_driver_init+0x74/0xa8
[4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
[4.377480] Caller[01b02f50]: kernel_init_freeable+0x1bc/0x228
[4.377721] Caller[00f37264]: kernel_init+0x18/0x134
[4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
[4.378140] Caller[]: 0x0
[4.378309] Instruction DUMP:
[4.378339] 80a22000
[4.378556] 1246
[4.378654] b0102001
[4.378746]
[4.378838] b0102000
[4.378930] 80a04019
[4.379022] b1653001
[4.379115] 81cfe008
[4.379507] 913a2000
[4.379617]
[4.380504] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0009
Reverting it fixes the problem. Bisect log attached.
The reason is that dma_supported() calls dma_4u_supported(), which
expects that dev->archdata.iommu has been initialized. However,
this is not the case for the virtio device. Result is a null pointer
dereference in dma_4u_supported().
static int dma_4u_supported(struct device *dev, u64 device_mask)
{
struct iommu *iommu = dev->archdata.iommu;
if (ali_sound_dma_hack(dev, device_mask))
return 1;
if (device_mask < iommu->dma_addr_mask)
Crash location
return 0;
return 1;
}
sparc64 does not support direct dma?
The virtio is a virtul device based on pci.
Do you know how we should adapt to sparc64?
It isn't exactly common or typical to set dev->dma_mask in driver
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Thu, Apr 06, 2023 at 10:20:09AM -0700, Guenter Roeck wrote:
> Hi,
>
> On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > caller can do dma operation in advance. The purpose is to keep memory
> > mapped across multiple add/get buf operations.
> >
> > Signed-off-by: Xuan Zhuo
> > Acked-by: Jason Wang
>
> On sparc64, this patch results in
>
> [4.364643] Unable to handle kernel NULL pointer dereference
> [4.365157] tsk->{mm,active_mm}->context =
> [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> [4.365611] \|/ \|/
> [4.365611] "@'/ .. \`@"
> [4.365611] /_| \__/ |_\
> [4.365611] \__U_/
> [4.366165] swapper/0(1): Oops [#1]
> [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G N
> 6.3.0-rc5-next-20230405 #1
> [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> 004375c4 Y: 0002Tainted: G N
> [4.367548] TPC:
> [4.368111] g0: g1: 01a93a50 g2:
> g3: 01a96170
> [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6: f8000417
> g7: 0005
> [4.368769] o0: o1: o2: 0001
> o3: f800040b78d8
> [4.369095] o4: o5: sp: f80004172d51
> ret_pc: 004375ac
> [4.369934] RPC:
> [4.370160] l0: 0002 l1: 0002 l2:
> l3:
> [4.370493] l4: 0001 l5: 0119d2b0 l6: f8000416e9a0
> l7: 018df000
> [4.370820] i0: 0001 i1: i2: f80004657758
> i3: 0125ac00
> [4.371145] i4: 02362400 i5: i6: f80004172e01
> i7: 0050e288
> [4.371473] I7:
> [4.371683] Call Trace:
> [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> [4.374330] [<00f37264>] kernel_init+0x18/0x134
> [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> [4.374738] [<>] 0x0
> [4.374981] Disabling lock debugging due to kernel taint
> [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> [4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
> [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> [4.377025] Caller[01b2a690]: virtio_net_driver_init+0x74/0xa8
> [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> [4.377480] Caller[01b02f50]: kernel_init_freeable+0x1bc/0x228
> [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> [4.378140] Caller[]: 0x0
> [4.378309] Instruction DUMP:
> [4.378339] 80a22000
> [4.378556] 1246
> [4.378654] b0102001
> [4.378746]
> [4.378838] b0102000
> [4.378930] 80a04019
> [4.379022] b1653001
> [4.379115] 81cfe008
> [4.379507] 913a2000
> [4.379617]
> [4.380504] Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x0009
>
> Reverting it fixes the problem. Bisect log attached.
>
> The reason is that dma_supported() calls dma_4u_supported(), which
> expects that dev->archdata.iommu has been initialized. However,
> this is not the case for the virtio device. Result is a null pointer
> dereference in dma_4u_supported().
>
> static int dma_4u_supported(struct device *dev, u64 device_mask)
> {
> struct iommu *iommu = dev->archdata.iommu;
>
> if (ali_sound_dma_hack(dev, device_mask))
> return 1;
>
> if (device_mask < iommu->dma_addr_mask)
> Crash location
> return 0;
> return 1;
> }
>
> Guenter
This
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
On Thu, 6 Apr 2023 10:20:09 -0700, Guenter Roeck wrote:
> Hi,
>
> On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > caller can do dma operation in advance. The purpose is to keep memory
> > mapped across multiple add/get buf operations.
> >
> > Signed-off-by: Xuan Zhuo
> > Acked-by: Jason Wang
>
> On sparc64, this patch results in
>
> [4.364643] Unable to handle kernel NULL pointer dereference
> [4.365157] tsk->{mm,active_mm}->context =
> [4.365394] tsk->{mm,active_mm}->pgd = f8402000
> [4.365611] \|/ \|/
> [4.365611] "@'/ .. \`@"
> [4.365611] /_| \__/ |_\
> [4.365611] \__U_/
> [4.366165] swapper/0(1): Oops [#1]
> [4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G N
> 6.3.0-rc5-next-20230405 #1
> [4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
> 004375c4 Y: 0002Tainted: G N
> [4.367548] TPC:
> [4.368111] g0: g1: 01a93a50 g2:
> g3: 01a96170
> [4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6: f8000417
> g7: 0005
> [4.368769] o0: o1: o2: 0001
> o3: f800040b78d8
> [4.369095] o4: o5: sp: f80004172d51
> ret_pc: 004375ac
> [4.369934] RPC:
> [4.370160] l0: 0002 l1: 0002 l2:
> l3:
> [4.370493] l4: 0001 l5: 0119d2b0 l6: f8000416e9a0
> l7: 018df000
> [4.370820] i0: 0001 i1: i2: f80004657758
> i3: 0125ac00
> [4.371145] i4: 02362400 i5: i6: f80004172e01
> i7: 0050e288
> [4.371473] I7:
> [4.371683] Call Trace:
> [4.371864] [<0050e288>] dma_set_mask+0x28/0x80
> [4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
> [4.372348] [<00ac7a18>] really_probe+0xb8/0x340
> [4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
> [4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
> [4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
> [4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
> [4.373440] [<00ac8d94>] driver_register+0x74/0x120
> [4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
> [4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
> [4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
> [4.374330] [<00f37264>] kernel_init+0x18/0x134
> [4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
> [4.374738] [<>] 0x0
> [4.374981] Disabling lock debugging due to kernel taint
> [4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
> [4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
> [4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
> [4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
> [4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
> [4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
> [4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
> [4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
> [4.377025] Caller[01b2a690]: virtio_net_driver_init+0x74/0xa8
> [4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
> [4.377480] Caller[01b02f50]: kernel_init_freeable+0x1bc/0x228
> [4.377721] Caller[00f37264]: kernel_init+0x18/0x134
> [4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
> [4.378140] Caller[]: 0x0
> [4.378309] Instruction DUMP:
> [4.378339] 80a22000
> [4.378556] 1246
> [4.378654] b0102001
> [4.378746]
> [4.378838] b0102000
> [4.378930] 80a04019
> [4.379022] b1653001
> [4.379115] 81cfe008
> [4.379507] 913a2000
> [4.379617]
> [4.380504] Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x0009
>
> Reverting it fixes the problem. Bisect log attached.
>
> The reason is that dma_supported() calls dma_4u_supported(), which
> expects that dev->archdata.iommu has been initialized. However,
> this is not the case for the virtio device. Result is a null pointer
> dereference in dma_4u_supported().
>
> static int dma_4u_supported(struct device *dev, u64 device_mask)
> {
> struct iommu *iommu = dev->archdata.iommu;
>
> if (ali_sound_dma_hack(dev, device_mask))
> return 1;
>
> if (device_mask < iommu->dma_addr_mask)
> Crash location
> return 0;
> return 1;
> }
sparc64 does not support direct
Re: [PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
Hi,
On Mon, Mar 27, 2023 at 12:05:33PM +0800, Xuan Zhuo wrote:
> Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> caller can do dma operation in advance. The purpose is to keep memory
> mapped across multiple add/get buf operations.
>
> Signed-off-by: Xuan Zhuo
> Acked-by: Jason Wang
On sparc64, this patch results in
[4.364643] Unable to handle kernel NULL pointer dereference
[4.365157] tsk->{mm,active_mm}->context =
[4.365394] tsk->{mm,active_mm}->pgd = f8402000
[4.365611] \|/ \|/
[4.365611] "@'/ .. \`@"
[4.365611] /_| \__/ |_\
[4.365611] \__U_/
[4.366165] swapper/0(1): Oops [#1]
[4.366630] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G N
6.3.0-rc5-next-20230405 #1
[4.367121] TSTATE: 004411001606 TPC: 004375c0 TNPC:
004375c4 Y: 0002Tainted: G N
[4.367548] TPC:
[4.368111] g0: g1: 01a93a50 g2:
g3: 01a96170
[4.368439] g4: f8000416e9a0 g5: f8001dc98000 g6: f8000417
g7: 0005
[4.368769] o0: o1: o2: 0001
o3: f800040b78d8
[4.369095] o4: o5: sp: f80004172d51
ret_pc: 004375ac
[4.369934] RPC:
[4.370160] l0: 0002 l1: 0002 l2:
l3:
[4.370493] l4: 0001 l5: 0119d2b0 l6: f8000416e9a0
l7: 018df000
[4.370820] i0: 0001 i1: i2: f80004657758
i3: 0125ac00
[4.371145] i4: 02362400 i5: i6: f80004172e01
i7: 0050e288
[4.371473] I7:
[4.371683] Call Trace:
[4.371864] [<0050e288>] dma_set_mask+0x28/0x80
[4.372123] [<00a83234>] virtio_dev_probe+0x14/0x400
[4.372348] [<00ac7a18>] really_probe+0xb8/0x340
[4.372555] [<00ac7d64>] driver_probe_device+0x24/0x120
[4.372794] [<00ac8010>] __driver_attach+0x90/0x1a0
[4.373012] [<00ac5b4c>] bus_for_each_dev+0x4c/0xa0
[4.373226] [<00ac6d80>] bus_add_driver+0x140/0x1e0
[4.373440] [<00ac8d94>] driver_register+0x74/0x120
[4.373653] [<01b2a690>] virtio_net_driver_init+0x74/0xa8
[4.373886] [<00427ee0>] do_one_initcall+0x60/0x340
[4.374099] [<01b02f50>] kernel_init_freeable+0x1bc/0x228
[4.374330] [<00f37264>] kernel_init+0x18/0x134
[4.374534] [<004060e8>] ret_from_fork+0x1c/0x2c
[4.374738] [<>] 0x0
[4.374981] Disabling lock debugging due to kernel taint
[4.375237] Caller[0050e288]: dma_set_mask+0x28/0x80
[4.375477] Caller[00a83234]: virtio_dev_probe+0x14/0x400
[4.375704] Caller[00ac7a18]: really_probe+0xb8/0x340
[4.375916] Caller[00ac7d64]: driver_probe_device+0x24/0x120
[4.376146] Caller[00ac8010]: __driver_attach+0x90/0x1a0
[4.376365] Caller[00ac5b4c]: bus_for_each_dev+0x4c/0xa0
[4.376583] Caller[00ac6d80]: bus_add_driver+0x140/0x1e0
[4.376805] Caller[00ac8d94]: driver_register+0x74/0x120
[4.377025] Caller[01b2a690]: virtio_net_driver_init+0x74/0xa8
[4.377262] Caller[00427ee0]: do_one_initcall+0x60/0x340
[4.377480] Caller[01b02f50]: kernel_init_freeable+0x1bc/0x228
[4.377721] Caller[00f37264]: kernel_init+0x18/0x134
[4.377930] Caller[004060e8]: ret_from_fork+0x1c/0x2c
[4.378140] Caller[]: 0x0
[4.378309] Instruction DUMP:
[4.378339] 80a22000
[4.378556] 1246
[4.378654] b0102001
[4.378746]
[4.378838] b0102000
[4.378930] 80a04019
[4.379022] b1653001
[4.379115] 81cfe008
[4.379507] 913a2000
[4.379617]
[4.380504] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0009
Reverting it fixes the problem. Bisect log attached.
The reason is that dma_supported() calls dma_4u_supported(), which
expects that dev->archdata.iommu has been initialized. However,
this is not the case for the virtio device. Result is a null pointer
dereference in dma_4u_supported().
static int dma_4u_supported(struct device *dev, u64 device_mask)
{
struct iommu *iommu = dev->archdata.iommu;
if (ali_sound_dma_hack(dev, device_mask))
return 1;
if (device_mask < iommu->dma_addr_mask)
Crash location
return 0;
return 1;
}
Guenter
---
# bad: [8417c8f5007bf4567ccffda850a3157c7d905f67] Add linux-next specific files
for 20230405
# good: [7e364e56293bb98cae1b55fd835f5991c4e96e7d] Linux 6.3-rc5
git bisect start 'HEAD' 'v6.3-rc5'
# good: [699c146eb5a03458f29e94cfde4d7dd7a36deeb4] Merge branch 'master' of
[PATCH vhost v6 08/11] virtio_ring: introduce virtqueue_dma_dev()
Added virtqueue_dma_dev() to get DMA device for virtio. Then the
caller can do dma operation in advance. The purpose is to keep memory
mapped across multiple add/get buf operations.
Signed-off-by: Xuan Zhuo
Acked-by: Jason Wang
---
drivers/virtio/virtio.c | 6 ++
drivers/virtio/virtio_ring.c | 17 +
include/linux/virtio.h | 2 ++
3 files changed, 25 insertions(+)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 3893dc29eb26..11c5035369e2 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include
#include
#include
#include
@@ -243,6 +244,11 @@ static int virtio_dev_probe(struct device *_d)
u64 driver_features;
u64 driver_features_legacy;
+ _d->dma_mask = &_d->coherent_dma_mask;
+ err = dma_set_mask_and_coherent(_d, DMA_BIT_MASK(64));
+ if (err)
+ return err;
+
/* We have a driver! */
virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 0b198ec3ff92..7b538c16c9b5 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2309,6 +2309,23 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
}
EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
+/**
+ * virtqueue_dma_dev - get the dma dev
+ * @_vq: the struct virtqueue we're talking about.
+ *
+ * Returns the dma dev. That can been used for dma api.
+ */
+struct device *virtqueue_dma_dev(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ if (vq->use_dma_api)
+ return vring_dma_dev(vq);
+ else
+ return >vq.vdev->dev;
+}
+EXPORT_SYMBOL_GPL(virtqueue_dma_dev);
+
/**
* virtqueue_kick_prepare - first half of split virtqueue_kick call.
* @_vq: the struct virtqueue
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 2b472514c49b..1fa50191cf0a 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -61,6 +61,8 @@ int virtqueue_add_sgs(struct virtqueue *vq,
void *data,
gfp_t gfp);
+struct device *virtqueue_dma_dev(struct virtqueue *vq);
+
bool virtqueue_kick(struct virtqueue *vq);
bool virtqueue_kick_prepare(struct virtqueue *vq);
--
2.32.0.3.g01195cf9f
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
