Re: [PATCH 0/4 v12] MSI-X MMIO support for KVM

Michael S. Tsirkin
2011-03-02T19:56:44Z

On Wed, Mar 02, 2011 at 03:51:20PM -0300, Marcelo Tosatti wrote:
> On Wed, Mar 02, 2011 at 11:23:14AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Mar 02, 2011 at 03:26:53PM +0800, Sheng Yang wrote:
> > > Change from v10:
> > > 1. Update according to the comments of Michael.
> > > 2. Use mmio_needed to exit to userspace according to Marcelo's comments.
> >
> > PCI-wise, I don't see anything to complain about.
> > So ack the PCI bits.
> > You guys decide on the rest.
> >
> > Several things I suggested previously that are not
> > related to the PCI point of view:
> >
> > 1. In msix_table_mmio_write, we fill in ext_data even if
> > we are not going to exit to userspace in the end.
> > It seems a trivial optimization to only do it if we exit.
> > 2. Instead of filling in ext_data, and then copying to vcpu,
> > we could fill the data in vcpu directly.
> > 3. MSIX is not an error. So returning -ENOTSYNC to signal
> > it is ugly. It would be cleaner to return negative
> > value on error, and positive exit code to trigger exit.
> > 4. Patch 4/4 adds whitespace errors that git complains about.
> >
> > With changes 2 and 3, arch/x86/kvm/x86.c would not
> > need to know about msix at all.
> >
> > As I said these are all suggestions unrelated to pci,
> > and I don't know what Avi/Marcelo think about 1 and 2.
> > 3 and 4 are easy to fix though.
>
> All minor IMO

Yes.

> (i prefer ENOTSYNC as its meaningful),

When this is merged I'll post a patch on top and
we can discuss.

> whitespace
> can be fixed while applying.
>
> Avi, can you please ACK?
>
> Sheng, we will fix any further comments. Thanks!