* [Qemu-devel] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
@ 2018-12-21 0:34 ` Greg Kurz
2018-12-21 17:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
2019-01-03 0:32 ` [Qemu-devel] " David Gibson
2018-12-21 0:35 ` [Qemu-devel] [PATCH 02/15] spapr: move spapr_create_phb() to core machine code Greg Kurz
` (13 subsequent siblings)
14 siblings, 2 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Alexey Kardashevskiy <aik@ozlabs.ru>
SLOF receives a device tree and updates it with various properties
before switching to the guest kernel and QEMU is not aware of any changes
made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
sense to pass the SLOF final device tree to QEMU to let it implement
RTAS related tasks better, such as PCI host bus adapter hotplug.
Specifially, now QEMU can find out the actual XICS phandle (for PHB
hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
assisted NMI - FWNMI).
This stores the initial DT blob in the sPAPR machine and replaces it
in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
This adds an @update_dt_enabled machine property to allow backward
migration.
SLOF already has a hypercall since
https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
This makes use of the new fdt_check_full() helper. In order to allow
the configure script to pick the correct DTC version, this adjusts
the DTC presense test.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
configure | 2 +-
hw/ppc/spapr.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++
hw/ppc/trace-events | 3 +++
include/hw/ppc/spapr.h | 7 ++++++-
5 files changed, 94 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 224d3071ac61..baeeabc29f56 100755
--- a/configure
+++ b/configure
@@ -3916,7 +3916,7 @@ if test "$fdt" != "no" ; then
cat > $TMPC << EOF
#include <libfdt.h>
#include <libfdt_env.h>
-int main(void) { fdt_first_subnode(0, 0); return 0; }
+int main(void) { fdt_check_full(NULL, 0); return 0; }
EOF
if compile_prog "" "$fdt_libs" ; then
# system DTC is good - use it
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 17ad84396b31..8ea680fcde1e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1668,7 +1668,10 @@ static void spapr_machine_reset(void)
/* Load the fdt */
qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
- g_free(fdt);
+ g_free(spapr->fdt_blob);
+ spapr->fdt_size = fdt_totalsize(fdt);
+ spapr->fdt_initial_size = spapr->fdt_size;
+ spapr->fdt_blob = fdt;
/* Set up the entry state */
spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
@@ -1919,6 +1922,39 @@ static const VMStateDescription vmstate_spapr_irq_map = {
},
};
+static bool spapr_dtb_needed(void *opaque)
+{
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
+
+ return smc->update_dt_enabled;
+}
+
+static int spapr_dtb_pre_load(void *opaque)
+{
+ sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
+
+ g_free(spapr->fdt_blob);
+ spapr->fdt_blob = NULL;
+ spapr->fdt_size = 0;
+
+ return 0;
+}
+
+static const VMStateDescription vmstate_spapr_dtb = {
+ .name = "spapr_dtb",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = spapr_dtb_needed,
+ .pre_load = spapr_dtb_pre_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
+ VMSTATE_UINT32(fdt_size, sPAPRMachineState),
+ VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
+ fdt_size),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
static const VMStateDescription vmstate_spapr = {
.name = "spapr",
.version_id = 3,
@@ -1948,6 +1984,7 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_cap_ibs,
&vmstate_spapr_irq_map,
&vmstate_spapr_cap_nested_kvm_hv,
+ &vmstate_spapr_dtb,
NULL
}
};
@@ -3929,6 +3966,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
hc->unplug = spapr_machine_device_unplug;
smc->dr_lmb_enabled = true;
+ smc->update_dt_enabled = true;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
mc->has_hotpluggable_cpus = true;
smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
@@ -4024,9 +4062,12 @@ DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
static void spapr_machine_3_1_class_options(MachineClass *mc)
{
+ sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
spapr_machine_4_0_class_options(mc);
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
+ smc->update_dt_enabled = false;
}
DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index ae913d070f50..78fecc8fe906 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1717,6 +1717,46 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
args[0] = characteristics;
args[1] = behaviour;
+ return H_SUCCESS;
+}
+
+static target_ulong h_update_dt(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ target_ulong dt = ppc64_phys_to_real(args[0]);
+ struct fdt_header hdr = { 0 };
+ unsigned cb;
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+ void *fdt;
+
+ cpu_physical_memory_read(dt, &hdr, sizeof(hdr));
+ cb = fdt32_to_cpu(hdr.totalsize);
+
+ if (!smc->update_dt_enabled) {
+ return H_SUCCESS;
+ }
+
+ /* Check that the fdt did not grow out of proportion */
+ if (cb > spapr->fdt_initial_size * 2) {
+ trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
+ fdt32_to_cpu(hdr.magic));
+ return H_PARAMETER;
+ }
+
+ fdt = g_malloc0(cb);
+ cpu_physical_memory_read(dt, fdt, cb);
+
+ /* Check the fdt consistency */
+ if (fdt_check_full(fdt, cb)) {
+ trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
+ fdt32_to_cpu(hdr.magic));
+ return H_PARAMETER;
+ }
+
+ g_free(spapr->fdt_blob);
+ spapr->fdt_size = cb;
+ spapr->fdt_blob = fdt;
+ trace_spapr_update_dt(cb);
return H_SUCCESS;
}
@@ -1822,6 +1862,8 @@ static void hypercall_register_types(void)
/* ibm,client-architecture-support support */
spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
+
+ spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
}
type_init(hypercall_register_types)
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index dc5e65aee96d..0af155ed323d 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -22,6 +22,9 @@ spapr_cas_pvr_try(uint32_t pvr) "0x%x"
spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=0x%x, explicit_match=%u, new=0x%x"
spapr_h_resize_hpt_prepare(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
spapr_h_resize_hpt_commit(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
+spapr_update_dt(unsigned cb) "New blob %u bytes"
+spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
+spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
# hw/ppc/spapr_iommu.c
spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=0x%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 2c77a8ba8810..36033b89d31a 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -103,6 +103,7 @@ struct sPAPRMachineClass {
/*< public >*/
bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
+ bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
bool pre_2_10_has_unused_icps;
bool legacy_irq_allocation;
@@ -139,6 +140,9 @@ struct sPAPRMachineState {
int vrma_adjust;
ssize_t rtas_size;
void *rtas_blob;
+ uint32_t fdt_size;
+ uint32_t fdt_initial_size;
+ void *fdt_blob;
long kernel_size;
bool kernel_le;
uint32_t initrd_base;
@@ -480,7 +484,8 @@ struct sPAPRMachineState {
#define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
/* Client Architecture support */
#define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2)
-#define KVMPPC_HCALL_MAX KVMPPC_H_CAS
+#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
+#define KVMPPC_HCALL_MAX KVMPPC_H_UPDATE_DT
typedef struct sPAPRDeviceTreeUpdateHeader {
uint32_t version_id;
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF
2018-12-21 0:34 ` [Qemu-devel] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF Greg Kurz
@ 2018-12-21 17:39 ` Murilo Opsfelder Araujo
2019-01-02 3:25 ` David Gibson
2019-01-03 0:32 ` [Qemu-devel] " David Gibson
1 sibling, 1 reply; 36+ messages in thread
From: Murilo Opsfelder Araujo @ 2018-12-21 17:39 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, Cornelia Huck, Gerd Hoffmann, Eduardo Habkost,
Michael S. Tsirkin, David Hildenbrand, Michael Roth, qemu-s390x,
Dmitry Fleytman, qemu-ppc, Cédric Le Goater,
Marcel Apfelbaum, Paolo Bonzini, David Gibson
On Fri, Dec 21, 2018 at 01:34:48AM +0100, Greg Kurz wrote:
> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> SLOF receives a device tree and updates it with various properties
> before switching to the guest kernel and QEMU is not aware of any changes
> made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
> sense to pass the SLOF final device tree to QEMU to let it implement
> RTAS related tasks better, such as PCI host bus adapter hotplug.
>
> Specifially, now QEMU can find out the actual XICS phandle (for PHB
> hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
> assisted NMI - FWNMI).
>
> This stores the initial DT blob in the sPAPR machine and replaces it
> in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
>
> This adds an @update_dt_enabled machine property to allow backward
> migration.
>
> SLOF already has a hypercall since
> https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
>
> This makes use of the new fdt_check_full() helper. In order to allow
> the configure script to pick the correct DTC version, this adjusts
> the DTC presense test.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> configure | 2 +-
> hw/ppc/spapr.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/trace-events | 3 +++
> include/hw/ppc/spapr.h | 7 ++++++-
> 5 files changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 224d3071ac61..baeeabc29f56 100755
> --- a/configure
> +++ b/configure
> @@ -3916,7 +3916,7 @@ if test "$fdt" != "no" ; then
> cat > $TMPC << EOF
> #include <libfdt.h>
> #include <libfdt_env.h>
> -int main(void) { fdt_first_subnode(0, 0); return 0; }
> +int main(void) { fdt_check_full(NULL, 0); return 0; }
> EOF
> if compile_prog "" "$fdt_libs" ; then
> # system DTC is good - use it
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 17ad84396b31..8ea680fcde1e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1668,7 +1668,10 @@ static void spapr_machine_reset(void)
> /* Load the fdt */
> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> - g_free(fdt);
> + g_free(spapr->fdt_blob);
> + spapr->fdt_size = fdt_totalsize(fdt);
> + spapr->fdt_initial_size = spapr->fdt_size;
> + spapr->fdt_blob = fdt;
>
> /* Set up the entry state */
> spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
> @@ -1919,6 +1922,39 @@ static const VMStateDescription vmstate_spapr_irq_map = {
> },
> };
>
> +static bool spapr_dtb_needed(void *opaque)
> +{
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
> +
> + return smc->update_dt_enabled;
> +}
> +
> +static int spapr_dtb_pre_load(void *opaque)
> +{
> + sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
Should we use SPAPR_MACHINE here?
> +
> + g_free(spapr->fdt_blob);
> + spapr->fdt_blob = NULL;
> + spapr->fdt_size = 0;
> +
> + return 0;
> +}
> +
> +static const VMStateDescription vmstate_spapr_dtb = {
> + .name = "spapr_dtb",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = spapr_dtb_needed,
> + .pre_load = spapr_dtb_pre_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
> + VMSTATE_UINT32(fdt_size, sPAPRMachineState),
> + VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
> + fdt_size),
> + VMSTATE_END_OF_LIST()
> + },
> +};
> +
> static const VMStateDescription vmstate_spapr = {
> .name = "spapr",
> .version_id = 3,
> @@ -1948,6 +1984,7 @@ static const VMStateDescription vmstate_spapr = {
> &vmstate_spapr_cap_ibs,
> &vmstate_spapr_irq_map,
> &vmstate_spapr_cap_nested_kvm_hv,
> + &vmstate_spapr_dtb,
> NULL
> }
> };
> @@ -3929,6 +3966,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> hc->unplug = spapr_machine_device_unplug;
>
> smc->dr_lmb_enabled = true;
> + smc->update_dt_enabled = true;
> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> mc->has_hotpluggable_cpus = true;
> smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
> @@ -4024,9 +4062,12 @@ DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
>
> static void spapr_machine_3_1_class_options(MachineClass *mc)
> {
> + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
> spapr_machine_4_0_class_options(mc);
> SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> + smc->update_dt_enabled = false;
> }
>
> DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ae913d070f50..78fecc8fe906 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1717,6 +1717,46 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>
> args[0] = characteristics;
> args[1] = behaviour;
> + return H_SUCCESS;
> +}
> +
> +static target_ulong h_update_dt(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + target_ulong dt = ppc64_phys_to_real(args[0]);
> + struct fdt_header hdr = { 0 };
> + unsigned cb;
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> + void *fdt;
> +
> + cpu_physical_memory_read(dt, &hdr, sizeof(hdr));
> + cb = fdt32_to_cpu(hdr.totalsize);
> +
> + if (!smc->update_dt_enabled) {
> + return H_SUCCESS;
> + }
Does it make sense to move this check upper in the function so it can return
earler if update_dt_enabled is false?
> +
> + /* Check that the fdt did not grow out of proportion */
> + if (cb > spapr->fdt_initial_size * 2) {
> + trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
> + fdt32_to_cpu(hdr.magic));
> + return H_PARAMETER;
> + }
> +
> + fdt = g_malloc0(cb);
> + cpu_physical_memory_read(dt, fdt, cb);
> +
> + /* Check the fdt consistency */
> + if (fdt_check_full(fdt, cb)) {
> + trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
> + fdt32_to_cpu(hdr.magic));
> + return H_PARAMETER;
> + }
> +
> + g_free(spapr->fdt_blob);
> + spapr->fdt_size = cb;
> + spapr->fdt_blob = fdt;
> + trace_spapr_update_dt(cb);
>
> return H_SUCCESS;
> }
> @@ -1822,6 +1862,8 @@ static void hypercall_register_types(void)
>
> /* ibm,client-architecture-support support */
> spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
> +
> + spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
> }
>
> type_init(hypercall_register_types)
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index dc5e65aee96d..0af155ed323d 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -22,6 +22,9 @@ spapr_cas_pvr_try(uint32_t pvr) "0x%x"
> spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=0x%x, explicit_match=%u, new=0x%x"
> spapr_h_resize_hpt_prepare(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
> spapr_h_resize_hpt_commit(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
> +spapr_update_dt(unsigned cb) "New blob %u bytes"
> +spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
> +spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
>
> # hw/ppc/spapr_iommu.c
> spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=0x%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 2c77a8ba8810..36033b89d31a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -103,6 +103,7 @@ struct sPAPRMachineClass {
>
> /*< public >*/
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> + bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> bool pre_2_10_has_unused_icps;
> bool legacy_irq_allocation;
> @@ -139,6 +140,9 @@ struct sPAPRMachineState {
> int vrma_adjust;
> ssize_t rtas_size;
> void *rtas_blob;
> + uint32_t fdt_size;
> + uint32_t fdt_initial_size;
> + void *fdt_blob;
> long kernel_size;
> bool kernel_le;
> uint32_t initrd_base;
> @@ -480,7 +484,8 @@ struct sPAPRMachineState {
> #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
> /* Client Architecture support */
> #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2)
> -#define KVMPPC_HCALL_MAX KVMPPC_H_CAS
> +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
> +#define KVMPPC_HCALL_MAX KVMPPC_H_UPDATE_DT
>
> typedef struct sPAPRDeviceTreeUpdateHeader {
> uint32_t version_id;
>
>
--
Murilo
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF
2018-12-21 17:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
@ 2019-01-02 3:25 ` David Gibson
0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-01-02 3:25 UTC (permalink / raw)
To: Murilo Opsfelder Araujo
Cc: Greg Kurz, qemu-devel, Cornelia Huck, Gerd Hoffmann,
Eduardo Habkost, Michael S. Tsirkin, David Hildenbrand,
Michael Roth, qemu-s390x, Dmitry Fleytman, qemu-ppc,
Cédric Le Goater, Marcel Apfelbaum, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 10656 bytes --]
On Fri, Dec 21, 2018 at 03:39:24PM -0200, Murilo Opsfelder Araujo wrote:
> On Fri, Dec 21, 2018 at 01:34:48AM +0100, Greg Kurz wrote:
> > From: Alexey Kardashevskiy <aik@ozlabs.ru>
> >
> > SLOF receives a device tree and updates it with various properties
> > before switching to the guest kernel and QEMU is not aware of any changes
> > made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
> > sense to pass the SLOF final device tree to QEMU to let it implement
> > RTAS related tasks better, such as PCI host bus adapter hotplug.
> >
> > Specifially, now QEMU can find out the actual XICS phandle (for PHB
> > hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
> > assisted NMI - FWNMI).
> >
> > This stores the initial DT blob in the sPAPR machine and replaces it
> > in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
> >
> > This adds an @update_dt_enabled machine property to allow backward
> > migration.
> >
> > SLOF already has a hypercall since
> > https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
> >
> > This makes use of the new fdt_check_full() helper. In order to allow
> > the configure script to pick the correct DTC version, this adjusts
> > the DTC presense test.
> >
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > configure | 2 +-
> > hw/ppc/spapr.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> > hw/ppc/trace-events | 3 +++
> > include/hw/ppc/spapr.h | 7 ++++++-
> > 5 files changed, 94 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 224d3071ac61..baeeabc29f56 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3916,7 +3916,7 @@ if test "$fdt" != "no" ; then
> > cat > $TMPC << EOF
> > #include <libfdt.h>
> > #include <libfdt_env.h>
> > -int main(void) { fdt_first_subnode(0, 0); return 0; }
> > +int main(void) { fdt_check_full(NULL, 0); return 0; }
> > EOF
> > if compile_prog "" "$fdt_libs" ; then
> > # system DTC is good - use it
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 17ad84396b31..8ea680fcde1e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1668,7 +1668,10 @@ static void spapr_machine_reset(void)
> > /* Load the fdt */
> > qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> > cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> > - g_free(fdt);
> > + g_free(spapr->fdt_blob);
> > + spapr->fdt_size = fdt_totalsize(fdt);
> > + spapr->fdt_initial_size = spapr->fdt_size;
> > + spapr->fdt_blob = fdt;
> >
> > /* Set up the entry state */
> > spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
> > @@ -1919,6 +1922,39 @@ static const VMStateDescription vmstate_spapr_irq_map = {
> > },
> > };
> >
> > +static bool spapr_dtb_needed(void *opaque)
> > +{
> > + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
> > +
> > + return smc->update_dt_enabled;
> > +}
> > +
> > +static int spapr_dtb_pre_load(void *opaque)
> > +{
> > + sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
>
> Should we use SPAPR_MACHINE here?
I don't think so. SPAPR_MACHINE is safer if we know what we have is
definitely a QOM object of some sort. Here we have a void *, so
assuming it's an sPAPRMachineState * is no more dangerous than
assuming it is an Object *.
>
> > +
> > + g_free(spapr->fdt_blob);
> > + spapr->fdt_blob = NULL;
> > + spapr->fdt_size = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static const VMStateDescription vmstate_spapr_dtb = {
> > + .name = "spapr_dtb",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .needed = spapr_dtb_needed,
> > + .pre_load = spapr_dtb_pre_load,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
> > + VMSTATE_UINT32(fdt_size, sPAPRMachineState),
> > + VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
> > + fdt_size),
> > + VMSTATE_END_OF_LIST()
> > + },
> > +};
> > +
> > static const VMStateDescription vmstate_spapr = {
> > .name = "spapr",
> > .version_id = 3,
> > @@ -1948,6 +1984,7 @@ static const VMStateDescription vmstate_spapr = {
> > &vmstate_spapr_cap_ibs,
> > &vmstate_spapr_irq_map,
> > &vmstate_spapr_cap_nested_kvm_hv,
> > + &vmstate_spapr_dtb,
> > NULL
> > }
> > };
> > @@ -3929,6 +3966,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > hc->unplug = spapr_machine_device_unplug;
> >
> > smc->dr_lmb_enabled = true;
> > + smc->update_dt_enabled = true;
> > mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> > mc->has_hotpluggable_cpus = true;
> > smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
> > @@ -4024,9 +4062,12 @@ DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> >
> > static void spapr_machine_3_1_class_options(MachineClass *mc)
> > {
> > + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> > spapr_machine_4_0_class_options(mc);
> > SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> > mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> > + smc->update_dt_enabled = false;
> > }
> >
> > DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index ae913d070f50..78fecc8fe906 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1717,6 +1717,46 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
> >
> > args[0] = characteristics;
> > args[1] = behaviour;
> > + return H_SUCCESS;
> > +}
> > +
> > +static target_ulong h_update_dt(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > + target_ulong opcode, target_ulong *args)
> > +{
> > + target_ulong dt = ppc64_phys_to_real(args[0]);
> > + struct fdt_header hdr = { 0 };
> > + unsigned cb;
> > + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> > + void *fdt;
> > +
> > + cpu_physical_memory_read(dt, &hdr, sizeof(hdr));
> > + cb = fdt32_to_cpu(hdr.totalsize);
> > +
> > + if (!smc->update_dt_enabled) {
> > + return H_SUCCESS;
> > + }
>
> Does it make sense to move this check upper in the function so it can return
> earler if update_dt_enabled is false?
>
> > +
> > + /* Check that the fdt did not grow out of proportion */
> > + if (cb > spapr->fdt_initial_size * 2) {
> > + trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
> > + fdt32_to_cpu(hdr.magic));
> > + return H_PARAMETER;
> > + }
> > +
> > + fdt = g_malloc0(cb);
> > + cpu_physical_memory_read(dt, fdt, cb);
> > +
> > + /* Check the fdt consistency */
> > + if (fdt_check_full(fdt, cb)) {
> > + trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
> > + fdt32_to_cpu(hdr.magic));
> > + return H_PARAMETER;
> > + }
> > +
> > + g_free(spapr->fdt_blob);
> > + spapr->fdt_size = cb;
> > + spapr->fdt_blob = fdt;
> > + trace_spapr_update_dt(cb);
> >
> > return H_SUCCESS;
> > }
> > @@ -1822,6 +1862,8 @@ static void hypercall_register_types(void)
> >
> > /* ibm,client-architecture-support support */
> > spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
> > +
> > + spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
> > }
> >
> > type_init(hypercall_register_types)
> > diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> > index dc5e65aee96d..0af155ed323d 100644
> > --- a/hw/ppc/trace-events
> > +++ b/hw/ppc/trace-events
> > @@ -22,6 +22,9 @@ spapr_cas_pvr_try(uint32_t pvr) "0x%x"
> > spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=0x%x, explicit_match=%u, new=0x%x"
> > spapr_h_resize_hpt_prepare(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
> > spapr_h_resize_hpt_commit(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
> > +spapr_update_dt(unsigned cb) "New blob %u bytes"
> > +spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
> > +spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
> >
> > # hw/ppc/spapr_iommu.c
> > spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=0x%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 2c77a8ba8810..36033b89d31a 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -103,6 +103,7 @@ struct sPAPRMachineClass {
> >
> > /*< public >*/
> > bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> > + bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
> > bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> > bool pre_2_10_has_unused_icps;
> > bool legacy_irq_allocation;
> > @@ -139,6 +140,9 @@ struct sPAPRMachineState {
> > int vrma_adjust;
> > ssize_t rtas_size;
> > void *rtas_blob;
> > + uint32_t fdt_size;
> > + uint32_t fdt_initial_size;
> > + void *fdt_blob;
> > long kernel_size;
> > bool kernel_le;
> > uint32_t initrd_base;
> > @@ -480,7 +484,8 @@ struct sPAPRMachineState {
> > #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
> > /* Client Architecture support */
> > #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2)
> > -#define KVMPPC_HCALL_MAX KVMPPC_H_CAS
> > +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
> > +#define KVMPPC_HCALL_MAX KVMPPC_H_UPDATE_DT
> >
> > typedef struct sPAPRDeviceTreeUpdateHeader {
> > uint32_t version_id;
> >
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF
2018-12-21 0:34 ` [Qemu-devel] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF Greg Kurz
2018-12-21 17:39 ` [Qemu-devel] [Qemu-ppc] " Murilo Opsfelder Araujo
@ 2019-01-03 0:32 ` David Gibson
1 sibling, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-01-03 0:32 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 9869 bytes --]
On Fri, Dec 21, 2018 at 01:34:48AM +0100, Greg Kurz wrote:
> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> SLOF receives a device tree and updates it with various properties
> before switching to the guest kernel and QEMU is not aware of any changes
> made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
> sense to pass the SLOF final device tree to QEMU to let it implement
> RTAS related tasks better, such as PCI host bus adapter hotplug.
>
> Specifially, now QEMU can find out the actual XICS phandle (for PHB
> hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
> assisted NMI - FWNMI).
>
> This stores the initial DT blob in the sPAPR machine and replaces it
> in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
>
> This adds an @update_dt_enabled machine property to allow backward
> migration.
>
> SLOF already has a hypercall since
> https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
>
> This makes use of the new fdt_check_full() helper. In order to allow
> the configure script to pick the correct DTC version, this adjusts
> the DTC presense test.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Greg Kurz <groug@kaod.org>
I've applied this again. Last time it seemed to create a mysterious
crash in the arm target, but I can't reproduce it any more, so fingers
crossed.
> ---
> configure | 2 +-
> hw/ppc/spapr.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/trace-events | 3 +++
> include/hw/ppc/spapr.h | 7 ++++++-
> 5 files changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 224d3071ac61..baeeabc29f56 100755
> --- a/configure
> +++ b/configure
> @@ -3916,7 +3916,7 @@ if test "$fdt" != "no" ; then
> cat > $TMPC << EOF
> #include <libfdt.h>
> #include <libfdt_env.h>
> -int main(void) { fdt_first_subnode(0, 0); return 0; }
> +int main(void) { fdt_check_full(NULL, 0); return 0; }
> EOF
> if compile_prog "" "$fdt_libs" ; then
> # system DTC is good - use it
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 17ad84396b31..8ea680fcde1e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1668,7 +1668,10 @@ static void spapr_machine_reset(void)
> /* Load the fdt */
> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> - g_free(fdt);
> + g_free(spapr->fdt_blob);
> + spapr->fdt_size = fdt_totalsize(fdt);
> + spapr->fdt_initial_size = spapr->fdt_size;
> + spapr->fdt_blob = fdt;
>
> /* Set up the entry state */
> spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
> @@ -1919,6 +1922,39 @@ static const VMStateDescription vmstate_spapr_irq_map = {
> },
> };
>
> +static bool spapr_dtb_needed(void *opaque)
> +{
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
> +
> + return smc->update_dt_enabled;
> +}
> +
> +static int spapr_dtb_pre_load(void *opaque)
> +{
> + sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
> +
> + g_free(spapr->fdt_blob);
> + spapr->fdt_blob = NULL;
> + spapr->fdt_size = 0;
> +
> + return 0;
> +}
> +
> +static const VMStateDescription vmstate_spapr_dtb = {
> + .name = "spapr_dtb",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = spapr_dtb_needed,
> + .pre_load = spapr_dtb_pre_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
> + VMSTATE_UINT32(fdt_size, sPAPRMachineState),
> + VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
> + fdt_size),
> + VMSTATE_END_OF_LIST()
> + },
> +};
> +
> static const VMStateDescription vmstate_spapr = {
> .name = "spapr",
> .version_id = 3,
> @@ -1948,6 +1984,7 @@ static const VMStateDescription vmstate_spapr = {
> &vmstate_spapr_cap_ibs,
> &vmstate_spapr_irq_map,
> &vmstate_spapr_cap_nested_kvm_hv,
> + &vmstate_spapr_dtb,
> NULL
> }
> };
> @@ -3929,6 +3966,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> hc->unplug = spapr_machine_device_unplug;
>
> smc->dr_lmb_enabled = true;
> + smc->update_dt_enabled = true;
> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> mc->has_hotpluggable_cpus = true;
> smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
> @@ -4024,9 +4062,12 @@ DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
>
> static void spapr_machine_3_1_class_options(MachineClass *mc)
> {
> + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
> spapr_machine_4_0_class_options(mc);
> SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> + smc->update_dt_enabled = false;
> }
>
> DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ae913d070f50..78fecc8fe906 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1717,6 +1717,46 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>
> args[0] = characteristics;
> args[1] = behaviour;
> + return H_SUCCESS;
> +}
> +
> +static target_ulong h_update_dt(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + target_ulong dt = ppc64_phys_to_real(args[0]);
> + struct fdt_header hdr = { 0 };
> + unsigned cb;
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> + void *fdt;
> +
> + cpu_physical_memory_read(dt, &hdr, sizeof(hdr));
> + cb = fdt32_to_cpu(hdr.totalsize);
> +
> + if (!smc->update_dt_enabled) {
> + return H_SUCCESS;
> + }
> +
> + /* Check that the fdt did not grow out of proportion */
> + if (cb > spapr->fdt_initial_size * 2) {
> + trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
> + fdt32_to_cpu(hdr.magic));
> + return H_PARAMETER;
> + }
> +
> + fdt = g_malloc0(cb);
> + cpu_physical_memory_read(dt, fdt, cb);
> +
> + /* Check the fdt consistency */
> + if (fdt_check_full(fdt, cb)) {
> + trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
> + fdt32_to_cpu(hdr.magic));
> + return H_PARAMETER;
> + }
> +
> + g_free(spapr->fdt_blob);
> + spapr->fdt_size = cb;
> + spapr->fdt_blob = fdt;
> + trace_spapr_update_dt(cb);
>
> return H_SUCCESS;
> }
> @@ -1822,6 +1862,8 @@ static void hypercall_register_types(void)
>
> /* ibm,client-architecture-support support */
> spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
> +
> + spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
> }
>
> type_init(hypercall_register_types)
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index dc5e65aee96d..0af155ed323d 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -22,6 +22,9 @@ spapr_cas_pvr_try(uint32_t pvr) "0x%x"
> spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=0x%x, explicit_match=%u, new=0x%x"
> spapr_h_resize_hpt_prepare(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
> spapr_h_resize_hpt_commit(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64
> +spapr_update_dt(unsigned cb) "New blob %u bytes"
> +spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
> +spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
>
> # hw/ppc/spapr_iommu.c
> spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=0x%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 2c77a8ba8810..36033b89d31a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -103,6 +103,7 @@ struct sPAPRMachineClass {
>
> /*< public >*/
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> + bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> bool pre_2_10_has_unused_icps;
> bool legacy_irq_allocation;
> @@ -139,6 +140,9 @@ struct sPAPRMachineState {
> int vrma_adjust;
> ssize_t rtas_size;
> void *rtas_blob;
> + uint32_t fdt_size;
> + uint32_t fdt_initial_size;
> + void *fdt_blob;
> long kernel_size;
> bool kernel_le;
> uint32_t initrd_base;
> @@ -480,7 +484,8 @@ struct sPAPRMachineState {
> #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
> /* Client Architecture support */
> #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2)
> -#define KVMPPC_HCALL_MAX KVMPPC_H_CAS
> +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
> +#define KVMPPC_HCALL_MAX KVMPPC_H_UPDATE_DT
>
> typedef struct sPAPRDeviceTreeUpdateHeader {
> uint32_t version_id;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 02/15] spapr: move spapr_create_phb() to core machine code
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
2018-12-21 0:34 ` [Qemu-devel] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF Greg Kurz
@ 2018-12-21 0:35 ` Greg Kurz
2019-01-03 0:33 ` David Gibson
2018-12-21 0:35 ` [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses Greg Kurz
` (12 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
This function is only used when creating the default PHB. Let's rename
it and move it to the core machine code for clarity.
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 13 ++++++++++++-
hw/ppc/spapr_pci.c | 11 -----------
include/hw/pci-host/spapr.h | 2 --
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8ea680fcde1e..1f17b5d01f4f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2551,6 +2551,17 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
}
}
+static PCIHostState *spapr_create_default_phb(void)
+{
+ DeviceState *dev;
+
+ dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
+ qdev_prop_set_uint32(dev, "index", 0);
+ qdev_init_nofail(dev);
+
+ return PCI_HOST_BRIDGE(dev);
+}
+
/* pSeries LPAR / sPAPR hardware init */
static void spapr_machine_init(MachineState *machine)
{
@@ -2782,7 +2793,7 @@ static void spapr_machine_init(MachineState *machine)
/* Set up PCI */
spapr_pci_rtas_init();
- phb = spapr_create_phb(spapr, 0);
+ phb = spapr_create_default_phb();
for (i = 0; i < nb_nics; i++) {
NICInfo *nd = &nd_table[i];
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 2374d55fc112..e59adbe706bb 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1979,17 +1979,6 @@ static const TypeInfo spapr_phb_info = {
}
};
-PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index)
-{
- DeviceState *dev;
-
- dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
- qdev_prop_set_uint32(dev, "index", index);
- qdev_init_nofail(dev);
-
- return PCI_HOST_BRIDGE(dev);
-}
-
typedef struct sPAPRFDT {
void *fdt;
int node_off;
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 7c66c3872f96..a65cfef16945 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -111,8 +111,6 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
return spapr_qirq(spapr, phb->lsi_table[pin].irq);
}
-PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index);
-
int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
uint32_t nr_msis);
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 02/15] spapr: move spapr_create_phb() to core machine code
2018-12-21 0:35 ` [Qemu-devel] [PATCH 02/15] spapr: move spapr_create_phb() to core machine code Greg Kurz
@ 2019-01-03 0:33 ` David Gibson
0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-01-03 0:33 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 2941 bytes --]
On Fri, Dec 21, 2018 at 01:35:09AM +0100, Greg Kurz wrote:
> This function is only used when creating the default PHB. Let's rename
> it and move it to the core machine code for clarity.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Applied to ppc-for-4.0, thanks.
> ---
> hw/ppc/spapr.c | 13 ++++++++++++-
> hw/ppc/spapr_pci.c | 11 -----------
> include/hw/pci-host/spapr.h | 2 --
> 3 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8ea680fcde1e..1f17b5d01f4f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2551,6 +2551,17 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> }
> }
>
> +static PCIHostState *spapr_create_default_phb(void)
> +{
> + DeviceState *dev;
> +
> + dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
> + qdev_prop_set_uint32(dev, "index", 0);
> + qdev_init_nofail(dev);
> +
> + return PCI_HOST_BRIDGE(dev);
> +}
> +
> /* pSeries LPAR / sPAPR hardware init */
> static void spapr_machine_init(MachineState *machine)
> {
> @@ -2782,7 +2793,7 @@ static void spapr_machine_init(MachineState *machine)
> /* Set up PCI */
> spapr_pci_rtas_init();
>
> - phb = spapr_create_phb(spapr, 0);
> + phb = spapr_create_default_phb();
>
> for (i = 0; i < nb_nics; i++) {
> NICInfo *nd = &nd_table[i];
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2374d55fc112..e59adbe706bb 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1979,17 +1979,6 @@ static const TypeInfo spapr_phb_info = {
> }
> };
>
> -PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index)
> -{
> - DeviceState *dev;
> -
> - dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
> - qdev_prop_set_uint32(dev, "index", index);
> - qdev_init_nofail(dev);
> -
> - return PCI_HOST_BRIDGE(dev);
> -}
> -
> typedef struct sPAPRFDT {
> void *fdt;
> int node_off;
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 7c66c3872f96..a65cfef16945 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -111,8 +111,6 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
> return spapr_qirq(spapr, phb->lsi_table[pin].irq);
> }
>
> -PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index);
> -
> int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
> uint32_t nr_msis);
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
2018-12-21 0:34 ` [Qemu-devel] [PATCH 01/15] ppc/spapr: Receive and store device tree blob from SLOF Greg Kurz
2018-12-21 0:35 ` [Qemu-devel] [PATCH 02/15] spapr: move spapr_create_phb() to core machine code Greg Kurz
@ 2018-12-21 0:35 ` Greg Kurz
2018-12-21 16:19 ` Michael S. Tsirkin
2018-12-21 0:35 ` [Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path Greg Kurz
` (11 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
This adds cleanup counterparts to pci_register_root_bus(),
pci_root_bus_new(), and pci_bus_irqs().
These cleanup routines are needed in the case of hotpluggable
PCIHostBridge implementations. Currently we can rely on the
object_unparent()'ing of the PCIHostState recursively unparenting
and cleaning up it's child buses, but we need explicit calls
to also:
1) remove the PCIHostState from pci_host_bridges global list.
otherwise, we risk accessing freed memory when we access
the list later
2) clean up memory allocated in pci_bus_irqs()
Both are handled outside the context of any particular bus or
host bridge's init/realize functions, making it difficult to
avoid the need for explicit cleanup functions without remodeling
how PCIHostBridges are created. So keep it simple and just add
them for now.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/pci/pci.c | 33 +++++++++++++++++++++++++++++++++
include/hw/pci/pci.h | 3 +++
2 files changed, 36 insertions(+)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index efb5ce196ffb..16354f91206c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
}
+static void pci_host_bus_unregister(DeviceState *host)
+{
+ PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
+
+ QLIST_REMOVE(host_bridge, next);
+}
+
PCIBus *pci_device_root_bus(const PCIDevice *d)
{
PCIBus *bus = pci_get_bus(d);
@@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
pci_host_bus_register(parent);
}
+static void pci_bus_uninit(PCIBus *bus)
+{
+ pci_host_bus_unregister(BUS(bus)->parent);
+}
+
bool pci_bus_is_express(PCIBus *bus)
{
return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
@@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
return bus;
}
+void pci_root_bus_cleanup(PCIBus *bus)
+{
+ pci_bus_uninit(bus);
+ object_unparent(OBJECT(bus));
+}
+
void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int nirq)
{
@@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
}
+void pci_bus_irqs_cleanup(PCIBus *bus)
+{
+ bus->set_irq = NULL;
+ bus->map_irq = NULL;
+ bus->irq_opaque = NULL;
+ bus->nirq = 0;
+ g_free(bus->irq_count);
+}
+
PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque,
@@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
return bus;
}
+void pci_unregister_root_bus(PCIBus *bus)
+{
+ pci_bus_irqs_cleanup(bus);
+ pci_root_bus_cleanup(bus);
+}
+
int pci_bus_num(PCIBus *s)
{
return PCI_BUS_GET_CLASS(s)->bus_num(s);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e6514bba23aa..8998e3be3390 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename);
+void pci_root_bus_cleanup(PCIBus *bus);
void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int nirq);
+void pci_bus_irqs_cleanup(PCIBus *bus);
int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
/* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
@@ -417,6 +419,7 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
MemoryRegion *address_space_io,
uint8_t devfn_min, int nirq,
const char *typename);
+void pci_unregister_root_bus(PCIBus *bus);
void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses
2018-12-21 0:35 ` [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses Greg Kurz
@ 2018-12-21 16:19 ` Michael S. Tsirkin
2019-01-03 0:36 ` David Gibson
0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2018-12-21 16:19 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, David Gibson,
Alexey Kardashevskiy, Cédric Le Goater, Michael Roth,
Paolo Bonzini, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> This adds cleanup counterparts to pci_register_root_bus(),
> pci_root_bus_new(), and pci_bus_irqs().
>
> These cleanup routines are needed in the case of hotpluggable
> PCIHostBridge implementations. Currently we can rely on the
> object_unparent()'ing of the PCIHostState recursively unparenting
> and cleaning up it's child buses, but we need explicit calls
> to also:
>
> 1) remove the PCIHostState from pci_host_bridges global list.
> otherwise, we risk accessing freed memory when we access
> the list later
> 2) clean up memory allocated in pci_bus_irqs()
>
> Both are handled outside the context of any particular bus or
> host bridge's init/realize functions, making it difficult to
> avoid the need for explicit cleanup functions without remodeling
> how PCIHostBridges are created. So keep it simple and just add
> them for now.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/pci/pci.c | 33 +++++++++++++++++++++++++++++++++
> include/hw/pci/pci.h | 3 +++
> 2 files changed, 36 insertions(+)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index efb5ce196ffb..16354f91206c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
> QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> }
>
> +static void pci_host_bus_unregister(DeviceState *host)
> +{
> + PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> +
> + QLIST_REMOVE(host_bridge, next);
> +}
> +
> PCIBus *pci_device_root_bus(const PCIDevice *d)
> {
> PCIBus *bus = pci_get_bus(d);
> @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
> pci_host_bus_register(parent);
> }
>
> +static void pci_bus_uninit(PCIBus *bus)
> +{
> + pci_host_bus_unregister(BUS(bus)->parent);
> +}
> +
> bool pci_bus_is_express(PCIBus *bus)
> {
> return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> return bus;
> }
>
> +void pci_root_bus_cleanup(PCIBus *bus)
> +{
> + pci_bus_uninit(bus);
> + object_unparent(OBJECT(bus));
> +}
> +
> void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque, int nirq)
> {
> @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> }
>
> +void pci_bus_irqs_cleanup(PCIBus *bus)
> +{
> + bus->set_irq = NULL;
> + bus->map_irq = NULL;
> + bus->irq_opaque = NULL;
> + bus->nirq = 0;
> + g_free(bus->irq_count);
> +}
> +
> PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> return bus;
> }
>
> +void pci_unregister_root_bus(PCIBus *bus)
> +{
> + pci_bus_irqs_cleanup(bus);
> + pci_root_bus_cleanup(bus);
> +}
> +
> int pci_bus_num(PCIBus *s)
> {
> return PCI_BUS_GET_CLASS(s)->bus_num(s);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index e6514bba23aa..8998e3be3390 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename);
> +void pci_root_bus_cleanup(PCIBus *bus);
> void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque, int nirq);
> +void pci_bus_irqs_cleanup(PCIBus *bus);
> int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> @@ -417,6 +419,7 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, int nirq,
> const char *typename);
> +void pci_unregister_root_bus(PCIBus *bus);
> void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
> PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
> bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses
2018-12-21 16:19 ` Michael S. Tsirkin
@ 2019-01-03 0:36 ` David Gibson
2019-01-03 3:27 ` Michael S. Tsirkin
0 siblings, 1 reply; 36+ messages in thread
From: David Gibson @ 2019-01-03 0:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Greg Kurz, qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Marcel Apfelbaum, Eduardo Habkost, David Hildenbrand,
Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 5726 bytes --]
On Fri, Dec 21, 2018 at 11:19:18AM -0500, Michael S. Tsirkin wrote:
> On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> > From: Michael Roth <mdroth@linux.vnet.ibm.com>
> >
> > This adds cleanup counterparts to pci_register_root_bus(),
> > pci_root_bus_new(), and pci_bus_irqs().
> >
> > These cleanup routines are needed in the case of hotpluggable
> > PCIHostBridge implementations. Currently we can rely on the
> > object_unparent()'ing of the PCIHostState recursively unparenting
> > and cleaning up it's child buses, but we need explicit calls
> > to also:
> >
> > 1) remove the PCIHostState from pci_host_bridges global list.
> > otherwise, we risk accessing freed memory when we access
> > the list later
> > 2) clean up memory allocated in pci_bus_irqs()
> >
> > Both are handled outside the context of any particular bus or
> > host bridge's init/realize functions, making it difficult to
> > avoid the need for explicit cleanup functions without remodeling
> > how PCIHostBridges are created. So keep it simple and just add
> > them for now.
> >
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
I've applied this tentatively to ppc-for-4.0. Let me know, Michael,
if you'd prefer to take it through your tree.
>
> > ---
> > hw/pci/pci.c | 33 +++++++++++++++++++++++++++++++++
> > include/hw/pci/pci.h | 3 +++
> > 2 files changed, 36 insertions(+)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index efb5ce196ffb..16354f91206c 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
> > QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> > }
> >
> > +static void pci_host_bus_unregister(DeviceState *host)
> > +{
> > + PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> > +
> > + QLIST_REMOVE(host_bridge, next);
> > +}
> > +
> > PCIBus *pci_device_root_bus(const PCIDevice *d)
> > {
> > PCIBus *bus = pci_get_bus(d);
> > @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
> > pci_host_bus_register(parent);
> > }
> >
> > +static void pci_bus_uninit(PCIBus *bus)
> > +{
> > + pci_host_bus_unregister(BUS(bus)->parent);
> > +}
> > +
> > bool pci_bus_is_express(PCIBus *bus)
> > {
> > return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> > return bus;
> > }
> >
> > +void pci_root_bus_cleanup(PCIBus *bus)
> > +{
> > + pci_bus_uninit(bus);
> > + object_unparent(OBJECT(bus));
> > +}
> > +
> > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > void *irq_opaque, int nirq)
> > {
> > @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> > }
> >
> > +void pci_bus_irqs_cleanup(PCIBus *bus)
> > +{
> > + bus->set_irq = NULL;
> > + bus->map_irq = NULL;
> > + bus->irq_opaque = NULL;
> > + bus->nirq = 0;
> > + g_free(bus->irq_count);
> > +}
> > +
> > PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > void *irq_opaque,
> > @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > return bus;
> > }
> >
> > +void pci_unregister_root_bus(PCIBus *bus)
> > +{
> > + pci_bus_irqs_cleanup(bus);
> > + pci_root_bus_cleanup(bus);
> > +}
> > +
> > int pci_bus_num(PCIBus *s)
> > {
> > return PCI_BUS_GET_CLASS(s)->bus_num(s);
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index e6514bba23aa..8998e3be3390 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> > MemoryRegion *address_space_mem,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, const char *typename);
> > +void pci_root_bus_cleanup(PCIBus *bus);
> > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > void *irq_opaque, int nirq);
> > +void pci_bus_irqs_cleanup(PCIBus *bus);
> > int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > @@ -417,6 +419,7 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, int nirq,
> > const char *typename);
> > +void pci_unregister_root_bus(PCIBus *bus);
> > void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
> > PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
> > bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses
2019-01-03 0:36 ` David Gibson
@ 2019-01-03 3:27 ` Michael S. Tsirkin
2019-01-03 3:41 ` David Gibson
0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2019-01-03 3:27 UTC (permalink / raw)
To: David Gibson
Cc: Greg Kurz, qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Marcel Apfelbaum, Eduardo Habkost, David Hildenbrand,
Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
On Thu, Jan 03, 2019 at 11:36:33AM +1100, David Gibson wrote:
> On Fri, Dec 21, 2018 at 11:19:18AM -0500, Michael S. Tsirkin wrote:
> > On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> > > From: Michael Roth <mdroth@linux.vnet.ibm.com>
> > >
> > > This adds cleanup counterparts to pci_register_root_bus(),
> > > pci_root_bus_new(), and pci_bus_irqs().
> > >
> > > These cleanup routines are needed in the case of hotpluggable
> > > PCIHostBridge implementations. Currently we can rely on the
> > > object_unparent()'ing of the PCIHostState recursively unparenting
> > > and cleaning up it's child buses, but we need explicit calls
> > > to also:
> > >
> > > 1) remove the PCIHostState from pci_host_bridges global list.
> > > otherwise, we risk accessing freed memory when we access
> > > the list later
> > > 2) clean up memory allocated in pci_bus_irqs()
> > >
> > > Both are handled outside the context of any particular bus or
> > > host bridge's init/realize functions, making it difficult to
> > > avoid the need for explicit cleanup functions without remodeling
> > > how PCIHostBridges are created. So keep it simple and just add
> > > them for now.
> > >
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> >
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> I've applied this tentatively to ppc-for-4.0. Let me know, Michael,
> if you'd prefer to take it through your tree.
I think your tree makes sense for this.
> >
> > > ---
> > > hw/pci/pci.c | 33 +++++++++++++++++++++++++++++++++
> > > include/hw/pci/pci.h | 3 +++
> > > 2 files changed, 36 insertions(+)
> > >
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index efb5ce196ffb..16354f91206c 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
> > > QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> > > }
> > >
> > > +static void pci_host_bus_unregister(DeviceState *host)
> > > +{
> > > + PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> > > +
> > > + QLIST_REMOVE(host_bridge, next);
> > > +}
> > > +
> > > PCIBus *pci_device_root_bus(const PCIDevice *d)
> > > {
> > > PCIBus *bus = pci_get_bus(d);
> > > @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
> > > pci_host_bus_register(parent);
> > > }
> > >
> > > +static void pci_bus_uninit(PCIBus *bus)
> > > +{
> > > + pci_host_bus_unregister(BUS(bus)->parent);
> > > +}
> > > +
> > > bool pci_bus_is_express(PCIBus *bus)
> > > {
> > > return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > > @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> > > return bus;
> > > }
> > >
> > > +void pci_root_bus_cleanup(PCIBus *bus)
> > > +{
> > > + pci_bus_uninit(bus);
> > > + object_unparent(OBJECT(bus));
> > > +}
> > > +
> > > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > void *irq_opaque, int nirq)
> > > {
> > > @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> > > }
> > >
> > > +void pci_bus_irqs_cleanup(PCIBus *bus)
> > > +{
> > > + bus->set_irq = NULL;
> > > + bus->map_irq = NULL;
> > > + bus->irq_opaque = NULL;
> > > + bus->nirq = 0;
> > > + g_free(bus->irq_count);
> > > +}
> > > +
> > > PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > void *irq_opaque,
> > > @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > return bus;
> > > }
> > >
> > > +void pci_unregister_root_bus(PCIBus *bus)
> > > +{
> > > + pci_bus_irqs_cleanup(bus);
> > > + pci_root_bus_cleanup(bus);
> > > +}
> > > +
> > > int pci_bus_num(PCIBus *s)
> > > {
> > > return PCI_BUS_GET_CLASS(s)->bus_num(s);
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index e6514bba23aa..8998e3be3390 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> > > MemoryRegion *address_space_mem,
> > > MemoryRegion *address_space_io,
> > > uint8_t devfn_min, const char *typename);
> > > +void pci_root_bus_cleanup(PCIBus *bus);
> > > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > void *irq_opaque, int nirq);
> > > +void pci_bus_irqs_cleanup(PCIBus *bus);
> > > int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > > /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > > int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > > @@ -417,6 +419,7 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > MemoryRegion *address_space_io,
> > > uint8_t devfn_min, int nirq,
> > > const char *typename);
> > > +void pci_unregister_root_bus(PCIBus *bus);
> > > void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
> > > PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
> > > bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
> >
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses
2019-01-03 3:27 ` Michael S. Tsirkin
@ 2019-01-03 3:41 ` David Gibson
0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-01-03 3:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Greg Kurz, qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Marcel Apfelbaum, Eduardo Habkost, David Hildenbrand,
Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 6492 bytes --]
On Wed, Jan 02, 2019 at 10:27:14PM -0500, Michael S. Tsirkin wrote:
> On Thu, Jan 03, 2019 at 11:36:33AM +1100, David Gibson wrote:
> > On Fri, Dec 21, 2018 at 11:19:18AM -0500, Michael S. Tsirkin wrote:
> > > On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> > > > From: Michael Roth <mdroth@linux.vnet.ibm.com>
> > > >
> > > > This adds cleanup counterparts to pci_register_root_bus(),
> > > > pci_root_bus_new(), and pci_bus_irqs().
> > > >
> > > > These cleanup routines are needed in the case of hotpluggable
> > > > PCIHostBridge implementations. Currently we can rely on the
> > > > object_unparent()'ing of the PCIHostState recursively unparenting
> > > > and cleaning up it's child buses, but we need explicit calls
> > > > to also:
> > > >
> > > > 1) remove the PCIHostState from pci_host_bridges global list.
> > > > otherwise, we risk accessing freed memory when we access
> > > > the list later
> > > > 2) clean up memory allocated in pci_bus_irqs()
> > > >
> > > > Both are handled outside the context of any particular bus or
> > > > host bridge's init/realize functions, making it difficult to
> > > > avoid the need for explicit cleanup functions without remodeling
> > > > how PCIHostBridges are created. So keep it simple and just add
> > > > them for now.
> > > >
> > > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > >
> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > I've applied this tentatively to ppc-for-4.0. Let me know, Michael,
> > if you'd prefer to take it through your tree.
>
>
> I think your tree makes sense for this.
Great, will do.
>
> > >
> > > > ---
> > > > hw/pci/pci.c | 33 +++++++++++++++++++++++++++++++++
> > > > include/hw/pci/pci.h | 3 +++
> > > > 2 files changed, 36 insertions(+)
> > > >
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index efb5ce196ffb..16354f91206c 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
> > > > QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> > > > }
> > > >
> > > > +static void pci_host_bus_unregister(DeviceState *host)
> > > > +{
> > > > + PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> > > > +
> > > > + QLIST_REMOVE(host_bridge, next);
> > > > +}
> > > > +
> > > > PCIBus *pci_device_root_bus(const PCIDevice *d)
> > > > {
> > > > PCIBus *bus = pci_get_bus(d);
> > > > @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
> > > > pci_host_bus_register(parent);
> > > > }
> > > >
> > > > +static void pci_bus_uninit(PCIBus *bus)
> > > > +{
> > > > + pci_host_bus_unregister(BUS(bus)->parent);
> > > > +}
> > > > +
> > > > bool pci_bus_is_express(PCIBus *bus)
> > > > {
> > > > return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > > > @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> > > > return bus;
> > > > }
> > > >
> > > > +void pci_root_bus_cleanup(PCIBus *bus)
> > > > +{
> > > > + pci_bus_uninit(bus);
> > > > + object_unparent(OBJECT(bus));
> > > > +}
> > > > +
> > > > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > > void *irq_opaque, int nirq)
> > > > {
> > > > @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > > bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> > > > }
> > > >
> > > > +void pci_bus_irqs_cleanup(PCIBus *bus)
> > > > +{
> > > > + bus->set_irq = NULL;
> > > > + bus->map_irq = NULL;
> > > > + bus->irq_opaque = NULL;
> > > > + bus->nirq = 0;
> > > > + g_free(bus->irq_count);
> > > > +}
> > > > +
> > > > PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > > pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > > void *irq_opaque,
> > > > @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > > return bus;
> > > > }
> > > >
> > > > +void pci_unregister_root_bus(PCIBus *bus)
> > > > +{
> > > > + pci_bus_irqs_cleanup(bus);
> > > > + pci_root_bus_cleanup(bus);
> > > > +}
> > > > +
> > > > int pci_bus_num(PCIBus *s)
> > > > {
> > > > return PCI_BUS_GET_CLASS(s)->bus_num(s);
> > > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > > index e6514bba23aa..8998e3be3390 100644
> > > > --- a/include/hw/pci/pci.h
> > > > +++ b/include/hw/pci/pci.h
> > > > @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
> > > > MemoryRegion *address_space_mem,
> > > > MemoryRegion *address_space_io,
> > > > uint8_t devfn_min, const char *typename);
> > > > +void pci_root_bus_cleanup(PCIBus *bus);
> > > > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > > > void *irq_opaque, int nirq);
> > > > +void pci_bus_irqs_cleanup(PCIBus *bus);
> > > > int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > > > /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > > > int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > > > @@ -417,6 +419,7 @@ PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > > MemoryRegion *address_space_io,
> > > > uint8_t devfn_min, int nirq,
> > > > const char *typename);
> > > > +void pci_unregister_root_bus(PCIBus *bus);
> > > > void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
> > > > PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
> > > > bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
> > >
> >
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (2 preceding siblings ...)
2018-12-21 0:35 ` [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses Greg Kurz
@ 2018-12-21 0:35 ` Greg Kurz
2019-01-03 1:59 ` David Gibson
2018-12-21 0:36 ` [Qemu-devel] [PATCH 05/15] spapr_pci: add PHB unrealize Greg Kurz
` (10 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
The current realize code assumes the PHB is coldplugged, ie, QEMU will
terminate if an error is detected, and does not bother to free anything
it has already allocated.
In order to support PHB hotplug, let's first ensure spapr_phb_realize()
doesn't leak anything in case of error.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr_pci.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index e59adbe706bb..46d7062dd143 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1570,6 +1570,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
sPAPRTCETable *tcet;
const unsigned windows_supported =
sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
+ Object *drcs[PCI_SLOT_MAX * 8];
if (!spapr) {
error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries machine");
@@ -1733,7 +1734,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
spapr_irq_claim(spapr, irq, true, &local_err);
if (local_err) {
error_propagate_prepend(errp, local_err, "can't allocate LSIs: ");
- return;
+ while (--i >= 0) {
+ spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
+ }
+ goto fail_del_msiwindow;
}
sphb->lsi_table[i].irq = irq;
@@ -1741,9 +1745,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
/* allocate connectors for child PCI devices */
if (sphb->dr_enabled) {
- for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
- spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
- (sphb->index << 16) | i);
+ for (i = 0; i < ARRAY_SIZE(drcs); i++) {
+ drcs[i] =
+ OBJECT(spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
+ (sphb->index << 16) | i));
}
}
@@ -1753,13 +1758,38 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
if (!tcet) {
error_setg(errp, "Creating window#%d failed for %s",
i, sphb->dtbusname);
- return;
+ while (--i >= 0) {
+ tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
+ assert(tcet);
+ memory_region_del_subregion(&sphb->iommu_root,
+ spapr_tce_get_iommu(tcet));
+ object_unparent(OBJECT(tcet));
+ }
+ goto fail_free_drcs;
}
memory_region_add_subregion(&sphb->iommu_root, 0,
spapr_tce_get_iommu(tcet));
}
sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
+ return;
+
+fail_free_drcs:
+ if (sphb->dr_enabled) {
+ for (i = 0; i < ARRAY_SIZE(drcs); i++) {
+ object_unparent(drcs[i]);
+ }
+ }
+fail_del_msiwindow:
+ memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
+ address_space_destroy(&sphb->iommu_as);
+ qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
+ pci_unregister_root_bus(phb->bus);
+ memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
+ if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
+ memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
+ }
+ memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
}
static int spapr_phb_children_reset(Object *child, void *opaque)
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path
2018-12-21 0:35 ` [Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path Greg Kurz
@ 2019-01-03 1:59 ` David Gibson
2019-01-07 16:40 ` Greg Kurz
0 siblings, 1 reply; 36+ messages in thread
From: David Gibson @ 2019-01-03 1:59 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 4581 bytes --]
On Fri, Dec 21, 2018 at 01:35:52AM +0100, Greg Kurz wrote:
> The current realize code assumes the PHB is coldplugged, ie, QEMU will
> terminate if an error is detected, and does not bother to free anything
> it has already allocated.
>
> In order to support PHB hotplug, let's first ensure spapr_phb_realize()
> doesn't leak anything in case of error.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
This looks ok as far as it goes. However, it looks like there will be
a lot of fragments duplicated between the rollback paths and
unrealize() when it's added in the next patch.
A common pattern to avoid that is to make unrealize() safe to call on
a partially realized object, then call that from the realize() failure
paths. Is it possible to do that here? I imagine that would involve
folding this patch with the next as well.
> ---
> hw/ppc/spapr_pci.c | 40 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index e59adbe706bb..46d7062dd143 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1570,6 +1570,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> sPAPRTCETable *tcet;
> const unsigned windows_supported =
> sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
> + Object *drcs[PCI_SLOT_MAX * 8];
>
> if (!spapr) {
> error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries machine");
> @@ -1733,7 +1734,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> spapr_irq_claim(spapr, irq, true, &local_err);
> if (local_err) {
> error_propagate_prepend(errp, local_err, "can't allocate LSIs: ");
> - return;
> + while (--i >= 0) {
> + spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
> + }
> + goto fail_del_msiwindow;
> }
>
> sphb->lsi_table[i].irq = irq;
> @@ -1741,9 +1745,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>
> /* allocate connectors for child PCI devices */
> if (sphb->dr_enabled) {
> - for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> - spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> - (sphb->index << 16) | i);
> + for (i = 0; i < ARRAY_SIZE(drcs); i++) {
> + drcs[i] =
> + OBJECT(spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> + (sphb->index << 16) | i));
> }
> }
>
> @@ -1753,13 +1758,38 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> if (!tcet) {
> error_setg(errp, "Creating window#%d failed for %s",
> i, sphb->dtbusname);
> - return;
> + while (--i >= 0) {
> + tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
> + assert(tcet);
> + memory_region_del_subregion(&sphb->iommu_root,
> + spapr_tce_get_iommu(tcet));
> + object_unparent(OBJECT(tcet));
> + }
> + goto fail_free_drcs;
> }
> memory_region_add_subregion(&sphb->iommu_root, 0,
> spapr_tce_get_iommu(tcet));
> }
>
> sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> + return;
> +
> +fail_free_drcs:
> + if (sphb->dr_enabled) {
> + for (i = 0; i < ARRAY_SIZE(drcs); i++) {
> + object_unparent(drcs[i]);
> + }
> + }
> +fail_del_msiwindow:
> + memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
> + address_space_destroy(&sphb->iommu_as);
> + qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
> + pci_unregister_root_bus(phb->bus);
> + memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
> + if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
> + memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
> + }
> + memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
> }
>
> static int spapr_phb_children_reset(Object *child, void *opaque)
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path
2019-01-03 1:59 ` David Gibson
@ 2019-01-07 16:40 ` Greg Kurz
0 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2019-01-07 16:40 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 4722 bytes --]
On Thu, 3 Jan 2019 12:59:06 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Dec 21, 2018 at 01:35:52AM +0100, Greg Kurz wrote:
> > The current realize code assumes the PHB is coldplugged, ie, QEMU will
> > terminate if an error is detected, and does not bother to free anything
> > it has already allocated.
> >
> > In order to support PHB hotplug, let's first ensure spapr_phb_realize()
> > doesn't leak anything in case of error.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> This looks ok as far as it goes. However, it looks like there will be
> a lot of fragments duplicated between the rollback paths and
> unrealize() when it's added in the next patch.
>
> A common pattern to avoid that is to make unrealize() safe to call on
> a partially realized object, then call that from the realize() failure
> paths. Is it possible to do that here? I imagine that would involve
> folding this patch with the next as well.
>
Makes sense. I'll give a try.
> > ---
> > hw/ppc/spapr_pci.c | 40 +++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index e59adbe706bb..46d7062dd143 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1570,6 +1570,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > sPAPRTCETable *tcet;
> > const unsigned windows_supported =
> > sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
> > + Object *drcs[PCI_SLOT_MAX * 8];
> >
> > if (!spapr) {
> > error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries machine");
> > @@ -1733,7 +1734,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > spapr_irq_claim(spapr, irq, true, &local_err);
> > if (local_err) {
> > error_propagate_prepend(errp, local_err, "can't allocate LSIs: ");
> > - return;
> > + while (--i >= 0) {
> > + spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
> > + }
> > + goto fail_del_msiwindow;
> > }
> >
> > sphb->lsi_table[i].irq = irq;
> > @@ -1741,9 +1745,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> >
> > /* allocate connectors for child PCI devices */
> > if (sphb->dr_enabled) {
> > - for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> > - spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> > - (sphb->index << 16) | i);
> > + for (i = 0; i < ARRAY_SIZE(drcs); i++) {
> > + drcs[i] =
> > + OBJECT(spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> > + (sphb->index << 16) | i));
> > }
> > }
> >
> > @@ -1753,13 +1758,38 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > if (!tcet) {
> > error_setg(errp, "Creating window#%d failed for %s",
> > i, sphb->dtbusname);
> > - return;
> > + while (--i >= 0) {
> > + tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
> > + assert(tcet);
> > + memory_region_del_subregion(&sphb->iommu_root,
> > + spapr_tce_get_iommu(tcet));
> > + object_unparent(OBJECT(tcet));
> > + }
> > + goto fail_free_drcs;
> > }
> > memory_region_add_subregion(&sphb->iommu_root, 0,
> > spapr_tce_get_iommu(tcet));
> > }
> >
> > sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
> > + return;
> > +
> > +fail_free_drcs:
> > + if (sphb->dr_enabled) {
> > + for (i = 0; i < ARRAY_SIZE(drcs); i++) {
> > + object_unparent(drcs[i]);
> > + }
> > + }
> > +fail_del_msiwindow:
> > + memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
> > + address_space_destroy(&sphb->iommu_as);
> > + qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
> > + pci_unregister_root_bus(phb->bus);
> > + memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
> > + if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
> > + memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
> > + }
> > + memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
> > }
> >
> > static int spapr_phb_children_reset(Object *child, void *opaque)
> >
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 05/15] spapr_pci: add PHB unrealize
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (3 preceding siblings ...)
2018-12-21 0:35 ` [Qemu-devel] [PATCH 04/15] spapr_pci: add proper rollback on PHB realize error path Greg Kurz
@ 2018-12-21 0:36 ` Greg Kurz
2018-12-21 0:36 ` [Qemu-devel] [PATCH 06/15] spapr: enable PHB hotplug for default pseries machine type Greg Kurz
` (9 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To support PHB hotplug we need to clean up lingering references,
memory, child properties, etc. prior to the PHB object being
finalized. Generally this will be called as a result of calling
object_unparent() on the PHB object, which in turn would normally
be called as the result of an unplug() operation.
When the PHB is finalized, child objects will be unparented in
turn, and finalized if the PHB was the only reference holder. so
we don't bother to explicitly unparent child objects of the PHB
(spapr_iommu, spapr_drc, etc).
The formula that gives the number of DMA windows is moved to an
inline function in the hw/pci-host/spapr.h header because it
will have other users.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr_pci.c | 56 +++++++++++++++++++++++++++++++++++++++++--
include/hw/pci-host/spapr.h | 4 +++
2 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 46d7062dd143..b772b72d6a48 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1551,6 +1551,57 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
}
}
+static void spapr_phb_finalizefn(Object *obj)
+{
+ sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(obj);
+
+ g_free(sphb->dtbusname);
+ sphb->dtbusname = NULL;
+}
+
+static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+ SysBusDevice *s = SYS_BUS_DEVICE(dev);
+ PCIHostState *phb = PCI_HOST_BRIDGE(s);
+ sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(phb);
+ sPAPRTCETable *tcet;
+ int i;
+ const unsigned windows_supported = spapr_phb_windows_supported(sphb);
+
+ g_hash_table_unref(sphb->msi);
+
+ /*
+ * Remove IO/MMIO subregions and aliases, rest should get cleaned
+ * via PHB's unrealize->object_finalize
+ */
+ for (i = windows_supported - 1; i >= 0; i--) {
+ tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
+ assert(tcet);
+ memory_region_del_subregion(&sphb->iommu_root,
+ spapr_tce_get_iommu(tcet));
+ }
+
+ for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
+ spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
+ }
+
+ QLIST_REMOVE(sphb, list);
+
+ memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
+
+ address_space_destroy(&sphb->iommu_as);
+
+ qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
+ pci_unregister_root_bus(phb->bus);
+
+ memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
+ if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
+ memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
+ }
+ memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
+}
+
static void spapr_phb_realize(DeviceState *dev, Error **errp)
{
/* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
@@ -1568,8 +1619,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
PCIBus *bus;
uint64_t msi_window_size = 4096;
sPAPRTCETable *tcet;
- const unsigned windows_supported =
- sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
+ const unsigned windows_supported = spapr_phb_windows_supported(sphb);
Object *drcs[PCI_SLOT_MAX * 8];
if (!spapr) {
@@ -1988,6 +2038,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
hc->root_bus_path = spapr_phb_root_bus_path;
dc->realize = spapr_phb_realize;
+ dc->unrealize = spapr_phb_unrealize;
dc->props = spapr_phb_properties;
dc->reset = spapr_phb_reset;
dc->vmsd = &vmstate_spapr_pci;
@@ -2002,6 +2053,7 @@ static const TypeInfo spapr_phb_info = {
.name = TYPE_SPAPR_PCI_HOST_BRIDGE,
.parent = TYPE_PCI_HOST_BRIDGE,
.instance_size = sizeof(sPAPRPHBState),
+ .instance_finalize = spapr_phb_finalizefn,
.class_init = spapr_phb_class_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_HOTPLUG_HANDLER },
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index a65cfef16945..9d2ec1a410e8 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -162,4 +162,8 @@ static inline void spapr_phb_vfio_reset(DeviceState *qdev)
void spapr_phb_dma_reset(sPAPRPHBState *sphb);
+static inline unsigned spapr_phb_windows_supported(sPAPRPHBState *sphb)
+{
+ return sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
+}
#endif /* PCI_HOST_SPAPR_H */
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 06/15] spapr: enable PHB hotplug for default pseries machine type
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (4 preceding siblings ...)
2018-12-21 0:36 ` [Qemu-devel] [PATCH 05/15] spapr_pci: add PHB unrealize Greg Kurz
@ 2018-12-21 0:36 ` Greg Kurz
2019-01-03 2:00 ` David Gibson
2018-12-21 0:36 ` [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h Greg Kurz
` (8 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
The 'dr_phb_enabled' field of that class can be set as part of
machine-specific init code. It will be used to conditionally
enable creation of DRC objects and device-tree description to
facilitate hotplug of PHBs.
Since we can't migrate this state to older machine types,
default the option to true and disable it for older machine
types.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 2 ++
include/hw/ppc/spapr.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1f17b5d01f4f..621006eaa862 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4011,6 +4011,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
spapr_caps_add_properties(smc, &error_abort);
smc->irq = &spapr_irq_xics;
+ smc->dr_phb_enabled = true;
}
static const TypeInfo spapr_machine_info = {
@@ -4079,6 +4080,7 @@ static void spapr_machine_3_1_class_options(MachineClass *mc)
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
smc->update_dt_enabled = false;
+ smc->dr_phb_enabled = false;
}
DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 36033b89d31a..e96deefa30de 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -104,6 +104,7 @@ struct sPAPRMachineClass {
/*< public >*/
bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
+ bool dr_phb_enabled; /* enable dynamic-reconfig/hotplug of PHBs */
bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
bool pre_2_10_has_unused_icps;
bool legacy_irq_allocation;
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 06/15] spapr: enable PHB hotplug for default pseries machine type
2018-12-21 0:36 ` [Qemu-devel] [PATCH 06/15] spapr: enable PHB hotplug for default pseries machine type Greg Kurz
@ 2019-01-03 2:00 ` David Gibson
2019-01-08 9:55 ` Greg Kurz
0 siblings, 1 reply; 36+ messages in thread
From: David Gibson @ 2019-01-03 2:00 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]
On Fri, Dec 21, 2018 at 01:36:32AM +0100, Greg Kurz wrote:
> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> The 'dr_phb_enabled' field of that class can be set as part of
> machine-specific init code. It will be used to conditionally
> enable creation of DRC objects and device-tree description to
> facilitate hotplug of PHBs.
>
> Since we can't migrate this state to older machine types,
> default the option to true and disable it for older machine
> types.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Although it makes sense to have this function first while
developing, it's usually best to have it last when you push, so you
don't have a potential bisection breakage where the support is
advertised but not fully working.
> ---
> hw/ppc/spapr.c | 2 ++
> include/hw/ppc/spapr.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1f17b5d01f4f..621006eaa862 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4011,6 +4011,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
> spapr_caps_add_properties(smc, &error_abort);
> smc->irq = &spapr_irq_xics;
> + smc->dr_phb_enabled = true;
> }
>
> static const TypeInfo spapr_machine_info = {
> @@ -4079,6 +4080,7 @@ static void spapr_machine_3_1_class_options(MachineClass *mc)
> SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> smc->update_dt_enabled = false;
> + smc->dr_phb_enabled = false;
> }
>
> DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 36033b89d31a..e96deefa30de 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -104,6 +104,7 @@ struct sPAPRMachineClass {
> /*< public >*/
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
> + bool dr_phb_enabled; /* enable dynamic-reconfig/hotplug of PHBs */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> bool pre_2_10_has_unused_icps;
> bool legacy_irq_allocation;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 06/15] spapr: enable PHB hotplug for default pseries machine type
2019-01-03 2:00 ` David Gibson
@ 2019-01-08 9:55 ` Greg Kurz
0 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2019-01-08 9:55 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]
On Thu, 3 Jan 2019 13:00:58 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Dec 21, 2018 at 01:36:32AM +0100, Greg Kurz wrote:
> > From: Michael Roth <mdroth@linux.vnet.ibm.com>
> >
> > The 'dr_phb_enabled' field of that class can be set as part of
> > machine-specific init code. It will be used to conditionally
> > enable creation of DRC objects and device-tree description to
> > facilitate hotplug of PHBs.
> >
> > Since we can't migrate this state to older machine types,
> > default the option to true and disable it for older machine
> > types.
> >
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> Although it makes sense to have this function first while
> developing, it's usually best to have it last when you push, so you
> don't have a potential bisection breakage where the support is
> advertised but not fully working.
>
Yes you're right. I've done so for v2.
> > ---
> > hw/ppc/spapr.c | 2 ++
> > include/hw/ppc/spapr.h | 1 +
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 1f17b5d01f4f..621006eaa862 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -4011,6 +4011,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
> > spapr_caps_add_properties(smc, &error_abort);
> > smc->irq = &spapr_irq_xics;
> > + smc->dr_phb_enabled = true;
> > }
> >
> > static const TypeInfo spapr_machine_info = {
> > @@ -4079,6 +4080,7 @@ static void spapr_machine_3_1_class_options(MachineClass *mc)
> > SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
> > mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> > smc->update_dt_enabled = false;
> > + smc->dr_phb_enabled = false;
> > }
> >
> > DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 36033b89d31a..e96deefa30de 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -104,6 +104,7 @@ struct sPAPRMachineClass {
> > /*< public >*/
> > bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> > bool update_dt_enabled; /* enable KVMPPC_H_UPDATE_DT */
> > + bool dr_phb_enabled; /* enable dynamic-reconfig/hotplug of PHBs */
> > bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> > bool pre_2_10_has_unused_icps;
> > bool legacy_irq_allocation;
> >
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (5 preceding siblings ...)
2018-12-21 0:36 ` [Qemu-devel] [PATCH 06/15] spapr: enable PHB hotplug for default pseries machine type Greg Kurz
@ 2018-12-21 0:36 ` Greg Kurz
2018-12-21 8:03 ` Cédric Le Goater
2019-01-03 2:07 ` David Gibson
2018-12-21 0:37 ` [Qemu-devel] [PATCH 08/15] spapr: create DR connectors for PHBs Greg Kurz
` (7 subsequent siblings)
14 siblings, 2 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
PHB hotplug will bring more users for it. Let's define it along with
the PHB defines from which it is derived for simplicity.
While here fix a misleading comment about manual placement, which was
abandoned with 30b3bc5aa9f4.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 2 --
include/hw/pci-host/spapr.h | 6 ++++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 621006eaa862..fe3f9829ae6c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3838,8 +3838,6 @@ static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
* 1TiB 64-bit MMIO windows for each PHB.
*/
const uint64_t base_buid = 0x800000020000000ULL;
-#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
- SPAPR_PCI_MEM64_WIN_SIZE - 1)
int i;
/* Sanity check natural alignments */
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 9d2ec1a410e8..83d5075a6ef3 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -94,11 +94,13 @@ struct sPAPRPHBState {
((1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET)
#define SPAPR_PCI_MEM64_WIN_SIZE 0x10000000000ULL /* 1 TiB */
-/* Without manual configuration, all PCI outbound windows will be
- * within this range */
+/* All PCI outbound windows will be within this range */
#define SPAPR_PCI_BASE (1ULL << 45) /* 32 TiB */
#define SPAPR_PCI_LIMIT (1ULL << 46) /* 64 TiB */
+#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
+ SPAPR_PCI_MEM64_WIN_SIZE - 1)
+
#define SPAPR_PCI_2_7_MMIO_WIN_SIZE 0xf80000000
#define SPAPR_PCI_IO_WIN_SIZE 0x10000
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h
2018-12-21 0:36 ` [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h Greg Kurz
@ 2018-12-21 8:03 ` Cédric Le Goater
2018-12-21 9:50 ` Greg Kurz
2019-01-03 2:07 ` David Gibson
1 sibling, 1 reply; 36+ messages in thread
From: Cédric Le Goater @ 2018-12-21 8:03 UTC (permalink / raw)
To: Greg Kurz, qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Michael Roth, Paolo Bonzini, Michael S. Tsirkin, Marcel Apfelbaum,
Eduardo Habkost, David Hildenbrand, Cornelia Huck, Gerd Hoffmann,
Dmitry Fleytman
On 12/21/18 1:36 AM, Greg Kurz wrote:
> PHB hotplug will bring more users for it. Let's define it along with
> the PHB defines from which it is derived for simplicity.
>
> While here fix a misleading comment about manual placement, which was
> abandoned with 30b3bc5aa9f4.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/ppc/spapr.c | 2 --
> include/hw/pci-host/spapr.h | 6 ++++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 621006eaa862..fe3f9829ae6c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3838,8 +3838,6 @@ static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
> * 1TiB 64-bit MMIO windows for each PHB.
> */
> const uint64_t base_buid = 0x800000020000000ULL;
> -#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> - SPAPR_PCI_MEM64_WIN_SIZE - 1)
> int i;
>
> /* Sanity check natural alignments */
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 9d2ec1a410e8..83d5075a6ef3 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -94,11 +94,13 @@ struct sPAPRPHBState {
> ((1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET)
> #define SPAPR_PCI_MEM64_WIN_SIZE 0x10000000000ULL /* 1 TiB */
>
> -/* Without manual configuration, all PCI outbound windows will be
> - * within this range */
> +/* All PCI outbound windows will be within this range */
> #define SPAPR_PCI_BASE (1ULL << 45) /* 32 TiB */
> #define SPAPR_PCI_LIMIT (1ULL << 46) /* 64 TiB */
>
> +#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> + SPAPR_PCI_MEM64_WIN_SIZE - 1)
> +
Which is 32. Good, this is in sync with the IRQ number ranges.
C.
> #define SPAPR_PCI_2_7_MMIO_WIN_SIZE 0xf80000000
> #define SPAPR_PCI_IO_WIN_SIZE 0x10000
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h
2018-12-21 8:03 ` Cédric Le Goater
@ 2018-12-21 9:50 ` Greg Kurz
0 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 9:50 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, qemu-ppc, qemu-s390x, David Gibson,
Alexey Kardashevskiy, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
On Fri, 21 Dec 2018 09:03:49 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> On 12/21/18 1:36 AM, Greg Kurz wrote:
> > PHB hotplug will bring more users for it. Let's define it along with
> > the PHB defines from which it is derived for simplicity.
> >
> > While here fix a misleading comment about manual placement, which was
> > abandoned with 30b3bc5aa9f4.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> > ---
> > hw/ppc/spapr.c | 2 --
> > include/hw/pci-host/spapr.h | 6 ++++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 621006eaa862..fe3f9829ae6c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3838,8 +3838,6 @@ static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
> > * 1TiB 64-bit MMIO windows for each PHB.
> > */
> > const uint64_t base_buid = 0x800000020000000ULL;
> > -#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> > - SPAPR_PCI_MEM64_WIN_SIZE - 1)
> > int i;
> >
> > /* Sanity check natural alignments */
> > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > index 9d2ec1a410e8..83d5075a6ef3 100644
> > --- a/include/hw/pci-host/spapr.h
> > +++ b/include/hw/pci-host/spapr.h
> > @@ -94,11 +94,13 @@ struct sPAPRPHBState {
> > ((1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET)
> > #define SPAPR_PCI_MEM64_WIN_SIZE 0x10000000000ULL /* 1 TiB */
> >
> > -/* Without manual configuration, all PCI outbound windows will be
> > - * within this range */
> > +/* All PCI outbound windows will be within this range */
> > #define SPAPR_PCI_BASE (1ULL << 45) /* 32 TiB */
> > #define SPAPR_PCI_LIMIT (1ULL << 46) /* 64 TiB */
> >
> > +#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> > + SPAPR_PCI_MEM64_WIN_SIZE - 1)
> > +
>
> Which is 32. Good, this is in sync with the IRQ number ranges.
>
> C.
>
Yeah 32 * 4 LSIs fit well in the 0x1200-0x127f range :)
> > #define SPAPR_PCI_2_7_MMIO_WIN_SIZE 0xf80000000
> > #define SPAPR_PCI_IO_WIN_SIZE 0x10000
> >
> >
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h
2018-12-21 0:36 ` [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h Greg Kurz
2018-12-21 8:03 ` Cédric Le Goater
@ 2019-01-03 2:07 ` David Gibson
1 sibling, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-01-03 2:07 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]
On Fri, Dec 21, 2018 at 01:36:53AM +0100, Greg Kurz wrote:
> PHB hotplug will bring more users for it. Let's define it along with
> the PHB defines from which it is derived for simplicity.
>
> While here fix a misleading comment about manual placement, which was
> abandoned with 30b3bc5aa9f4.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Applied to ppc-for-4.0, thanks.
> ---
> hw/ppc/spapr.c | 2 --
> include/hw/pci-host/spapr.h | 6 ++++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 621006eaa862..fe3f9829ae6c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3838,8 +3838,6 @@ static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
> * 1TiB 64-bit MMIO windows for each PHB.
> */
> const uint64_t base_buid = 0x800000020000000ULL;
> -#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> - SPAPR_PCI_MEM64_WIN_SIZE - 1)
> int i;
>
> /* Sanity check natural alignments */
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 9d2ec1a410e8..83d5075a6ef3 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -94,11 +94,13 @@ struct sPAPRPHBState {
> ((1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET)
> #define SPAPR_PCI_MEM64_WIN_SIZE 0x10000000000ULL /* 1 TiB */
>
> -/* Without manual configuration, all PCI outbound windows will be
> - * within this range */
> +/* All PCI outbound windows will be within this range */
> #define SPAPR_PCI_BASE (1ULL << 45) /* 32 TiB */
> #define SPAPR_PCI_LIMIT (1ULL << 46) /* 64 TiB */
>
> +#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> + SPAPR_PCI_MEM64_WIN_SIZE - 1)
> +
> #define SPAPR_PCI_2_7_MMIO_WIN_SIZE 0xf80000000
> #define SPAPR_PCI_IO_WIN_SIZE 0x10000
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 08/15] spapr: create DR connectors for PHBs
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (6 preceding siblings ...)
2018-12-21 0:36 ` [Qemu-devel] [PATCH 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h Greg Kurz
@ 2018-12-21 0:37 ` Greg Kurz
2018-12-21 0:37 ` [Qemu-devel] [PATCH 09/15] spapr: populate PHB DRC entries for root DT node Greg Kurz
` (6 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 13 +++++++++++++
hw/ppc/spapr_drc.c | 17 +++++++++++++++++
include/hw/ppc/spapr_drc.h | 8 ++++++++
3 files changed, 38 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fe3f9829ae6c..280e45037704 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2790,6 +2790,19 @@ static void spapr_machine_init(MachineState *machine)
/* We always have at least the nvram device on VIO */
spapr_create_nvram(spapr);
+ /*
+ * Setup hotplug / dynamic-reconfiguration connectors. top-level
+ * connectors (described in root DT node's "ibm,drc-types" property)
+ * are pre-initialized here. additional child connectors (such as
+ * connectors for a PHBs PCI slots) are added as needed during their
+ * parent's realization.
+ */
+ if (smc->dr_phb_enabled) {
+ for (i = 0; i < SPAPR_MAX_PHBS; i++) {
+ spapr_dr_connector_new(OBJECT(machine), TYPE_SPAPR_DRC_PHB, i);
+ }
+ }
+
/* Set up PCI */
spapr_pci_rtas_init();
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 2edb7d1e9c8c..189ee681062a 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -696,6 +696,15 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
drck->release = spapr_lmb_release;
}
+static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
+{
+ sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+ drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
+ drck->typename = "PHB";
+ drck->drc_name_prefix = "PHB ";
+}
+
static const TypeInfo spapr_dr_connector_info = {
.name = TYPE_SPAPR_DR_CONNECTOR,
.parent = TYPE_DEVICE,
@@ -739,6 +748,13 @@ static const TypeInfo spapr_drc_lmb_info = {
.class_init = spapr_drc_lmb_class_init,
};
+static const TypeInfo spapr_drc_phb_info = {
+ .name = TYPE_SPAPR_DRC_PHB,
+ .parent = TYPE_SPAPR_DRC_LOGICAL,
+ .instance_size = sizeof(sPAPRDRConnector),
+ .class_init = spapr_drc_phb_class_init,
+};
+
/* helper functions for external users */
sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
@@ -1189,6 +1205,7 @@ static void spapr_drc_register_types(void)
type_register_static(&spapr_drc_cpu_info);
type_register_static(&spapr_drc_pci_info);
type_register_static(&spapr_drc_lmb_info);
+ type_register_static(&spapr_drc_phb_info);
spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
rtas_set_indicator);
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index f6ff32e7e2f2..56bba36ad4da 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -70,6 +70,14 @@
#define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
TYPE_SPAPR_DRC_LMB)
+#define TYPE_SPAPR_DRC_PHB "spapr-drc-phb"
+#define SPAPR_DRC_PHB_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHB)
+#define SPAPR_DRC_PHB_CLASS(klass) \
+ OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PHB)
+#define SPAPR_DRC_PHB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+ TYPE_SPAPR_DRC_PHB)
+
/*
* Various hotplug types managed by sPAPRDRConnector
*
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 09/15] spapr: populate PHB DRC entries for root DT node
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (7 preceding siblings ...)
2018-12-21 0:37 ` [Qemu-devel] [PATCH 08/15] spapr: create DR connectors for PHBs Greg Kurz
@ 2018-12-21 0:37 ` Greg Kurz
2018-12-21 0:37 ` [Qemu-devel] [PATCH 10/15] spapr_events: add support for phb hotplug events Greg Kurz
` (5 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
This add entries to the root OF node to advertise our PHBs as being
DR-capable in accordance with PAPR specification.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 280e45037704..4f9d11b14666 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1350,6 +1350,14 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
exit(1);
}
+ if (smc->dr_phb_enabled) {
+ ret = spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_PHB);
+ if (ret < 0) {
+ error_report("Couldn't set up PHB DR device tree properties");
+ exit(1);
+ }
+ }
+
return fdt;
}
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 10/15] spapr_events: add support for phb hotplug events
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (8 preceding siblings ...)
2018-12-21 0:37 ` [Qemu-devel] [PATCH 09/15] spapr: populate PHB DRC entries for root DT node Greg Kurz
@ 2018-12-21 0:37 ` Greg Kurz
2018-12-21 0:38 ` [Qemu-devel] [PATCH 11/15] qdev: pass an Object * to qbus_set_hotplug_handler() Greg Kurz
` (4 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Extend the existing EPOW event format we use for PCI
devices to emit PHB plug/unplug events.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr_events.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 32719a1b72d0..baf30c55710a 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -526,6 +526,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
case SPAPR_DR_CONNECTOR_TYPE_CPU:
hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_CPU;
break;
+ case SPAPR_DR_CONNECTOR_TYPE_PHB:
+ hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_PHB;
+ break;
default:
/* we shouldn't be signaling hotplug events for resources
* that don't support them
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 11/15] qdev: pass an Object * to qbus_set_hotplug_handler()
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (9 preceding siblings ...)
2018-12-21 0:37 ` [Qemu-devel] [PATCH 10/15] spapr_events: add support for phb hotplug events Greg Kurz
@ 2018-12-21 0:38 ` Greg Kurz
2018-12-21 0:38 ` [Qemu-devel] [PATCH 12/15] spapr_pci: provide node start offset via spapr_populate_pci_dt() Greg Kurz
` (3 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Certain devices types, like memory/CPU, are now being handled using a
hotplug interface provided by a top-level MachineClass. Hotpluggable
host bridges are another such device where it makes sense to use a
machine-level hotplug handler. However, unlike those devices,
host-bridges have a parent bus (the main system bus), and devices with
a parent bus use a different mechanism for registering their hotplug
handlers: qbus_set_hotplug_handler(). This interface currently expects
a handler to be a subclass of DeviceClass, but this is not the case
for MachineClass, which derives directly from ObjectClass.
Internally, the interface only requires an ObjectClass, so expose that
in qbus_set_hotplug_handler().
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/acpi/piix4.c | 2 +-
hw/char/virtio-serial-bus.c | 2 +-
hw/core/bus.c | 11 ++---------
hw/pci/pcie.c | 2 +-
hw/pci/shpc.c | 2 +-
hw/ppc/spapr_pci.c | 2 +-
hw/s390x/css-bridge.c | 2 +-
hw/s390x/s390-pci-bus.c | 6 +++---
hw/scsi/virtio-scsi.c | 2 +-
hw/scsi/vmw_pvscsi.c | 2 +-
hw/usb/dev-smartcard-reader.c | 2 +-
include/hw/qdev-core.h | 3 +--
12 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index e330f24c71e0..d72aba555c5c 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -441,7 +441,7 @@ static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque)
/* pci_bus cannot outlive PIIX4PMState, because /machine keeps it alive
* and it's not hot-unpluggable */
- qbus_set_hotplug_handler(BUS(pci_bus), DEVICE(s), &error_abort);
+ qbus_set_hotplug_handler(BUS(pci_bus), OBJECT(s), &error_abort);
}
static void piix4_pm_machine_ready(Notifier *n, void *opaque)
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 04e3ebe3526a..e4310c78f2dc 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1052,7 +1052,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
/* Spawn a new virtio-serial bus on which the ports will ride as devices */
qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
dev, vdev->bus_name);
- qbus_set_hotplug_handler(BUS(&vser->bus), DEVICE(vser), errp);
+ qbus_set_hotplug_handler(BUS(&vser->bus), OBJECT(vser), errp);
vser->bus.vser = vser;
QTAILQ_INIT(&vser->ports);
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f244864c..e09843f6abea 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -22,22 +22,15 @@
#include "hw/qdev.h"
#include "qapi/error.h"
-static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
- Error **errp)
+void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
{
-
object_property_set_link(OBJECT(bus), OBJECT(handler),
QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
}
-void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp)
-{
- qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
-}
-
void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
{
- qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
+ qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
}
int qbus_walk_children(BusState *bus,
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6c91bd44a0a5..d74b121e8b6e 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -444,7 +444,7 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
dev->exp.hpev_notified = false;
qbus_set_hotplug_handler(BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev))),
- DEVICE(dev), NULL);
+ OBJECT(dev), NULL);
}
void pcie_cap_slot_reset(PCIDevice *dev)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 96a43d2f709a..377aedeb27be 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -639,7 +639,7 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar,
shpc_cap_update_dword(d);
memory_region_add_subregion(bar, offset, &shpc->mmio);
- qbus_set_hotplug_handler(BUS(sec_bus), DEVICE(d), NULL);
+ qbus_set_hotplug_handler(BUS(sec_bus), OBJECT(d), NULL);
d->cap_present |= QEMU_PCI_CAP_SHPC;
return 0;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index b772b72d6a48..292dd95cbef9 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1723,7 +1723,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
&sphb->memspace, &sphb->iospace,
PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
phb->bus = bus;
- qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
+ qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb), NULL);
/*
* Initialize PHB address space.
diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index 1bd6c8b45860..7573c40badbd 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -108,7 +108,7 @@ VirtualCssBus *virtual_css_bus_init(void)
cbus = VIRTUAL_CSS_BUS(bus);
/* Enable hotplugging */
- qbus_set_hotplug_handler(bus, dev, &error_abort);
+ qbus_set_hotplug_handler(bus, OBJECT(dev), &error_abort);
css_register_io_adapters(CSS_IO_ADAPTER_VIRTIO, true, false,
0, &error_abort);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index f7458445c0f5..01eea2752912 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -708,7 +708,7 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
pci_setup_iommu(b, s390_pci_dma_iommu, s);
bus = BUS(b);
- qbus_set_hotplug_handler(bus, dev, &local_err);
+ qbus_set_hotplug_handler(bus, OBJECT(dev), &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -716,7 +716,7 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
phb->bus = b;
s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, dev, NULL));
- qbus_set_hotplug_handler(BUS(s->bus), dev, &local_err);
+ qbus_set_hotplug_handler(BUS(s->bus), OBJECT(dev), &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -844,7 +844,7 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
bus = BUS(&pb->sec_bus);
- qbus_set_hotplug_handler(bus, DEVICE(s), errp);
+ qbus_set_hotplug_handler(bus, OBJECT(s), errp);
if (dev->hotplugged) {
pci_default_write_config(pdev, PCI_PRIMARY_BUS, s->bus_no, 1);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3aa99717e235..9cf6290c2986 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -893,7 +893,7 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
scsi_bus_new(&s->bus, sizeof(s->bus), dev,
&virtio_scsi_scsi_info, vdev->bus_name);
/* override default SCSI bus hotplug-handler, with virtio-scsi's one */
- qbus_set_hotplug_handler(BUS(&s->bus), dev, &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
virtio_scsi_dataplane_setup(s, errp);
}
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index a3a019e30a74..584b4be07e79 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1142,7 +1142,7 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(pci_dev),
&pvscsi_scsi_info, NULL);
/* override default SCSI bus hotplug-handler, with pvscsi's one */
- qbus_set_hotplug_handler(BUS(&s->bus), DEVICE(s), &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(s), &error_abort);
pvscsi_reset_state(s);
}
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 8f716fc165a3..6b0137bb7699 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1322,7 +1322,7 @@ static void ccid_realize(USBDevice *dev, Error **errp)
usb_desc_init(dev);
qbus_create_inplace(&s->bus, sizeof(s->bus), TYPE_CCID_BUS, DEVICE(dev),
NULL);
- qbus_set_hotplug_handler(BUS(&s->bus), DEVICE(dev), &error_abort);
+ qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
s->bulk = usb_ep_get(dev, USB_TOKEN_IN, CCID_BULK_IN_EP);
s->card = NULL;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 92851e55dfa9..3fa33901dfcf 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -422,8 +422,7 @@ char *qdev_get_dev_path(DeviceState *dev);
GSList *qdev_build_hotpluggable_device_list(Object *peripheral);
-void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
- Error **errp);
+void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp);
void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 12/15] spapr_pci: provide node start offset via spapr_populate_pci_dt()
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (10 preceding siblings ...)
2018-12-21 0:38 ` [Qemu-devel] [PATCH 11/15] qdev: pass an Object * to qbus_set_hotplug_handler() Greg Kurz
@ 2018-12-21 0:38 ` Greg Kurz
2018-12-21 0:38 ` [Qemu-devel] [PATCH 13/15] spapr_pci: add ibm, my-drc-index property for PHB hotplug Greg Kurz
` (2 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
PHB hotplug re-uses PHB device tree generation code and passes
it to a guest via RTAS. Doing this requires knowledge of where
exactly in the device tree the node describing the PHB begins.
Provide this via a new optional pointer that can be used to
store the PHB node's start offset.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 2 +-
hw/ppc/spapr_pci.c | 5 ++++-
include/hw/pci-host/spapr.h | 2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4f9d11b14666..5c405a5fafca 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1297,7 +1297,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
QLIST_FOREACH(phb, &spapr->phbs, list) {
ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt,
- spapr->irq->nr_msis);
+ spapr->irq->nr_msis, NULL);
if (ret < 0) {
error_report("couldn't setup PCI devices in fdt");
exit(1);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 292dd95cbef9..5e7b40a8c910 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2140,7 +2140,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
}
int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
- uint32_t nr_msis)
+ uint32_t nr_msis, int *node_offset)
{
int bus_off, i, j, ret;
gchar *nodename;
@@ -2195,6 +2195,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
_FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
g_free(nodename);
+ if (node_offset) {
+ *node_offset = bus_off;
+ }
/* Write PHB properties */
_FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 83d5075a6ef3..6ef8f141ea92 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -114,7 +114,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
}
int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
- uint32_t nr_msis);
+ uint32_t nr_msis, int *node_offset);
void spapr_pci_rtas_init(void);
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 13/15] spapr_pci: add ibm, my-drc-index property for PHB hotplug
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (11 preceding siblings ...)
2018-12-21 0:38 ` [Qemu-devel] [PATCH 12/15] spapr_pci: provide node start offset via spapr_populate_pci_dt() Greg Kurz
@ 2018-12-21 0:38 ` Greg Kurz
2018-12-21 6:35 ` [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node Greg Kurz
2018-12-21 6:36 ` [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug Greg Kurz
14 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 0:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
This is needed to denote a boot-time PHB as being hot-pluggable.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr_pci.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5e7b40a8c910..688cca83ef2f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2190,6 +2190,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
sPAPRTCETable *tcet;
PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
sPAPRFDT s_fdt;
+ sPAPRDRConnector *drc;
/* Start populating the FDT */
nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
@@ -2256,6 +2257,14 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
tcet->liobn, tcet->bus_offset,
tcet->nb_table << tcet->page_shift);
+ drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, phb->index);
+ if (drc) {
+ uint32_t drc_index = cpu_to_be32(spapr_drc_index(drc));
+
+ _FDT(fdt_setprop(fdt, bus_off, "ibm,my-drc-index", &drc_index,
+ sizeof(drc_index)));
+ }
+
/* Walk the bridges and program the bus numbers*/
spapr_phb_pci_enumerate(phb);
_FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (12 preceding siblings ...)
2018-12-21 0:38 ` [Qemu-devel] [PATCH 13/15] spapr_pci: add ibm, my-drc-index property for PHB hotplug Greg Kurz
@ 2018-12-21 6:35 ` Greg Kurz
2018-12-21 8:12 ` Cédric Le Goater
2018-12-21 6:36 ` [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug Greg Kurz
14 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
This will be needed by PHB hotplug in order to access the phandle property.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/intc/spapr_xive.c | 9 +++++++--
hw/intc/xics_spapr.c | 9 ++++++++-
hw/ppc/spapr_irq.c | 3 +++
include/hw/ppc/spapr_irq.h | 1 +
include/hw/ppc/spapr_xive.h | 1 +
include/hw/ppc/xics.h | 1 +
6 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 87424de26c1c..0540aac88d2a 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -1410,6 +1410,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
spapr_register_hypercall(H_INT_RESET, h_int_reset);
}
+gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr)
+{
+ return g_strdup_printf("interrupt-controller@%" PRIx64,
+ spapr->xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
+}
+
void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle)
{
@@ -1444,8 +1450,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
timas[3] = cpu_to_be64(1ull << TM_SHIFT);
- nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
- xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
+ nodename = spapr_xive_get_nodename(spapr);
_FDT(node = fdt_add_subnode(fdt, 0, nodename));
g_free(nodename);
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index f67d3c80bf3a..75d40daf518d 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -244,6 +244,13 @@ void xics_spapr_init(sPAPRMachineState *spapr)
spapr_register_hypercall(H_IPOLL, h_ipoll);
}
+#define NODENAME "interrupt-controller"
+
+gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr)
+{
+ return g_strdup(NODENAME);
+}
+
void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle)
{
@@ -252,7 +259,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
};
int node;
- _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
+ _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
_FDT(fdt_setprop_string(fdt, node, "device_type",
"PowerPC-External-Interrupt-Presentation"));
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 0999a2b2d69c..703c3a3c20d5 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -223,6 +223,7 @@ sPAPRIrq spapr_irq_xics = {
.qirq = spapr_qirq_xics,
.print_info = spapr_irq_print_info_xics,
.dt_populate = spapr_dt_xics,
+ .get_nodename = spapr_xics_get_nodename,
.cpu_intc_create = spapr_irq_cpu_intc_create_xics,
.post_load = spapr_irq_post_load_xics,
};
@@ -349,6 +350,7 @@ sPAPRIrq spapr_irq_xive = {
.qirq = spapr_qirq_xive,
.print_info = spapr_irq_print_info_xive,
.dt_populate = spapr_dt_xive,
+ .get_nodename = spapr_xive_get_nodename,
.cpu_intc_create = spapr_irq_cpu_intc_create_xive,
.post_load = spapr_irq_post_load_xive,
.reset = spapr_irq_reset_xive,
@@ -462,6 +464,7 @@ sPAPRIrq spapr_irq_xics_legacy = {
.qirq = spapr_qirq_xics,
.print_info = spapr_irq_print_info_xics,
.dt_populate = spapr_dt_xics,
+ .get_nodename = spapr_xics_get_nodename,
.cpu_intc_create = spapr_irq_cpu_intc_create_xics,
.post_load = spapr_irq_post_load_xics,
};
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index b34d5a00381b..59a1cf8bbc1d 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -42,6 +42,7 @@ typedef struct sPAPRIrq {
void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
void (*dt_populate)(sPAPRMachineState *spapr, uint32_t nr_servers,
void *fdt, uint32_t phandle);
+ gchar *(*get_nodename)(sPAPRMachineState *spapr);
Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
Error **errp);
int (*post_load)(sPAPRMachineState *spapr, int version_id);
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 728735dbcfbe..d280310ed587 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -47,6 +47,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
void spapr_xive_hcall_init(sPAPRMachineState *spapr);
void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle);
+gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr);
void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
#endif /* PPC_SPAPR_XIVE_H */
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 14afda198cdb..eafb6428787f 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -204,6 +204,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle);
+gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr);
int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
void xics_spapr_init(sPAPRMachineState *spapr);
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node
2018-12-21 6:35 ` [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node Greg Kurz
@ 2018-12-21 8:12 ` Cédric Le Goater
2018-12-21 9:53 ` Greg Kurz
0 siblings, 1 reply; 36+ messages in thread
From: Cédric Le Goater @ 2018-12-21 8:12 UTC (permalink / raw)
To: Greg Kurz, qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Michael Roth, Paolo Bonzini, Michael S. Tsirkin, Marcel Apfelbaum,
Eduardo Habkost, David Hildenbrand, Cornelia Huck, Gerd Hoffmann,
Dmitry Fleytman
On 12/21/18 7:35 AM, Greg Kurz wrote:
> This will be needed by PHB hotplug in order to access the phandle property.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
I would have used the prefix 'spapr_dt_', but it's minor.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> hw/intc/spapr_xive.c | 9 +++++++--
> hw/intc/xics_spapr.c | 9 ++++++++-
> hw/ppc/spapr_irq.c | 3 +++
> include/hw/ppc/spapr_irq.h | 1 +
> include/hw/ppc/spapr_xive.h | 1 +
> include/hw/ppc/xics.h | 1 +
> 6 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 87424de26c1c..0540aac88d2a 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -1410,6 +1410,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
> spapr_register_hypercall(H_INT_RESET, h_int_reset);
> }
>
> +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr)
> +{
> + return g_strdup_printf("interrupt-controller@%" PRIx64,
> + spapr->xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> +}
> +
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle)
> {
> @@ -1444,8 +1450,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
> timas[3] = cpu_to_be64(1ull << TM_SHIFT);
>
> - nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> - xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> + nodename = spapr_xive_get_nodename(spapr);
> _FDT(node = fdt_add_subnode(fdt, 0, nodename));
> g_free(nodename);
>
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index f67d3c80bf3a..75d40daf518d 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -244,6 +244,13 @@ void xics_spapr_init(sPAPRMachineState *spapr)
> spapr_register_hypercall(H_IPOLL, h_ipoll);
> }
>
> +#define NODENAME "interrupt-controller"
> +
> +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr)
> +{
> + return g_strdup(NODENAME);
> +}
> +
> void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle)
> {
> @@ -252,7 +259,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> };
> int node;
>
> - _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> + _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
>
> _FDT(fdt_setprop_string(fdt, node, "device_type",
> "PowerPC-External-Interrupt-Presentation"));
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 0999a2b2d69c..703c3a3c20d5 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -223,6 +223,7 @@ sPAPRIrq spapr_irq_xics = {
> .qirq = spapr_qirq_xics,
> .print_info = spapr_irq_print_info_xics,
> .dt_populate = spapr_dt_xics,
> + .get_nodename = spapr_xics_get_nodename,
> .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> .post_load = spapr_irq_post_load_xics,
> };
> @@ -349,6 +350,7 @@ sPAPRIrq spapr_irq_xive = {
> .qirq = spapr_qirq_xive,
> .print_info = spapr_irq_print_info_xive,
> .dt_populate = spapr_dt_xive,
> + .get_nodename = spapr_xive_get_nodename,
> .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> .post_load = spapr_irq_post_load_xive,
> .reset = spapr_irq_reset_xive,
> @@ -462,6 +464,7 @@ sPAPRIrq spapr_irq_xics_legacy = {
> .qirq = spapr_qirq_xics,
> .print_info = spapr_irq_print_info_xics,
> .dt_populate = spapr_dt_xics,
> + .get_nodename = spapr_xics_get_nodename,
> .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> .post_load = spapr_irq_post_load_xics,
> };
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index b34d5a00381b..59a1cf8bbc1d 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -42,6 +42,7 @@ typedef struct sPAPRIrq {
> void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
> void (*dt_populate)(sPAPRMachineState *spapr, uint32_t nr_servers,
> void *fdt, uint32_t phandle);
> + gchar *(*get_nodename)(sPAPRMachineState *spapr);
> Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
> Error **errp);
> int (*post_load)(sPAPRMachineState *spapr, int version_id);
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 728735dbcfbe..d280310ed587 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -47,6 +47,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
> void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr);
> void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>
> #endif /* PPC_SPAPR_XIVE_H */
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 14afda198cdb..eafb6428787f 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -204,6 +204,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
>
> void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr);
> int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> void xics_spapr_init(sPAPRMachineState *spapr);
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node
2018-12-21 8:12 ` Cédric Le Goater
@ 2018-12-21 9:53 ` Greg Kurz
2019-01-03 2:13 ` David Gibson
0 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 9:53 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, qemu-ppc, qemu-s390x, David Gibson,
Alexey Kardashevskiy, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
On Fri, 21 Dec 2018 09:12:24 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> On 12/21/18 7:35 AM, Greg Kurz wrote:
> > This will be needed by PHB hotplug in order to access the phandle property.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> I would have used the prefix 'spapr_dt_', but it's minor.
>
I guess there might be a v2. I'll do the change.
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
Thanks!
> Thanks,
>
> C.
>
> > ---
> > hw/intc/spapr_xive.c | 9 +++++++--
> > hw/intc/xics_spapr.c | 9 ++++++++-
> > hw/ppc/spapr_irq.c | 3 +++
> > include/hw/ppc/spapr_irq.h | 1 +
> > include/hw/ppc/spapr_xive.h | 1 +
> > include/hw/ppc/xics.h | 1 +
> > 6 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> > index 87424de26c1c..0540aac88d2a 100644
> > --- a/hw/intc/spapr_xive.c
> > +++ b/hw/intc/spapr_xive.c
> > @@ -1410,6 +1410,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
> > spapr_register_hypercall(H_INT_RESET, h_int_reset);
> > }
> >
> > +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr)
> > +{
> > + return g_strdup_printf("interrupt-controller@%" PRIx64,
> > + spapr->xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> > +}
> > +
> > void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > uint32_t phandle)
> > {
> > @@ -1444,8 +1450,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
> > timas[3] = cpu_to_be64(1ull << TM_SHIFT);
> >
> > - nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> > - xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> > + nodename = spapr_xive_get_nodename(spapr);
> > _FDT(node = fdt_add_subnode(fdt, 0, nodename));
> > g_free(nodename);
> >
> > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> > index f67d3c80bf3a..75d40daf518d 100644
> > --- a/hw/intc/xics_spapr.c
> > +++ b/hw/intc/xics_spapr.c
> > @@ -244,6 +244,13 @@ void xics_spapr_init(sPAPRMachineState *spapr)
> > spapr_register_hypercall(H_IPOLL, h_ipoll);
> > }
> >
> > +#define NODENAME "interrupt-controller"
> > +
> > +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr)
> > +{
> > + return g_strdup(NODENAME);
> > +}
> > +
> > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > uint32_t phandle)
> > {
> > @@ -252,7 +259,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > };
> > int node;
> >
> > - _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> > + _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
> >
> > _FDT(fdt_setprop_string(fdt, node, "device_type",
> > "PowerPC-External-Interrupt-Presentation"));
> > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > index 0999a2b2d69c..703c3a3c20d5 100644
> > --- a/hw/ppc/spapr_irq.c
> > +++ b/hw/ppc/spapr_irq.c
> > @@ -223,6 +223,7 @@ sPAPRIrq spapr_irq_xics = {
> > .qirq = spapr_qirq_xics,
> > .print_info = spapr_irq_print_info_xics,
> > .dt_populate = spapr_dt_xics,
> > + .get_nodename = spapr_xics_get_nodename,
> > .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> > .post_load = spapr_irq_post_load_xics,
> > };
> > @@ -349,6 +350,7 @@ sPAPRIrq spapr_irq_xive = {
> > .qirq = spapr_qirq_xive,
> > .print_info = spapr_irq_print_info_xive,
> > .dt_populate = spapr_dt_xive,
> > + .get_nodename = spapr_xive_get_nodename,
> > .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> > .post_load = spapr_irq_post_load_xive,
> > .reset = spapr_irq_reset_xive,
> > @@ -462,6 +464,7 @@ sPAPRIrq spapr_irq_xics_legacy = {
> > .qirq = spapr_qirq_xics,
> > .print_info = spapr_irq_print_info_xics,
> > .dt_populate = spapr_dt_xics,
> > + .get_nodename = spapr_xics_get_nodename,
> > .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> > .post_load = spapr_irq_post_load_xics,
> > };
> > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> > index b34d5a00381b..59a1cf8bbc1d 100644
> > --- a/include/hw/ppc/spapr_irq.h
> > +++ b/include/hw/ppc/spapr_irq.h
> > @@ -42,6 +42,7 @@ typedef struct sPAPRIrq {
> > void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
> > void (*dt_populate)(sPAPRMachineState *spapr, uint32_t nr_servers,
> > void *fdt, uint32_t phandle);
> > + gchar *(*get_nodename)(sPAPRMachineState *spapr);
> > Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
> > Error **errp);
> > int (*post_load)(sPAPRMachineState *spapr, int version_id);
> > diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> > index 728735dbcfbe..d280310ed587 100644
> > --- a/include/hw/ppc/spapr_xive.h
> > +++ b/include/hw/ppc/spapr_xive.h
> > @@ -47,6 +47,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
> > void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> > void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > uint32_t phandle);
> > +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr);
> > void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
> >
> > #endif /* PPC_SPAPR_XIVE_H */
> > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> > index 14afda198cdb..eafb6428787f 100644
> > --- a/include/hw/ppc/xics.h
> > +++ b/include/hw/ppc/xics.h
> > @@ -204,6 +204,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
> >
> > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > uint32_t phandle);
> > +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr);
> > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> > void xics_spapr_init(sPAPRMachineState *spapr);
> >
> >
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node
2018-12-21 9:53 ` Greg Kurz
@ 2019-01-03 2:13 ` David Gibson
0 siblings, 0 replies; 36+ messages in thread
From: David Gibson @ 2019-01-03 2:13 UTC (permalink / raw)
To: Greg Kurz
Cc: Cédric Le Goater, qemu-devel, qemu-ppc, qemu-s390x,
Alexey Kardashevskiy, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 7038 bytes --]
On Fri, Dec 21, 2018 at 10:53:13AM +0100, Greg Kurz wrote:
> On Fri, 21 Dec 2018 09:12:24 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
>
> > On 12/21/18 7:35 AM, Greg Kurz wrote:
> > > This will be needed by PHB hotplug in order to access the phandle property.
> > >
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> >
> > I would have used the prefix 'spapr_dt_', but it's minor.
> >
>
> I guess there might be a v2. I'll do the change.
Actually, don't. I'm standardizing on spapr_dt_* for functions which
generate pieces of the device tree. This helper is related, but
doesn't really fit that pattern.
>
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> >
>
> Thanks!
>
> > Thanks,
> >
> > C.
> >
> > > ---
> > > hw/intc/spapr_xive.c | 9 +++++++--
> > > hw/intc/xics_spapr.c | 9 ++++++++-
> > > hw/ppc/spapr_irq.c | 3 +++
> > > include/hw/ppc/spapr_irq.h | 1 +
> > > include/hw/ppc/spapr_xive.h | 1 +
> > > include/hw/ppc/xics.h | 1 +
> > > 6 files changed, 21 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> > > index 87424de26c1c..0540aac88d2a 100644
> > > --- a/hw/intc/spapr_xive.c
> > > +++ b/hw/intc/spapr_xive.c
> > > @@ -1410,6 +1410,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
> > > spapr_register_hypercall(H_INT_RESET, h_int_reset);
> > > }
> > >
> > > +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr)
> > > +{
> > > + return g_strdup_printf("interrupt-controller@%" PRIx64,
> > > + spapr->xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> > > +}
> > > +
> > > void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > > uint32_t phandle)
> > > {
> > > @@ -1444,8 +1450,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > > XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
> > > timas[3] = cpu_to_be64(1ull << TM_SHIFT);
> > >
> > > - nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> > > - xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> > > + nodename = spapr_xive_get_nodename(spapr);
> > > _FDT(node = fdt_add_subnode(fdt, 0, nodename));
> > > g_free(nodename);
> > >
> > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> > > index f67d3c80bf3a..75d40daf518d 100644
> > > --- a/hw/intc/xics_spapr.c
> > > +++ b/hw/intc/xics_spapr.c
> > > @@ -244,6 +244,13 @@ void xics_spapr_init(sPAPRMachineState *spapr)
> > > spapr_register_hypercall(H_IPOLL, h_ipoll);
> > > }
> > >
> > > +#define NODENAME "interrupt-controller"
> > > +
> > > +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr)
> > > +{
> > > + return g_strdup(NODENAME);
> > > +}
> > > +
> > > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > > uint32_t phandle)
> > > {
> > > @@ -252,7 +259,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > > };
> > > int node;
> > >
> > > - _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> > > + _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
> > >
> > > _FDT(fdt_setprop_string(fdt, node, "device_type",
> > > "PowerPC-External-Interrupt-Presentation"));
> > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > > index 0999a2b2d69c..703c3a3c20d5 100644
> > > --- a/hw/ppc/spapr_irq.c
> > > +++ b/hw/ppc/spapr_irq.c
> > > @@ -223,6 +223,7 @@ sPAPRIrq spapr_irq_xics = {
> > > .qirq = spapr_qirq_xics,
> > > .print_info = spapr_irq_print_info_xics,
> > > .dt_populate = spapr_dt_xics,
> > > + .get_nodename = spapr_xics_get_nodename,
> > > .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> > > .post_load = spapr_irq_post_load_xics,
> > > };
> > > @@ -349,6 +350,7 @@ sPAPRIrq spapr_irq_xive = {
> > > .qirq = spapr_qirq_xive,
> > > .print_info = spapr_irq_print_info_xive,
> > > .dt_populate = spapr_dt_xive,
> > > + .get_nodename = spapr_xive_get_nodename,
> > > .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> > > .post_load = spapr_irq_post_load_xive,
> > > .reset = spapr_irq_reset_xive,
> > > @@ -462,6 +464,7 @@ sPAPRIrq spapr_irq_xics_legacy = {
> > > .qirq = spapr_qirq_xics,
> > > .print_info = spapr_irq_print_info_xics,
> > > .dt_populate = spapr_dt_xics,
> > > + .get_nodename = spapr_xics_get_nodename,
> > > .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> > > .post_load = spapr_irq_post_load_xics,
> > > };
> > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> > > index b34d5a00381b..59a1cf8bbc1d 100644
> > > --- a/include/hw/ppc/spapr_irq.h
> > > +++ b/include/hw/ppc/spapr_irq.h
> > > @@ -42,6 +42,7 @@ typedef struct sPAPRIrq {
> > > void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
> > > void (*dt_populate)(sPAPRMachineState *spapr, uint32_t nr_servers,
> > > void *fdt, uint32_t phandle);
> > > + gchar *(*get_nodename)(sPAPRMachineState *spapr);
> > > Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
> > > Error **errp);
> > > int (*post_load)(sPAPRMachineState *spapr, int version_id);
> > > diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> > > index 728735dbcfbe..d280310ed587 100644
> > > --- a/include/hw/ppc/spapr_xive.h
> > > +++ b/include/hw/ppc/spapr_xive.h
> > > @@ -47,6 +47,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
> > > void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> > > void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > > uint32_t phandle);
> > > +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr);
> > > void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
> > >
> > > #endif /* PPC_SPAPR_XIVE_H */
> > > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> > > index 14afda198cdb..eafb6428787f 100644
> > > --- a/include/hw/ppc/xics.h
> > > +++ b/include/hw/ppc/xics.h
> > > @@ -204,6 +204,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
> > >
> > > void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> > > uint32_t phandle);
> > > +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr);
> > > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> > > void xics_spapr_init(sPAPRMachineState *spapr);
> > >
> > >
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug
2018-12-21 0:34 [Qemu-devel] [PATCH 00/15] spapr: Add support for PHB hotplug Greg Kurz
` (13 preceding siblings ...)
2018-12-21 6:35 ` [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node Greg Kurz
@ 2018-12-21 6:36 ` Greg Kurz
2019-01-03 2:17 ` David Gibson
14 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2018-12-21 6:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, qemu-s390x, David Gibson, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
From: Michael Roth <mdroth@linux.vnet.ibm.com>
Hotplugging PHBs is a machine-level operation, but PHBs reside on the
main system bus, so we register spapr machine as the handler for the
main system bus.
We re-get the phandle of the interrupt controller systematically for
simplicity.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_drc.c | 1
hw/ppc/spapr_pci.c | 16 -----
include/hw/ppc/spapr.h | 1
4 files changed, 149 insertions(+), 16 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5c405a5fafca..065c9f19700e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2923,6 +2923,10 @@ static void spapr_machine_init(MachineState *machine)
register_savevm_live(NULL, "spapr/htab", -1, 1,
&savevm_htab_handlers, spapr);
+ if (smc->dr_phb_enabled) {
+ qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), NULL);
+ }
+
qemu_register_boot_set(spapr_boot_set, spapr);
if (kvm_enabled()) {
@@ -3716,6 +3720,135 @@ out:
error_propagate(errp, local_err);
}
+static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+ sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+ const unsigned windows_supported = spapr_phb_windows_supported(sphb);
+
+ if (sphb->index == (uint32_t)-1) {
+ error_setg(errp, "\"index\" for PAPR PHB is mandatory");
+ return;
+ }
+
+ /*
+ * This will check that sphb->index doesn't exceed the maximum number of
+ * PHBs for the current machine type.
+ */
+ smc->phb_placement(spapr, sphb->index,
+ &sphb->buid, &sphb->io_win_addr,
+ &sphb->mem_win_addr, &sphb->mem64_win_addr,
+ windows_supported, sphb->dma_liobn, errp);
+}
+
+static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+ sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
+ void *fdt = NULL;
+ int fdt_start_offset;
+ int fdt_size;
+ Error *local_err = NULL;
+ sPAPRDRConnector *drc;
+ int ret;
+ bool hotplugged = spapr_drc_hotplugged(dev);
+ int offset, phandle = 0;
+ gchar *nodename = NULL;
+
+ if (!smc->dr_phb_enabled) {
+ return;
+ }
+
+ drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
+ /* hotplug hooks should check it's enabled before getting this far */
+ assert(drc);
+
+ if (hotplugged) {
+ if (spapr->fdt_blob) {
+ /*
+ * SLOF might have pushed an updated FDT with new phandle values.
+ * Re-get the one of our interrupt controller.
+ */
+ nodename = spapr->irq->get_nodename(spapr);
+
+ offset = fdt_subnode_offset(spapr->fdt_blob, 0, nodename);
+ if (offset < 0) {
+ error_setg(errp, "Can't find node \"%s\": %s",
+ nodename, fdt_strerror(offset));
+ goto out;
+ }
+
+ phandle = fdt_get_phandle(spapr->fdt_blob, offset);
+ if (phandle < 0) {
+ error_setg(errp, "Can't get phandle of node \"%s\": %s",
+ nodename, fdt_strerror(offset));
+ goto out;
+ }
+ }
+ DEVICE_GET_CLASS(dev)->reset(dev);
+ }
+
+ /* For cold-plugged at initial boot and fallback for hotplug */
+ if (!phandle) {
+ phandle = PHANDLE_XICP;
+ }
+
+ fdt = create_device_tree(&fdt_size);
+ ret = spapr_populate_pci_dt(sphb, phandle, fdt, spapr->irq->nr_msis,
+ &fdt_start_offset);
+ if (ret < 0) {
+ error_setg(&local_err, "unable to create FDT for %sPHB",
+ dev->hotplugged ? "hotplugged " : "");
+ goto out;
+ }
+
+ if (hotplugged) {
+ /* generally SLOF creates these, for hotplug it's up to QEMU */
+ _FDT(fdt_setprop_string(fdt, fdt_start_offset, "name", "pci"));
+ }
+
+ spapr_drc_attach(drc, DEVICE(dev), fdt, fdt_start_offset, &local_err);
+
+out:
+ g_free(nodename);
+
+ if (local_err) {
+ error_propagate(errp, local_err);
+ g_free(fdt);
+ return;
+ }
+
+ if (hotplugged) {
+ spapr_hotplug_req_add_by_index(drc);
+ } else if (drc) {
+ spapr_drc_reset(drc);
+ }
+}
+
+void spapr_phb_release(DeviceState *dev)
+{
+ object_unparent(OBJECT(dev));
+}
+
+static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
+ sPAPRDRConnector *drc;
+
+ drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
+ assert(drc);
+
+ if (!spapr_drc_unplug_requested(drc)) {
+ spapr_drc_detach(drc);
+ spapr_hotplug_req_remove_by_index(drc);
+ }
+}
+
static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -3723,6 +3856,8 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
spapr_memory_plug(hotplug_dev, dev, errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
spapr_core_plug(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+ spapr_phb_plug(hotplug_dev, dev, errp);
}
}
@@ -3741,6 +3876,7 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
{
sPAPRMachineState *sms = SPAPR_MACHINE(OBJECT(hotplug_dev));
MachineClass *mc = MACHINE_GET_CLASS(sms);
+ sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
@@ -3760,6 +3896,12 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
return;
}
spapr_core_unplug_request(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+ if (!smc->dr_phb_enabled) {
+ error_setg(errp, "PHB hot unplug not supported on this machine");
+ return;
+ }
+ spapr_phb_unplug_request(hotplug_dev, dev, errp);
}
}
@@ -3770,6 +3912,8 @@ static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev,
spapr_memory_pre_plug(hotplug_dev, dev, errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
spapr_core_pre_plug(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+ spapr_phb_pre_plug(hotplug_dev, dev, errp);
}
}
@@ -3777,7 +3921,8 @@ static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine,
DeviceState *dev)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
- object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
+ object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE) ||
+ object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
return HOTPLUG_HANDLER(machine);
}
return NULL;
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 189ee681062a..7a2676716364 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -703,6 +703,7 @@ static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
drck->typename = "PHB";
drck->drc_name_prefix = "PHB ";
+ drck->release = spapr_phb_release;
}
static const TypeInfo spapr_dr_connector_info = {
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 688cca83ef2f..5bc912aa0028 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1627,21 +1627,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
return;
}
- if (sphb->index != (uint32_t)-1) {
- Error *local_err = NULL;
-
- smc->phb_placement(spapr, sphb->index,
- &sphb->buid, &sphb->io_win_addr,
- &sphb->mem_win_addr, &sphb->mem64_win_addr,
- windows_supported, sphb->dma_liobn, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- } else {
- error_setg(errp, "\"index\" for PAPR PHB is mandatory");
- return;
- }
+ assert(sphb->index != (uint32_t)-1); /* checked in spapr_phb_pre_plug() */
if (sphb->mem64_win_size != 0) {
if (sphb->mem_win_size > SPAPR_PCI_MEM32_WIN_SIZE) {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e96deefa30de..eff479b0a019 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -764,6 +764,7 @@ int spapr_max_server_number(sPAPRMachineState *spapr);
/* CPU and LMB DRC release callbacks. */
void spapr_core_release(DeviceState *dev);
void spapr_lmb_release(DeviceState *dev);
+void spapr_phb_release(DeviceState *dev);
void spapr_rtc_read(sPAPRRTCState *rtc, struct tm *tm, uint32_t *ns);
int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug
2018-12-21 6:36 ` [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug Greg Kurz
@ 2019-01-03 2:17 ` David Gibson
2019-01-08 10:18 ` Greg Kurz
0 siblings, 1 reply; 36+ messages in thread
From: David Gibson @ 2019-01-03 2:17 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 10880 bytes --]
On Fri, Dec 21, 2018 at 07:36:12AM +0100, Greg Kurz wrote:
> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> Hotplugging PHBs is a machine-level operation, but PHBs reside on the
> main system bus, so we register spapr machine as the handler for the
> main system bus.
>
> We re-get the phandle of the interrupt controller systematically for
> simplicity.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> hw/ppc/spapr.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_drc.c | 1
> hw/ppc/spapr_pci.c | 16 -----
> include/hw/ppc/spapr.h | 1
> 4 files changed, 149 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5c405a5fafca..065c9f19700e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2923,6 +2923,10 @@ static void spapr_machine_init(MachineState *machine)
> register_savevm_live(NULL, "spapr/htab", -1, 1,
> &savevm_htab_handlers, spapr);
>
> + if (smc->dr_phb_enabled) {
> + qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), NULL);
> + }
> +
> qemu_register_boot_set(spapr_boot_set, spapr);
>
> if (kvm_enabled()) {
> @@ -3716,6 +3720,135 @@ out:
> error_propagate(errp, local_err);
> }
>
> +static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> + Error **errp)
> +{
> + sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> + const unsigned windows_supported = spapr_phb_windows_supported(sphb);
> +
> + if (sphb->index == (uint32_t)-1) {
> + error_setg(errp, "\"index\" for PAPR PHB is mandatory");
> + return;
> + }
> +
> + /*
> + * This will check that sphb->index doesn't exceed the maximum number of
> + * PHBs for the current machine type.
> + */
> + smc->phb_placement(spapr, sphb->index,
> + &sphb->buid, &sphb->io_win_addr,
> + &sphb->mem_win_addr, &sphb->mem64_win_addr,
> + windows_supported, sphb->dma_liobn, errp);
> +}
> +
> +static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> + Error **errp)
> +{
> + sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> + void *fdt = NULL;
> + int fdt_start_offset;
> + int fdt_size;
> + Error *local_err = NULL;
> + sPAPRDRConnector *drc;
> + int ret;
> + bool hotplugged = spapr_drc_hotplugged(dev);
> + int offset, phandle = 0;
> + gchar *nodename = NULL;
> +
> + if (!smc->dr_phb_enabled) {
> + return;
> + }
> +
> + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
> + /* hotplug hooks should check it's enabled before getting this far */
> + assert(drc);
> +
> + if (hotplugged) {
> + if (spapr->fdt_blob) {
> + /*
> + * SLOF might have pushed an updated FDT with new phandle values.
> + * Re-get the one of our interrupt controller.
> + */
> + nodename = spapr->irq->get_nodename(spapr);
> +
> + offset = fdt_subnode_offset(spapr->fdt_blob, 0, nodename);
> + if (offset < 0) {
> + error_setg(errp, "Can't find node \"%s\": %s",
> + nodename, fdt_strerror(offset));
> + goto out;
> + }
> +
> + phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> + if (phandle < 0) {
> + error_setg(errp, "Can't get phandle of node \"%s\": %s",
> + nodename, fdt_strerror(offset));
> + goto out;
> + }
> + }
> + DEVICE_GET_CLASS(dev)->reset(dev);
> + }
> +
> + /* For cold-plugged at initial boot and fallback for hotplug */
> + if (!phandle) {
> + phandle = PHANDLE_XICP;
> + }
> +
> + fdt = create_device_tree(&fdt_size);
> + ret = spapr_populate_pci_dt(sphb, phandle, fdt, spapr->irq->nr_msis,
> + &fdt_start_offset);
> + if (ret < 0) {
> + error_setg(&local_err, "unable to create FDT for %sPHB",
> + dev->hotplugged ? "hotplugged " : "");
> + goto out;
> + }
> +
> + if (hotplugged) {
> + /* generally SLOF creates these, for hotplug it's up to QEMU */
> + _FDT(fdt_setprop_string(fdt, fdt_start_offset, "name", "pci"));
> + }
> +
> + spapr_drc_attach(drc, DEVICE(dev), fdt, fdt_start_offset, &local_err);
> +
> +out:
> + g_free(nodename);
> +
> + if (local_err) {
> + error_propagate(errp, local_err);
> + g_free(fdt);
> + return;
> + }
> +
> + if (hotplugged) {
> + spapr_hotplug_req_add_by_index(drc);
> + } else if (drc) {
> + spapr_drc_reset(drc);
> + }
> +}
> +
> +void spapr_phb_release(DeviceState *dev)
> +{
> + object_unparent(OBJECT(dev));
> +}
> +
> +static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> + sPAPRDRConnector *drc;
> +
> + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
> + assert(drc);
> +
> + if (!spapr_drc_unplug_requested(drc)) {
> + spapr_drc_detach(drc);
> + spapr_hotplug_req_remove_by_index(drc);
> + }
> +}
> +
> static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> @@ -3723,6 +3856,8 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> spapr_memory_plug(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> spapr_core_plug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> + spapr_phb_plug(hotplug_dev, dev, errp);
> }
> }
>
> @@ -3741,6 +3876,7 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> {
> sPAPRMachineState *sms = SPAPR_MACHINE(OBJECT(hotplug_dev));
> MachineClass *mc = MACHINE_GET_CLASS(sms);
> + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> @@ -3760,6 +3896,12 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> return;
> }
> spapr_core_unplug_request(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> + if (!smc->dr_phb_enabled) {
> + error_setg(errp, "PHB hot unplug not supported on this machine");
> + return;
> + }
> + spapr_phb_unplug_request(hotplug_dev, dev, errp);
> }
> }
>
> @@ -3770,6 +3912,8 @@ static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev,
> spapr_memory_pre_plug(hotplug_dev, dev, errp);
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> spapr_core_pre_plug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> + spapr_phb_pre_plug(hotplug_dev, dev, errp);
> }
> }
>
> @@ -3777,7 +3921,8 @@ static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine,
> DeviceState *dev)
> {
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> - object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> + object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> return HOTPLUG_HANDLER(machine);
> }
> return NULL;
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 189ee681062a..7a2676716364 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -703,6 +703,7 @@ static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
> drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
> drck->typename = "PHB";
> drck->drc_name_prefix = "PHB ";
> + drck->release = spapr_phb_release;
> }
>
> static const TypeInfo spapr_dr_connector_info = {
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 688cca83ef2f..5bc912aa0028 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1627,21 +1627,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - if (sphb->index != (uint32_t)-1) {
> - Error *local_err = NULL;
> -
> - smc->phb_placement(spapr, sphb->index,
> - &sphb->buid, &sphb->io_win_addr,
> - &sphb->mem_win_addr, &sphb->mem64_win_addr,
> - windows_supported, sphb->dma_liobn, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> - } else {
> - error_setg(errp, "\"index\" for PAPR PHB is mandatory");
> - return;
> - }
> + assert(sphb->index != (uint32_t)-1); /* checked in spapr_phb_pre_plug() */
Could this cause an unexpected assert if you're using an older machine
type that doesn't install the hotplug handler, then don't supply an
index on the command line?
> if (sphb->mem64_win_size != 0) {
> if (sphb->mem_win_size > SPAPR_PCI_MEM32_WIN_SIZE) {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index e96deefa30de..eff479b0a019 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -764,6 +764,7 @@ int spapr_max_server_number(sPAPRMachineState *spapr);
> /* CPU and LMB DRC release callbacks. */
> void spapr_core_release(DeviceState *dev);
> void spapr_lmb_release(DeviceState *dev);
> +void spapr_phb_release(DeviceState *dev);
>
> void spapr_rtc_read(sPAPRRTCState *rtc, struct tm *tm, uint32_t *ns);
> int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug
2019-01-03 2:17 ` David Gibson
@ 2019-01-08 10:18 ` Greg Kurz
0 siblings, 0 replies; 36+ messages in thread
From: Greg Kurz @ 2019-01-08 10:18 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, qemu-ppc, qemu-s390x, Alexey Kardashevskiy,
Cédric Le Goater, Michael Roth, Paolo Bonzini,
Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, Gerd Hoffmann, Dmitry Fleytman
[-- Attachment #1: Type: text/plain, Size: 11962 bytes --]
On Thu, 3 Jan 2019 13:17:03 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Dec 21, 2018 at 07:36:12AM +0100, Greg Kurz wrote:
> > From: Michael Roth <mdroth@linux.vnet.ibm.com>
> >
> > Hotplugging PHBs is a machine-level operation, but PHBs reside on the
> > main system bus, so we register spapr machine as the handler for the
> > main system bus.
> >
> > We re-get the phandle of the interrupt controller systematically for
> > simplicity.
> >
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > hw/ppc/spapr.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++
> > hw/ppc/spapr_drc.c | 1
> > hw/ppc/spapr_pci.c | 16 -----
> > include/hw/ppc/spapr.h | 1
> > 4 files changed, 149 insertions(+), 16 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5c405a5fafca..065c9f19700e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2923,6 +2923,10 @@ static void spapr_machine_init(MachineState *machine)
> > register_savevm_live(NULL, "spapr/htab", -1, 1,
> > &savevm_htab_handlers, spapr);
> >
> > + if (smc->dr_phb_enabled) {
> > + qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), NULL);
> > + }
> > +
> > qemu_register_boot_set(spapr_boot_set, spapr);
> >
> > if (kvm_enabled()) {
> > @@ -3716,6 +3720,135 @@ out:
> > error_propagate(errp, local_err);
> > }
> >
> > +static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > + Error **errp)
> > +{
> > + sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> > + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> > + const unsigned windows_supported = spapr_phb_windows_supported(sphb);
> > +
> > + if (sphb->index == (uint32_t)-1) {
> > + error_setg(errp, "\"index\" for PAPR PHB is mandatory");
> > + return;
> > + }
> > +
> > + /*
> > + * This will check that sphb->index doesn't exceed the maximum number of
> > + * PHBs for the current machine type.
> > + */
> > + smc->phb_placement(spapr, sphb->index,
> > + &sphb->buid, &sphb->io_win_addr,
> > + &sphb->mem_win_addr, &sphb->mem64_win_addr,
> > + windows_supported, sphb->dma_liobn, errp);
> > +}
> > +
> > +static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > + Error **errp)
> > +{
> > + sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> > + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> > + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > + void *fdt = NULL;
> > + int fdt_start_offset;
> > + int fdt_size;
> > + Error *local_err = NULL;
> > + sPAPRDRConnector *drc;
> > + int ret;
> > + bool hotplugged = spapr_drc_hotplugged(dev);
> > + int offset, phandle = 0;
> > + gchar *nodename = NULL;
> > +
> > + if (!smc->dr_phb_enabled) {
> > + return;
> > + }
> > +
> > + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
> > + /* hotplug hooks should check it's enabled before getting this far */
> > + assert(drc);
> > +
> > + if (hotplugged) {
> > + if (spapr->fdt_blob) {
> > + /*
> > + * SLOF might have pushed an updated FDT with new phandle values.
> > + * Re-get the one of our interrupt controller.
> > + */
> > + nodename = spapr->irq->get_nodename(spapr);
> > +
> > + offset = fdt_subnode_offset(spapr->fdt_blob, 0, nodename);
> > + if (offset < 0) {
> > + error_setg(errp, "Can't find node \"%s\": %s",
> > + nodename, fdt_strerror(offset));
> > + goto out;
> > + }
> > +
> > + phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> > + if (phandle < 0) {
> > + error_setg(errp, "Can't get phandle of node \"%s\": %s",
> > + nodename, fdt_strerror(offset));
> > + goto out;
> > + }
> > + }
> > + DEVICE_GET_CLASS(dev)->reset(dev);
> > + }
> > +
> > + /* For cold-plugged at initial boot and fallback for hotplug */
> > + if (!phandle) {
> > + phandle = PHANDLE_XICP;
> > + }
> > +
> > + fdt = create_device_tree(&fdt_size);
> > + ret = spapr_populate_pci_dt(sphb, phandle, fdt, spapr->irq->nr_msis,
> > + &fdt_start_offset);
> > + if (ret < 0) {
> > + error_setg(&local_err, "unable to create FDT for %sPHB",
> > + dev->hotplugged ? "hotplugged " : "");
> > + goto out;
> > + }
> > +
> > + if (hotplugged) {
> > + /* generally SLOF creates these, for hotplug it's up to QEMU */
> > + _FDT(fdt_setprop_string(fdt, fdt_start_offset, "name", "pci"));
> > + }
> > +
> > + spapr_drc_attach(drc, DEVICE(dev), fdt, fdt_start_offset, &local_err);
> > +
> > +out:
> > + g_free(nodename);
> > +
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + g_free(fdt);
> > + return;
> > + }
> > +
> > + if (hotplugged) {
> > + spapr_hotplug_req_add_by_index(drc);
> > + } else if (drc) {
> > + spapr_drc_reset(drc);
> > + }
> > +}
> > +
> > +void spapr_phb_release(DeviceState *dev)
> > +{
> > + object_unparent(OBJECT(dev));
> > +}
> > +
> > +static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev,
> > + DeviceState *dev, Error **errp)
> > +{
> > + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> > + sPAPRDRConnector *drc;
> > +
> > + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
> > + assert(drc);
> > +
> > + if (!spapr_drc_unplug_requested(drc)) {
> > + spapr_drc_detach(drc);
> > + spapr_hotplug_req_remove_by_index(drc);
> > + }
> > +}
> > +
> > static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> > DeviceState *dev, Error **errp)
> > {
> > @@ -3723,6 +3856,8 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> > spapr_memory_plug(hotplug_dev, dev, errp);
> > } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> > spapr_core_plug(hotplug_dev, dev, errp);
> > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> > + spapr_phb_plug(hotplug_dev, dev, errp);
> > }
> > }
> >
> > @@ -3741,6 +3876,7 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> > {
> > sPAPRMachineState *sms = SPAPR_MACHINE(OBJECT(hotplug_dev));
> > MachineClass *mc = MACHINE_GET_CLASS(sms);
> > + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> >
> > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> > if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> > @@ -3760,6 +3896,12 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> > return;
> > }
> > spapr_core_unplug_request(hotplug_dev, dev, errp);
> > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> > + if (!smc->dr_phb_enabled) {
> > + error_setg(errp, "PHB hot unplug not supported on this machine");
> > + return;
> > + }
> > + spapr_phb_unplug_request(hotplug_dev, dev, errp);
> > }
> > }
> >
> > @@ -3770,6 +3912,8 @@ static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev,
> > spapr_memory_pre_plug(hotplug_dev, dev, errp);
> > } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> > spapr_core_pre_plug(hotplug_dev, dev, errp);
> > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> > + spapr_phb_pre_plug(hotplug_dev, dev, errp);
> > }
> > }
> >
> > @@ -3777,7 +3921,8 @@ static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine,
> > DeviceState *dev)
> > {
> > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> > - object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> > + object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE) ||
> > + object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> > return HOTPLUG_HANDLER(machine);
> > }
> > return NULL;
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index 189ee681062a..7a2676716364 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -703,6 +703,7 @@ static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
> > drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
> > drck->typename = "PHB";
> > drck->drc_name_prefix = "PHB ";
> > + drck->release = spapr_phb_release;
> > }
> >
> > static const TypeInfo spapr_dr_connector_info = {
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 688cca83ef2f..5bc912aa0028 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1627,21 +1627,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > return;
> > }
> >
> > - if (sphb->index != (uint32_t)-1) {
> > - Error *local_err = NULL;
> > -
> > - smc->phb_placement(spapr, sphb->index,
> > - &sphb->buid, &sphb->io_win_addr,
> > - &sphb->mem_win_addr, &sphb->mem64_win_addr,
> > - windows_supported, sphb->dma_liobn, &local_err);
> > - if (local_err) {
> > - error_propagate(errp, local_err);
> > - return;
> > - }
> > - } else {
> > - error_setg(errp, "\"index\" for PAPR PHB is mandatory");
> > - return;
> > - }
> > + assert(sphb->index != (uint32_t)-1); /* checked in spapr_phb_pre_plug() */
>
> Could this cause an unexpected assert if you're using an older machine
> type that doesn't install the hotplug handler, then don't supply an
> index on the command line?
>
AFAICS all machine types do install the hotplug handler.
static void spapr_machine_class_init(ObjectClass *oc, void *data)
{
...
HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
...
hc->pre_plug = spapr_machine_device_pre_plug;
hc->plug = spapr_machine_device_plug;
and so we get:
$ ppc64-softmmu/qemu-system-ppc64 -machine pseries-2.1 -device spapr-pci-host-bridge
qemu-system-ppc64: -device spapr-pci-host-bridge: "index" for PAPR PHB is mandatory
which is expected since we deliberately broke backward compatibility when we
made "index" mandatory (see commit 30b3bc5aa9f4 for details).
> > if (sphb->mem64_win_size != 0) {
> > if (sphb->mem_win_size > SPAPR_PCI_MEM32_WIN_SIZE) {
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index e96deefa30de..eff479b0a019 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -764,6 +764,7 @@ int spapr_max_server_number(sPAPRMachineState *spapr);
> > /* CPU and LMB DRC release callbacks. */
> > void spapr_core_release(DeviceState *dev);
> > void spapr_lmb_release(DeviceState *dev);
> > +void spapr_phb_release(DeviceState *dev);
> >
> > void spapr_rtc_read(sPAPRRTCState *rtc, struct tm *tm, uint32_t *ns);
> > int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
> >
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread