From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1A84C6FA8B for ; Tue, 13 Sep 2022 13:35:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231439AbiIMNfD (ORCPT ); Tue, 13 Sep 2022 09:35:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40842 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231740AbiIMNfC (ORCPT ); Tue, 13 Sep 2022 09:35:02 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8813E1928D for ; Tue, 13 Sep 2022 06:35:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663076099; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vptXUYceNtwYVyKwfHyA3dKZqDg4HJHh7ydKyqgQ4H0=; b=S4b3e69zJq8HEaY7lpGW8BZAdT8bF3g68oKSbO5j61ZdgoPK4J8wGW5Lq19dAPqv5ouP4S tNRi+kFlUD/bcOVLKKpKN1uEHcDc2UDmWmYgwykDYYn5KIf8K+hYsC3WpbNuQXIPw9+77N zIYxWv5WNt/aCqTegoe7FAG87SVeO18= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-530-jDoLI4neOamvpWS-NEF5_Q-1; Tue, 13 Sep 2022 09:34:54 -0400 X-MC-Unique: jDoLI4neOamvpWS-NEF5_Q-1 Received: by mail-ej1-f71.google.com with SMTP id xh12-20020a170906da8c00b007413144e87fso4565451ejb.14 for ; Tue, 13 Sep 2022 06:34:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date; bh=vptXUYceNtwYVyKwfHyA3dKZqDg4HJHh7ydKyqgQ4H0=; b=0M4WseD8L3bSfWBnRog1AO6qegsy82UPlTuDzELIVSJ8/CSCR3MSpwrFMVRwnqg/Mk fM1CoOD52E8SlwJ4qdnN5bSZmvr1cACagM9BytrlxM/181uFUPIChn9jHgBpi+dHqLEQ aHKlITTB1Cenm+9CbKSAUV/XXWIF5F2niJvu4D1jNdpOOQ47vlzHSeAF9ZfbNZQKyMSR vIdYmZjGtvkTXDo+PbVqjqbKvcM+Zo0GFqLQ0tbFfSVkt9Dq/oPgmVCZFxEpTFnFZdg7 Ypg8Akh1YUQJSPLBqj+5lhP18Hs5eI7olc/oP0RPftzXyqSEEohPWmc5qYhB2EmwzG0c CLzw== X-Gm-Message-State: ACgBeo1uwI34RrgsdDha1X1c31NLS9/ITbh9kbyAOCEIVKIMXQjXNCL+ mqDonzE8PhsGDn1jsDpFkc1xuaTredRC4j2mdhnMZD9Y9n2e+7Yf3968F8ah6DSrRAnh44h6Z9Y OPRiFb5+if7jDfDzE0oIe X-Received: by 2002:a17:907:7f91:b0:77f:c4c7:9155 with SMTP id qk17-20020a1709077f9100b0077fc4c79155mr3137420ejc.476.1663076093151; Tue, 13 Sep 2022 06:34:53 -0700 (PDT) X-Google-Smtp-Source: AA6agR6avRwtqutArvSmcEzg7QncTSscL/a0gwHDnK2tQKDnZWQ3sHpo7gh8eU4Pf6A1+IXk9R/ikw== X-Received: by 2002:a17:907:7f91:b0:77f:c4c7:9155 with SMTP id qk17-20020a1709077f9100b0077fc4c79155mr3137408ejc.476.1663076092870; Tue, 13 Sep 2022 06:34:52 -0700 (PDT) Received: from fedora (nat-2.ign.cz. [91.219.240.2]) by smtp.gmail.com with ESMTPSA id 1-20020a170906218100b00730b61d8a5esm6099500eju.61.2022.09.13.06.34.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Sep 2022 06:34:51 -0700 (PDT) From: Vitaly Kuznetsov To: Ajay Kaher Cc: "x86@kernel.org" , "hpa@zytor.com" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "rostedt@goodmis.org" , Srivatsa Bhat , "srivatsa@csail.mit.edu" , Alexey Makhalov , Vasavi Sirnapalli , "er.ajay.kaher@gmail.com" , "willy@infradead.org" , Nadav Amit , "linux-hyperv@vger.kernel.org" , "kvm@vger.kernel.org" , "jailhouse-dev@googlegroups.com" , "xen-devel@lists.xenproject.org" , "acrn-dev@lists.projectacrn.org" , "helgaas@kernel.org" , "bhelgaas@google.com" , "tglx@linutronix.de" , "mingo@redhat.com" , "bp@alien8.de" , "dave.hansen@linux.intel.com" , Alexander Graf Subject: Re: [PATCH v2] x86/PCI: Prefer MMIO over PIO on all hypervisor In-Reply-To: <9FEC6622-780D-41E6-B7CA-8D39EDB2C093@vmware.com> References: <9FEC6622-780D-41E6-B7CA-8D39EDB2C093@vmware.com> Date: Tue, 13 Sep 2022 15:34:50 +0200 Message-ID: <87zgf3pfd1.fsf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Ajay Kaher writes: > Note: Corrected the Subject. > >> =EF=BB=BFOn 07/09/22, 8:50 PM, "Vitaly Kuznetsov" = wrote: >> >>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c >>> index ddb7986..1e5a8f7 100644 >>> --- a/arch/x86/pci/common.c >>> +++ b/arch/x86/pci/common.c >>> @@ -20,6 +20,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> unsigned int pci_probe =3D PCI_PROBE_BIOS | PCI_PROBE_CONF1 | PCI_PROB= E_CONF2 | >>> PCI_PROBE_MMCONF; >>> @@ -57,14 +58,58 @@ int raw_pci_write(unsigned int domain, unsigned int= bus, unsigned int devfn, >>> return -EINVAL; >>> } >>> >>> +#ifdef CONFIG_HYPERVISOR_GUEST >>> +static int vm_raw_pci_read(unsigned int domain, unsigned int bus, unsi= gned int devfn, >>> + int reg, int len, u32 *va= l) >>> +{ >>> + if (raw_pci_ext_ops) >>> + return raw_pci_ext_ops->read(domain, bus, devfn, reg, len= , val); >>> + if (domain =3D=3D 0 && reg < 256 && raw_pci_ops) >>> + return raw_pci_ops->read(domain, bus, devfn, reg, len, va= l); >>> + return -EINVAL; >>> +} >>> + >>> +static int vm_raw_pci_write(unsigned int domain, unsigned int bus, uns= igned int devfn, >>> + int reg, int len, u32 val) >>> +{ >>> + if (raw_pci_ext_ops) >>> + return raw_pci_ext_ops->write(domain, bus, devfn, reg, le= n, val); >>> + if (domain =3D=3D 0 && reg < 256 && raw_pci_ops) >>> + return raw_pci_ops->write(domain, bus, devfn, reg, len, v= al); >>> + return -EINVAL; >>> +} >> >> These look exactly like raw_pci_read()/raw_pci_write() but with inverted >> priority. We could've added a parameter but to be more flexible, I'd >> suggest we add a 'priority' field to 'struct pci_raw_ops' and make >> raw_pci_read()/raw_pci_write() check it before deciding what to use >> first. To be on the safe side, you can leave raw_pci_ops's priority >> higher than raw_pci_ext_ops's by default and only tweak it in >> arch/x86/kernel/cpu/vmware.c > > Thanks Vitaly for your response. > > 1. we have multiple objects of struct pci_raw_ops, 2. adding 'priority' f= ield to struct pci_raw_ops > doesn't seems to be appropriate as need to take decision which object of = struct pci_raw_ops has > to be used, not something with-in struct pci_raw_ops. I'm not sure I follow, you have two instances of 'struct pci_raw_ops' which are called 'raw_pci_ops' and 'raw_pci_ext_ops'. What if you do something like (completely untested): diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h index 70533fdcbf02..fb8270fa6c78 100644 --- a/arch/x86/include/asm/pci_x86.h +++ b/arch/x86/include/asm/pci_x86.h @@ -116,6 +116,7 @@ extern void (*pcibios_disable_irq)(struct pci_dev *dev); extern bool mp_should_keep_irq(struct device *dev); =20 struct pci_raw_ops { + int rating; int (*read)(unsigned int domain, unsigned int bus, unsigned int dev= fn, int reg, int len, u32 *val); int (*write)(unsigned int domain, unsigned int bus, unsigned int de= vfn, diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c index ddb798603201..e9965fd11576 100644 --- a/arch/x86/pci/common.c +++ b/arch/x86/pci/common.c @@ -40,7 +40,8 @@ const struct pci_raw_ops *__read_mostly raw_pci_ext_ops; int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn, int reg, int len, u32 *val) { - if (domain =3D=3D 0 && reg < 256 && raw_pci_ops) + if (domain =3D=3D 0 && reg < 256 && raw_pci_ops && + (!raw_pci_ext_ops || raw_pci_ext_ops->rating <=3D raw_pci_ops->= rating)) return raw_pci_ops->read(domain, bus, devfn, reg, len, val); if (raw_pci_ext_ops) return raw_pci_ext_ops->read(domain, bus, devfn, reg, len, = val); @@ -50,7 +51,8 @@ int raw_pci_read(unsigned int domain, unsigned int bus, u= nsigned int devfn, int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devf= n, int reg, int len, u32 val) { - if (domain =3D=3D 0 && reg < 256 && raw_pci_ops) + if (domain =3D=3D 0 && reg < 256 && raw_pci_ops && + (!raw_pci_ext_ops || raw_pci_ext_ops->rating <=3D raw_pci_ops->= rating)) return raw_pci_ops->write(domain, bus, devfn, reg, len, val= ); if (raw_pci_ext_ops) return raw_pci_ext_ops->write(domain, bus, devfn, reg, len,= val); and then somewhere in Vmware hypervisor initialization code (arch/x86/kernel/cpu/vmware.c) you do raw_pci_ext_ops->rating =3D 100; why wouldn't it work?=20 (diclaimer: completely untested, raw_pci_ops/raw_pci_ext_ops initialization has to be checked so 'rating' is not garbage). > > It's a generic solution for all hypervisor (sorry for earlier wrong > Subject), not specific to VMware. Further looking for feedback if it's > impacting to any hypervisor. That's the tricky part. We can check modern hypervisor versions, but what about all other versions in existence? How can we know that there's no QEMU/Hyper-V/... version out there where MMIO path is broken? I'd suggest we limit the change to Vmware hypervisor, other hypervisors may use the same mechanism (like the one above) later (but the person suggesting the patch is always responsible for the research why it is safe to do so). --=20 Vitaly