[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/1] pci: pass along the return value of dma_memory_rw
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH 1/1] pci: pass along the return value of dma_memory_rw |
Date: |
Mon, 11 Nov 2019 06:00:42 -0500 |
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 <address@hidden>
> > > > > ---
> > > > > 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é <address@hidden>
> > >
> > > 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 <address@hidden>
and mention that the return code has no existing
users so it's safe to change.