On Fri, Sep 23, 2016 at 03:25:42PM +0800, Peter Xu wrote:
> > + width = ops->io_readb(&test->width);
> > + if (width != 1 && width != 2 && width != 4)
> > + return false;
>
> IIUC we only have 1?
I guess it boils what *have* does mean here.
pci-testdev protocol allows it to be any, but hw/misc/pci-testdev.c
implements just 1 (yet?).
> > +
> > + sig = ops->io_readl(&test->data);
> > + off = ops->io_readl(&test->offset);
> > +
> > + for (i = 0; i < nr_writes; i++) {
> > + switch (width) {
> > + case 1: ops->io_writeb(sig, (void *)test + off); break;
> > + case 2: ops->io_writew(sig, (void *)test + off); break;
> > + case 4: ops->io_writel(sig, (void *)test + off); break;
>
> Here as well. Could I ask why we are handling 2/4?
Basically, because x86 had it and this implementation mimics it.
> [...]
>
> > +static int pci_testdev_all(struct pci_test_dev_hdr *test,
> > + struct pci_testdev_ops *ops)
> > +{
> > + int i;
> > +
> > + for (i = 0;; i++) {
> > + if (!pci_testdev_one(test, i, ops))
> > + break;
>
> Since we have defined PCI_TESTDEV_NUM_TESTS, shall we use it here to
> stop the loop rather than depending on a failure code from
> pci_testdev_one()?
No, I think we indeed need to inquiry the device this way and
go ahead and test if it reported the size is supported.
> [...]
> x86/vmexit.c is using pci-testdev as well. Maybe we can generalize the
> init part and share it? (Actually there is patch in my local tree for
> this, but haven't posted :)
Yep, I have x86 enabler and it is very simple. But x86 is just too
different to try to generalize and we're not pursuing it right now.
> Thanks!
>
> -- peterx
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
