On Mon, Nov 11, 2019 at 11:33:17AM +0100, Klaus Birkelund wrote: > 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
It's probably best to just include it in the series. When you do feel free to include Reviewed-by: Michael S. Tsirkin <m...@redhat.com> and mention that the return code has no existing users so it's safe to change.