On 11/30/20 6:32 PM, Cédric Le Goater wrote: > On 11/30/20 5:52 PM, Greg Kurz wrote: >> The machine tells the IC backend the number of vCPU ids it will be >> exposed to, in order to: >> - fill the "ibm,interrupt-server-ranges" property in the DT (XICS) >> - size the VP block used by the in-kernel chip (XICS-on-XIVE, XIVE) >> >> The current "nr_servers" and "spapr_max_server_number" naming can >> mislead people info thinking it is about a quantity of CPUs. Make >> it clear this is all about vCPU ids. > > OK. This looks fine. > > But, XIVE does not care about vCPU ids. Only the count of vCPUs > is relevant. So, it would be nice to add a comment in the code > that we got it wrong at some point or that XICS imposed the use > of max vCPU ids.
Which you do in the next patch, Reviewed-by: Cédric Le Goater <c...@kaod.org> Thanks, C. > >> Signed-off-by: Greg Kurz <gr...@kaod.org> >> --- >> include/hw/ppc/spapr.h | 2 +- >> include/hw/ppc/spapr_irq.h | 8 ++++---- >> include/hw/ppc/spapr_xive.h | 2 +- >> include/hw/ppc/xics_spapr.h | 2 +- >> hw/intc/spapr_xive.c | 8 ++++---- >> hw/intc/spapr_xive_kvm.c | 4 ++-- >> hw/intc/xics_kvm.c | 4 ++-- >> hw/intc/xics_spapr.c | 8 ++++---- >> hw/ppc/spapr.c | 8 ++++---- >> hw/ppc/spapr_irq.c | 18 +++++++++--------- >> 10 files changed, 32 insertions(+), 32 deletions(-) >> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >> index b7ced9faebf5..dc99d45e2852 100644 >> --- a/include/hw/ppc/spapr.h >> +++ b/include/hw/ppc/spapr.h >> @@ -849,7 +849,7 @@ int spapr_hpt_shift_for_ramsize(uint64_t ramsize); >> int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp); >> void spapr_clear_pending_events(SpaprMachineState *spapr); >> void spapr_clear_pending_hotplug_events(SpaprMachineState *spapr); >> -int spapr_max_server_number(SpaprMachineState *spapr); >> +int spapr_max_vcpu_ids(SpaprMachineState *spapr); >> void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex, >> uint64_t pte0, uint64_t pte1); >> void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered); >> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h >> index c22a72c9e270..2e53fc9e6cbb 100644 >> --- a/include/hw/ppc/spapr_irq.h >> +++ b/include/hw/ppc/spapr_irq.h >> @@ -43,7 +43,7 @@ DECLARE_CLASS_CHECKERS(SpaprInterruptControllerClass, >> SPAPR_INTC, >> struct SpaprInterruptControllerClass { >> InterfaceClass parent; >> >> - int (*activate)(SpaprInterruptController *intc, uint32_t nr_servers, >> + int (*activate)(SpaprInterruptController *intc, uint32_t max_vcpu_ids, >> Error **errp); >> void (*deactivate)(SpaprInterruptController *intc); >> >> @@ -62,7 +62,7 @@ struct SpaprInterruptControllerClass { >> /* These methods should only be called on the active intc */ >> void (*set_irq)(SpaprInterruptController *intc, int irq, int val); >> void (*print_info)(SpaprInterruptController *intc, Monitor *mon); >> - void (*dt)(SpaprInterruptController *intc, uint32_t nr_servers, >> + void (*dt)(SpaprInterruptController *intc, uint32_t max_vcpu_ids, >> void *fdt, uint32_t phandle); >> int (*post_load)(SpaprInterruptController *intc, int version_id); >> }; >> @@ -74,7 +74,7 @@ int spapr_irq_cpu_intc_create(struct SpaprMachineState >> *spapr, >> void spapr_irq_cpu_intc_reset(struct SpaprMachineState *spapr, PowerPCCPU >> *cpu); >> void spapr_irq_cpu_intc_destroy(struct SpaprMachineState *spapr, PowerPCCPU >> *cpu); >> void spapr_irq_print_info(struct SpaprMachineState *spapr, Monitor *mon); >> -void spapr_irq_dt(struct SpaprMachineState *spapr, uint32_t nr_servers, >> +void spapr_irq_dt(struct SpaprMachineState *spapr, uint32_t max_vcpu_ids, >> void *fdt, uint32_t phandle); >> >> uint32_t spapr_irq_nr_msis(struct SpaprMachineState *spapr); >> @@ -105,7 +105,7 @@ typedef int >> (*SpaprInterruptControllerInitKvm)(SpaprInterruptController *, >> >> int spapr_irq_init_kvm(SpaprInterruptControllerInitKvm fn, >> SpaprInterruptController *intc, >> - uint32_t nr_servers, >> + uint32_t max_vcpu_ids, >> Error **errp); >> >> /* >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h >> index 26c8d90d7196..643129b13536 100644 >> --- a/include/hw/ppc/spapr_xive.h >> +++ b/include/hw/ppc/spapr_xive.h >> @@ -79,7 +79,7 @@ int spapr_xive_end_to_target(uint8_t end_blk, uint32_t >> end_idx, >> /* >> * KVM XIVE device helpers >> */ >> -int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, >> +int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t >> max_vcpu_ids, >> Error **errp); >> void kvmppc_xive_disconnect(SpaprInterruptController *intc); >> void kvmppc_xive_reset(SpaprXive *xive, Error **errp); >> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h >> index de752c0d2c7e..5c0e9430a964 100644 >> --- a/include/hw/ppc/xics_spapr.h >> +++ b/include/hw/ppc/xics_spapr.h >> @@ -35,7 +35,7 @@ >> DECLARE_INSTANCE_CHECKER(ICSState, ICS_SPAPR, >> TYPE_ICS_SPAPR) >> >> -int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, >> +int xics_kvm_connect(SpaprInterruptController *intc, uint32_t max_vcpu_ids, >> Error **errp); >> void xics_kvm_disconnect(SpaprInterruptController *intc); >> bool xics_kvm_has_broken_disconnect(void); >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >> index 12dd6d3ce357..d0a0ca822367 100644 >> --- a/hw/intc/spapr_xive.c >> +++ b/hw/intc/spapr_xive.c >> @@ -669,7 +669,7 @@ static void >> spapr_xive_print_info(SpaprInterruptController *intc, Monitor *mon) >> spapr_xive_pic_print_info(xive, mon); >> } >> >> -static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t >> nr_servers, >> +static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t >> max_vcpu_ids, >> void *fdt, uint32_t phandle) >> { >> SpaprXive *xive = SPAPR_XIVE(intc); >> @@ -678,7 +678,7 @@ static void spapr_xive_dt(SpaprInterruptController >> *intc, uint32_t nr_servers, >> /* Interrupt number ranges for the IPIs */ >> uint32_t lisn_ranges[] = { >> cpu_to_be32(SPAPR_IRQ_IPI), >> - cpu_to_be32(SPAPR_IRQ_IPI + nr_servers), >> + cpu_to_be32(SPAPR_IRQ_IPI + max_vcpu_ids), >> }; >> /* >> * EQ size - the sizes of pages supported by the system 4K, 64K, >> @@ -733,12 +733,12 @@ static void spapr_xive_dt(SpaprInterruptController >> *intc, uint32_t nr_servers, >> } >> >> static int spapr_xive_activate(SpaprInterruptController *intc, >> - uint32_t nr_servers, Error **errp) >> + uint32_t max_vcpu_ids, Error **errp) >> { >> SpaprXive *xive = SPAPR_XIVE(intc); >> >> if (kvm_enabled()) { >> - int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, nr_servers, >> + int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, max_vcpu_ids, >> errp); >> if (rc < 0) { >> return rc; >> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c >> index e8667ce5f621..2a938b4429a8 100644 >> --- a/hw/intc/spapr_xive_kvm.c >> +++ b/hw/intc/spapr_xive_kvm.c >> @@ -716,7 +716,7 @@ static void *kvmppc_xive_mmap(SpaprXive *xive, int >> pgoff, size_t len, >> * All the XIVE memory regions are now backed by mappings from the KVM >> * XIVE device. >> */ >> -int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, >> +int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t >> max_vcpu_ids, >> Error **errp) >> { >> SpaprXive *xive = SPAPR_XIVE(intc); >> @@ -753,7 +753,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, >> uint32_t nr_servers, >> if (kvm_device_check_attr(xive->fd, KVM_DEV_XIVE_GRP_CTRL, >> KVM_DEV_XIVE_NR_SERVERS)) { >> ret = kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_CTRL, >> - KVM_DEV_XIVE_NR_SERVERS, &nr_servers, true, >> + KVM_DEV_XIVE_NR_SERVERS, &max_vcpu_ids, >> true, >> errp); >> if (ret < 0) { >> goto fail; >> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c >> index 570d635bcc08..74e47752185c 100644 >> --- a/hw/intc/xics_kvm.c >> +++ b/hw/intc/xics_kvm.c >> @@ -347,7 +347,7 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val) >> } >> } >> >> -int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, >> +int xics_kvm_connect(SpaprInterruptController *intc, uint32_t max_vcpu_ids, >> Error **errp) >> { >> ICSState *ics = ICS_SPAPR(intc); >> @@ -408,7 +408,7 @@ int xics_kvm_connect(SpaprInterruptController *intc, >> uint32_t nr_servers, >> if (kvm_device_check_attr(rc, KVM_DEV_XICS_GRP_CTRL, >> KVM_DEV_XICS_NR_SERVERS)) { >> if (kvm_device_access(rc, KVM_DEV_XICS_GRP_CTRL, >> - KVM_DEV_XICS_NR_SERVERS, &nr_servers, true, >> + KVM_DEV_XICS_NR_SERVERS, &max_vcpu_ids, true, >> &local_err)) { >> goto fail; >> } >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c >> index 8ae4f41459c3..8f753a858cc2 100644 >> --- a/hw/intc/xics_spapr.c >> +++ b/hw/intc/xics_spapr.c >> @@ -308,11 +308,11 @@ static void ics_spapr_realize(DeviceState *dev, Error >> **errp) >> spapr_register_hypercall(H_IPOLL, h_ipoll); >> } >> >> -static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t >> nr_servers, >> +static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t >> max_vcpu_ids, >> void *fdt, uint32_t phandle) >> { >> uint32_t interrupt_server_ranges_prop[] = { >> - 0, cpu_to_be32(nr_servers), >> + 0, cpu_to_be32(max_vcpu_ids), >> }; >> int node; >> >> @@ -423,10 +423,10 @@ static int >> xics_spapr_post_load(SpaprInterruptController *intc, int version_id) >> } >> >> static int xics_spapr_activate(SpaprInterruptController *intc, >> - uint32_t nr_servers, Error **errp) >> + uint32_t max_vcpu_ids, Error **errp) >> { >> if (kvm_enabled()) { >> - return spapr_irq_init_kvm(xics_kvm_connect, intc, nr_servers, errp); >> + return spapr_irq_init_kvm(xics_kvm_connect, intc, max_vcpu_ids, >> errp); >> } >> return 0; >> } >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 7e954bc84bed..ab59bfe941d0 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -161,7 +161,7 @@ static void pre_2_10_vmstate_unregister_dummy_icp(int i) >> (void *)(uintptr_t) i); >> } >> >> -int spapr_max_server_number(SpaprMachineState *spapr) >> +int spapr_max_vcpu_ids(SpaprMachineState *spapr) >> { >> MachineState *ms = MACHINE(spapr); >> >> @@ -1164,7 +1164,7 @@ void *spapr_build_fdt(SpaprMachineState *spapr, bool >> reset, size_t space) >> _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2)); >> >> /* /interrupt controller */ >> - spapr_irq_dt(spapr, spapr_max_server_number(spapr), fdt, PHANDLE_INTC); >> + spapr_irq_dt(spapr, spapr_max_vcpu_ids(spapr), fdt, PHANDLE_INTC); >> >> ret = spapr_dt_memory(spapr, fdt); >> if (ret < 0) { >> @@ -2558,7 +2558,7 @@ static void spapr_init_cpus(SpaprMachineState *spapr) >> if (smc->pre_2_10_has_unused_icps) { >> int i; >> >> - for (i = 0; i < spapr_max_server_number(spapr); i++) { >> + for (i = 0; i < spapr_max_vcpu_ids(spapr); i++) { >> /* Dummy entries get deregistered when real ICPState objects >> * are registered during CPU core hotplug. >> */ >> @@ -2709,7 +2709,7 @@ static void spapr_machine_init(MachineState *machine) >> >> /* >> * VSMT must be set in order to be able to compute VCPU ids, ie to >> - * call spapr_max_server_number() or spapr_vcpu_id(). >> + * call spapr_max_vcpu_ids() or spapr_vcpu_id(). >> */ >> spapr_set_vsmt_mode(spapr, &error_fatal); >> >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c >> index a0d1e1298e1e..552e30e93036 100644 >> --- a/hw/ppc/spapr_irq.c >> +++ b/hw/ppc/spapr_irq.c >> @@ -72,13 +72,13 @@ void spapr_irq_msi_free(SpaprMachineState *spapr, int >> irq, uint32_t num) >> >> int spapr_irq_init_kvm(SpaprInterruptControllerInitKvm fn, >> SpaprInterruptController *intc, >> - uint32_t nr_servers, >> + uint32_t max_vcpu_ids, >> Error **errp) >> { >> Error *local_err = NULL; >> >> if (kvm_enabled() && kvm_kernel_irqchip_allowed()) { >> - if (fn(intc, nr_servers, &local_err) < 0) { >> + if (fn(intc, max_vcpu_ids, &local_err) < 0) { >> if (kvm_kernel_irqchip_required()) { >> error_prepend(&local_err, >> "kernel_irqchip requested but unavailable: "); >> @@ -271,13 +271,13 @@ void spapr_irq_print_info(SpaprMachineState *spapr, >> Monitor *mon) >> sicc->print_info(spapr->active_intc, mon); >> } >> >> -void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, >> +void spapr_irq_dt(SpaprMachineState *spapr, uint32_t max_vcpu_ids, >> void *fdt, uint32_t phandle) >> { >> SpaprInterruptControllerClass *sicc >> = SPAPR_INTC_GET_CLASS(spapr->active_intc); >> >> - sicc->dt(spapr->active_intc, nr_servers, fdt, phandle); >> + sicc->dt(spapr->active_intc, max_vcpu_ids, fdt, phandle); >> } >> >> uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr) >> @@ -324,7 +324,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error >> **errp) >> } >> >> if (spapr->irq->xive) { >> - uint32_t nr_servers = spapr_max_server_number(spapr); >> + uint32_t max_vcpu_ids = spapr_max_vcpu_ids(spapr); >> DeviceState *dev; >> int i; >> >> @@ -334,7 +334,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error >> **errp) >> * 8 XIVE END structures per CPU. One for each available >> * priority >> */ >> - qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3); >> + qdev_prop_set_uint32(dev, "nr-ends", max_vcpu_ids << 3); >> object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr), >> &error_abort); >> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >> @@ -342,7 +342,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error >> **errp) >> spapr->xive = SPAPR_XIVE(dev); >> >> /* Enable the CPU IPIs */ >> - for (i = 0; i < nr_servers; ++i) { >> + for (i = 0; i < max_vcpu_ids; ++i) { >> SpaprInterruptControllerClass *sicc >> = SPAPR_INTC_GET_CLASS(spapr->xive); >> >> @@ -479,7 +479,7 @@ static void set_active_intc(SpaprMachineState *spapr, >> SpaprInterruptController *new_intc) >> { >> SpaprInterruptControllerClass *sicc; >> - uint32_t nr_servers = spapr_max_server_number(spapr); >> + uint32_t max_vcpu_ids = spapr_max_vcpu_ids(spapr); >> >> assert(new_intc); >> >> @@ -497,7 +497,7 @@ static void set_active_intc(SpaprMachineState *spapr, >> >> sicc = SPAPR_INTC_GET_CLASS(new_intc); >> if (sicc->activate) { >> - sicc->activate(new_intc, nr_servers, &error_fatal); >> + sicc->activate(new_intc, max_vcpu_ids, &error_fatal); >> } >> >> spapr->active_intc = new_intc; >> >