From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33027) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZZdx-0005ll-6A for qemu-devel@nongnu.org; Wed, 09 Sep 2015 03:18:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZZdt-0001es-WE for qemu-devel@nongnu.org; Wed, 09 Sep 2015 03:18:09 -0400 Received: from mail-pa0-f43.google.com ([209.85.220.43]:35761) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZZdt-0001eO-Oc for qemu-devel@nongnu.org; Wed, 09 Sep 2015 03:18:05 -0400 Received: by pacfv12 with SMTP id fv12so1718788pac.2 for ; Wed, 09 Sep 2015 00:18:04 -0700 (PDT) References: <1437293970-6727-1-git-send-email-aik@ozlabs.ru> <1437310218.1391.660.camel@redhat.com> <1437326126.1391.710.camel@redhat.com> <55AC5FAC.4000508@ozlabs.ru> From: Alexey Kardashevskiy Message-ID: <55EFDD26.5040403@ozlabs.ru> Date: Wed, 9 Sep 2015 17:17:58 +1000 MIME-Version: 1.0 In-Reply-To: <55AC5FAC.4000508@ozlabs.ru> 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 12:40 PM, Alexey Kardashevskiy wrote: > 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. Ping? The only proper automatic disabling on POWER I can think of would be: 1) (still) add a "allow_quirks" property 2) in the pseries machine init code, set the property in the compat_props to "false". But this still requires a property. Better ideas? > > > > >> >> 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