Re: [PATCH kvm-unit-tests 13/17] pci: edu: introduce pci-edu helpers

[Peter Xu <peterx@redhat.com>]

On Fri, Nov 04, 2016 at 06:24:48PM +0100, Andrew Jones wrote:
> On Wed, Oct 26, 2016 at 03:47:16PM +0800, Peter Xu wrote:
> > QEMU edu device is a pci device that is originally written for
> > educational purpose, however it also suits for IOMMU unit test. Adding
> > helpers for this specific device to implement the device logic.
> > 
> > The device supports lots of functions, here only DMA operation is
> > supported.
> > 
> > The spec of the device can be found at:
> > 
> >   https://github.com/qemu/qemu/blob/master/docs/specs/edu.txt
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  lib/pci-edu.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/pci-edu.h | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 153 insertions(+)
> >  create mode 100644 lib/pci-edu.c
> >  create mode 100644 lib/pci-edu.h
> > 
> > diff --git a/lib/pci-edu.c b/lib/pci-edu.c
> > new file mode 100644
> > index 0000000..c594e08
> > --- /dev/null
> > +++ b/lib/pci-edu.c
> > @@ -0,0 +1,74 @@
> > +/*
> > + * Edu PCI device.
> > + *
> > + * Copyright (C) 2016 Red Hat, Inc.
> > + *
> > + * Authors:
> > + *   Peter Xu <peterx@redhat.com>,
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2 or
> > + * later.
> > + */
> > +
> > +#include "pci-edu.h"
> > +#include "asm/barrier.h"
> > +
> > +/* Return true if alive */
> > +static inline bool edu_check_alive(struct pci_edu_dev *dev)
> > +{
> > +	static uint32_t live_count = 1;
> > +	uint32_t value;
> > +
> > +	edu_reg_write(dev, EDU_REG_ALIVE, live_count++);
> > +	value = edu_reg_read(dev, EDU_REG_ALIVE);
> > +	return (live_count - 1 == ~value);
> > +}
> > +
> > +int edu_init(struct pci_edu_dev *dev)
> > +{
> > +	int ret;
> > +
> > +	ret = pci_find_dev(&dev->pci_dev, PCI_VENDOR_ID_QEMU,
> > +			   PCI_DEVICE_ID_EDU);
> > +	if (ret)
> > +		return ret;
> 
> Another comment: as I mentioned earlier, I think pci_find_dev should
> return a bool. Here too, i.e. edu_init returns true when init
> succeeds, else false. The only reason to go with a zero success,
> non-zero (negative) failure is if multiple error codes will be
> introduced. I don't think that'll happen for kvm-unit-tests though.

Ok. Will change pci_find_dev's return code into bool. Thanks,

-- peterx