From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53942) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZH10j-0001R3-6y for qemu-devel@nongnu.org; Sun, 19 Jul 2015 22:40:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZH10b-0006Ab-Ss for qemu-devel@nongnu.org; Sun, 19 Jul 2015 22:40:57 -0400 Received: from mail-pd0-f171.google.com ([209.85.192.171]:36018) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZH10b-0006AI-Np for qemu-devel@nongnu.org; Sun, 19 Jul 2015 22:40:49 -0400 Received: by pdjr16 with SMTP id r16so96503042pdj.3 for ; Sun, 19 Jul 2015 19:40:49 -0700 (PDT) References: <1437293970-6727-1-git-send-email-aik@ozlabs.ru> <1437310218.1391.660.camel@redhat.com> <1437326126.1391.710.camel@redhat.com> From: Alexey Kardashevskiy Message-ID: <55AC5FAC.4000508@ozlabs.ru> Date: Mon, 20 Jul 2015 12:40:44 +1000 MIME-Version: 1.0 In-Reply-To: <1437326126.1391.710.camel@redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH qemu] vfio_pci: Allow disabling quirks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org On 07/20/2015 03:15 AM, Alex Williamson wrote: > On Sun, 2015-07-19 at 06:50 -0600, Alex Williamson wrote: >> On Sun, 2015-07-19 at 18:19 +1000, Alexey Kardashevskiy wrote: >>> The existing quirks aim config space and MSIX BAR accesses interception. >>> These are not always needed, for example, on pseries machines, >>> config space and MSI/MSIX configuration are handled by hypervisor. >>> >>> This adds a "quirks" property to control whether to enable quirks or not; >>> the property is set to "true" by default. >>> >>> Signed-off-by: Alexey Kardashevskiy >>> --- >>> >>> Helps to get >>> VGA compatible controller: NVIDIA Corporation GM107GL [Quadro K2200] (rev a2) >>> (which does not need the quirk anyway) working on POWER8 system. > > BTW, as I was working on the rtl quirk last week, I re-realized > something; page size doesn't matter for quirks. If we want to carve a > 4k hole for a PCI extended config space window, we can do that > regardless of the host page size. The rtl quirk only carves out an 8 > byte window. It's the Memory API's problem to figure out the extent of > the region that needs to fault into QEMU and which parts go through the > quirk vs the slow backing of the BAR. But the memory API cannot handle it right now, right? > That's why we create the slow > backing across the entire BAR. So if this quirk isn't working for you, > page size is probably not the reason. Quirks do not install - KVM fails to register these memory regions. And when I fix this, they do not for that another unknown reason, so it is not probably, it is definitely :) > That said, there are some > gratuitous uses of target page size in the quirk code. Part of it is > just a convenience factor for packing data structures, part of it is > completely unnecessary, like the TARGET_PAGE_ALIGN setting up the quirk > in question here. Thanks, Regarding automatic disabling of quirks on POWER - I'd love to do that but how can I do this without adding a property? #ifdef PPC64 in hw/vfio/pci.c? We were avoiding this. > > Alex > >>> --- >>> hw/vfio/pci.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >>> index 2ed877f..ba47301 100644 >>> --- a/hw/vfio/pci.c >>> +++ b/hw/vfio/pci.c >>> @@ -168,6 +168,7 @@ typedef struct VFIOPCIDevice { >>> bool has_flr; >>> bool has_pm_reset; >>> bool rom_read_failed; >>> + bool allow_quirks; >>> } VFIOPCIDevice; >>> >>> typedef struct VFIORomBlacklistEntry { >>> @@ -2442,7 +2443,9 @@ static void vfio_map_bar(VFIOPCIDevice *vdev, int nr) >>> } >>> } >>> >>> - vfio_bar_quirk_setup(vdev, nr); >>> + if (vdev->allow_quirks) { >>> + vfio_bar_quirk_setup(vdev, nr); >>> + } >>> } >>> >>> static void vfio_map_bars(VFIOPCIDevice *vdev) >>> @@ -3753,6 +3756,7 @@ static Property vfio_pci_dev_properties[] = { >>> VFIO_FEATURE_ENABLE_REQ_BIT, true), >>> DEFINE_PROP_INT32("bootindex", VFIOPCIDevice, bootindex, -1), >>> DEFINE_PROP_BOOL("x-mmap", VFIOPCIDevice, vbasedev.allow_mmap, true), >>> + DEFINE_PROP_BOOL("quirks", VFIOPCIDevice, allow_quirks, true), >>> /* >>> * TODO - support passed fds... is this necessary? >>> * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name), >> >> >> NAK, if you don't need it for power then make it automatically disabled >> for power. >> Otherwise you're just pushing the burden onto the >> user/libvirt to know when to use this option. Thanks, -- Alexey