From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH RFC] virtio-pci: share config interrupt between virtio devices Date: Sun, 21 Sep 2014 20:53:37 +0300 Message-ID: <20140921175337.GA28640@redhat.com> References: <1409550114-8186-1-git-send-email-akong@redhat.com> <20140901063730.GB20186@redhat.com> <11860049.kd4R4PIiz4@k> <20140921080914.GA2489@redhat.com> <541ED707.4050100@oracle.com> <20140921150259.GA27059@redhat.com> <541EEC88.5070804@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <541EEC88.5070804@oracle.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Sasha Levin Cc: kvm@vger.kernel.org, will.deacon@arm.com, virtualization@lists.linux-foundation.org, Stefan Fritsch , penberg@iki.fi List-Id: virtualization@lists.linuxfoundation.org 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