On Mon, Nov 11, 2019 at 05:16:41AM -0500, Michael S. Tsirkin wrote:
> On Mon, Nov 11, 2019 at 10:30:07AM +0100, Klaus Birkelund wrote:
> > On Thu, Oct 24, 2019 at 01:13:36AM +0200, Philippe Mathieu-Daudé wrote:
> > > On 10/11/19 9:01 AM, Klaus Jensen wrote:
> > > > Some might actually care about the return value of dma_memory_rw. So
> > > > let us pass it along instead of ignoring it.
> > > >
> > > > Signed-off-by: Klaus Jensen <k.jen...@samsung.com>
> > > > ---
> > > > include/hw/pci/pci.h | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > > index f3f0ffd5fb78..4e95bb847857 100644
> > > > --- a/include/hw/pci/pci.h
> > > > +++ b/include/hw/pci/pci.h
> > > > @@ -779,8 +779,7 @@ static inline AddressSpace
> > > > *pci_get_address_space(PCIDevice *dev)
> > > > static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
> > > > void *buf, dma_addr_t len, DMADirection
> > > > dir)
> > > > {
> > > > - dma_memory_rw(pci_get_address_space(dev), addr, buf, len, dir);
> > > > - return 0;
> > > > + return dma_memory_rw(pci_get_address_space(dev), addr, buf, len,
> > > > dir);
> > > > }
> > > > static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr,
> > > >
> > >
> > > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> >
> > Gentle ping on this.
> >
> > This fix is required for the nvme device to start passing some of the
> > nasty tests from blktests that flips bus mastering while doing I/O.
> >
> >
> > Cheers,
> > Klaus
>
> So I looked and it does not seem like anyone at all checks the
> return value. While this makes the patch safe, how come it
> helps anyone at all?
I have a series[1] under review for the nvme device (I should have
mentioned that). There is a certain test (block/011) from blktests[2],
that disables the PCI device while doing I/O by flipping the bus master
register. QEMU correctly understands this which causes the dma_memory_rw
calls to fail while the device is not a bus master. For the NVMe device
to pass that test, it needs to know that it could not do the DMA,
otherwise it will just think that a completion queue entry was
successfully posted or data was correctly read.
> Maybe this is just infrastructure to allow checks in the
> future, in this case do we need this for the freeze?
> Or can it wait until after the release?
>
The series I'm mentioning won't be going into the release, so yeah - it
can surely wait. I was just pinging to make sure someone would pick it
up at some point :)
[1]: https://patchwork.kernel.org/cover/11190045/
[2]: https://github.com/osandov/blktests
