On Wed, 21 Jun 2023 13:24:42 -0400 Joel Upham <jupham...@gmail.com> wrote:
> On Wed, Jun 21, 2023 at 7:28 AM Igor Mammedov <imamm...@redhat.com> wrote: > > > On Tue, 20 Jun 2023 13:24:36 -0400 > > Joel Upham <jupham...@gmail.com> wrote: > > > > > On Q35 we still need to assign BSEL property to bus(es) for PCI device > > > add/hotplug to work. > > > Extend acpi_set_pci_info() function to support Q35 as well. This patch > > adds new (trivial) > > > function find_q35() which returns root PCIBus object on Q35, in a way > > > similar to what find_i440fx does. > > > > I think patch is mostly obsolete, q35 ACPI PCI hotplug is supported in > > upstream QEMU. > > > > Also see comment below. > > > > I make use of the find_q35() function in later patches, but I agree now a > majority of this patch is a bit different. There is likely an existing alternative already. (probably introduced by ACPI PIC hotplug for q35) > > > > > > > Signed-off-by: Alexey Gerasimenko <x1917x@xxxxxxxxx> > > > Signed-off-by: Joel Upham <jupham...@gmail.com> > > > --- > > > hw/acpi/pcihp.c | 4 +++- > > > hw/pci-host/q35.c | 9 +++++++++ > > > include/hw/i386/pc.h | 3 +++ > > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > > > index cdd6f775a1..f4e39d7a9c 100644 > > > --- a/hw/acpi/pcihp.c > > > +++ b/hw/acpi/pcihp.c > > > @@ -40,6 +40,7 @@ > > > #include "qapi/error.h" > > > #include "qom/qom-qobject.h" > > > #include "trace.h" > > > +#include "sysemu/xen.h" > > > > > > #define ACPI_PCIHP_SIZE 0x0018 > > > #define PCI_UP_BASE 0x0000 > > > @@ -84,7 +85,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > > > bool is_bridge = IS_PCI_BRIDGE(br); > > > > > > /* hotplugged bridges can't be described in ACPI ignore them */ > > > - if (qbus_is_hotpluggable(BUS(bus))) { > > > > > + /* Xen requires hotplugging to the root device, even on the Q35 > > chipset */ > > pls explain what 'root device' is. > > Why can't you use root-ports for hotplug? > > > > Wording may have been incorrect. Root port is correct. This may not be > needed anymore, > and may have been left over for when I was debugging PCIe hotplugging > problems. > I will retest and fix patch once I know more. Xen expects the PCIe device > to be on the root port. > > I can move the function to a different patch that uses it. > > > > + if (qbus_is_hotpluggable(BUS(bus)) || xen_enabled()) { > > > if (!is_bridge || (!br->hotplugged && > > info->has_bridge_hotplug)) { > > > bus_bsel = g_malloc(sizeof *bus_bsel); > > > > > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > > > index fd18920e7f..fe5fc0f47c 100644 > > > --- a/hw/pci-host/q35.c > > > +++ b/hw/pci-host/q35.c > > > @@ -259,6 +259,15 @@ static void q35_host_initfn(Object *obj) > > > qdev_prop_allow_set_link_before_realize, > > 0); > > > } > > > > > > +PCIBus *find_q35(void) > > > +{ > > > + PCIHostState *s = OBJECT_CHECK(PCIHostState, > > > + object_resolve_path("/machine/q35", > > NULL), > > > + TYPE_PCI_HOST_BRIDGE); > > > + return s ? s->bus : NULL; > > > +} > > > + > > > + > > > static const TypeInfo q35_host_info = { > > > .name = TYPE_Q35_HOST_DEVICE, > > > .parent = TYPE_PCIE_HOST_BRIDGE, > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > index c661e9cc80..550f8fa221 100644 > > > --- a/include/hw/i386/pc.h > > > +++ b/include/hw/i386/pc.h > > > @@ -196,6 +196,9 @@ void pc_madt_cpu_entry(int uid, const CPUArchIdList > > *apic_ids, > > > /* sgx.c */ > > > void pc_machine_init_sgx_epc(PCMachineState *pcms); > > > > > > +/* q35.c */ > > > +PCIBus *find_q35(void); > > > + > > > extern GlobalProperty pc_compat_8_0[]; > > > extern const size_t pc_compat_8_0_len; > > > > > > >