From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52855) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dhhRr-0003Hg-Fp for qemu-devel@nongnu.org; Tue, 15 Aug 2017 15:24:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dhhRn-0003ju-FH for qemu-devel@nongnu.org; Tue, 15 Aug 2017 15:24:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55442) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dhhRn-0003iF-5Y for qemu-devel@nongnu.org; Tue, 15 Aug 2017 15:24:15 -0400 Date: Tue, 15 Aug 2017 22:24:08 +0300 From: "Michael S. Tsirkin" Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170815140751.2d432a46@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.10 v2 1/2] hw/acpi: Call acpi_set_pci_info when no ACPI tables needed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Anthony PERARD , qemu-devel@nongnu.org, xen-devel@lists.xenproject.org, Stefano Stabellini , Bruce Rogers , Paolo Bonzini , Richard Henderson , Eduardo Habkost 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. > > > 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)); > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH for-2.10 v2 1/2] hw/acpi: Call acpi_set_pci_info when no ACPI tables needed Date: Tue, 15 Aug 2017 22:24:08 +0300 Message-ID: <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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dhhRo-0005R4-OR for xen-devel@lists.xenproject.org; Tue, 15 Aug 2017 19:24:16 +0000 Content-Disposition: inline In-Reply-To: <20170815140751.2d432a46@nial.brq.redhat.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Igor Mammedov Cc: Stefano Stabellini , Eduardo Habkost , qemu-devel@nongnu.org, Bruce Rogers , Paolo Bonzini , Anthony PERARD , xen-devel@lists.xenproject.org, Richard Henderson List-Id: xen-devel@lists.xenproject.org T24gVHVlLCBBdWcgMTUsIDIwMTcgYXQgMDI6MDc6NTFQTSArMDIwMCwgSWdvciBNYW1tZWRvdiB3 cm90ZToKPiBPbiBUdWUsIDE1IEF1ZyAyMDE3IDEyOjE1OjQ4ICswMTAwCj4gQW50aG9ueSBQRVJB UkQgPGFudGhvbnkucGVyYXJkQGNpdHJpeC5jb20+IHdyb3RlOgo+IAo+ID4gVG8gZG8gUENJIHBh c3N0aHJvdWdoIHdpdGggWGVuLCB0aGUgcHJvcGVydHkgYWNwaS1wY2locC1ic2VsIG5lZWRzIHRv IGJlCj4gPiBzZXQsIGJ1dCB0aGlzIHdhcyBkb25lIG9ubHkgd2hlbiBBQ1BJIHRhYmxlcyBhcmUg YnVpbHQgd2hpY2ggaXMgbm90Cj4gPiBuZWVkZWQgZm9yIGEgWGVuIGd1ZXN0LiBUaGUgbmVlZCBm b3IgdGhlIHByb3BlcnR5IHN0YXJ0cyB3aXRoIGNvbW1pdAo+ID4gInBjOiBwY2locDogYXZvaWQg YWRkaW5nIEFDUElfUENJSFBfUFJPUF9CU0VMIHR3aWNlIgo+ID4gKGYwYzlkNjRhNjhiNzc2Mzc0 ZWM0NzMyNDI0YTNlMjc3NTNjZTM3YjYpLgo+ID4gCj4gPiBTZXQgcGNpIGluZm8gYmVmb3JlIGNo ZWNraW5nIGZvciB0aGUgbmVlZHMgdG8gYnVpbGQgQUNQSSB0YWJsZXMuCj4gPiAKPiA+IEFzc2ln biBic2VsPTAgcHJvcGVydHkgb25seSB0byB0aGUgcm9vdCBidXMgb24gWGVuIGFzIHRoZXJlIGlz IG5vCj4gPiBzdXBwb3J0IGluIHRoZSBYZW4gQUNQSSB0YWJsZXMgZm9yIGEgZGlmZmVyZW50IHZh bHVlLgo+IAo+IGxvb2tpbmcgYXQgaHcvYWNwaS9wY2locC5jIGFuZCBic2VsIHVzYWdlIHRoZXJl IGl0IGxvb2tzIGxpa2UKPiBic2VsIHByb3BlcnR5IGlzIG93bmVkIGJ5IGl0IGFuZCBub3QgYnkg QUNQSSB0YWJsZXMsIHNvIGluc3RlYWQgb2YKPiBzaHVmZmxpbmcgaXQgaW4gYWNwaV9zZXR1cCgp LCBob3cgYWJvdXQgbW92aW5nIGJzZWwgaW5pdGlhbGl6YXRpb24KPiB0byBody9hY3BpL3BjaWhw LmMgYW5kIGluaXRpYWxpemUgaXQgdGhlcmUgdW5jb25kaXRpb25hbGx5Pwo+IAo+IEl0IGNvdWxk IGJlIGFzIHNpbXBsZSBhcyBtb3ZpbmcgYWNwaV9zZXRfcGNpX2luZm8oKS9hY3BpX3NldF9ic2Vs KCkKPiB0aGVyZSBhbmQgY2FsbGluZyBpdCBmcm9tIGFjcGlfcGNpaHBfcmVzZXQoKS4KPiAKPiBU aGVuIHRoZXJlIHdvbid0IGJlIG5lZWQgZm9yIFhlbiBzcGVjaWZpYyBicmFuY2hlcywgYXMgcm9v dCBidXMKPiB3aWxsIGhhdmUgYnNlbCBzZXQgYXV0b21hdGljYWxseSB3aGljaCBpcyBzdWZmaWNp ZW50IGZvciBYZW4gYW5kCj4gdGhlIHJlc3Qgb2YgYnNlbC1zIChicmlkZ2VzKSB3aWxsIGJlIGp1 c3QgdW51c2VkIGJ5IFhlbiwKPiB3aGljaCBjb3VsZCBsYXRlciBleHRlbmQgaXRzIEFDUEkgdGFi bGUgaW1wbGVtZW50YXRpb24gdG8gdXRpbGl6ZSB0aGVtLiAKCkxhdGVyIGlzIGV4YWN0bHkgd2hh dCBJJ2QgbGlrZSB0byB0cnkgdG8gYXZvaWQuCldob2V2ZXIgd2FudHMgYWNwaSBob3RwbHVnIGZv ciBicmlkZ2VzIG5lZWRzIHRvIGdldAp0aGUgYnNlbCBpbmZvIGZyb20gcWVtdSBzdXBwbGllZCBh Y3BpIHRhYmxlcy4KCj4gCj4gPiBSZXBvcnRlZC1ieTogU2FuZGVyIEVpa2VsZW5ib29tIDxsaW51 eEBlaWtlbGVuYm9vbS5pdD4KPiA+IFNpZ25lZC1vZmYtYnk6IEFudGhvbnkgUEVSQVJEIDxhbnRo b255LnBlcmFyZEBjaXRyaXguY29tPgo+ID4gCj4gPiAtLS0KPiA+IENoYW5nZXMgaW4gVjI6Cj4g PiAgIC0gY2hlY2sgZm9yIGFjcGlfZW5hYmxlZCBiZWZvcmUgY2FsbGluZyBhY3BpX3NldF9wY2lf aW5mby4KPiA+ICAgLSBzZXQgdGhlIHByb3BlcnR5IG9uIHRoZSByb290IGJ1cyBvbmx5Lgo+ID4g Cj4gPiBUaGlzIHBhdGNoIHdvdWxkIGJlIGEgY2FuZGl0YWRlIHRvIGJhY2twb3J0IHRvIDIuOS4K PiA+IAo+ID4gQ0M6IFN0ZWZhbm8gU3RhYmVsbGluaSA8c3N0YWJlbGxpbmlAa2VybmVsLm9yZz4K PiA+IENDOiBCcnVjZSBSb2dlcnMgPGJyb2dlcnNAc3VzZS5jb20+Cj4gPiAtLS0KPiA+ICBody9p Mzg2L2FjcGktYnVpbGQuYyB8IDI1ICsrKysrKysrKysrKysrKystLS0tLS0tLS0KPiA+ICAxIGZp bGUgY2hhbmdlZCwgMTYgaW5zZXJ0aW9ucygrKSwgOSBkZWxldGlvbnMoLSkKPiA+IAo+ID4gZGlm ZiAtLWdpdCBhL2h3L2kzODYvYWNwaS1idWlsZC5jIGIvaHcvaTM4Ni9hY3BpLWJ1aWxkLmMKPiA+ IGluZGV4IDk4ZGQ0MjQ2NzguLmMwNDgzYjk2Y2YgMTAwNjQ0Cj4gPiAtLS0gYS9ody9pMzg2L2Fj cGktYnVpbGQuYwo+ID4gKysrIGIvaHcvaTM4Ni9hY3BpLWJ1aWxkLmMKPiA+IEBAIC00Niw2ICs0 Niw3IEBACj4gPiAgI2luY2x1ZGUgInN5c2VtdS90cG1fYmFja2VuZC5oIgo+ID4gICNpbmNsdWRl ICJody90aW1lci9tYzE0NjgxOHJ0Y19yZWdzLmgiCj4gPiAgI2luY2x1ZGUgInN5c2VtdS9udW1h LmgiCj4gPiArI2luY2x1ZGUgImh3L3hlbi94ZW4uaCIKPiA+ICAKPiA+ICAvKiBTdXBwb3J0ZWQg Y2hpcHNldHM6ICovCj4gPiAgI2luY2x1ZGUgImh3L2FjcGkvcGlpeDQuaCIKPiA+IEBAIC01MTgs OCArNTE5LDEzIEBAIHN0YXRpYyB2b2lkIGFjcGlfc2V0X3BjaV9pbmZvKHZvaWQpCj4gPiAgICAg IHVuc2lnbmVkIGJzZWxfYWxsb2MgPSBBQ1BJX1BDSUhQX0JTRUxfREVGQVVMVDsKPiA+ICAKPiA+ ICAgICAgaWYgKGJ1cykgewo+ID4gLSAgICAgICAgLyogU2NhbiBhbGwgUENJIGJ1c2VzLiBTZXQg cHJvcGVydHkgdG8gZW5hYmxlIGFjcGkgYmFzZWQgaG90cGx1Zy4gKi8KPiA+IC0gICAgICAgIHBj aV9mb3JfZWFjaF9idXNfZGVwdGhfZmlyc3QoYnVzLCBhY3BpX3NldF9ic2VsLCBOVUxMLCAmYnNl bF9hbGxvYyk7Cj4gPiArICAgICAgICBpZiAoeGVuX2VuYWJsZWQoKSkgewo+ID4gKyAgICAgICAg ICAgIC8qIEFzc2lnbiBCU0VMIHByb3BlcnR5IHRvIHJvb3QgYnVzIG9ubHkuICovCj4gPiArICAg ICAgICAgICAgYWNwaV9zZXRfYnNlbChidXMsICZic2VsX2FsbG9jKTsKPiA+ICsgICAgICAgIH0g ZWxzZSB7Cj4gPiArICAgICAgICAgICAgLyogU2NhbiBhbGwgUENJIGJ1c2VzLiBTZXQgcHJvcGVy dHkgdG8gZW5hYmxlIGFjcGkgYmFzZWQgaG90cGx1Zy4gKi8KPiA+ICsgICAgICAgICAgICBwY2lf Zm9yX2VhY2hfYnVzX2RlcHRoX2ZpcnN0KGJ1cywgYWNwaV9zZXRfYnNlbCwgTlVMTCwgJmJzZWxf YWxsb2MpOwo+ID4gKyAgICAgICAgfQo+ID4gICAgICB9Cj4gPiAgfQo+ID4gIAo+ID4gQEAgLTI4 NzEsNiArMjg3NywxNCBAQCB2b2lkIGFjcGlfc2V0dXAodm9pZCkKPiA+ICAgICAgQWNwaUJ1aWxk U3RhdGUgKmJ1aWxkX3N0YXRlOwo+ID4gICAgICBPYmplY3QgKnZtZ2VuaWRfZGV2Owo+ID4gIAo+ ID4gKyAgICBpZiAoIWFjcGlfZW5hYmxlZCkgewo+ID4gKyAgICAgICAgQUNQSV9CVUlMRF9EUFJJ TlRGKCJBQ1BJIGRpc2FibGVkLiBCYWlsaW5nIG91dC5cbiIpOwo+ID4gKyAgICAgICAgcmV0dXJu Owo+ID4gKyAgICB9Cj4gPiArCj4gPiArICAgIC8qIEFzc2lnbiBCU0VMIHByb3BlcnR5IG9uIGhv dHBsdWdnYWJsZSBQQ0kgYnVzZXMuICovCj4gPiArICAgIGFjcGlfc2V0X3BjaV9pbmZvKCk7Cj4g PiArCj4gPiAgICAgIGlmICghcGNtcy0+ZndfY2ZnKSB7Cj4gPiAgICAgICAgICBBQ1BJX0JVSUxE X0RQUklOVEYoIk5vIGZ3IGNmZy4gQmFpbGluZyBvdXQuXG4iKTsKPiA+ICAgICAgICAgIHJldHVy bjsKPiA+IEBAIC0yODgxLDE1ICsyODk1LDggQEAgdm9pZCBhY3BpX3NldHVwKHZvaWQpCj4gPiAg ICAgICAgICByZXR1cm47Cj4gPiAgICAgIH0KPiA+ICAKPiA+IC0gICAgaWYgKCFhY3BpX2VuYWJs ZWQpIHsKPiA+IC0gICAgICAgIEFDUElfQlVJTERfRFBSSU5URigiQUNQSSBkaXNhYmxlZC4gQmFp bGluZyBvdXQuXG4iKTsKPiA+IC0gICAgICAgIHJldHVybjsKPiA+IC0gICAgfQo+ID4gLQo+ID4g ICAgICBidWlsZF9zdGF0ZSA9IGdfbWFsbG9jMChzaXplb2YgKmJ1aWxkX3N0YXRlKTsKPiA+ICAK PiA+IC0gICAgYWNwaV9zZXRfcGNpX2luZm8oKTsKPiA+IC0KPiA+ICAgICAgYWNwaV9idWlsZF90 YWJsZXNfaW5pdCgmdGFibGVzKTsKPiA+ICAgICAgYWNwaV9idWlsZCgmdGFibGVzLCBNQUNISU5F KHBjbXMpKTsKPiA+ICAKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fClhlbi1kZXZlbCBtYWlsaW5nIGxpc3QKWGVuLWRldmVsQGxpc3RzLnhlbi5vcmcKaHR0 cHM6Ly9saXN0cy54ZW4ub3JnL3hlbi1kZXZlbAo=