From 1b6e74824659ef10bd2a2924a98df388b78e175e Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Wed, 24 May 2017 14:10:48 +0200 Subject: [PATCH 01/17] migration: remove register_savevm() We can replace the four remaining calls of register_savevm() by calls to register_savevm_live(). So we can remove the function and as we don't allocate anymore the ops pointer with g_new0() we don't have to free it then. Signed-off-by: Laurent Vivier Reviewed-by: Juan Quintela Signed-off-by: David Gibson --- hw/net/vmxnet3.c | 8 ++++++-- hw/s390x/s390-skeys.c | 9 +++++++-- hw/s390x/s390-virtio-ccw.c | 8 ++++++-- include/migration/vmstate.h | 8 -------- migration/savevm.c | 16 ---------------- slirp/slirp.c | 8 ++++++-- 6 files changed, 25 insertions(+), 32 deletions(-) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 8b1fab24fd..4df31101ec 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2262,6 +2262,11 @@ static const MemoryRegionOps b1_ops = { }, }; +static SaveVMHandlers savevm_vmxnet3_msix = { + .save_state = vmxnet3_msix_save, + .load_state = vmxnet3_msix_load, +}; + static uint64_t vmxnet3_device_serial_num(VMXNET3State *s) { uint64_t dsn_payload; @@ -2331,8 +2336,7 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) vmxnet3_device_serial_num(s)); } - register_savevm(dev, "vmxnet3-msix", -1, 1, - vmxnet3_msix_save, vmxnet3_msix_load, s); + register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s); } static void vmxnet3_instance_init(Object *obj) diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c index 619152cc37..35e7f6316f 100644 --- a/hw/s390x/s390-skeys.c +++ b/hw/s390x/s390-skeys.c @@ -362,6 +362,11 @@ static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp) return ss->migration_enabled; } +static SaveVMHandlers savevm_s390_storage_keys = { + .save_state = s390_storage_keys_save, + .load_state = s390_storage_keys_load, +}; + static inline void s390_skeys_set_migration_enabled(Object *obj, bool value, Error **errp) { @@ -375,8 +380,8 @@ static inline void s390_skeys_set_migration_enabled(Object *obj, bool value, ss->migration_enabled = value; if (ss->migration_enabled) { - register_savevm(NULL, TYPE_S390_SKEYS, 0, 1, s390_storage_keys_save, - s390_storage_keys_load, ss); + register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1, + &savevm_s390_storage_keys, ss); } else { unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss); } diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index c9021f2fa9..a806345276 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -104,6 +104,11 @@ void s390_memory_init(ram_addr_t mem_size) s390_skeys_init(); } +static SaveVMHandlers savevm_gtod = { + .save_state = gtod_save, + .load_state = gtod_load, +}; + static void ccw_init(MachineState *machine) { int ret; @@ -151,8 +156,7 @@ static void ccw_init(MachineState *machine) s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw"); /* Register savevm handler for guest TOD clock */ - register_savevm(NULL, "todclock", 0, 1, - gtod_save, gtod_load, kvm_state); + register_savevm_live(NULL, "todclock", 0, 1, &savevm_gtod, kvm_state); } static void s390_cpu_plug(HotplugHandler *hotplug_dev, diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 66895623da..8a3e9e6088 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -59,14 +59,6 @@ typedef struct SaveVMHandlers { LoadStateHandler *load_state; } SaveVMHandlers; -int register_savevm(DeviceState *dev, - const char *idstr, - int instance_id, - int version_id, - SaveStateHandler *save_state, - LoadStateHandler *load_state, - void *opaque); - int register_savevm_live(DeviceState *dev, const char *idstr, int instance_id, diff --git a/migration/savevm.c b/migration/savevm.c index 9c320f59d0..035c127fdd 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -645,21 +645,6 @@ int register_savevm_live(DeviceState *dev, return 0; } -int register_savevm(DeviceState *dev, - const char *idstr, - int instance_id, - int version_id, - SaveStateHandler *save_state, - LoadStateHandler *load_state, - void *opaque) -{ - SaveVMHandlers *ops = g_new0(SaveVMHandlers, 1); - ops->save_state = save_state; - ops->load_state = load_state; - return register_savevm_live(dev, idstr, instance_id, version_id, - ops, opaque); -} - void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) { SaveStateEntry *se, *new_se; @@ -679,7 +664,6 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) { QTAILQ_REMOVE(&savevm_state.handlers, se, entry); g_free(se->compat); - g_free(se->ops); g_free(se); } } diff --git a/slirp/slirp.c b/slirp/slirp.c index e79345bdfc..23864938f7 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -272,6 +272,11 @@ static void slirp_init_once(void) static void slirp_state_save(QEMUFile *f, void *opaque); static int slirp_state_load(QEMUFile *f, void *opaque, int version_id); +static SaveVMHandlers savevm_slirp_state = { + .save_state = slirp_state_save, + .load_state = slirp_state_load, +}; + Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork, struct in_addr vnetmask, struct in_addr vhost, bool in6_enabled, @@ -321,8 +326,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork, slirp->opaque = opaque; - register_savevm(NULL, "slirp", 0, 4, - slirp_state_save, slirp_state_load, slirp); + register_savevm_live(NULL, "slirp", 0, 4, &savevm_slirp_state, slirp); QTAILQ_INSERT_TAIL(&slirp_instances, slirp, entry); From 75e972dab51ba3bc02760e8142fc0c22eb611e3a Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 26 May 2017 14:46:28 +1000 Subject: [PATCH 02/17] migration: Mark CPU states dirty before incoming migration/loadvm As a rule, CPU internal state should never be updated when !cpu->kvm_vcpu_dirty (or the HAX equivalent). If that is done, then subsequent calls to cpu_synchronize_state() - usually safe and idempotent - will clobber state. However, we routinely do this during a loadvm or incoming migration. Usually this is called shortly after a reset, which will clear all the cpu dirty flags with cpu_synchronize_all_post_reset(). Nothing is expected to set the dirty flags again before the cpu state is loaded from the incoming stream. This means that it isn't safe to call cpu_synchronize_state() from a post_load handler, which is non-obvious and potentially inconvenient. We could cpu_synchronize_all_state() before the loadvm, but that would be overkill since a) we expect the state to already be synchronized from the reset and b) we expect to completely rewrite the state with a call to cpu_synchronize_all_post_init() at the end of qemu_loadvm_state(). To clear this up, this patch introduces cpu_synchronize_pre_loadvm() and associated helpers, which simply marks the cpu state as dirty without actually changing anything. i.e. it says we want to discard any existing KVM (or HAX) state and replace it with what we're going to load. Cc: Juan Quintela Cc: Dave Gilbert Signed-off-by: David Gibson Reviewed-by: Juan Quintela --- cpus.c | 9 +++++++++ include/sysemu/cpus.h | 1 + include/sysemu/hax.h | 1 + include/sysemu/hw_accel.h | 10 ++++++++++ include/sysemu/kvm.h | 1 + kvm-all.c | 10 ++++++++++ migration/savevm.c | 2 ++ target/i386/hax-all.c | 10 ++++++++++ 8 files changed, 44 insertions(+) diff --git a/cpus.c b/cpus.c index 516e5cbac1..6398439946 100644 --- a/cpus.c +++ b/cpus.c @@ -921,6 +921,15 @@ void cpu_synchronize_all_post_init(void) } } +void cpu_synchronize_all_pre_loadvm(void) +{ + CPUState *cpu; + + CPU_FOREACH(cpu) { + cpu_synchronize_pre_loadvm(cpu); + } +} + static int do_vm_stop(RunState state) { int ret = 0; diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h index a8053f1715..731756d948 100644 --- a/include/sysemu/cpus.h +++ b/include/sysemu/cpus.h @@ -27,6 +27,7 @@ void qemu_timer_notify_cb(void *opaque, QEMUClockType type); void cpu_synchronize_all_states(void); void cpu_synchronize_all_post_reset(void); void cpu_synchronize_all_post_init(void); +void cpu_synchronize_all_pre_loadvm(void); void qtest_clock_warp(int64_t dest); diff --git a/include/sysemu/hax.h b/include/sysemu/hax.h index d9f023918e..232a68ab1b 100644 --- a/include/sysemu/hax.h +++ b/include/sysemu/hax.h @@ -33,6 +33,7 @@ int hax_populate_ram(uint64_t va, uint32_t size); void hax_cpu_synchronize_state(CPUState *cpu); void hax_cpu_synchronize_post_reset(CPUState *cpu); void hax_cpu_synchronize_post_init(CPUState *cpu); +void hax_cpu_synchronize_pre_loadvm(CPUState *cpu); #ifdef CONFIG_HAX diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h index c9b3105bc7..469ffda460 100644 --- a/include/sysemu/hw_accel.h +++ b/include/sysemu/hw_accel.h @@ -45,4 +45,14 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) } } +static inline void cpu_synchronize_pre_loadvm(CPUState *cpu) +{ + if (kvm_enabled()) { + kvm_cpu_synchronize_pre_loadvm(cpu); + } + if (hax_enabled()) { + hax_cpu_synchronize_pre_loadvm(cpu); + } +} + #endif /* QEMU_HW_ACCEL_H */ diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 5cc83f2003..a45c145560 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -459,6 +459,7 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, void kvm_cpu_synchronize_state(CPUState *cpu); void kvm_cpu_synchronize_post_reset(CPUState *cpu); void kvm_cpu_synchronize_post_init(CPUState *cpu); +void kvm_cpu_synchronize_pre_loadvm(CPUState *cpu); void kvm_init_cpu_signals(CPUState *cpu); diff --git a/kvm-all.c b/kvm-all.c index 7df27c8522..494b9256aa 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1896,6 +1896,16 @@ void kvm_cpu_synchronize_post_init(CPUState *cpu) run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, RUN_ON_CPU_NULL); } +static void do_kvm_cpu_synchronize_pre_loadvm(CPUState *cpu, run_on_cpu_data arg) +{ + cpu->kvm_vcpu_dirty = true; +} + +void kvm_cpu_synchronize_pre_loadvm(CPUState *cpu) +{ + run_on_cpu(cpu, do_kvm_cpu_synchronize_pre_loadvm, RUN_ON_CPU_NULL); +} + #ifdef KVM_HAVE_MCE_INJECTION static __thread void *pending_sigbus_addr; static __thread int pending_sigbus_code; diff --git a/migration/savevm.c b/migration/savevm.c index 035c127fdd..1993ca23fe 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1999,6 +1999,8 @@ int qemu_loadvm_state(QEMUFile *f) } } + cpu_synchronize_all_pre_loadvm(); + ret = qemu_loadvm_state_main(f, mis); qemu_event_set(&mis->main_thread_load_event); diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c index 73469311d6..097db5cae1 100644 --- a/target/i386/hax-all.c +++ b/target/i386/hax-all.c @@ -635,6 +635,16 @@ void hax_cpu_synchronize_post_init(CPUState *cpu) run_on_cpu(cpu, do_hax_cpu_synchronize_post_init, RUN_ON_CPU_NULL); } +static void do_hax_cpu_synchronize_pre_loadvm(CPUState *cpu, run_on_cpu_data arg) +{ + cpu->hax_vcpu_dirty = true; +} + +void hax_cpu_synchronize_pre_loadvm(CPUState *cpu) +{ + run_on_cpu(cpu, do_hax_cpu_synchronize_pre_loadvm, RUN_ON_CPU_NULL); +} + int hax_smp_cpu_exec(CPUState *cpu) { CPUArchState *env = (CPUArchState *) (cpu->env_ptr); From b89b3d39299559bc7bf7030e55f99c75dab70b2b Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 1 Jun 2017 10:30:00 +1000 Subject: [PATCH 03/17] spapr: Move DRC RTAS calls into spapr_drc.c Currently implementations of the RTAS calls related to DRCs are in spapr_rtas.c. They belong better in spapr_drc.c - that way they're closer to related code, and we'll be able to make some more things local. spapr_rtas.c was intended to contain the RTAS infrastructure and core calls that don't belong anywhere else, not every RTAS implementation. Code motion only. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 322 +++++++++++++++++++++++++++++++++++++++++++- hw/ppc/spapr_rtas.c | 304 ----------------------------------------- 2 files changed, 315 insertions(+), 311 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index cc2400bcd5..ae8800da23 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -27,6 +27,34 @@ #define DRC_INDEX_TYPE_SHIFT 28 #define DRC_INDEX_ID_MASK ((1ULL << DRC_INDEX_TYPE_SHIFT) - 1) +static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr, + uint32_t drc_index) +{ + sPAPRConfigureConnectorState *ccs = NULL; + + QTAILQ_FOREACH(ccs, &spapr->ccs_list, next) { + if (ccs->drc_index == drc_index) { + break; + } + } + + return ccs; +} + +static void spapr_ccs_add(sPAPRMachineState *spapr, + sPAPRConfigureConnectorState *ccs) +{ + g_assert(!spapr_ccs_find(spapr, ccs->drc_index)); + QTAILQ_INSERT_HEAD(&spapr->ccs_list, ccs, next); +} + +static void spapr_ccs_remove(sPAPRMachineState *spapr, + sPAPRConfigureConnectorState *ccs) +{ + QTAILQ_REMOVE(&spapr->ccs_list, ccs, next); + g_free(ccs); +} + static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type) { uint32_t shift = 0; @@ -747,13 +775,6 @@ static const TypeInfo spapr_dr_connector_info = { .class_init = spapr_dr_connector_class_init, }; -static void spapr_drc_register_types(void) -{ - type_register_static(&spapr_dr_connector_info); -} - -type_init(spapr_drc_register_types) - /* helper functions for external users */ sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index) @@ -932,3 +953,290 @@ out: return ret; } + +/* + * RTAS calls + */ + +static bool sensor_type_is_dr(uint32_t sensor_type) +{ + switch (sensor_type) { + case RTAS_SENSOR_TYPE_ISOLATION_STATE: + case RTAS_SENSOR_TYPE_DR: + case RTAS_SENSOR_TYPE_ALLOCATION_STATE: + return true; + } + + return false; +} + +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ + uint32_t sensor_type; + uint32_t sensor_index; + uint32_t sensor_state; + uint32_t ret = RTAS_OUT_SUCCESS; + sPAPRDRConnector *drc; + sPAPRDRConnectorClass *drck; + + if (nargs != 3 || nret != 1) { + ret = RTAS_OUT_PARAM_ERROR; + goto out; + } + + sensor_type = rtas_ld(args, 0); + sensor_index = rtas_ld(args, 1); + sensor_state = rtas_ld(args, 2); + + if (!sensor_type_is_dr(sensor_type)) { + goto out_unimplemented; + } + + /* if this is a DR sensor we can assume sensor_index == drc_index */ + drc = spapr_dr_connector_by_index(sensor_index); + if (!drc) { + trace_spapr_rtas_set_indicator_invalid(sensor_index); + ret = RTAS_OUT_PARAM_ERROR; + goto out; + } + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + + switch (sensor_type) { + case RTAS_SENSOR_TYPE_ISOLATION_STATE: + /* if the guest is configuring a device attached to this + * DRC, we should reset the configuration state at this + * point since it may no longer be reliable (guest released + * device and needs to start over, or unplug occurred so + * the FDT is no longer valid) + */ + if (sensor_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) { + sPAPRConfigureConnectorState *ccs = spapr_ccs_find(spapr, + sensor_index); + if (ccs) { + spapr_ccs_remove(spapr, ccs); + } + } + ret = drck->set_isolation_state(drc, sensor_state); + break; + case RTAS_SENSOR_TYPE_DR: + ret = drck->set_indicator_state(drc, sensor_state); + break; + case RTAS_SENSOR_TYPE_ALLOCATION_STATE: + ret = drck->set_allocation_state(drc, sensor_state); + break; + default: + goto out_unimplemented; + } + +out: + rtas_st(rets, 0, ret); + return; + +out_unimplemented: + /* currently only DR-related sensors are implemented */ + trace_spapr_rtas_set_indicator_not_supported(sensor_index, sensor_type); + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); +} + +static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ + uint32_t sensor_type; + uint32_t sensor_index; + uint32_t sensor_state = 0; + sPAPRDRConnector *drc; + sPAPRDRConnectorClass *drck; + uint32_t ret = RTAS_OUT_SUCCESS; + + if (nargs != 2 || nret != 2) { + ret = RTAS_OUT_PARAM_ERROR; + goto out; + } + + sensor_type = rtas_ld(args, 0); + sensor_index = rtas_ld(args, 1); + + if (sensor_type != RTAS_SENSOR_TYPE_ENTITY_SENSE) { + /* currently only DR-related sensors are implemented */ + trace_spapr_rtas_get_sensor_state_not_supported(sensor_index, + sensor_type); + ret = RTAS_OUT_NOT_SUPPORTED; + goto out; + } + + drc = spapr_dr_connector_by_index(sensor_index); + if (!drc) { + trace_spapr_rtas_get_sensor_state_invalid(sensor_index); + ret = RTAS_OUT_PARAM_ERROR; + goto out; + } + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + ret = drck->entity_sense(drc, &sensor_state); + +out: + rtas_st(rets, 0, ret); + rtas_st(rets, 1, sensor_state); +} + +/* configure-connector work area offsets, int32_t units for field + * indexes, bytes for field offset/len values. + * + * as documented by PAPR+ v2.7, 13.5.3.5 + */ +#define CC_IDX_NODE_NAME_OFFSET 2 +#define CC_IDX_PROP_NAME_OFFSET 2 +#define CC_IDX_PROP_LEN 3 +#define CC_IDX_PROP_DATA_OFFSET 4 +#define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4) +#define CC_WA_LEN 4096 + +static void configure_connector_st(target_ulong addr, target_ulong offset, + const void *buf, size_t len) +{ + cpu_physical_memory_write(ppc64_phys_to_real(addr + offset), + buf, MIN(len, CC_WA_LEN - offset)); +} + +void spapr_ccs_reset_hook(void *opaque) +{ + sPAPRMachineState *spapr = opaque; + sPAPRConfigureConnectorState *ccs, *ccs_tmp; + + QTAILQ_FOREACH_SAFE(ccs, &spapr->ccs_list, next, ccs_tmp) { + spapr_ccs_remove(spapr, ccs); + } +} + +static void rtas_ibm_configure_connector(PowerPCCPU *cpu, + sPAPRMachineState *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ + uint64_t wa_addr; + uint64_t wa_offset; + uint32_t drc_index; + sPAPRDRConnector *drc; + sPAPRDRConnectorClass *drck; + sPAPRConfigureConnectorState *ccs; + sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE; + int rc; + const void *fdt; + + if (nargs != 2 || nret != 1) { + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); + return; + } + + wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0); + + drc_index = rtas_ld(wa_addr, 0); + drc = spapr_dr_connector_by_index(drc_index); + if (!drc) { + trace_spapr_rtas_ibm_configure_connector_invalid(drc_index); + rc = RTAS_OUT_PARAM_ERROR; + goto out; + } + + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + fdt = drck->get_fdt(drc, NULL); + if (!fdt) { + trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index); + rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE; + goto out; + } + + ccs = spapr_ccs_find(spapr, drc_index); + if (!ccs) { + ccs = g_new0(sPAPRConfigureConnectorState, 1); + (void)drck->get_fdt(drc, &ccs->fdt_offset); + ccs->drc_index = drc_index; + spapr_ccs_add(spapr, ccs); + } + + do { + uint32_t tag; + const char *name; + const struct fdt_property *prop; + int fdt_offset_next, prop_len; + + tag = fdt_next_tag(fdt, ccs->fdt_offset, &fdt_offset_next); + + switch (tag) { + case FDT_BEGIN_NODE: + ccs->fdt_depth++; + name = fdt_get_name(fdt, ccs->fdt_offset, NULL); + + /* provide the name of the next OF node */ + wa_offset = CC_VAL_DATA_OFFSET; + rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset); + configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1); + resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD; + break; + case FDT_END_NODE: + ccs->fdt_depth--; + if (ccs->fdt_depth == 0) { + /* done sending the device tree, don't need to track + * the state anymore + */ + drck->set_configured(drc); + spapr_ccs_remove(spapr, ccs); + ccs = NULL; + resp = SPAPR_DR_CC_RESPONSE_SUCCESS; + } else { + resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT; + } + break; + case FDT_PROP: + prop = fdt_get_property_by_offset(fdt, ccs->fdt_offset, + &prop_len); + name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); + + /* provide the name of the next OF property */ + wa_offset = CC_VAL_DATA_OFFSET; + rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset); + configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1); + + /* provide the length and value of the OF property. data gets + * placed immediately after NULL terminator of the OF property's + * name string + */ + wa_offset += strlen(name) + 1, + rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len); + rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset); + configure_connector_st(wa_addr, wa_offset, prop->data, prop_len); + resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY; + break; + case FDT_END: + resp = SPAPR_DR_CC_RESPONSE_ERROR; + default: + /* keep seeking for an actionable tag */ + break; + } + if (ccs) { + ccs->fdt_offset = fdt_offset_next; + } + } while (resp == SPAPR_DR_CC_RESPONSE_CONTINUE); + + rc = resp; +out: + rtas_st(rets, 0, rc); +} + +static void spapr_drc_register_types(void) +{ + type_register_static(&spapr_dr_connector_info); + + spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", + rtas_set_indicator); + spapr_rtas_register(RTAS_GET_SENSOR_STATE, "get-sensor-state", + rtas_get_sensor_state); + spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector", + rtas_ibm_configure_connector); +} +type_init(spapr_drc_register_types) diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index b666a4c15c..707c4d4936 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -47,44 +47,6 @@ #include "trace.h" #include "hw/ppc/fdt.h" -static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr, - uint32_t drc_index) -{ - sPAPRConfigureConnectorState *ccs = NULL; - - QTAILQ_FOREACH(ccs, &spapr->ccs_list, next) { - if (ccs->drc_index == drc_index) { - break; - } - } - - return ccs; -} - -static void spapr_ccs_add(sPAPRMachineState *spapr, - sPAPRConfigureConnectorState *ccs) -{ - g_assert(!spapr_ccs_find(spapr, ccs->drc_index)); - QTAILQ_INSERT_HEAD(&spapr->ccs_list, ccs, next); -} - -static void spapr_ccs_remove(sPAPRMachineState *spapr, - sPAPRConfigureConnectorState *ccs) -{ - QTAILQ_REMOVE(&spapr->ccs_list, ccs, next); - g_free(ccs); -} - -void spapr_ccs_reset_hook(void *opaque) -{ - sPAPRMachineState *spapr = opaque; - sPAPRConfigureConnectorState *ccs, *ccs_tmp; - - QTAILQ_FOREACH_SAFE(ccs, &spapr->ccs_list, next, ccs_tmp) { - spapr_ccs_remove(spapr, ccs); - } -} - static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr, uint32_t token, uint32_t nargs, target_ulong args, @@ -389,266 +351,6 @@ static void rtas_get_power_level(PowerPCCPU *cpu, sPAPRMachineState *spapr, rtas_st(rets, 1, 100); } -static bool sensor_type_is_dr(uint32_t sensor_type) -{ - switch (sensor_type) { - case RTAS_SENSOR_TYPE_ISOLATION_STATE: - case RTAS_SENSOR_TYPE_DR: - case RTAS_SENSOR_TYPE_ALLOCATION_STATE: - return true; - } - - return false; -} - -static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr, - uint32_t token, uint32_t nargs, - target_ulong args, uint32_t nret, - target_ulong rets) -{ - uint32_t sensor_type; - uint32_t sensor_index; - uint32_t sensor_state; - uint32_t ret = RTAS_OUT_SUCCESS; - sPAPRDRConnector *drc; - sPAPRDRConnectorClass *drck; - - if (nargs != 3 || nret != 1) { - ret = RTAS_OUT_PARAM_ERROR; - goto out; - } - - sensor_type = rtas_ld(args, 0); - sensor_index = rtas_ld(args, 1); - sensor_state = rtas_ld(args, 2); - - if (!sensor_type_is_dr(sensor_type)) { - goto out_unimplemented; - } - - /* if this is a DR sensor we can assume sensor_index == drc_index */ - drc = spapr_dr_connector_by_index(sensor_index); - if (!drc) { - trace_spapr_rtas_set_indicator_invalid(sensor_index); - ret = RTAS_OUT_PARAM_ERROR; - goto out; - } - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - - switch (sensor_type) { - case RTAS_SENSOR_TYPE_ISOLATION_STATE: - /* if the guest is configuring a device attached to this - * DRC, we should reset the configuration state at this - * point since it may no longer be reliable (guest released - * device and needs to start over, or unplug occurred so - * the FDT is no longer valid) - */ - if (sensor_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) { - sPAPRConfigureConnectorState *ccs = spapr_ccs_find(spapr, - sensor_index); - if (ccs) { - spapr_ccs_remove(spapr, ccs); - } - } - ret = drck->set_isolation_state(drc, sensor_state); - break; - case RTAS_SENSOR_TYPE_DR: - ret = drck->set_indicator_state(drc, sensor_state); - break; - case RTAS_SENSOR_TYPE_ALLOCATION_STATE: - ret = drck->set_allocation_state(drc, sensor_state); - break; - default: - goto out_unimplemented; - } - -out: - rtas_st(rets, 0, ret); - return; - -out_unimplemented: - /* currently only DR-related sensors are implemented */ - trace_spapr_rtas_set_indicator_not_supported(sensor_index, sensor_type); - rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); -} - -static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr, - uint32_t token, uint32_t nargs, - target_ulong args, uint32_t nret, - target_ulong rets) -{ - uint32_t sensor_type; - uint32_t sensor_index; - uint32_t sensor_state = 0; - sPAPRDRConnector *drc; - sPAPRDRConnectorClass *drck; - uint32_t ret = RTAS_OUT_SUCCESS; - - if (nargs != 2 || nret != 2) { - ret = RTAS_OUT_PARAM_ERROR; - goto out; - } - - sensor_type = rtas_ld(args, 0); - sensor_index = rtas_ld(args, 1); - - if (sensor_type != RTAS_SENSOR_TYPE_ENTITY_SENSE) { - /* currently only DR-related sensors are implemented */ - trace_spapr_rtas_get_sensor_state_not_supported(sensor_index, - sensor_type); - ret = RTAS_OUT_NOT_SUPPORTED; - goto out; - } - - drc = spapr_dr_connector_by_index(sensor_index); - if (!drc) { - trace_spapr_rtas_get_sensor_state_invalid(sensor_index); - ret = RTAS_OUT_PARAM_ERROR; - goto out; - } - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - ret = drck->entity_sense(drc, &sensor_state); - -out: - rtas_st(rets, 0, ret); - rtas_st(rets, 1, sensor_state); -} - -/* configure-connector work area offsets, int32_t units for field - * indexes, bytes for field offset/len values. - * - * as documented by PAPR+ v2.7, 13.5.3.5 - */ -#define CC_IDX_NODE_NAME_OFFSET 2 -#define CC_IDX_PROP_NAME_OFFSET 2 -#define CC_IDX_PROP_LEN 3 -#define CC_IDX_PROP_DATA_OFFSET 4 -#define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4) -#define CC_WA_LEN 4096 - -static void configure_connector_st(target_ulong addr, target_ulong offset, - const void *buf, size_t len) -{ - cpu_physical_memory_write(ppc64_phys_to_real(addr + offset), - buf, MIN(len, CC_WA_LEN - offset)); -} - -static void rtas_ibm_configure_connector(PowerPCCPU *cpu, - sPAPRMachineState *spapr, - uint32_t token, uint32_t nargs, - target_ulong args, uint32_t nret, - target_ulong rets) -{ - uint64_t wa_addr; - uint64_t wa_offset; - uint32_t drc_index; - sPAPRDRConnector *drc; - sPAPRDRConnectorClass *drck; - sPAPRConfigureConnectorState *ccs; - sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE; - int rc; - const void *fdt; - - if (nargs != 2 || nret != 1) { - rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); - return; - } - - wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0); - - drc_index = rtas_ld(wa_addr, 0); - drc = spapr_dr_connector_by_index(drc_index); - if (!drc) { - trace_spapr_rtas_ibm_configure_connector_invalid(drc_index); - rc = RTAS_OUT_PARAM_ERROR; - goto out; - } - - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - fdt = drck->get_fdt(drc, NULL); - if (!fdt) { - trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index); - rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE; - goto out; - } - - ccs = spapr_ccs_find(spapr, drc_index); - if (!ccs) { - ccs = g_new0(sPAPRConfigureConnectorState, 1); - (void)drck->get_fdt(drc, &ccs->fdt_offset); - ccs->drc_index = drc_index; - spapr_ccs_add(spapr, ccs); - } - - do { - uint32_t tag; - const char *name; - const struct fdt_property *prop; - int fdt_offset_next, prop_len; - - tag = fdt_next_tag(fdt, ccs->fdt_offset, &fdt_offset_next); - - switch (tag) { - case FDT_BEGIN_NODE: - ccs->fdt_depth++; - name = fdt_get_name(fdt, ccs->fdt_offset, NULL); - - /* provide the name of the next OF node */ - wa_offset = CC_VAL_DATA_OFFSET; - rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset); - configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1); - resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD; - break; - case FDT_END_NODE: - ccs->fdt_depth--; - if (ccs->fdt_depth == 0) { - /* done sending the device tree, don't need to track - * the state anymore - */ - drck->set_configured(drc); - spapr_ccs_remove(spapr, ccs); - ccs = NULL; - resp = SPAPR_DR_CC_RESPONSE_SUCCESS; - } else { - resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT; - } - break; - case FDT_PROP: - prop = fdt_get_property_by_offset(fdt, ccs->fdt_offset, - &prop_len); - name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); - - /* provide the name of the next OF property */ - wa_offset = CC_VAL_DATA_OFFSET; - rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset); - configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1); - - /* provide the length and value of the OF property. data gets - * placed immediately after NULL terminator of the OF property's - * name string - */ - wa_offset += strlen(name) + 1, - rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len); - rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset); - configure_connector_st(wa_addr, wa_offset, prop->data, prop_len); - resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY; - break; - case FDT_END: - resp = SPAPR_DR_CC_RESPONSE_ERROR; - default: - /* keep seeking for an actionable tag */ - break; - } - if (ccs) { - ccs->fdt_offset = fdt_offset_next; - } - } while (resp == SPAPR_DR_CC_RESPONSE_CONTINUE); - - rc = resp; -out: - rtas_st(rets, 0, rc); -} - static struct rtas_call { const char *name; spapr_rtas_fn fn; @@ -790,12 +492,6 @@ static void core_rtas_register_types(void) rtas_set_power_level); spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level", rtas_get_power_level); - spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", - rtas_set_indicator); - spapr_rtas_register(RTAS_GET_SENSOR_STATE, "get-sensor-state", - rtas_get_sensor_state); - spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector", - rtas_ibm_configure_connector); } type_init(core_rtas_register_types) From 88af6ea5687b1fdad47dad44eb9ff67c459c7dcc Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 1 Jun 2017 10:36:24 +1000 Subject: [PATCH 04/17] spapr: Abolish DRC get_fdt method The DRConnectorClass includes a get_fdt method. However * There's only one implementation, and there's only likely to ever be one * Both callers are local to spapr_drc * Each caller only uses one half of the actual implementation So abolish get_fdt() entirely, and just open-code what we need. Signed-off-by: David Gibson Reviewed-by: Laurent Vivier Reviewed-by: Greg Kurz Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 23 ++++++----------------- include/hw/ppc/spapr_drc.h | 1 - 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index ae8800da23..f5b7b68090 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -199,14 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc) return drc->name; } -static const void *get_fdt(sPAPRDRConnector *drc, int *fdt_start_offset) -{ - if (fdt_start_offset) { - *fdt_start_offset = drc->fdt_start_offset; - } - return drc->fdt; -} - static void set_configured(sPAPRDRConnector *drc) { trace_spapr_drc_set_configured(get_index(drc)); @@ -753,7 +745,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) drck->get_index = get_index; drck->get_type = get_type; drck->get_name = get_name; - drck->get_fdt = get_fdt; drck->set_configured = set_configured; drck->entity_sense = entity_sense; drck->attach = attach; @@ -1126,7 +1117,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, sPAPRConfigureConnectorState *ccs; sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE; int rc; - const void *fdt; if (nargs != 2 || nret != 1) { rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); @@ -1144,8 +1134,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, } drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - fdt = drck->get_fdt(drc, NULL); - if (!fdt) { + if (!drc->fdt) { trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index); rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE; goto out; @@ -1154,7 +1143,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, ccs = spapr_ccs_find(spapr, drc_index); if (!ccs) { ccs = g_new0(sPAPRConfigureConnectorState, 1); - (void)drck->get_fdt(drc, &ccs->fdt_offset); + ccs->fdt_offset = drc->fdt_start_offset; ccs->drc_index = drc_index; spapr_ccs_add(spapr, ccs); } @@ -1165,12 +1154,12 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, const struct fdt_property *prop; int fdt_offset_next, prop_len; - tag = fdt_next_tag(fdt, ccs->fdt_offset, &fdt_offset_next); + tag = fdt_next_tag(drc->fdt, ccs->fdt_offset, &fdt_offset_next); switch (tag) { case FDT_BEGIN_NODE: ccs->fdt_depth++; - name = fdt_get_name(fdt, ccs->fdt_offset, NULL); + name = fdt_get_name(drc->fdt, ccs->fdt_offset, NULL); /* provide the name of the next OF node */ wa_offset = CC_VAL_DATA_OFFSET; @@ -1193,9 +1182,9 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, } break; case FDT_PROP: - prop = fdt_get_property_by_offset(fdt, ccs->fdt_offset, + prop = fdt_get_property_by_offset(drc->fdt, ccs->fdt_offset, &prop_len); - name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); + name = fdt_string(drc->fdt, fdt32_to_cpu(prop->nameoff)); /* provide the name of the next OF property */ wa_offset = CC_VAL_DATA_OFFSET; diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 813b9ffd60..80db9553c6 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -178,7 +178,6 @@ typedef struct sPAPRDRConnectorClass { uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state); /* QEMU interfaces for managing FDT/configure-connector */ - const void *(*get_fdt)(sPAPRDRConnector *drc, int *fdt_start_offset); void (*set_configured)(sPAPRDRConnector *drc); /* QEMU interfaces for managing hotplug operations */ From 4f65ce00ab9b059ed1aadc18718760ffd0c60a68 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 2 Jun 2017 13:36:10 +1000 Subject: [PATCH 05/17] spapr: Abolish DRC set_configured method DRConnectorClass has a set_configured method, however: * There is only one implementation, and only ever likely to be one * There's exactly one caller, and that's (now) local * The implementation is very straightforward So abolish the method entirely, and just open-code what we need. Signed-off-by: David Gibson Reviewed-by: Laurent Vivier Reviewed-by: Greg Kurz Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 24 ++++++++---------------- include/hw/ppc/spapr_drc.h | 3 --- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index f5b7b68090..693571af9f 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -199,18 +199,6 @@ static const char *get_name(sPAPRDRConnector *drc) return drc->name; } -static void set_configured(sPAPRDRConnector *drc) -{ - trace_spapr_drc_set_configured(get_index(drc)); - - if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_UNISOLATED) { - /* guest should be not configuring an isolated device */ - trace_spapr_drc_set_configured_skipping(get_index(drc)); - return; - } - drc->configured = true; -} - /* has the guest been notified of device attachment? */ static void set_signalled(sPAPRDRConnector *drc) { @@ -745,7 +733,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) drck->get_index = get_index; drck->get_type = get_type; drck->get_name = get_name; - drck->set_configured = set_configured; drck->entity_sense = entity_sense; drck->attach = attach; drck->detach = detach; @@ -1113,7 +1100,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, uint64_t wa_offset; uint32_t drc_index; sPAPRDRConnector *drc; - sPAPRDRConnectorClass *drck; sPAPRConfigureConnectorState *ccs; sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE; int rc; @@ -1133,7 +1119,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, goto out; } - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); if (!drc->fdt) { trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index); rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE; @@ -1170,10 +1155,17 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, case FDT_END_NODE: ccs->fdt_depth--; if (ccs->fdt_depth == 0) { + sPAPRDRIsolationState state = drc->isolation_state; /* done sending the device tree, don't need to track * the state anymore */ - drck->set_configured(drc); + trace_spapr_drc_set_configured(get_index(drc)); + if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) { + drc->configured = true; + } else { + /* guest should be not configuring an isolated device */ + trace_spapr_drc_set_configured_skipping(get_index(drc)); + } spapr_ccs_remove(spapr, ccs); ccs = NULL; resp = SPAPR_DR_CC_RESPONSE_SUCCESS; diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 80db9553c6..90f49534d6 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -177,9 +177,6 @@ typedef struct sPAPRDRConnectorClass { uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state); - /* QEMU interfaces for managing FDT/configure-connector */ - void (*set_configured)(sPAPRDRConnector *drc); - /* QEMU interfaces for managing hotplug operations */ void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt, int fdt_start_offset, bool coldplug, Error **errp); From 0b55aa91c976b30b527c123fb66d25f5db43d083 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 2 Jun 2017 13:49:20 +1000 Subject: [PATCH 06/17] spapr: Make DRC get_index and get_type methods into plain functions These two methods only have one implementation, and the spec they're implementing means any other implementation is unlikely, verging on impossible. So replace them with simple functions. Signed-off-by: David Gibson Reviewed-by: Laurent Vivier Tested-by: Daniel Barboza --- hw/ppc/spapr.c | 13 +++----- hw/ppc/spapr_drc.c | 66 ++++++++++++++++++-------------------- hw/ppc/spapr_events.c | 10 +++--- hw/ppc/spapr_pci.c | 4 +-- include/hw/ppc/spapr_drc.h | 5 +-- 5 files changed, 44 insertions(+), 54 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index ab3aab1279..5d10366fdd 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -456,15 +456,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)}; int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu)); sPAPRDRConnector *drc; - sPAPRDRConnectorClass *drck; int drc_index; uint32_t radix_AP_encodings[PPC_PAGE_SIZES_MAX_SZ]; int i; drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index); if (drc) { - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - drc_index = drck->get_index(drc); + drc_index = spapr_drc_index(drc); _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index))); } @@ -654,15 +652,13 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt) if (i >= hotplug_lmb_start) { sPAPRDRConnector *drc; - sPAPRDRConnectorClass *drck; drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, i); g_assert(drc); - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); dynamic_memory[0] = cpu_to_be32(addr >> 32); dynamic_memory[1] = cpu_to_be32(addr & 0xffffffff); - dynamic_memory[2] = cpu_to_be32(drck->get_index(drc)); + dynamic_memory[2] = cpu_to_be32(spapr_drc_index(drc)); dynamic_memory[3] = cpu_to_be32(0); /* reserved */ dynamic_memory[4] = cpu_to_be32(numa_get_node(addr, NULL)); if (memory_region_present(get_system_memory(), addr)) { @@ -2560,7 +2556,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); spapr_hotplug_req_add_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs, - drck->get_index(drc)); + spapr_drc_index(drc)); } else { spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs); @@ -2770,8 +2766,7 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, addr_start / SPAPR_MEMORY_BLOCK_SIZE); drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); spapr_hotplug_req_remove_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB, - nr_lmbs, - drck->get_index(drc)); + nr_lmbs, spapr_drc_index(drc)); out: error_propagate(errp, local_err); } diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 693571af9f..a35314e099 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -70,7 +70,7 @@ static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type) return shift; } -static uint32_t get_index(sPAPRDRConnector *drc) +uint32_t spapr_drc_index(sPAPRDRConnector *drc) { /* no set format for a drc index: it only needs to be globally * unique. this is how we encode the DRC type on bare-metal @@ -85,7 +85,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc, { sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - trace_spapr_drc_set_isolation_state(get_index(drc), state); + trace_spapr_drc_set_isolation_state(spapr_drc_index(drc), state); if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) { /* cannot unisolate a non-existent resource, and, or resources @@ -126,11 +126,12 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc, * PAPR+ 2.7, 13.4 */ if (drc->awaiting_release) { + uint32_t drc_index = spapr_drc_index(drc); if (drc->configured) { - trace_spapr_drc_set_isolation_state_finalizing(get_index(drc)); + trace_spapr_drc_set_isolation_state_finalizing(drc_index); drck->detach(drc, DEVICE(drc->dev), NULL); } else { - trace_spapr_drc_set_isolation_state_deferring(get_index(drc)); + trace_spapr_drc_set_isolation_state_deferring(drc_index); } } drc->configured = false; @@ -142,7 +143,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc, static uint32_t set_indicator_state(sPAPRDRConnector *drc, sPAPRDRIndicatorState state) { - trace_spapr_drc_set_indicator_state(get_index(drc), state); + trace_spapr_drc_set_indicator_state(spapr_drc_index(drc), state); drc->indicator_state = state; return RTAS_OUT_SUCCESS; } @@ -152,7 +153,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc, { sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - trace_spapr_drc_set_allocation_state(get_index(drc), state); + trace_spapr_drc_set_allocation_state(spapr_drc_index(drc), state); if (state == SPAPR_DR_ALLOCATION_STATE_USABLE) { /* if there's no resource/device associated with the DRC, there's @@ -180,7 +181,8 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc, drc->allocation_state = state; if (drc->awaiting_release && drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { - trace_spapr_drc_set_allocation_state_finalizing(get_index(drc)); + uint32_t drc_index = spapr_drc_index(drc); + trace_spapr_drc_set_allocation_state_finalizing(drc_index); drck->detach(drc, DEVICE(drc->dev), NULL); } else if (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) { drc->awaiting_allocation = false; @@ -189,7 +191,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc, return RTAS_OUT_SUCCESS; } -static uint32_t get_type(sPAPRDRConnector *drc) +sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc) { return drc->type; } @@ -243,7 +245,7 @@ static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state) } } - trace_spapr_drc_entity_sense(get_index(drc), *state); + trace_spapr_drc_entity_sense(spapr_drc_index(drc), *state); return RTAS_OUT_SUCCESS; } @@ -251,8 +253,7 @@ static void prop_get_index(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - uint32_t value = (uint32_t)drck->get_index(drc); + uint32_t value = spapr_drc_index(drc); visit_type_uint32(v, name, &value, errp); } @@ -260,8 +261,7 @@ static void prop_get_type(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - uint32_t value = (uint32_t)drck->get_type(drc); + uint32_t value = (uint32_t)spapr_drc_type(drc); visit_type_uint32(v, name, &value, errp); } @@ -362,7 +362,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name, static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, int fdt_start_offset, bool coldplug, Error **errp) { - trace_spapr_drc_attach(get_index(drc)); + trace_spapr_drc_attach(spapr_drc_index(drc)); if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) { error_setg(errp, "an attached device is still awaiting release"); @@ -413,7 +413,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) { - trace_spapr_drc_detach(get_index(drc)); + trace_spapr_drc_detach(spapr_drc_index(drc)); /* if we've signalled device presence to the guest, or if the guest * has gone ahead and configured the device (via manually-executed @@ -436,14 +436,14 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) } if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) { - trace_spapr_drc_awaiting_isolated(get_index(drc)); + trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc)); drc->awaiting_release = true; return; } if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI && drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { - trace_spapr_drc_awaiting_unusable(get_index(drc)); + trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc)); drc->awaiting_release = true; return; } @@ -451,7 +451,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) if (drc->awaiting_allocation) { if (!drc->awaiting_allocation_skippable) { drc->awaiting_release = true; - trace_spapr_drc_awaiting_allocation(get_index(drc)); + trace_spapr_drc_awaiting_allocation(spapr_drc_index(drc)); return; } } @@ -495,7 +495,7 @@ static void reset(DeviceState *d) sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); sPAPRDREntitySense state; - trace_spapr_drc_reset(drck->get_index(drc)); + trace_spapr_drc_reset(spapr_drc_index(drc)); /* immediately upon reset we can safely assume DRCs whose devices * are pending removal can be safely removed, and that they will * subsequently be left in an ISOLATED state. move the DRC to this @@ -584,13 +584,12 @@ static const VMStateDescription vmstate_spapr_drc = { static void realize(DeviceState *d, Error **errp) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d); - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); Object *root_container; char link_name[256]; gchar *child_name; Error *err = NULL; - trace_spapr_drc_realize(drck->get_index(drc)); + trace_spapr_drc_realize(spapr_drc_index(drc)); /* NOTE: we do this as part of realize/unrealize due to the fact * that the guest will communicate with the DRC via RTAS calls * referencing the global DRC index. By unlinking the DRC @@ -599,9 +598,9 @@ static void realize(DeviceState *d, Error **errp) * existing in the composition tree */ root_container = container_get(object_get_root(), DRC_CONTAINER_PATH); - snprintf(link_name, sizeof(link_name), "%x", drck->get_index(drc)); + snprintf(link_name, sizeof(link_name), "%x", spapr_drc_index(drc)); child_name = object_get_canonical_path_component(OBJECT(drc)); - trace_spapr_drc_realize_child(drck->get_index(drc), child_name); + trace_spapr_drc_realize_child(spapr_drc_index(drc), child_name); object_property_add_alias(root_container, link_name, drc->owner, child_name, &err); if (err) { @@ -609,22 +608,21 @@ static void realize(DeviceState *d, Error **errp) object_unref(OBJECT(drc)); } g_free(child_name); - vmstate_register(DEVICE(drc), drck->get_index(drc), &vmstate_spapr_drc, + vmstate_register(DEVICE(drc), spapr_drc_index(drc), &vmstate_spapr_drc, drc); - trace_spapr_drc_realize_complete(drck->get_index(drc)); + trace_spapr_drc_realize_complete(spapr_drc_index(drc)); } static void unrealize(DeviceState *d, Error **errp) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d); - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); Object *root_container; char name[256]; Error *err = NULL; - trace_spapr_drc_unrealize(drck->get_index(drc)); + trace_spapr_drc_unrealize(spapr_drc_index(drc)); root_container = container_get(object_get_root(), DRC_CONTAINER_PATH); - snprintf(name, sizeof(name), "%x", drck->get_index(drc)); + snprintf(name, sizeof(name), "%x", spapr_drc_index(drc)); object_property_del(root_container, name, &err); if (err) { error_report_err(err); @@ -645,7 +643,8 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, drc->type = type; drc->id = id; drc->owner = owner; - prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", get_index(drc)); + prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", + spapr_drc_index(drc)); object_property_add_child(owner, prop_name, OBJECT(drc), NULL); object_property_set_bool(OBJECT(drc), true, "realized", NULL); g_free(prop_name); @@ -730,8 +729,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) drck->set_isolation_state = set_isolation_state; drck->set_indicator_state = set_indicator_state; drck->set_allocation_state = set_allocation_state; - drck->get_index = get_index; - drck->get_type = get_type; drck->get_name = get_name; drck->entity_sense = entity_sense; drck->attach = attach; @@ -868,7 +865,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, drc_count++; /* ibm,drc-indexes */ - drc_index = cpu_to_be32(drck->get_index(drc)); + drc_index = cpu_to_be32(spapr_drc_index(drc)); g_array_append_val(drc_indexes, drc_index); /* ibm,drc-power-domains */ @@ -1156,15 +1153,16 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, ccs->fdt_depth--; if (ccs->fdt_depth == 0) { sPAPRDRIsolationState state = drc->isolation_state; + uint32_t drc_index = spapr_drc_index(drc); /* done sending the device tree, don't need to track * the state anymore */ - trace_spapr_drc_set_configured(get_index(drc)); + trace_spapr_drc_set_configured(drc_index); if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) { drc->configured = true; } else { /* guest should be not configuring an isolated device */ - trace_spapr_drc_set_configured_skipping(get_index(drc)); + trace_spapr_drc_set_configured_skipping(drc_index); } spapr_ccs_remove(spapr, ccs); ccs = NULL; diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index 57acd85a87..d349ae5981 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -570,22 +570,20 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action, void spapr_hotplug_req_add_by_index(sPAPRDRConnector *drc) { - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - sPAPRDRConnectorType drc_type = drck->get_type(drc); + sPAPRDRConnectorType drc_type = spapr_drc_type(drc); union drc_identifier drc_id; - drc_id.index = drck->get_index(drc); + drc_id.index = spapr_drc_index(drc); spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX, RTAS_LOG_V6_HP_ACTION_ADD, drc_type, &drc_id); } void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc) { - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - sPAPRDRConnectorType drc_type = drck->get_type(drc); + sPAPRDRConnectorType drc_type = spapr_drc_type(drc); union drc_identifier drc_id; - drc_id.index = drck->get_index(drc); + drc_id.index = spapr_drc_index(drc); spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX, RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id); } diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index e4daf8d5f1..7a208a720e 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1417,14 +1417,12 @@ static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, PCIDevice *pdev) { sPAPRDRConnector *drc = spapr_phb_get_pci_drc(phb, pdev); - sPAPRDRConnectorClass *drck; if (!drc) { return 0; } - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - return drck->get_index(drc); + return spapr_drc_index(drc); } static void spapr_phb_hot_plug_child(HotplugHandler *plug_handler, diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 90f49534d6..10e7c2473c 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -171,8 +171,6 @@ typedef struct sPAPRDRConnectorClass { sPAPRDRIndicatorState state); uint32_t (*set_allocation_state)(sPAPRDRConnector *drc, sPAPRDRAllocationState state); - uint32_t (*get_index)(sPAPRDRConnector *drc); - uint32_t (*get_type)(sPAPRDRConnector *drc); const char *(*get_name)(sPAPRDRConnector *drc); uint32_t (*entity_sense)(sPAPRDRConnector *drc, sPAPRDREntitySense *state); @@ -185,6 +183,9 @@ typedef struct sPAPRDRConnectorClass { void (*set_signalled)(sPAPRDRConnector *drc); } sPAPRDRConnectorClass; +uint32_t spapr_drc_index(sPAPRDRConnector *drc); +sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc); + sPAPRDRConnector *spapr_dr_connector_new(Object *owner, sPAPRDRConnectorType type, uint32_t id); From a09f7443bcccd1703adf7c2954af695629c784a5 Mon Sep 17 00:00:00 2001 From: Aaron Larson Date: Fri, 2 Jun 2017 04:32:59 -0700 Subject: [PATCH 07/17] target-ppc: Fix openpic timer read register offset openpic_tmr_read() is incorrectly computing register offset of the TCCR, TBCR, TVPR, and TDR registers when accessing the open pic timer registers. Specifically the offset of timer registers for openpic_tmr_read() is not accounting for the timer frequency reporting register (TFFR) which is the first register in the "tmr" memory region. openpic_tmr_write() *is* correctly computing the offset by adding 0x10f0 to the address prior to computing the register index. This patch instead subtracts 0x10 in both the read and write routines and eliminates some other gratuitous differences between the functions. Signed-off-by: Aaron Larson Signed-off-by: David Gibson --- hw/intc/openpic.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index 4349e45e04..f966d0604a 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -796,27 +796,24 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr addr, unsigned len) } static void openpic_tmr_write(void *opaque, hwaddr addr, uint64_t val, - unsigned len) + unsigned len) { OpenPICState *opp = opaque; int idx; - addr += 0x10f0; - DPRINTF("%s: addr %#" HWADDR_PRIx " <= %08" PRIx64 "\n", - __func__, addr, val); + __func__, (addr + 0x10f0), val); if (addr & 0xF) { return; } - if (addr == 0x10f0) { + if (addr == 0) { /* TFRR */ opp->tfrr = val; return; } - + addr -= 0x10; /* correct for TFRR */ idx = (addr >> 6) & 0x3; - addr = addr & 0x30; switch (addr & 0x30) { case 0x00: /* TCCR */ @@ -844,16 +841,17 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len) uint32_t retval = -1; int idx; - DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr); + DPRINTF("%s: addr %#" HWADDR_PRIx "\n", __func__, addr + 0x10f0); if (addr & 0xF) { goto out; } - idx = (addr >> 6) & 0x3; - if (addr == 0x0) { + if (addr == 0) { /* TFRR */ retval = opp->tfrr; goto out; } + addr -= 0x10; /* correct for TFRR */ + idx = (addr >> 6) & 0x3; switch (addr & 0x30) { case 0x00: /* TCCR */ retval = opp->timers[idx].tccr; @@ -861,10 +859,10 @@ static uint64_t openpic_tmr_read(void *opaque, hwaddr addr, unsigned len) case 0x10: /* TBCR */ retval = opp->timers[idx].tbcr; break; - case 0x20: /* TIPV */ + case 0x20: /* TVPR */ retval = read_IRQreg_ivpr(opp, opp->irq_tim0 + idx); break; - case 0x30: /* TIDE (TIDR) */ + case 0x30: /* TDR */ retval = read_IRQreg_idr(opp, opp->irq_tim0 + idx); break; } From 60694bc67837ac1678fa9d67e0f5f83f8cf5436e Mon Sep 17 00:00:00 2001 From: Suraj Jitindar Singh Date: Mon, 5 Jun 2017 10:49:51 +1000 Subject: [PATCH 08/17] target/ppc: Fixup set_spr error in h_register_process_table set_spr is used in the function h_register_process_table() to update the LPCR_GTSE and LPCR_UPRT values based on the flags passed by the guest. The set_spr function takes the last two arguments mask and value used to mask and set the value of the spr respectively. The current call site passes these arguments in the wrong order and thus bot GTSE and UPRT will be set irrespective, which is obviously incorrect. Rearrange the function call so that these arguments are passed in the correct order and the correct behaviour is exhibited. It is worth noting that this wasn't detected earlier since these were always both set in all cases where this H_CALL was made. Fixes: 6de833070ca2 ("target/ppc: Set UPRT and GTSE on all cpus in H_REGISTER_PROCESS_TABLE") Signed-off-by: Suraj Jitindar Singh Signed-off-by: David Gibson --- hw/ppc/spapr_hcall.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index aae5a62a61..aa1ffea9e5 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -992,9 +992,10 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, /* Update the UPRT and GTSE bits in the LPCR for all cpus */ CPU_FOREACH(cs) { - set_spr(cs, SPR_LPCR, LPCR_UPRT | LPCR_GTSE, + set_spr(cs, SPR_LPCR, ((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ? LPCR_UPRT : 0) | - ((flags & FLAG_GTSE) ? LPCR_GTSE : 0)); + ((flags & FLAG_GTSE) ? LPCR_GTSE : 0), + LPCR_UPRT | LPCR_GTSE); } if (kvm_enabled()) { From 052495178821fdc97b4125a8677c1b68eb458db9 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 5 Jun 2017 16:14:17 +0100 Subject: [PATCH 09/17] spapr_nvram: Check return value from blk_getlength() The blk_getlength() function can return an error value if the image size cannot be determined. Check for this rather than ploughing on and trying to g_malloc0() a negative number. (Spotted by Coverity, CID 1288484.) Signed-off-by: Peter Maydell Signed-off-by: David Gibson --- hw/nvram/spapr_nvram.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c index aa5d2c1f5f..bc355a4348 100644 --- a/hw/nvram/spapr_nvram.c +++ b/hw/nvram/spapr_nvram.c @@ -144,7 +144,15 @@ static void spapr_nvram_realize(VIOsPAPRDevice *dev, Error **errp) int ret; if (nvram->blk) { - nvram->size = blk_getlength(nvram->blk); + int64_t len = blk_getlength(nvram->blk); + + if (len < 0) { + error_setg_errno(errp, -len, + "could not get length of backing image"); + return; + } + + nvram->size = len; ret = blk_set_perm(nvram->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, From 7032d92ac83c13987be302b96ab1f26d4b0f404e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Mon, 5 Jun 2017 17:44:21 +0200 Subject: [PATCH 10/17] ppc/pnv: check the return value of fdt_setprop() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédric Le Goater [dwg: Correct typo in commit message] Signed-off-by: David Gibson --- hw/ppc/pnv.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 231ed9735b..89b6801f67 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -378,8 +378,9 @@ static void powernv_populate_ipmi_bt(ISADevice *d, void *fdt, int lpc_off) _FDT(node); g_free(name); - fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs)); - fdt_setprop(fdt, node, "compatible", compatible, sizeof(compatible)); + _FDT((fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs)))); + _FDT((fdt_setprop(fdt, node, "compatible", compatible, + sizeof(compatible)))); /* Mark it as reserved to avoid Linux trying to claim it */ _FDT((fdt_setprop_string(fdt, node, "status", "reserved"))); From c4e13492afad2c49d41ec2abf4727c69be637f9f Mon Sep 17 00:00:00 2001 From: Felipe Franciosi Date: Mon, 5 Jun 2017 16:55:18 +0100 Subject: [PATCH 11/17] spapr: Allow boot from vhost-*-scsi backends The current implementation of spapr_get_fw_dev_path() doesn't take into consideration vhost-*-scsi devices. This makes said devices unbootable on PPC as SLOF is unable to work out the path to scan boot disks. This makes VMs bootable on spapr when using vhost-*-scsi by implementing a disk path for VHostSCSICommon (which currently includes both vhost-user-scsi and vhost-scsi). Signed-off-by: Felipe Franciosi Signed-off-by: Mike Cui Signed-off-by: David Gibson --- hw/ppc/spapr.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 5d10366fdd..c1c095163c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -57,6 +57,7 @@ #include "hw/pci/pci.h" #include "hw/scsi/scsi.h" #include "hw/virtio/virtio-scsi.h" +#include "hw/virtio/vhost-scsi-common.h" #include "exec/address-spaces.h" #include "hw/usb.h" @@ -2384,6 +2385,7 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, ((type *)object_dynamic_cast(OBJECT(obj), (name))) SCSIDevice *d = CAST(SCSIDevice, dev, TYPE_SCSI_DEVICE); sPAPRPHBState *phb = CAST(sPAPRPHBState, dev, TYPE_SPAPR_PCI_HOST_BRIDGE); + VHostSCSICommon *vsc = CAST(VHostSCSICommon, dev, TYPE_VHOST_SCSI_COMMON); if (d) { void *spapr = CAST(void, bus->parent, "spapr-vscsi"); @@ -2440,6 +2442,12 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, return g_strdup_printf("pci@%"PRIX64, phb->buid); } + if (vsc) { + /* Same logic as virtio above */ + unsigned id = 0x1000000 | (vsc->target << 16) | vsc->lun; + return g_strdup_printf("disk@%"PRIX64, (uint64_t)id << 32); + } + return NULL; } From a32e900b8abf3541414ef192ef72b6381fde8d0c Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 2 Jun 2017 12:09:35 +0200 Subject: [PATCH 12/17] spapr/drc: don't migrate DRC of cold-plugged CPUs and LMBs As explained in commit 5c0139a8c2f0 ("spapr: fix default DRC state for coldplugged LMBs"), guests expect cold-plugged LMBs to be pre-allocated and unisolated. The same goes for cold-plugged CPUs. While here, let's convert g_assert(false) to the better self documenting g_assert_not_reached(). Signed-off-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_drc.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index a35314e099..c1d4b1041c 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -546,20 +546,16 @@ static bool spapr_drc_needed(void *opaque) */ switch (drc->type) { case SPAPR_DR_CONNECTOR_TYPE_PCI: - rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) && - (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) && - drc->configured && drc->signalled && !drc->awaiting_release); - break; case SPAPR_DR_CONNECTOR_TYPE_CPU: case SPAPR_DR_CONNECTOR_TYPE_LMB: - rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) && - (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) && + rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) && + (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) && drc->configured && drc->signalled && !drc->awaiting_release); break; case SPAPR_DR_CONNECTOR_TYPE_PHB: case SPAPR_DR_CONNECTOR_TYPE_VIO: default: - g_assert(false); + g_assert_not_reached(); } return rc; } From 2d33581899edcaba9b7e48def11c4ddfcd2423ec Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 4 Jun 2017 20:25:17 +1000 Subject: [PATCH 13/17] spapr: Introduce DRC subclasses Currently we only have a single QOM type for all DRCs, but lots of places where we switch behaviour based on the DRC's PAPR defined type. This is a poor use of our existing type system. So, instead create QOM subclasses for each PAPR defined DRC type. We also introduce intermediate subclasses for physical and logical DRCs, a division which will be useful later on. Instead of being stored in the DRC object itself, the PAPR type is now stored in the class structure. There are still many places where we switch directly on the PAPR type value, but this at least provides the basis to start to remove those. Signed-off-by: David Gibson Reviewed-by: Michael Roth Acked-by: Michael Roth --- hw/ppc/spapr.c | 5 +- hw/ppc/spapr_drc.c | 123 +++++++++++++++++++++++++++---------- hw/ppc/spapr_pci.c | 3 +- include/hw/ppc/spapr_drc.h | 47 +++++++++++++- 4 files changed, 139 insertions(+), 39 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index c1c095163c..11a04d1aef 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1912,7 +1912,7 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr) uint64_t addr; addr = i * lmb_size + spapr->hotplug_memory.base; - drc = spapr_dr_connector_new(OBJECT(spapr), SPAPR_DR_CONNECTOR_TYPE_LMB, + drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB, addr/lmb_size); qemu_register_reset(spapr_drc_reset, drc); } @@ -2009,8 +2009,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr) if (mc->has_hotpluggable_cpus) { sPAPRDRConnector *drc = - spapr_dr_connector_new(OBJECT(spapr), - SPAPR_DR_CONNECTOR_TYPE_CPU, + spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU, (core_id / smp_threads) * smt); qemu_register_reset(spapr_drc_reset, drc); diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index c1d4b1041c..969d727b72 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -70,14 +70,23 @@ static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type) return shift; } +sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc) +{ + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + + return 1 << drck->typeshift; +} + uint32_t spapr_drc_index(sPAPRDRConnector *drc) { + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); + /* no set format for a drc index: it only needs to be globally * unique. this is how we encode the DRC type on bare-metal * however, so might as well do that here */ - return (get_type_shift(drc->type) << DRC_INDEX_TYPE_SHIFT) | - (drc->id & DRC_INDEX_ID_MASK); + return (drck->typeshift << DRC_INDEX_TYPE_SHIFT) + | (drc->id & DRC_INDEX_ID_MASK); } static uint32_t set_isolation_state(sPAPRDRConnector *drc, @@ -107,7 +116,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc, * If the LMB being removed doesn't belong to a DIMM device that is * actually being unplugged, fail the isolation request here. */ - if (drc->type == SPAPR_DR_CONNECTOR_TYPE_LMB) { + if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) { if ((state == SPAPR_DR_ISOLATION_STATE_ISOLATED) && !drc->awaiting_release) { return RTAS_OUT_HW_ERROR; @@ -177,7 +186,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc, } } - if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) { + if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) { drc->allocation_state = state; if (drc->awaiting_release && drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { @@ -191,11 +200,6 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc, return RTAS_OUT_SUCCESS; } -sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc) -{ - return drc->type; -} - static const char *get_name(sPAPRDRConnector *drc) { return drc->name; @@ -217,7 +221,7 @@ static void set_signalled(sPAPRDRConnector *drc) static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state) { if (drc->dev) { - if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI && + if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI && drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { /* for logical DR, we return a state of UNUSABLE * iff the allocation state UNUSABLE. @@ -235,7 +239,7 @@ static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state) *state = SPAPR_DR_ENTITY_SENSE_PRESENT; } } else { - if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) { + if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) { /* PCI devices, and only PCI devices, use EMPTY * in cases where we'd otherwise use UNUSABLE */ @@ -368,7 +372,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, error_setg(errp, "an attached device is still awaiting release"); return; } - if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) { + if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) { g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE); } g_assert(fdt || coldplug); @@ -380,7 +384,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, * may be accessing the device, we can easily crash the guest, so we * we defer completion of removal in such cases to the reset() hook. */ - if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) { + if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) { drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED; } drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE; @@ -398,10 +402,10 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, * 'physical' DR resources such as PCI where each device/resource is * signalled individually. */ - drc->signalled = (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) + drc->signalled = (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) ? true : coldplug; - if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) { + if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) { drc->awaiting_allocation = true; } @@ -441,7 +445,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) return; } - if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI && + if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI && drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc)); drc->awaiting_release = true; @@ -458,8 +462,8 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE; - /* Calling release callbacks based on drc->type. */ - switch (drc->type) { + /* Calling release callbacks based on spapr_drc_type(drc). */ + switch (spapr_drc_type(drc)) { case SPAPR_DR_CONNECTOR_TYPE_CPU: spapr_core_release(drc->dev); break; @@ -515,7 +519,7 @@ static void reset(DeviceState *d) } /* non-PCI devices may be awaiting a transition to UNUSABLE */ - if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI && + if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI && drc->awaiting_release) { drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_UNUSABLE); } @@ -544,7 +548,7 @@ static bool spapr_drc_needed(void *opaque) * If there is dev plugged in, we need to migrate the DRC state when * it is different from cold-plugged state */ - switch (drc->type) { + switch (spapr_drc_type(drc)) { case SPAPR_DR_CONNECTOR_TYPE_PCI: case SPAPR_DR_CONNECTOR_TYPE_CPU: case SPAPR_DR_CONNECTOR_TYPE_LMB: @@ -626,17 +630,12 @@ static void unrealize(DeviceState *d, Error **errp) } } -sPAPRDRConnector *spapr_dr_connector_new(Object *owner, - sPAPRDRConnectorType type, +sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type, uint32_t id) { - sPAPRDRConnector *drc = - SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR)); + sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type)); char *prop_name; - g_assert(type); - - drc->type = type; drc->id = id; drc->owner = owner; prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", @@ -666,7 +665,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, * DRC names as documented by PAPR+ v2.7, 13.5.2.4 * location codes as documented by PAPR+ v2.7, 12.3.1.5 */ - switch (drc->type) { + switch (spapr_drc_type(drc)) { case SPAPR_DR_CONNECTOR_TYPE_CPU: drc->name = g_strdup_printf("CPU %d", id); break; @@ -685,7 +684,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, } /* PCI slot always start in a USABLE state, and stay there */ - if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) { + if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) { drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE; } @@ -737,6 +736,27 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) dk->user_creatable = false; } +static void spapr_drc_cpu_class_init(ObjectClass *k, void *data) +{ + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); + + drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU; +} + +static void spapr_drc_pci_class_init(ObjectClass *k, void *data) +{ + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); + + drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI; +} + +static void spapr_drc_lmb_class_init(ObjectClass *k, void *data) +{ + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); + + drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB; +} + static const TypeInfo spapr_dr_connector_info = { .name = TYPE_SPAPR_DR_CONNECTOR, .parent = TYPE_DEVICE, @@ -744,6 +764,42 @@ static const TypeInfo spapr_dr_connector_info = { .instance_init = spapr_dr_connector_instance_init, .class_size = sizeof(sPAPRDRConnectorClass), .class_init = spapr_dr_connector_class_init, + .abstract = true, +}; + +static const TypeInfo spapr_drc_physical_info = { + .name = TYPE_SPAPR_DRC_PHYSICAL, + .parent = TYPE_SPAPR_DR_CONNECTOR, + .instance_size = sizeof(sPAPRDRConnector), + .abstract = true, +}; + +static const TypeInfo spapr_drc_logical_info = { + .name = TYPE_SPAPR_DRC_LOGICAL, + .parent = TYPE_SPAPR_DR_CONNECTOR, + .instance_size = sizeof(sPAPRDRConnector), + .abstract = true, +}; + +static const TypeInfo spapr_drc_cpu_info = { + .name = TYPE_SPAPR_DRC_CPU, + .parent = TYPE_SPAPR_DRC_LOGICAL, + .instance_size = sizeof(sPAPRDRConnector), + .class_init = spapr_drc_cpu_class_init, +}; + +static const TypeInfo spapr_drc_pci_info = { + .name = TYPE_SPAPR_DRC_PCI, + .parent = TYPE_SPAPR_DRC_PHYSICAL, + .instance_size = sizeof(sPAPRDRConnector), + .class_init = spapr_drc_pci_class_init, +}; + +static const TypeInfo spapr_drc_lmb_info = { + .name = TYPE_SPAPR_DRC_LMB, + .parent = TYPE_SPAPR_DRC_LOGICAL, + .instance_size = sizeof(sPAPRDRConnector), + .class_init = spapr_drc_lmb_class_init, }; /* helper functions for external users */ @@ -854,7 +910,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, continue; } - if ((drc->type & drc_type_mask) == 0) { + if ((spapr_drc_type(drc) & drc_type_mask) == 0) { continue; } @@ -874,7 +930,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, /* ibm,drc-types */ drc_types = g_string_append(drc_types, - spapr_drc_get_type_str(drc->type)); + spapr_drc_get_type_str(spapr_drc_type(drc))); drc_types = g_string_insert_len(drc_types, -1, "\0", 1); } @@ -1206,6 +1262,11 @@ out: static void spapr_drc_register_types(void) { type_register_static(&spapr_dr_connector_info); + type_register_static(&spapr_drc_physical_info); + type_register_static(&spapr_drc_logical_info); + type_register_static(&spapr_drc_cpu_info); + type_register_static(&spapr_drc_pci_info); + type_register_static(&spapr_drc_lmb_info); spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator", rtas_set_indicator); diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 7a208a720e..50d673bace 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1761,8 +1761,7 @@ 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), - SPAPR_DR_CONNECTOR_TYPE_PCI, + spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI, (sphb->index << 16) | i); } } diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 10e7c2473c..969c16d885 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -26,6 +26,48 @@ #define SPAPR_DR_CONNECTOR(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \ TYPE_SPAPR_DR_CONNECTOR) +#define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical" +#define SPAPR_DRC_PHYSICAL_GET_CLASS(obj) \ + OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHYSICAL) +#define SPAPR_DRC_PHYSICAL_CLASS(klass) \ + OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \ + TYPE_SPAPR_DRC_PHYSICAL) +#define SPAPR_DRC_PHYSICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \ + TYPE_SPAPR_DRC_PHYSICAL) + +#define TYPE_SPAPR_DRC_LOGICAL "spapr-drc-logical" +#define SPAPR_DRC_LOGICAL_GET_CLASS(obj) \ + OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LOGICAL) +#define SPAPR_DRC_LOGICAL_CLASS(klass) \ + OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \ + TYPE_SPAPR_DRC_LOGICAL) +#define SPAPR_DRC_LOGICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \ + TYPE_SPAPR_DRC_LOGICAL) + +#define TYPE_SPAPR_DRC_CPU "spapr-drc-cpu" +#define SPAPR_DRC_CPU_GET_CLASS(obj) \ + OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_CPU) +#define SPAPR_DRC_CPU_CLASS(klass) \ + OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_CPU) +#define SPAPR_DRC_CPU(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \ + TYPE_SPAPR_DRC_CPU) + +#define TYPE_SPAPR_DRC_PCI "spapr-drc-pci" +#define SPAPR_DRC_PCI_GET_CLASS(obj) \ + OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PCI) +#define SPAPR_DRC_PCI_CLASS(klass) \ + OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PCI) +#define SPAPR_DRC_PCI(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \ + TYPE_SPAPR_DRC_PCI) + +#define TYPE_SPAPR_DRC_LMB "spapr-drc-lmb" +#define SPAPR_DRC_LMB_GET_CLASS(obj) \ + OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LMB) +#define SPAPR_DRC_LMB_CLASS(klass) \ + OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_LMB) +#define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \ + TYPE_SPAPR_DRC_LMB) + /* * Various hotplug types managed by sPAPRDRConnector * @@ -134,7 +176,6 @@ typedef struct sPAPRDRConnector { /*< private >*/ DeviceState parent; - sPAPRDRConnectorType type; uint32_t id; Object *owner; const char *name; @@ -163,6 +204,7 @@ typedef struct sPAPRDRConnectorClass { DeviceClass parent; /*< public >*/ + sPAPRDRConnectorTypeShift typeshift; /* accessors for guest-visible (generally via RTAS) DR state */ uint32_t (*set_isolation_state)(sPAPRDRConnector *drc, @@ -186,8 +228,7 @@ typedef struct sPAPRDRConnectorClass { uint32_t spapr_drc_index(sPAPRDRConnector *drc); sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc); -sPAPRDRConnector *spapr_dr_connector_new(Object *owner, - sPAPRDRConnectorType type, +sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type, uint32_t id); sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index); sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type, From fbf553971898aee18b5933d335e2fa3e74bb9be7 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 4 Jun 2017 20:26:03 +1000 Subject: [PATCH 14/17] spapr: Clean up spapr_dr_connector_by_*() * Change names to something less ludicrously verbose * Now that we have QOM subclasses for the different DRC types, use a QOM typename instead of a PAPR type value parameter The latter allows removal of the get_type_shift() helper. Signed-off-by: David Gibson Reviewed-by: Michael Roth Acked-by: Michael Roth --- hw/ppc/spapr.c | 28 ++++++++++++++-------------- hw/ppc/spapr_drc.c | 34 ++++++++++------------------------ hw/ppc/spapr_events.c | 2 +- hw/ppc/spapr_pci.c | 6 ++---- include/hw/ppc/spapr_drc.h | 5 ++--- 5 files changed, 29 insertions(+), 46 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 11a04d1aef..3fa92632dd 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -461,7 +461,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, uint32_t radix_AP_encodings[PPC_PAGE_SIZES_MAX_SZ]; int i; - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index); if (drc) { drc_index = spapr_drc_index(drc); _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index))); @@ -654,7 +654,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt) if (i >= hotplug_lmb_start) { sPAPRDRConnector *drc; - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, i); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, i); g_assert(drc); dynamic_memory[0] = cpu_to_be32(addr >> 32); @@ -2536,8 +2536,8 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, uint64_t addr = addr_start; for (i = 0; i < nr_lmbs; i++) { - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, - addr/SPAPR_MEMORY_BLOCK_SIZE); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, + addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); fdt = create_device_tree(&fdt_size); @@ -2558,8 +2558,8 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, */ if (dev->hotplugged) { if (dedicated_hp_event_source) { - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, - addr_start / SPAPR_MEMORY_BLOCK_SIZE); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, + addr_start / SPAPR_MEMORY_BLOCK_SIZE); drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); spapr_hotplug_req_add_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs, @@ -2676,8 +2676,8 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms, addr = addr_start; for (i = 0; i < nr_lmbs; i++) { - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, - addr / SPAPR_MEMORY_BLOCK_SIZE); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, + addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); if (drc->indicator_state != SPAPR_DR_INDICATOR_STATE_INACTIVE) { avail_lmbs++; @@ -2760,8 +2760,8 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, addr = addr_start; for (i = 0; i < nr_lmbs; i++) { - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, - addr / SPAPR_MEMORY_BLOCK_SIZE); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, + addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); @@ -2769,8 +2769,8 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, addr += SPAPR_MEMORY_BLOCK_SIZE; } - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, - addr_start / SPAPR_MEMORY_BLOCK_SIZE); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, + addr_start / SPAPR_MEMORY_BLOCK_SIZE); drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); spapr_hotplug_req_remove_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs, spapr_drc_index(drc)); @@ -2841,7 +2841,7 @@ void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, return; } - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index * smt); g_assert(drc); drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); @@ -2876,7 +2876,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev, cc->core_id); return; } - drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index * smt); g_assert(drc || !mc->has_hotpluggable_cpus); diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 969d727b72..7ed2de1c25 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -55,21 +55,6 @@ static void spapr_ccs_remove(sPAPRMachineState *spapr, g_free(ccs); } -static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type) -{ - uint32_t shift = 0; - - /* make sure this isn't SPAPR_DR_CONNECTOR_TYPE_ANY, or some - * other wonky value. - */ - g_assert(is_power_of_2(type)); - - while (type != (1 << shift)) { - shift++; - } - return shift; -} - sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc) { sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); @@ -804,7 +789,7 @@ static const TypeInfo spapr_drc_lmb_info = { /* helper functions for external users */ -sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index) +sPAPRDRConnector *spapr_drc_by_index(uint32_t index) { Object *obj; char name[256]; @@ -815,12 +800,13 @@ sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index) return !obj ? NULL : SPAPR_DR_CONNECTOR(obj); } -sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type, - uint32_t id) +sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id) { - return spapr_dr_connector_by_index( - (get_type_shift(type) << DRC_INDEX_TYPE_SHIFT) | - (id & DRC_INDEX_ID_MASK)); + sPAPRDRConnectorClass *drck + = SPAPR_DR_CONNECTOR_CLASS(object_class_by_name(type)); + + return spapr_drc_by_index(drck->typeshift << DRC_INDEX_TYPE_SHIFT + | (id & DRC_INDEX_ID_MASK)); } /* generate a string the describes the DRC to encode into the @@ -1023,7 +1009,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr, } /* if this is a DR sensor we can assume sensor_index == drc_index */ - drc = spapr_dr_connector_by_index(sensor_index); + drc = spapr_drc_by_index(sensor_index); if (!drc) { trace_spapr_rtas_set_indicator_invalid(sensor_index); ret = RTAS_OUT_PARAM_ERROR; @@ -1096,7 +1082,7 @@ static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr, goto out; } - drc = spapr_dr_connector_by_index(sensor_index); + drc = spapr_drc_by_index(sensor_index); if (!drc) { trace_spapr_rtas_get_sensor_state_invalid(sensor_index); ret = RTAS_OUT_PARAM_ERROR; @@ -1161,7 +1147,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0); drc_index = rtas_ld(wa_addr, 0); - drc = spapr_dr_connector_by_index(drc_index); + drc = spapr_drc_by_index(drc_index); if (!drc) { trace_spapr_rtas_ibm_configure_connector_invalid(drc_index); rc = RTAS_OUT_PARAM_ERROR; diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index d349ae5981..171aedc7e0 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -477,7 +477,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque) static void spapr_hotplug_set_signalled(uint32_t drc_index) { - sPAPRDRConnector *drc = spapr_dr_connector_by_index(drc_index); + sPAPRDRConnector *drc = spapr_drc_by_index(drc_index); sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); drck->set_signalled(drc); } diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 50d673bace..0c181bbca5 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1400,10 +1400,8 @@ static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb, uint32_t busnr, int32_t devfn) { - return spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_PCI, - (phb->index << 16) | - (busnr << 8) | - devfn); + return spapr_drc_by_id(TYPE_SPAPR_DRC_PCI, + (phb->index << 16) | (busnr << 8) | devfn); } static sPAPRDRConnector *spapr_phb_get_pci_drc(sPAPRPHBState *phb, diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 969c16d885..e4a25c8aa1 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -230,9 +230,8 @@ sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc); sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type, uint32_t id); -sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index); -sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type, - uint32_t id); +sPAPRDRConnector *spapr_drc_by_index(uint32_t index); +sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id); int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, uint32_t drc_type_mask); From b8fdd530be3450940130b63d930bb0aee1538e7e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 4 Jun 2017 20:26:25 +1000 Subject: [PATCH 15/17] spapr: Move configure-connector state into DRC Currently the sPAPRMachineState contains a list of sPAPRConfigureConnector structures which store intermediate state for the ibm,configure-connector RTAS call. This was an attempt to separate this state from the core of the DRC state. However the configure connector process is intimately tied to the DRC model, so there's really no point trying to have two levels of interface here. Moving the configure-connector state into its corresponding DRC allows removal of a number of helpers for maintaining the anciliary list. Signed-off-by: David Gibson Reviewed-by: Michael Roth Acked-by: Michael Roth --- hw/ppc/spapr.c | 4 --- hw/ppc/spapr_drc.c | 73 ++++++++++---------------------------- include/hw/ppc/spapr.h | 14 -------- include/hw/ppc/spapr_drc.h | 7 ++++ 4 files changed, 25 insertions(+), 73 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 3fa92632dd..86e622834f 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2340,10 +2340,6 @@ static void ppc_spapr_init(MachineState *machine) register_savevm_live(NULL, "spapr/htab", -1, 1, &savevm_htab_handlers, spapr); - /* used by RTAS */ - QTAILQ_INIT(&spapr->ccs_list); - qemu_register_reset(spapr_ccs_reset_hook, spapr); - qemu_register_boot_set(spapr_boot_set, spapr); if (kvm_enabled()) { diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 7ed2de1c25..783c621551 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -27,34 +27,6 @@ #define DRC_INDEX_TYPE_SHIFT 28 #define DRC_INDEX_ID_MASK ((1ULL << DRC_INDEX_TYPE_SHIFT) - 1) -static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr, - uint32_t drc_index) -{ - sPAPRConfigureConnectorState *ccs = NULL; - - QTAILQ_FOREACH(ccs, &spapr->ccs_list, next) { - if (ccs->drc_index == drc_index) { - break; - } - } - - return ccs; -} - -static void spapr_ccs_add(sPAPRMachineState *spapr, - sPAPRConfigureConnectorState *ccs) -{ - g_assert(!spapr_ccs_find(spapr, ccs->drc_index)); - QTAILQ_INSERT_HEAD(&spapr->ccs_list, ccs, next); -} - -static void spapr_ccs_remove(sPAPRMachineState *spapr, - sPAPRConfigureConnectorState *ccs) -{ - QTAILQ_REMOVE(&spapr->ccs_list, ccs, next); - g_free(ccs); -} - sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc) { sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); @@ -81,6 +53,16 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc, trace_spapr_drc_set_isolation_state(spapr_drc_index(drc), state); + /* if the guest is configuring a device attached to this DRC, we + * should reset the configuration state at this point since it may + * no longer be reliable (guest released device and needs to start + * over, or unplug occurred so the FDT is no longer valid) + */ + if (state == SPAPR_DR_ISOLATION_STATE_ISOLATED) { + g_free(drc->ccs); + drc->ccs = NULL; + } + if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) { /* cannot unisolate a non-existent resource, and, or resources * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, 13.5.3.5) @@ -485,6 +467,10 @@ static void reset(DeviceState *d) sPAPRDREntitySense state; trace_spapr_drc_reset(spapr_drc_index(drc)); + + g_free(drc->ccs); + drc->ccs = NULL; + /* immediately upon reset we can safely assume DRCs whose devices * are pending removal can be safely removed, and that they will * subsequently be left in an ISOLATED state. move the DRC to this @@ -1019,19 +1005,6 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr, switch (sensor_type) { case RTAS_SENSOR_TYPE_ISOLATION_STATE: - /* if the guest is configuring a device attached to this - * DRC, we should reset the configuration state at this - * point since it may no longer be reliable (guest released - * device and needs to start over, or unplug occurred so - * the FDT is no longer valid) - */ - if (sensor_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) { - sPAPRConfigureConnectorState *ccs = spapr_ccs_find(spapr, - sensor_index); - if (ccs) { - spapr_ccs_remove(spapr, ccs); - } - } ret = drck->set_isolation_state(drc, sensor_state); break; case RTAS_SENSOR_TYPE_DR: @@ -1115,16 +1088,6 @@ static void configure_connector_st(target_ulong addr, target_ulong offset, buf, MIN(len, CC_WA_LEN - offset)); } -void spapr_ccs_reset_hook(void *opaque) -{ - sPAPRMachineState *spapr = opaque; - sPAPRConfigureConnectorState *ccs, *ccs_tmp; - - QTAILQ_FOREACH_SAFE(ccs, &spapr->ccs_list, next, ccs_tmp) { - spapr_ccs_remove(spapr, ccs); - } -} - static void rtas_ibm_configure_connector(PowerPCCPU *cpu, sPAPRMachineState *spapr, uint32_t token, uint32_t nargs, @@ -1160,12 +1123,11 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, goto out; } - ccs = spapr_ccs_find(spapr, drc_index); + ccs = drc->ccs; if (!ccs) { ccs = g_new0(sPAPRConfigureConnectorState, 1); ccs->fdt_offset = drc->fdt_start_offset; - ccs->drc_index = drc_index; - spapr_ccs_add(spapr, ccs); + drc->ccs = ccs; } do { @@ -1202,7 +1164,8 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, /* guest should be not configuring an isolated device */ trace_spapr_drc_set_configured_skipping(drc_index); } - spapr_ccs_remove(spapr, ccs); + g_free(ccs); + drc->ccs = NULL; ccs = NULL; resp = SPAPR_DR_CC_RESPONSE_SUCCESS; } else { diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 98fb78b012..f973b02845 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -11,7 +11,6 @@ struct VIOsPAPRBus; struct sPAPRPHBState; struct sPAPRNVRAM; -typedef struct sPAPRConfigureConnectorState sPAPRConfigureConnectorState; typedef struct sPAPREventLogEntry sPAPREventLogEntry; typedef struct sPAPREventSource sPAPREventSource; @@ -102,9 +101,6 @@ struct sPAPRMachineState { bool htab_first_pass; int htab_fd; - /* RTAS state */ - QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list; - /* Pending DIMM unplug cache. It is populated when a LMB * unplug starts. It can be regenerated if a migration * occurs during the unplug process. */ @@ -646,16 +642,6 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset, void spapr_core_release(DeviceState *dev); void spapr_lmb_release(DeviceState *dev); -/* rtas-configure-connector state */ -struct sPAPRConfigureConnectorState { - uint32_t drc_index; - int fdt_offset; - int fdt_depth; - QTAILQ_ENTRY(sPAPRConfigureConnectorState) next; -}; - -void spapr_ccs_reset_hook(void *opaque); - void spapr_rtc_read(sPAPRRTCState *rtc, struct tm *tm, uint32_t *ns); int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset); diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index e4a25c8aa1..7dbb478e8b 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -172,6 +172,12 @@ typedef enum { SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE = -9003, } sPAPRDRCCResponse; +/* rtas-configure-connector state */ +typedef struct sPAPRConfigureConnectorState { + int fdt_offset; + int fdt_depth; +} sPAPRConfigureConnectorState; + typedef struct sPAPRDRConnector { /*< private >*/ DeviceState parent; @@ -189,6 +195,7 @@ typedef struct sPAPRDRConnector { void *fdt; int fdt_start_offset; bool configured; + sPAPRConfigureConnectorState *ccs; bool awaiting_release; bool signalled; From 1693ea168575849bc2784e193002c317ef3eee39 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 4 Jun 2017 20:26:54 +1000 Subject: [PATCH 16/17] spapr: Eliminate spapr_drc_get_type_str() This function was used in generating the device tree. However, now that we have different QOM types for different DRC types we can easily store the information we need in the class structure and avoid this specialized lookup function. Signed-off-by: David Gibson Reviewed-by: Michael Roth Acked-by: Michael Roth --- hw/ppc/spapr_drc.c | 31 ++++--------------------------- include/hw/ppc/spapr_drc.h | 1 + 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 783c621551..06df5d0b13 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -712,6 +712,7 @@ static void spapr_drc_cpu_class_init(ObjectClass *k, void *data) sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU; + drck->typename = "CPU"; } static void spapr_drc_pci_class_init(ObjectClass *k, void *data) @@ -719,6 +720,7 @@ static void spapr_drc_pci_class_init(ObjectClass *k, void *data) sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI; + drck->typename = "28"; } static void spapr_drc_lmb_class_init(ObjectClass *k, void *data) @@ -726,6 +728,7 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, void *data) sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB; + drck->typename = "MEM"; } static const TypeInfo spapr_dr_connector_info = { @@ -795,31 +798,6 @@ sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id) | (id & DRC_INDEX_ID_MASK)); } -/* generate a string the describes the DRC to encode into the - * device tree. - * - * as documented by PAPR+ v2.7, 13.5.2.6 and C.6.1 - */ -static const char *spapr_drc_get_type_str(sPAPRDRConnectorType type) -{ - switch (type) { - case SPAPR_DR_CONNECTOR_TYPE_CPU: - return "CPU"; - case SPAPR_DR_CONNECTOR_TYPE_PHB: - return "PHB"; - case SPAPR_DR_CONNECTOR_TYPE_VIO: - return "SLOT"; - case SPAPR_DR_CONNECTOR_TYPE_PCI: - return "28"; - case SPAPR_DR_CONNECTOR_TYPE_LMB: - return "MEM"; - default: - g_assert(false); - } - - return NULL; -} - /** * spapr_drc_populate_dt * @@ -901,8 +879,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, drc_names = g_string_insert_len(drc_names, -1, "\0", 1); /* ibm,drc-types */ - drc_types = g_string_append(drc_types, - spapr_drc_get_type_str(spapr_drc_type(drc))); + drc_types = g_string_append(drc_types, drck->typename); drc_types = g_string_insert_len(drc_types, -1, "\0", 1); } diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 7dbb478e8b..c88e1beed4 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -212,6 +212,7 @@ typedef struct sPAPRDRConnectorClass { /*< public >*/ sPAPRDRConnectorTypeShift typeshift; + const char *typename; /* used in device tree, PAPR 13.5.2.6 & C.6.1 */ /* accessors for guest-visible (generally via RTAS) DR state */ uint32_t (*set_isolation_state)(sPAPRDRConnector *drc, From 91dcb1ffa67cfa41a13dcc89dd44e2310b5773b0 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 4 Jun 2017 20:28:04 +1000 Subject: [PATCH 17/17] spapr: Remove some non-useful properties on DRC objects * 'connector_type' is easily derived from the 'index' property, so there's no point to it (it's also implicit in the QOM type of the DRC) * 'isolation-state', 'indicator-state' and 'allocation-state' are part of the transaction between qemu and guest during PAPR hotplug operations, and outside tools really have no business looking at it (especially not changing, and these were RW properties) * 'entity-sense' is basically just a weird PAPR encoding of whether there is a device connected to this DRC Strictly speaking removing these properties is breaking the qemu interface. However, I'm pretty sure no management tools have ever used these. For debugging there are better alternatives. Therefore, I think removing these broken interfaces is the better option. Signed-off-by: David Gibson Reviewed-by: Michael Roth Acked-by: Michael Roth --- hw/ppc/spapr_drc.c | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 06df5d0b13..39e7f3080a 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -228,14 +228,6 @@ static void prop_get_index(Object *obj, Visitor *v, const char *name, visit_type_uint32(v, name, &value, errp); } -static void prop_get_type(Object *obj, Visitor *v, const char *name, - void *opaque, Error **errp) -{ - sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); - uint32_t value = (uint32_t)spapr_drc_type(drc); - visit_type_uint32(v, name, &value, errp); -} - static char *prop_get_name(Object *obj, Error **errp) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); @@ -243,17 +235,6 @@ static char *prop_get_name(Object *obj, Error **errp) return g_strdup(drck->get_name(drc)); } -static void prop_get_entity_sense(Object *obj, Visitor *v, const char *name, - void *opaque, Error **errp) -{ - sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - uint32_t value; - - drck->entity_sense(drc, &value); - visit_type_uint32(v, name, &value, errp); -} - static void prop_get_fdt(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { @@ -666,20 +647,10 @@ static void spapr_dr_connector_instance_init(Object *obj) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); - object_property_add_uint32_ptr(obj, "isolation-state", - &drc->isolation_state, NULL); - object_property_add_uint32_ptr(obj, "indicator-state", - &drc->indicator_state, NULL); - object_property_add_uint32_ptr(obj, "allocation-state", - &drc->allocation_state, NULL); object_property_add_uint32_ptr(obj, "id", &drc->id, NULL); object_property_add(obj, "index", "uint32", prop_get_index, NULL, NULL, NULL, NULL); - object_property_add(obj, "connector_type", "uint32", prop_get_type, - NULL, NULL, NULL, NULL); object_property_add_str(obj, "name", prop_get_name, NULL, NULL); - object_property_add(obj, "entity-sense", "uint32", prop_get_entity_sense, - NULL, NULL, NULL, NULL); object_property_add(obj, "fdt", "struct", prop_get_fdt, NULL, NULL, NULL, NULL); }