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

Marcelo Tosatti
2011-03-02T18:53:11Z

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 (i prefer ENOTSYNC as its meaningful), whitespace
can be fixed while applying.

Avi, can you please ACK?

Sheng, we will fix any further comments. Thanks!