From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37901) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1diWGi-0005TM-J7 for qemu-devel@nongnu.org; Thu, 17 Aug 2017 21:40:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1diWGe-00047G-Kb for qemu-devel@nongnu.org; Thu, 17 Aug 2017 21:40:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44552) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1diWGe-000433-BP for qemu-devel@nongnu.org; Thu, 17 Aug 2017 21:40:08 -0400 Date: Fri, 18 Aug 2017 04:40:02 +0300 From: "Michael S. Tsirkin" Message-ID: <20170818043655-mutt-send-email-mst@kernel.org> References: <20170817162347.1590-1-anthony.perard@citrix.com> <20170817162347.1590-3-anthony.perard@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170817162347.1590-3-anthony.perard@citrix.com> Subject: Re: [Qemu-devel] [PATCH for-2.10 v3 2/3] hw/acpi: Move acpi_set_pci_info to pcihp List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony PERARD Cc: qemu-devel@nongnu.org, xen-devel@lists.xenproject.org, Stefano Stabellini , Bruce Rogers , Igor Mammedov , Paolo Bonzini , Richard Henderson , Eduardo Habkost On Thu, Aug 17, 2017 at 05:23:46PM +0100, Anthony PERARD wrote: > This means that the function will be call and the property > acpi-pcihp-bsel will be set even if ACPI build is disable. > > 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). > > Reported-by: Sander Eikelenboom > Signed-off-by: Anthony PERARD > > --- > Changes in V3: > - move acpi_set_pci_info to pcihp instead > > 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, along with > "hw/acpi: Limit hotplug to root bus on legacy mode" > > CC: Stefano Stabellini > CC: Bruce Rogers > --- > hw/acpi/pcihp.c | 31 +++++++++++++++++++++++++++++++ > hw/i386/acpi-build.c | 32 -------------------------------- > 2 files changed, 31 insertions(+), 32 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index 9db3c2eaf2..44e8842db8 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -75,6 +75,36 @@ static int acpi_pcihp_get_bsel(PCIBus *bus) > } > } > > +/* Assign BSEL property to all buses. In the future, this can be changed > + * to only assign to buses that support hotplug. > + */ > +static void *acpi_set_bsel(PCIBus *bus, void *opaque) > +{ > + unsigned *bsel_alloc = opaque; > + unsigned *bus_bsel; > + > + if (qbus_is_hotpluggable(BUS(bus))) { > + bus_bsel = g_malloc(sizeof *bus_bsel); > + > + *bus_bsel = (*bsel_alloc)++; > + object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, > + bus_bsel, &error_abort); > + } > + > + return bsel_alloc; > +} > + > +static void acpi_set_pci_info(void) > +{ > + PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > + 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); > + } > +} > + > static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque) > { > AcpiPciHpFind *find = opaque; > @@ -177,6 +207,7 @@ static void acpi_pcihp_update(AcpiPciHpState *s) > > void acpi_pcihp_reset(AcpiPciHpState *s) > { > + acpi_set_pci_info(); > acpi_pcihp_update(s); > } IIUC doing this on reset will add property over and over again leaking memory. I think that we need to do it on machine done. Igor, I think reordering acpi-build like earlier version did is less intrusive and more appropriate for 2.10. For 2.10 I would like to see ideally some changes that are all if (xen) making it obvious non xen is not affected. I can then ack it and it will be merged in xen tree. Clean it up after 2.10. > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 98dd424678..4d19d91e1b 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -493,36 +493,6 @@ build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms) > table_data->len - madt_start, 1, NULL, NULL); > } > > -/* Assign BSEL property to all buses. In the future, this can be changed > - * to only assign to buses that support hotplug. > - */ > -static void *acpi_set_bsel(PCIBus *bus, void *opaque) > -{ > - unsigned *bsel_alloc = opaque; > - unsigned *bus_bsel; > - > - if (qbus_is_hotpluggable(BUS(bus))) { > - bus_bsel = g_malloc(sizeof *bus_bsel); > - > - *bus_bsel = (*bsel_alloc)++; > - object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, > - bus_bsel, &error_abort); > - } > - > - return bsel_alloc; > -} > - > -static void acpi_set_pci_info(void) > -{ > - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > - 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); > - } > -} > - > static void build_append_pcihp_notify_entry(Aml *method, int slot) > { > Aml *if_ctx; > @@ -2888,8 +2858,6 @@ void acpi_setup(void) > > build_state = g_malloc0(sizeof *build_state); > > - acpi_set_pci_info(); > - > acpi_build_tables_init(&tables); > acpi_build(&tables, MACHINE(pcms)); > > -- > Anthony PERARD From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH for-2.10 v3 2/3] hw/acpi: Move acpi_set_pci_info to pcihp Date: Fri, 18 Aug 2017 04:40:02 +0300 Message-ID: <20170818043655-mutt-send-email-mst@kernel.org> References: <20170817162347.1590-1-anthony.perard@citrix.com> <20170817162347.1590-3-anthony.perard@citrix.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 1diWGe-00039I-Fu for xen-devel@lists.xenproject.org; Fri, 18 Aug 2017 01:40:08 +0000 Content-Disposition: inline In-Reply-To: <20170817162347.1590-3-anthony.perard@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Anthony PERARD Cc: Stefano Stabellini , Eduardo Habkost , qemu-devel@nongnu.org, Bruce Rogers , Paolo Bonzini , Igor Mammedov , xen-devel@lists.xenproject.org, Richard Henderson List-Id: xen-devel@lists.xenproject.org T24gVGh1LCBBdWcgMTcsIDIwMTcgYXQgMDU6MjM6NDZQTSArMDEwMCwgQW50aG9ueSBQRVJBUkQg d3JvdGU6Cj4gVGhpcyBtZWFucyB0aGF0IHRoZSBmdW5jdGlvbiB3aWxsIGJlIGNhbGwgYW5kIHRo ZSBwcm9wZXJ0eQo+IGFjcGktcGNpaHAtYnNlbCB3aWxsIGJlIHNldCBldmVuIGlmIEFDUEkgYnVp bGQgaXMgZGlzYWJsZS4KPiAKPiBUbyBkbyBQQ0kgcGFzc3Rocm91Z2ggd2l0aCBYZW4sIHRoZSBw cm9wZXJ0eSBhY3BpLXBjaWhwLWJzZWwgbmVlZHMgdG8gYmUKPiBzZXQsIGJ1dCB0aGlzIHdhcyBk b25lIG9ubHkgd2hlbiBBQ1BJIHRhYmxlcyBhcmUgYnVpbHQgd2hpY2ggaXMgbm90Cj4gbmVlZGVk IGZvciBhIFhlbiBndWVzdC4gVGhlIG5lZWQgZm9yIHRoZSBwcm9wZXJ0eSBzdGFydHMgd2l0aCBj b21taXQKPiAicGM6IHBjaWhwOiBhdm9pZCBhZGRpbmcgQUNQSV9QQ0lIUF9QUk9QX0JTRUwgdHdp Y2UiCj4gKGYwYzlkNjRhNjhiNzc2Mzc0ZWM0NzMyNDI0YTNlMjc3NTNjZTM3YjYpLgo+IAo+IFJl cG9ydGVkLWJ5OiBTYW5kZXIgRWlrZWxlbmJvb20gPGxpbnV4QGVpa2VsZW5ib29tLml0Pgo+IFNp Z25lZC1vZmYtYnk6IEFudGhvbnkgUEVSQVJEIDxhbnRob255LnBlcmFyZEBjaXRyaXguY29tPgo+ IAo+IC0tLQo+IENoYW5nZXMgaW4gVjM6Cj4gICAtIG1vdmUgYWNwaV9zZXRfcGNpX2luZm8gdG8g cGNpaHAgaW5zdGVhZAo+IAo+IENoYW5nZXMgaW4gVjI6Cj4gICAtIGNoZWNrIGZvciBhY3BpX2Vu YWJsZWQgYmVmb3JlIGNhbGxpbmcgYWNwaV9zZXRfcGNpX2luZm8uCj4gICAtIHNldCB0aGUgcHJv cGVydHkgb24gdGhlIHJvb3QgYnVzIG9ubHkuCj4gCj4gVGhpcyBwYXRjaCB3b3VsZCBiZSBhIGNh bmRpdGFkZSB0byBiYWNrcG9ydCB0byAyLjksIGFsb25nIHdpdGgKPiAiaHcvYWNwaTogTGltaXQg aG90cGx1ZyB0byByb290IGJ1cyBvbiBsZWdhY3kgbW9kZSIKPiAKPiBDQzogU3RlZmFubyBTdGFi ZWxsaW5pIDxzc3RhYmVsbGluaUBrZXJuZWwub3JnPgo+IENDOiBCcnVjZSBSb2dlcnMgPGJyb2dl cnNAc3VzZS5jb20+Cj4gLS0tCj4gIGh3L2FjcGkvcGNpaHAuYyAgICAgIHwgMzEgKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKwo+ICBody9pMzg2L2FjcGktYnVpbGQuYyB8IDMyIC0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCj4gIDIgZmlsZXMgY2hhbmdlZCwgMzEgaW5zZXJ0 aW9ucygrKSwgMzIgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2h3L2FjcGkvcGNpaHAu YyBiL2h3L2FjcGkvcGNpaHAuYwo+IGluZGV4IDlkYjNjMmVhZjIuLjQ0ZTg4NDJkYjggMTAwNjQ0 Cj4gLS0tIGEvaHcvYWNwaS9wY2locC5jCj4gKysrIGIvaHcvYWNwaS9wY2locC5jCj4gQEAgLTc1 LDYgKzc1LDM2IEBAIHN0YXRpYyBpbnQgYWNwaV9wY2locF9nZXRfYnNlbChQQ0lCdXMgKmJ1cykK PiAgICAgIH0KPiAgfQo+ICAKPiArLyogQXNzaWduIEJTRUwgcHJvcGVydHkgdG8gYWxsIGJ1c2Vz LiAgSW4gdGhlIGZ1dHVyZSwgdGhpcyBjYW4gYmUgY2hhbmdlZAo+ICsgKiB0byBvbmx5IGFzc2ln biB0byBidXNlcyB0aGF0IHN1cHBvcnQgaG90cGx1Zy4KPiArICovCj4gK3N0YXRpYyB2b2lkICph Y3BpX3NldF9ic2VsKFBDSUJ1cyAqYnVzLCB2b2lkICpvcGFxdWUpCj4gK3sKPiArICAgIHVuc2ln bmVkICpic2VsX2FsbG9jID0gb3BhcXVlOwo+ICsgICAgdW5zaWduZWQgKmJ1c19ic2VsOwo+ICsK PiArICAgIGlmIChxYnVzX2lzX2hvdHBsdWdnYWJsZShCVVMoYnVzKSkpIHsKPiArICAgICAgICBi dXNfYnNlbCA9IGdfbWFsbG9jKHNpemVvZiAqYnVzX2JzZWwpOwo+ICsKPiArICAgICAgICAqYnVz X2JzZWwgPSAoKmJzZWxfYWxsb2MpKys7Cj4gKyAgICAgICAgb2JqZWN0X3Byb3BlcnR5X2FkZF91 aW50MzJfcHRyKE9CSkVDVChidXMpLCBBQ1BJX1BDSUhQX1BST1BfQlNFTCwKPiArICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgYnVzX2JzZWwsICZlcnJvcl9hYm9ydCk7Cj4g KyAgICB9Cj4gKwo+ICsgICAgcmV0dXJuIGJzZWxfYWxsb2M7Cj4gK30KPiArCj4gK3N0YXRpYyB2 b2lkIGFjcGlfc2V0X3BjaV9pbmZvKHZvaWQpCj4gK3sKPiArICAgIFBDSUJ1cyAqYnVzID0gZmlu ZF9pNDQwZngoKTsgLyogVE9ETzogUTM1IHN1cHBvcnQgKi8KPiArICAgIHVuc2lnbmVkIGJzZWxf YWxsb2MgPSBBQ1BJX1BDSUhQX0JTRUxfREVGQVVMVDsKPiArCj4gKyAgICBpZiAoYnVzKSB7Cj4g KyAgICAgICAgLyogU2NhbiBhbGwgUENJIGJ1c2VzLiBTZXQgcHJvcGVydHkgdG8gZW5hYmxlIGFj cGkgYmFzZWQgaG90cGx1Zy4gKi8KPiArICAgICAgICBwY2lfZm9yX2VhY2hfYnVzX2RlcHRoX2Zp cnN0KGJ1cywgYWNwaV9zZXRfYnNlbCwgTlVMTCwgJmJzZWxfYWxsb2MpOwo+ICsgICAgfQo+ICt9 Cj4gKwo+ICBzdGF0aWMgdm9pZCBhY3BpX3BjaWhwX3Rlc3RfaG90cGx1Z19idXMoUENJQnVzICpi dXMsIHZvaWQgKm9wYXF1ZSkKPiAgewo+ICAgICAgQWNwaVBjaUhwRmluZCAqZmluZCA9IG9wYXF1 ZTsKPiBAQCAtMTc3LDYgKzIwNyw3IEBAIHN0YXRpYyB2b2lkIGFjcGlfcGNpaHBfdXBkYXRlKEFj cGlQY2lIcFN0YXRlICpzKQo+ICAKPiAgdm9pZCBhY3BpX3BjaWhwX3Jlc2V0KEFjcGlQY2lIcFN0 YXRlICpzKQo+ICB7Cj4gKyAgICBhY3BpX3NldF9wY2lfaW5mbygpOwo+ICAgICAgYWNwaV9wY2lo cF91cGRhdGUocyk7Cj4gIH0KCklJVUMgZG9pbmcgdGhpcyBvbiByZXNldCB3aWxsIGFkZCBwcm9w ZXJ0eSBvdmVyIGFuZCBvdmVyIGFnYWluIGxlYWtpbmcKbWVtb3J5LgoKSSB0aGluayB0aGF0IHdl IG5lZWQgdG8gZG8gaXQgb24gbWFjaGluZSBkb25lLgoKSWdvciwgIEkgdGhpbmsgcmVvcmRlcmlu ZyBhY3BpLWJ1aWxkIGxpa2UgZWFybGllciB2ZXJzaW9uIGRpZAppcyBsZXNzIGludHJ1c2l2ZSBh bmQgbW9yZSBhcHByb3ByaWF0ZSBmb3IgMi4xMC4KCkZvciAyLjEwIEkgd291bGQgbGlrZSB0byBz ZWUgaWRlYWxseSBzb21lIGNoYW5nZXMgdGhhdAphcmUgYWxsIGlmICh4ZW4pIG1ha2luZyBpdCBv YnZpb3VzIG5vbiB4ZW4gaXMgbm90CmFmZmVjdGVkLiBJIGNhbiB0aGVuIGFjayBpdCBhbmQgaXQg d2lsbCBiZSBtZXJnZWQgaW4geGVuIHRyZWUuCgoKQ2xlYW4gaXQgdXAgYWZ0ZXIgMi4xMC4KCj4g IAo+IGRpZmYgLS1naXQgYS9ody9pMzg2L2FjcGktYnVpbGQuYyBiL2h3L2kzODYvYWNwaS1idWls ZC5jCj4gaW5kZXggOThkZDQyNDY3OC4uNGQxOWQ5MWUxYiAxMDA2NDQKPiAtLS0gYS9ody9pMzg2 L2FjcGktYnVpbGQuYwo+ICsrKyBiL2h3L2kzODYvYWNwaS1idWlsZC5jCj4gQEAgLTQ5MywzNiAr NDkzLDYgQEAgYnVpbGRfbWFkdChHQXJyYXkgKnRhYmxlX2RhdGEsIEJJT1NMaW5rZXIgKmxpbmtl ciwgUENNYWNoaW5lU3RhdGUgKnBjbXMpCj4gICAgICAgICAgICAgICAgICAgdGFibGVfZGF0YS0+ bGVuIC0gbWFkdF9zdGFydCwgMSwgTlVMTCwgTlVMTCk7Cj4gIH0KPiAgCj4gLS8qIEFzc2lnbiBC U0VMIHByb3BlcnR5IHRvIGFsbCBidXNlcy4gIEluIHRoZSBmdXR1cmUsIHRoaXMgY2FuIGJlIGNo YW5nZWQKPiAtICogdG8gb25seSBhc3NpZ24gdG8gYnVzZXMgdGhhdCBzdXBwb3J0IGhvdHBsdWcu Cj4gLSAqLwo+IC1zdGF0aWMgdm9pZCAqYWNwaV9zZXRfYnNlbChQQ0lCdXMgKmJ1cywgdm9pZCAq b3BhcXVlKQo+IC17Cj4gLSAgICB1bnNpZ25lZCAqYnNlbF9hbGxvYyA9IG9wYXF1ZTsKPiAtICAg IHVuc2lnbmVkICpidXNfYnNlbDsKPiAtCj4gLSAgICBpZiAocWJ1c19pc19ob3RwbHVnZ2FibGUo QlVTKGJ1cykpKSB7Cj4gLSAgICAgICAgYnVzX2JzZWwgPSBnX21hbGxvYyhzaXplb2YgKmJ1c19i c2VsKTsKPiAtCj4gLSAgICAgICAgKmJ1c19ic2VsID0gKCpic2VsX2FsbG9jKSsrOwo+IC0gICAg ICAgIG9iamVjdF9wcm9wZXJ0eV9hZGRfdWludDMyX3B0cihPQkpFQ1QoYnVzKSwgQUNQSV9QQ0lI UF9QUk9QX0JTRUwsCj4gLSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGJ1 c19ic2VsLCAmZXJyb3JfYWJvcnQpOwo+IC0gICAgfQo+IC0KPiAtICAgIHJldHVybiBic2VsX2Fs bG9jOwo+IC19Cj4gLQo+IC1zdGF0aWMgdm9pZCBhY3BpX3NldF9wY2lfaW5mbyh2b2lkKQo+IC17 Cj4gLSAgICBQQ0lCdXMgKmJ1cyA9IGZpbmRfaTQ0MGZ4KCk7IC8qIFRPRE86IFEzNSBzdXBwb3J0 ICovCj4gLSAgICB1bnNpZ25lZCBic2VsX2FsbG9jID0gQUNQSV9QQ0lIUF9CU0VMX0RFRkFVTFQ7 Cj4gLQo+IC0gICAgaWYgKGJ1cykgewo+IC0gICAgICAgIC8qIFNjYW4gYWxsIFBDSSBidXNlcy4g U2V0IHByb3BlcnR5IHRvIGVuYWJsZSBhY3BpIGJhc2VkIGhvdHBsdWcuICovCj4gLSAgICAgICAg cGNpX2Zvcl9lYWNoX2J1c19kZXB0aF9maXJzdChidXMsIGFjcGlfc2V0X2JzZWwsIE5VTEwsICZi c2VsX2FsbG9jKTsKPiAtICAgIH0KPiAtfQo+IC0KPiAgc3RhdGljIHZvaWQgYnVpbGRfYXBwZW5k X3BjaWhwX25vdGlmeV9lbnRyeShBbWwgKm1ldGhvZCwgaW50IHNsb3QpCj4gIHsKPiAgICAgIEFt bCAqaWZfY3R4Owo+IEBAIC0yODg4LDggKzI4NTgsNiBAQCB2b2lkIGFjcGlfc2V0dXAodm9pZCkK PiAgCj4gICAgICBidWlsZF9zdGF0ZSA9IGdfbWFsbG9jMChzaXplb2YgKmJ1aWxkX3N0YXRlKTsK PiAgCj4gLSAgICBhY3BpX3NldF9wY2lfaW5mbygpOwo+IC0KPiAgICAgIGFjcGlfYnVpbGRfdGFi bGVzX2luaXQoJnRhYmxlcyk7Cj4gICAgICBhY3BpX2J1aWxkKCZ0YWJsZXMsIE1BQ0hJTkUocGNt cykpOwo+ICAKPiAtLSAKPiBBbnRob255IFBFUkFSRAoKX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVsIG1haWxpbmcgbGlzdApYZW4tZGV2ZWxA bGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5vcmcveGVuLWRldmVsCg==