From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35259) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZiJrI-0006yz-Lj for qemu-devel@nongnu.org; Sat, 03 Oct 2015 06:16:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZiJrD-0002aN-Ju for qemu-devel@nongnu.org; Sat, 03 Oct 2015 06:16:04 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:35701) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZiJrD-0002Y7-C1 for qemu-devel@nongnu.org; Sat, 03 Oct 2015 06:15:59 -0400 Received: by pacfv12 with SMTP id fv12so131866953pac.2 for ; Sat, 03 Oct 2015 03:15:57 -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> <55EFDD26.5040403@ozlabs.ru> <1441823669.20355.521.camel@redhat.com> <560E391D.9020200@ozlabs.ru> <1443795607.26107.104.camel@redhat.com> From: Alexey Kardashevskiy Message-ID: <560FAAD8.8060406@ozlabs.ru> Date: Sat, 3 Oct 2015 20:15:52 +1000 MIME-Version: 1.0 In-Reply-To: <1443795607.26107.104.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: Paolo Bonzini , qemu-devel@nongnu.org On 10/03/2015 12:20 AM, Alex Williamson wrote: > On Fri, 2015-10-02 at 17:58 +1000, Alexey Kardashevskiy wrote: >> On 09/10/2015 04:34 AM, Alex Williamson wrote: >>> On Wed, 2015-09-09 at 17:17 +1000, Alexey Kardashevskiy wrote: >>>> 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? >>> >>> See the patch series I just posted that eliminates use of target page >>> size in quirks and then tell me if/why you still need to avoid quirks on >>> POWER. Thanks, >> >> I pulled the latest upstream which has "VFIO updates 2015-09-23", I presume >> this is the patchset you mentioned (which I am not sure about). >> >> The vfio_probe_nvidia_bar0_quirk() fails by exact same reason - the offset >> (0x88000) is not host page size (0x10000) aligned. It used to be >> &TARGET_PAGE_MASK, now there is no alignment. &qemu_real_host_page_mask >> would work if I wanted the quirk but as you mentioned before, it is a more >> straight approach if we just disable quirks when we do not need them. > > Sorry, I don't accept this answer. We have another quirk initialized > from vfio_probe_nvidia_bar0_quirk() that places a quirk at 0x1800. This > is not aligned with the host page size on x86 (0x1000), but it works as > expected. Ah. My ignorance. I did not realize that. So the problem is actually in kvm_set_phys_mem() in kvm-all.c which aligns to TARGET_PAGE_SIZE (not qemu_real_host_page_size), this is why it works for 4K pages. I'll try patching kvm-all.c. > This suggests that there's something wrong with how your > platform handles memory regions and if we need to worry about host page > sizes every time we want to drop a quirk into an MMIO region, this > problem is going to continue to plague us. The reason for disabling > this quirk should be performance or scalability, not functionality. The > functionality problem needs to be found and fixed. Thanks, I agree. I was only going that direction because you suggested disabling quirks on the platform which does not need it. -- Alexey