From patchwork Wed Aug 16 09:10:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Igor Mammedov X-Patchwork-Id: 9903317 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 93AFD60244 for ; Wed, 16 Aug 2017 09:13:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 862D6289A2 for ; Wed, 16 Aug 2017 09:13:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7B14D2899C; Wed, 16 Aug 2017 09:13:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 90A24289A8 for ; Wed, 16 Aug 2017 09:13:55 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dhuLp-00008Z-Eg; Wed, 16 Aug 2017 09:10:57 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dhuLo-00007e-7e for xen-devel@lists.xenproject.org; Wed, 16 Aug 2017 09:10:56 +0000 Received: from [85.158.137.68] by server-7.bemta-3.messagelabs.com id 81/6D-02177-F1C04995; Wed, 16 Aug 2017 09:10:55 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrIIsWRWlGSWpSXmKPExsVysWW7jK4cz5R Ig2Vz1Cy+b5nM5MDocfjDFZYAxijWzLyk/IoE1owTu44xFlzXr7i86B9jA+MhlS5GLg4hgTlM Ep9O9rJ1MXJysAg4SMyb2ccIYjMKlEn8W9MDZedKTNx9kgmiYR6jRMfHI1ANqhJHNmxlB7HZB DQlns+5AGaLANlPb70Ga2AWWMEkcaz5KNgkYYEEiWcz5rCA2LwClhJv/kFs5hSwkVjRu5YVYs MFRomHn89AFQlKnJz5BMxmFtCSePjrFpQtL7H97RxmEFtCQFviyO69bCDNEgJ9jBK3j99mmcA oNAtJ/ywk/bOQ9C9gZF7FqF6cWlSWWqRrqpdUlJmeUZKbmJmja2hgrJebWlycmJ6ak5hUrJec n7uJERjU9QwMjDsYL391OsQoycGkJMq76OykSCG+pPyUyozE4oz4otKc1OJDjDIcHEoSvEncU yKFBItS01Mr0jJzgPEFk5bg4FES4ZUASfMWFyTmFmemQ6ROMSpKifO+5wJKCIAkMkrz4NpgMX 2JUVZKmJeRgYFBiKcgtSg3swRV/hWjOAejkjCvOsh4nsy8Erjpr4AWMwEtvtI+CWRxSSJCSqq BsW7SrTRrz9Dqm19WJxncvnApp+G22e3pwnqh7yMcN4U6mZpXRrP1LWSdwJd6c1rBy8bLn5c/ fa1ZE5XCfu+uZ1rOtsf736vffhD04YXq5OQoC67ayPdvOkq454h5rTtoEbMwgiuzIfz7TqOa5 Y9jJy+6+9eMdYO41qxrb+3v8nok3+e9mWk9TYmlOCPRUIu5qDgRAKnF6brkAgAA X-Env-Sender: imammedo@redhat.com X-Msg-Ref: server-11.tower-31.messagelabs.com!1502874652!79345809!1 X-Originating-IP: [209.132.183.28] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMjA5LjEzMi4xODMuMjggPT4gNTQwNjQ=\n X-StarScan-Received: X-StarScan-Version: 9.4.45; banners=-,-,- X-VirusChecked: Checked Received: (qmail 34019 invoked from network); 16 Aug 2017 09:10:54 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by server-11.tower-31.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 16 Aug 2017 09:10:54 -0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3E8C92C96C9; Wed, 16 Aug 2017 09:10:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3E8C92C96C9 Received: from nial.brq.redhat.com (unknown [10.43.2.241]) by smtp.corp.redhat.com (Postfix) with ESMTP id 18A1A65EA8; Wed, 16 Aug 2017 09:10:47 +0000 (UTC) Date: Wed, 16 Aug 2017 11:10:46 +0200 From: Igor Mammedov To: "Michael S. Tsirkin" Message-ID: <20170816111046.7e0a935d@nial.brq.redhat.com> In-Reply-To: <20170815222133-mutt-send-email-mst@kernel.org> References: <20170815111549.6232-1-anthony.perard@citrix.com> <20170815111549.6232-2-anthony.perard@citrix.com> <20170815140751.2d432a46@nial.brq.redhat.com> <20170815222133-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 16 Aug 2017 09:10:52 +0000 (UTC) Cc: Stefano Stabellini , Eduardo Habkost , qemu-devel@nongnu.org, Bruce Rogers , Paolo Bonzini , Anthony PERARD , xen-devel@lists.xenproject.org, Richard Henderson Subject: Re: [Xen-devel] [PATCH for-2.10 v2 1/2] hw/acpi: Call acpi_set_pci_info when no ACPI tables needed X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 15 Aug 2017 22:24:08 +0300 "Michael S. Tsirkin" wrote: > On Tue, Aug 15, 2017 at 02:07:51PM +0200, Igor Mammedov wrote: > > On Tue, 15 Aug 2017 12:15:48 +0100 > > Anthony PERARD wrote: > > > > > To do PCI passthrough with Xen, the property acpi-pcihp-bsel needs to be > > > set, but this was done only when ACPI tables are built which is not > > > needed for a Xen guest. The need for the property starts with commit > > > "pc: pcihp: avoid adding ACPI_PCIHP_PROP_BSEL twice" > > > (f0c9d64a68b776374ec4732424a3e27753ce37b6). > > > > > > Set pci info before checking for the needs to build ACPI tables. > > > > > > Assign bsel=0 property only to the root bus on Xen as there is no > > > support in the Xen ACPI tables for a different value. > > > > looking at hw/acpi/pcihp.c and bsel usage there it looks like > > bsel property is owned by it and not by ACPI tables, so instead of > > shuffling it in acpi_setup(), how about moving bsel initialization > > to hw/acpi/pcihp.c and initialize it there unconditionally? > > > > It could be as simple as moving acpi_set_pci_info()/acpi_set_bsel() > > there and calling it from acpi_pcihp_reset(). > > > > Then there won't be need for Xen specific branches, as root bus > > will have bsel set automatically which is sufficient for Xen and > > the rest of bsel-s (bridges) will be just unused by Xen, > > which could later extend its ACPI table implementation to utilize them. > > Later is exactly what I'd like to try to avoid. > Whoever wants acpi hotplug for bridges needs to get > the bsel info from qemu supplied acpi tables. I'd prefer to have only one behavior in QEMU (on hw interface) side and let Xen to maintain their own ACPI tables dealing with issues that arise from it since they insist on doing job twice. The point is bsel is so embedded in HW part of impl. that it should be allocated/manged there, otherwise it leads to hacks where acpi_setup() is called but does partial init and then bails out to fix code pcihp.c that depend on it being run, pcihp.c (hw part) shouldn't depend on on ACPI tables generation (bios part). Anyway if you insist on capping Xen, it probably could be done with comat machinery, something like this: (where the 1st hunk should been there since, we've introduced "acpi-pci-hotplug-with-bridge-support") > > > Reported-by: Sander Eikelenboom > > > Signed-off-by: Anthony PERARD > > > > > > --- > > > Changes in V2: > > > - check for acpi_enabled before calling acpi_set_pci_info. > > > - set the property on the root bus only. > > > > > > This patch would be a canditade to backport to 2.9. > > > > > > CC: Stefano Stabellini > > > CC: Bruce Rogers > > > --- > > > hw/i386/acpi-build.c | 25 ++++++++++++++++--------- > > > 1 file changed, 16 insertions(+), 9 deletions(-) > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > index 98dd424678..c0483b96cf 100644 > > > --- a/hw/i386/acpi-build.c > > > +++ b/hw/i386/acpi-build.c > > > @@ -46,6 +46,7 @@ > > > #include "sysemu/tpm_backend.h" > > > #include "hw/timer/mc146818rtc_regs.h" > > > #include "sysemu/numa.h" > > > +#include "hw/xen/xen.h" > > > > > > /* Supported chipsets: */ > > > #include "hw/acpi/piix4.h" > > > @@ -518,8 +519,13 @@ static void acpi_set_pci_info(void) > > > unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; > > > > > > if (bus) { > > > - /* Scan all PCI buses. Set property to enable acpi based hotplug. */ > > > - pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); > > > + if (xen_enabled()) { > > > + /* Assign BSEL property to root bus only. */ > > > + acpi_set_bsel(bus, &bsel_alloc); > > > + } else { > > > + /* Scan all PCI buses. Set property to enable acpi based hotplug. */ > > > + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); > > > + } > > > } > > > } > > > > > > @@ -2871,6 +2877,14 @@ void acpi_setup(void) > > > AcpiBuildState *build_state; > > > Object *vmgenid_dev; > > > > > > + if (!acpi_enabled) { > > > + ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n"); > > > + return; > > > + } > > > + > > > + /* Assign BSEL property on hotpluggable PCI buses. */ > > > + acpi_set_pci_info(); > > > + > > > if (!pcms->fw_cfg) { > > > ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n"); > > > return; > > > @@ -2881,15 +2895,8 @@ void acpi_setup(void) > > > return; > > > } > > > > > > - if (!acpi_enabled) { > > > - ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n"); > > > - return; > > > - } > > > - > > > build_state = g_malloc0(sizeof *build_state); > > > > > > - acpi_set_pci_info(); > > > - > > > acpi_build_tables_init(&tables); > > > acpi_build(&tables, MACHINE(pcms)); > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index c420a38..a55f022 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -273,7 +273,7 @@ static void pci_write(void *opaque, hwaddr addr, uint64_t data, addr, data); break; case PCI_SEL_BASE: - s->hotplug_select = data; + s->hotplug_select = s->legacy_piix ? 0 : data; ACPI_PCIHP_DPRINTF("pcisel write %" HWADDR_PRIx " <== %" PRIu64 "\n", addr, data); default: diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 22dbef6..81b8c3e 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -1117,6 +1117,13 @@ static void xenfv_machine_options(MachineClass *m) m->max_cpus = HVM_MAX_VCPUS; m->default_machine_opts = "accel=xen"; m->hot_add_cpu = pc_hot_add_cpu; + SET_MACHINE_COMPAT(m, + {\ + .driver = "PIIX4_PM",\ + .property = "acpi-pci-hotplug-with-bridge-support",\ + .value = "off",\ + }, + ); } DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,