From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bF7jA-00059Y-Q7 for qemu-devel@nongnu.org; Mon, 20 Jun 2016 18:31:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bF7j6-0005eL-Nm for qemu-devel@nongnu.org; Mon, 20 Jun 2016 18:31:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40632) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bF7j6-0005eH-I1 for qemu-devel@nongnu.org; Mon, 20 Jun 2016 18:31:28 -0400 Date: Mon, 20 Jun 2016 16:31:32 -0600 From: Alex Williamson Message-ID: <20160620163132.6943a6d7@t450s.home> In-Reply-To: <57686CCB.6080101@redhat.com> References: <20160620215824.13390.52262.stgit@gimli.home> <57686CCB.6080101@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vfio/pci: Hide SR-IOV capability List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, chen.fan.fnst@cn.fujitsu.com, lersek@redhat.com, zhoujie2011@cn.fujitsu.com On Mon, 20 Jun 2016 16:23:07 -0600 Eric Blake wrote: > On 06/20/2016 04:04 PM, Alex Williamson wrote: > > The kernel currently exposes the SR-IOV capability as read-only > > through vfio-pci. This is sufficient to protect the host kernel, but > > has the potential to confuse guests without further virtualization. > > In particular, OVMF tries to size the VF BARs and comes up with absurd > > results, ending with an assert. There's not much point in adding > > virtualization to a read-only capability, so we simply hide it for > > now. If the kernel ever enables SR-IOV virtualization, we should > > easily be able to test it through VF BAR sizing or explicit flags. > > > > Testing whether we should parse extended capabilities is also pulled > > into the function to keep these assumptions in one place. > > > > Signed-off-by: Alex Williamson > > --- > > > + * Extended capabilities are chained with each pointing to the next, so we > > + * can drop anything other than the head of the chain simply by modifying > > + * the previous next pointer. For the head of the chain, we can modify the > > + * capability ID to something that cannot match a valid capability. ID > > + * 0 is reserved for this since absence of capabilities is indicated by > > + * 0 for the ID, version, AND next pointer. However, pcie_add_capability() > > + * uses ID 0 as reserved for list management and will incorrectly match and > > + * assert if we attempt to pre-load the head of the chain with with this > > + * ID. Use ID 0xFFFF temporarily since it is also seems to be reserved in > > + * part for identifying abscense of capabilities in a root complex register > > s/abscense/absence/ > Thanks, updated: diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 36d5e00..2418b93 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1796,7 +1796,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) * uses ID 0 as reserved for list management and will incorrectly match and * assert if we attempt to pre-load the head of the chain with with this * ID. Use ID 0xFFFF temporarily since it is also seems to be reserved in - * part for identifying abscense of capabilities in a root complex register + * part for identifying absence of capabilities in a root complex register * block. If the ID still exists after adding capabilities, switch back to * zero. We'll mark this entire first dword as emulated for this purpose. */