On Sun, Sep 21, 2014 at 11:19:36AM -0400, Sasha Levin wrote:
> On 09/21/2014 11:02 AM, Michael S. Tsirkin wrote:
> > On Sun, Sep 21, 2014 at 09:47:51AM -0400, Sasha Levin wrote:
> >> > On 09/21/2014 04:09 AM, Michael S. Tsirkin wrote:
> >>>> > >> The virtio 0.9.5 spec says that ISR is "unused" when in MSI-X mode. I
> >>>>> > >> > don't think that you can depend on the device to set the configuration
> >>>>> > >> > changed bit.
> >>>>> > >> > The virtio 1.0 spec seems to have fixed that.
> >>> > > Yes, virtio 0.9.5 has this bug. But in practice qemu always set this
> >>> > > bit, so for qemu we could do that unconditionally. Pekka's lkvm tool
> >>> > > doesn't unfortunately. It's easy to fix that, but it would be nicer to
> >>> > > additionally probe for old versions of the tool, and disable IRQF_SHARED
> >>> > > in that case.
> >>> > >
> >>> > > To complicate things, lkvm does not use a distinct subsystem vendor ID,
> >>> > > in spite of the fact the virtio spec always required this explicitly.
> >> >
> >> > I think I may be a bit confused here, but AFAIK we do set subsystem vendor
> >> > ID properly for our virtio-pci devices?
> >> >
> >> > vpci->pci_hdr = (struct pci_device_header) {
> >> > .vendor_id = cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET),
> >> > .device_id = cpu_to_le16(device_id),
> >> > [...]
> >> > .subsys_vendor_id = cpu_to_le16(PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET),
> >> >
> >> >
> >> > Thanks,
> >> > Sasha
> >
> > Yes but the spec says:
> > The Subsystem Vendor ID should reflect the PCI Vendor ID of the environment.
> >
> > IOW lkvm shouldn't reuse the ID from qemu, it should have its own
> > (qemu and lkvm hypervisors being a different environment).
> >
> > virtio 1.0 have weakened this requirement:
> > The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY
> > reflect the PCI Vendor and Device
> > ID of the environment (for informational purposes by the driver).
> >
> > I reasoned that since it's for informational purposes only, there's no
> > reason to make it a SHOULD.
> >
> > It might or might not be a good idea to change it back, worth
> > considering.
>
> Ow. The 0.9.5 spec also says:
>
> "(it's currently only used for informational purposes by the guest)."
>
> That and the combination of "should" rather then "must" (recommended rather than
> required) prompted us to just put something that works in there and leave it be.
>
>
> Thanks,
> Sasha
Note "currently" as well as "should" which means "before you don't, make
sure you understand the implications".
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
