From 58c46efa451caa3935224223f950216872e2eee3 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Fri, 30 Aug 2019 18:13:45 +0200 Subject: pseries: do not allow memory-less/cpu-less NUMA node When we hotplug a CPU on memory-less/cpu-less node, the linux kernel crashes. This happens because linux kernel needs to know the NUMA topology at start to be able to initialize the distance lookup table. On pseries, the topology is provided by the firmware via the existing CPUs and memory information. Thus a node without memory and CPU cannot be discovered by the kernel. To avoid the kernel crash, do not allow to start pseries with empty nodes. Signed-off-by: Laurent Vivier Message-Id: <20190830161345.22436-1-lvivier@redhat.com> [dwg: Rework to cope with movement of numa state from globals to MachineState] Signed-off-by: David Gibson --- hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 08a2a5a..f976d76 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2854,6 +2854,39 @@ static void spapr_machine_init(MachineState *machine) /* init CPUs */ spapr_init_cpus(spapr); + /* + * check we don't have a memory-less/cpu-less NUMA node + * Firmware relies on the existing memory/cpu topology to provide the + * NUMA topology to the kernel. + * And the linux kernel needs to know the NUMA topology at start + * to be able to hotplug CPUs later. + */ + if (machine->numa_state->num_nodes) { + for (i = 0; i < machine->numa_state->num_nodes; ++i) { + /* check for memory-less node */ + if (machine->numa_state->nodes[i].node_mem == 0) { + CPUState *cs; + int found = 0; + /* check for cpu-less node */ + CPU_FOREACH(cs) { + PowerPCCPU *cpu = POWERPC_CPU(cs); + if (cpu->node_id == i) { + found = 1; + break; + } + } + /* memory-less and cpu-less node */ + if (!found) { + error_report( + "Memory-less/cpu-less nodes are not supported (node %d)", + i); + exit(1); + } + } + } + + } + if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) && ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0, spapr->max_compat_pvr)) { -- cgit v1.1 From f42b6f535c290046d10aebf1796fb46957dfa8e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Mon, 2 Sep 2019 11:29:32 +0200 Subject: ppc/pnv: fix "bmc" node name in DT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the dtc output : ERROR (node_name_chars): //bmc: Bad character '/' in node name Warning (avoid_unnecessary_addr_size): /bmc: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property Signed-off-by: Cédric Le Goater Message-Id: <20190902092932.20200-1-clg@kaod.org> Signed-off-by: David Gibson --- hw/ppc/pnv_bmc.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c index e5eb6e5..dc5e918 100644 --- a/hw/ppc/pnv_bmc.c +++ b/hw/ppc/pnv_bmc.c @@ -77,13 +77,10 @@ void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt) const struct ipmi_sdr_compact *sdr; uint16_t nextrec; - offset = fdt_add_subnode(fdt, 0, "/bmc"); + offset = fdt_add_subnode(fdt, 0, "bmc"); _FDT(offset); _FDT((fdt_setprop_string(fdt, offset, "name", "bmc"))); - _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0x1))); - _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 0x0))); - offset = fdt_add_subnode(fdt, offset, "sensors"); _FDT(offset); -- cgit v1.1 From 226c9d15df0a9c9e94fc30c373cdefbc4156d8dd Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 9 Sep 2019 20:10:09 +0200 Subject: spapr-tpm-proxy: Drop misleading check Coverity is reporting in CID 1405304 that tpm_execute() may pass a NULL tpm_proxy->host_path pointer to open(). This is based on the fact that h_tpm_comm() does a NULL check on tpm_proxy->host_path and then passes tpm_proxy to tpm_execute(). The check in h_tpm_comm() is abusive actually since a spapr-proxy-tpm requires a non NULL host_path property, as checked during realize. Fixes: 0fb6bd073230 Signed-off-by: Greg Kurz Message-Id: <156805260916.1779401.11054185183758185247.stgit@bahia.lan> Reviewed-by: Michael Roth Signed-off-by: David Gibson --- hw/ppc/spapr_tpm_proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/ppc/spapr_tpm_proxy.c b/hw/ppc/spapr_tpm_proxy.c index b835d25..ca1caec 100644 --- a/hw/ppc/spapr_tpm_proxy.c +++ b/hw/ppc/spapr_tpm_proxy.c @@ -114,7 +114,7 @@ static target_ulong h_tpm_comm(PowerPCCPU *cpu, return H_FUNCTION; } - trace_spapr_h_tpm_comm(tpm_proxy->host_path ?: "null", op); + trace_spapr_h_tpm_comm(tpm_proxy->host_path, op); switch (op) { case TPM_COMM_OP_EXECUTE: -- cgit v1.1 From 59b7c1c283508ad5aa1c4064b17f8d368664029c Mon Sep 17 00:00:00 2001 From: Balamuruhan S Date: Wed, 11 Sep 2019 19:59:25 +0530 Subject: hw/ppc/pnv: fix checkpatch.pl coding style warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There were few trailing comments after `/*` instead in new line and line more than 80 character, these fixes are trivial and doesn't change any logic in code. Signed-off-by: Balamuruhan S Message-Id: <20190911142925.19197-5-bala24@linux.ibm.com> Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/ppc/pnv.c | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) (limited to 'hw') diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 3f08db7..13126d6 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -187,7 +187,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq))); _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq))); - _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size))); + _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", + cpu->hash64_opts->slb_size))); _FDT((fdt_setprop_string(fdt, offset, "status", "okay"))); _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0))); @@ -200,19 +201,23 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) segs, sizeof(segs)))); } - /* Advertise VMX/VSX (vector extensions) if available + /* + * Advertise VMX/VSX (vector extensions) if available * 0 / no property == no vector extensions * 1 == VMX / Altivec available - * 2 == VSX available */ + * 2 == VSX available + */ if (env->insns_flags & PPC_ALTIVEC) { uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1; _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx))); } - /* Advertise DFP (Decimal Floating Point) if available + /* + * Advertise DFP (Decimal Floating Point) if available * 0 / no property == no DFP - * 1 == DFP available */ + * 1 == DFP available + */ if (env->insns_flags2 & PPC2_DFP) { _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1))); } @@ -424,7 +429,8 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque) return 0; } -/* The default LPC bus of a multichip system is on chip 0. It's +/* + * The default LPC bus of a multichip system is on chip 0. It's * recognized by the firmware (skiboot) using a "primary" property. */ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt) @@ -442,8 +448,10 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt) assert(phandle > 0); _FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle))); - /* ISA devices are not necessarily parented to the ISA bus so we - * can not use object_child_foreach() */ + /* + * ISA devices are not necessarily parented to the ISA bus so we + * can not use object_child_foreach() + */ qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL, &args); } @@ -545,7 +553,8 @@ static void pnv_reset(MachineState *machine) qemu_devices_reset(); - /* OpenPOWER systems have a BMC, which can be defined on the + /* + * OpenPOWER systems have a BMC, which can be defined on the * command line with: * * -device ipmi-bmc-sim,id=bmc0 @@ -705,7 +714,8 @@ static void pnv_init(MachineState *machine) pnv->chips[i] = PNV_CHIP(chip); - /* TODO: put all the memory in one node on chip 0 until we find a + /* + * TODO: put all the memory in one node on chip 0 until we find a * way to specify different ranges for each chip */ if (i == 0) { @@ -732,8 +742,10 @@ static void pnv_init(MachineState *machine) /* Create an RTC ISA device too */ mc146818_rtc_init(pnv->isa_bus, 2000, NULL); - /* OpenPOWER systems use a IPMI SEL Event message to notify the - * host to powerdown */ + /* + * OpenPOWER systems use a IPMI SEL Event message to notify the + * host to powerdown + */ pnv->powerdown_notifier.notify = pnv_powerdown_notify; qemu_register_powerdown_notifier(&pnv->powerdown_notifier); } @@ -803,7 +815,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu, pnv_cpu->intc = obj; } -/* Allowed core identifiers on a POWER8 Processor Chip : +/* + * Allowed core identifiers on a POWER8 Processor Chip : * * * EX1 - Venice only @@ -923,8 +936,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) (uint64_t) PNV_XSCOM_BASE(chip), PNV_XSCOM_LPC_BASE); - /* Interrupt Management Area. This is the memory region holding - * all the Interrupt Control Presenter (ICP) registers */ + /* + * Interrupt Management Area. This is the memory region holding + * all the Interrupt Control Presenter (ICP) registers + */ pnv_chip_icp_realize(chip8, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -1404,8 +1419,8 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) mc->init = pnv_init; mc->reset = pnv_reset; mc->max_cpus = MAX_CPUS; - mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for - * storage */ + /* Pnv provides a AHCI device for storage */ + mc->block_default_type = IF_IDE; mc->no_parallel = 1; mc->default_boot_order = NULL; /* -- cgit v1.1 From f041d6af55708a75851bfc4ef1373a565703f976 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 12 Sep 2019 16:30:09 +0200 Subject: spapr: Report kvm_irqchip_in_kernel() in 'info pic' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unless the machine was started with kernel-irqchip=on, we cannot easily tell if we're actually using an in-kernel or an emulated irqchip. This information is important enough that it is worth printing it in 'info pic'. Signed-off-by: Greg Kurz Message-Id: <156829860985.2073005.5893493824873412773.stgit@bahia.tls.ibm.com> Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/ppc/spapr.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f976d76..2725b13 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -81,6 +81,8 @@ #include "hw/mem/memory-device.h" #include "hw/ppc/spapr_tpm_proxy.h" +#include "monitor/monitor.h" + #include /* SLOF memory layout: @@ -4354,6 +4356,8 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj, SpaprMachineState *spapr = SPAPR_MACHINE(obj); spapr->irq->print_info(spapr, mon); + monitor_printf(mon, "irqchip: %s\n", + kvm_irqchip_in_kernel() ? "in-kernel" : "emulated"); } int spapr_get_vcpu_id(PowerPCCPU *cpu) -- cgit v1.1 From 7454558c69b4fa4a9bb26d601b26330319b85f89 Mon Sep 17 00:00:00 2001 From: Balamuruhan S Date: Thu, 12 Sep 2019 15:00:52 +0530 Subject: hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During PowerNV boot skiboot populates the device tree by retrieving base address of homer/occ common area from PBA BARs and prd ipoll mask by accessing xscom read/write accesses. Reviewed-by: Cédric Le Goater Signed-off-by: Balamuruhan S Message-Id: <20190912093056.4516-2-bala24@linux.ibm.com> Signed-off-by: David Gibson --- hw/ppc/pnv_xscom.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c index 67aab98..f01d788 100644 --- a/hw/ppc/pnv_xscom.c +++ b/hw/ppc/pnv_xscom.c @@ -36,6 +36,16 @@ #define PRD_P9_IPOLL_REG_MASK 0x000F0033 #define PRD_P9_IPOLL_REG_STATUS 0x000F0034 +/* PBA BARs */ +#define P8_PBA_BAR0 0x2013f00 +#define P8_PBA_BAR2 0x2013f02 +#define P8_PBA_BARMASK0 0x2013f04 +#define P8_PBA_BARMASK2 0x2013f06 +#define P9_PBA_BAR0 0x5012b00 +#define P9_PBA_BAR2 0x5012b02 +#define P9_PBA_BARMASK0 0x5012b04 +#define P9_PBA_BARMASK2 0x5012b06 + static void xscom_complete(CPUState *cs, uint64_t hmer_bits) { /* @@ -74,6 +84,26 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba) case 0x18002: /* ECID2 */ return 0; + case P9_PBA_BAR0: + return PNV9_HOMER_BASE(chip); + case P8_PBA_BAR0: + return PNV_HOMER_BASE(chip); + + case P9_PBA_BARMASK0: /* P9 homer region size */ + return PNV9_HOMER_SIZE; + case P8_PBA_BARMASK0: /* P8 homer region size */ + return PNV_HOMER_SIZE; + + case P9_PBA_BAR2: /* P9 occ common area */ + return PNV9_OCC_COMMON_AREA(chip); + case P8_PBA_BAR2: /* P8 occ common area */ + return PNV_OCC_COMMON_AREA(chip); + + case P9_PBA_BARMASK2: /* P9 occ common area size */ + return PNV9_OCC_COMMON_AREA_SIZE; + case P8_PBA_BARMASK2: /* P8 occ common area size */ + return PNV_OCC_COMMON_AREA_SIZE; + case 0x1010c00: /* PIBAM FIR */ case 0x1010c03: /* PIBAM FIR MASK */ @@ -93,13 +123,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba) case 0x2020009: /* ADU stuff, error register */ case 0x202000f: /* ADU stuff, receive status register*/ return 0; - case 0x2013f00: /* PBA stuff */ case 0x2013f01: /* PBA stuff */ - case 0x2013f02: /* PBA stuff */ case 0x2013f03: /* PBA stuff */ - case 0x2013f04: /* PBA stuff */ case 0x2013f05: /* PBA stuff */ - case 0x2013f06: /* PBA stuff */ case 0x2013f07: /* PBA stuff */ return 0; case 0x2013028: /* CAPP stuff */ -- cgit v1.1 From f3db82660d22def1ffaa122f3952b560ac386147 Mon Sep 17 00:00:00 2001 From: Balamuruhan S Date: Thu, 12 Sep 2019 15:00:53 +0530 Subject: hw/ppc/pnv_occ: add sram device model for occ common area MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit emulate occ common area region with occ sram device model which occ and skiboot uses it to communicate regarding sensors, slw and HWMON in PowerNV emulated host. Reviewed-by: Cédric Le Goater Signed-off-by: Balamuruhan S Message-Id: <20190912093056.4516-3-bala24@linux.ibm.com> Signed-off-by: David Gibson --- hw/ppc/pnv.c | 8 ++++++ hw/ppc/pnv_occ.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) (limited to 'hw') diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 13126d6..09d2d7c 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -953,6 +953,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) return; } pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs); + + /* OCC SRAM model */ + memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip), + &chip8->occ.sram_regs); } static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data) @@ -1141,6 +1145,10 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp) return; } pnv_xscom_add_subregion(chip, PNV9_XSCOM_OCC_BASE, &chip9->occ.xscom_regs); + + /* OCC SRAM model */ + memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip), + &chip9->occ.sram_regs); } static void pnv_chip_power9_class_init(ObjectClass *klass, void *data) diff --git a/hw/ppc/pnv_occ.c b/hw/ppc/pnv_occ.c index 8bead2c..785653b 100644 --- a/hw/ppc/pnv_occ.c +++ b/hw/ppc/pnv_occ.c @@ -30,6 +30,24 @@ #define OCB_OCI_OCCMISC_AND 0x4021 #define OCB_OCI_OCCMISC_OR 0x4022 +/* OCC sensors */ +#define OCC_SENSOR_DATA_BLOCK_OFFSET 0x580000 +#define OCC_SENSOR_DATA_VALID 0x580001 +#define OCC_SENSOR_DATA_VERSION 0x580002 +#define OCC_SENSOR_DATA_READING_VERSION 0x580004 +#define OCC_SENSOR_DATA_NR_SENSORS 0x580008 +#define OCC_SENSOR_DATA_NAMES_OFFSET 0x580010 +#define OCC_SENSOR_DATA_READING_PING_OFFSET 0x580014 +#define OCC_SENSOR_DATA_READING_PONG_OFFSET 0x58000c +#define OCC_SENSOR_DATA_NAME_LENGTH 0x58000d +#define OCC_SENSOR_NAME_STRUCTURE_TYPE 0x580023 +#define OCC_SENSOR_LOC_CORE 0x580022 +#define OCC_SENSOR_LOC_GPU 0x580020 +#define OCC_SENSOR_TYPE_POWER 0x580003 +#define OCC_SENSOR_NAME 0x580005 +#define HWMON_SENSORS_MASK 0x58001e +#define SLW_IMAGE_BASE 0x0 + static void pnv_occ_set_misc(PnvOCC *occ, uint64_t val) { bool irq_state; @@ -82,6 +100,48 @@ static void pnv_occ_power8_xscom_write(void *opaque, hwaddr addr, } } +static uint64_t pnv_occ_common_area_read(void *opaque, hwaddr addr, + unsigned width) +{ + switch (addr) { + /* + * occ-sensor sanity check that asserts the sensor + * header block + */ + case OCC_SENSOR_DATA_BLOCK_OFFSET: + case OCC_SENSOR_DATA_VALID: + case OCC_SENSOR_DATA_VERSION: + case OCC_SENSOR_DATA_READING_VERSION: + case OCC_SENSOR_DATA_NR_SENSORS: + case OCC_SENSOR_DATA_NAMES_OFFSET: + case OCC_SENSOR_DATA_READING_PING_OFFSET: + case OCC_SENSOR_DATA_READING_PONG_OFFSET: + case OCC_SENSOR_NAME_STRUCTURE_TYPE: + return 1; + case OCC_SENSOR_DATA_NAME_LENGTH: + return 0x30; + case OCC_SENSOR_LOC_CORE: + return 0x0040; + case OCC_SENSOR_TYPE_POWER: + return 0x0080; + case OCC_SENSOR_NAME: + return 0x1000; + case HWMON_SENSORS_MASK: + case OCC_SENSOR_LOC_GPU: + return 0x8e00; + case SLW_IMAGE_BASE: + return 0x1000000000000000; + } + return 0; +} + +static void pnv_occ_common_area_write(void *opaque, hwaddr addr, + uint64_t val, unsigned width) +{ + /* callback function defined to occ common area write */ + return; +} + static const MemoryRegionOps pnv_occ_power8_xscom_ops = { .read = pnv_occ_power8_xscom_read, .write = pnv_occ_power8_xscom_write, @@ -92,12 +152,24 @@ static const MemoryRegionOps pnv_occ_power8_xscom_ops = { .endianness = DEVICE_BIG_ENDIAN, }; +const MemoryRegionOps pnv_occ_sram_ops = { + .read = pnv_occ_common_area_read, + .write = pnv_occ_common_area_write, + .valid.min_access_size = 1, + .valid.max_access_size = 8, + .impl.min_access_size = 1, + .impl.max_access_size = 8, + .endianness = DEVICE_BIG_ENDIAN, +}; + static void pnv_occ_power8_class_init(ObjectClass *klass, void *data) { PnvOCCClass *poc = PNV_OCC_CLASS(klass); poc->xscom_size = PNV_XSCOM_OCC_SIZE; + poc->sram_size = PNV_OCC_COMMON_AREA_SIZE; poc->xscom_ops = &pnv_occ_power8_xscom_ops; + poc->sram_ops = &pnv_occ_sram_ops; poc->psi_irq = PSIHB_IRQ_OCC; } @@ -168,7 +240,9 @@ static void pnv_occ_power9_class_init(ObjectClass *klass, void *data) PnvOCCClass *poc = PNV_OCC_CLASS(klass); poc->xscom_size = PNV9_XSCOM_OCC_SIZE; + poc->sram_size = PNV9_OCC_COMMON_AREA_SIZE; poc->xscom_ops = &pnv_occ_power9_xscom_ops; + poc->sram_ops = &pnv_occ_sram_ops; poc->psi_irq = PSIHB9_IRQ_OCC; } @@ -199,6 +273,10 @@ static void pnv_occ_realize(DeviceState *dev, Error **errp) /* XScom region for OCC registers */ pnv_xscom_region_init(&occ->xscom_regs, OBJECT(dev), poc->xscom_ops, occ, "xscom-occ", poc->xscom_size); + + /* XScom region for OCC SRAM registers */ + pnv_xscom_region_init(&occ->sram_regs, OBJECT(dev), poc->sram_ops, + occ, "occ-common-area", poc->sram_size); } static void pnv_occ_class_init(ObjectClass *klass, void *data) -- cgit v1.1 From 3887d241238c7b5960c7f153d2025891c5f5fc66 Mon Sep 17 00:00:00 2001 From: Balamuruhan S Date: Thu, 12 Sep 2019 15:00:54 +0530 Subject: hw/ppc/pnv_homer: add PowerNV homer device model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit add PnvHomer device model to emulate homer memory access for pstate table, occ-sensors, slw, occ static and dynamic values for Power8 and Power9 chips. Signed-off-by: Balamuruhan S Message-Id: <20190912093056.4516-4-bala24@linux.ibm.com> Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/ppc/Makefile.objs | 1 + hw/ppc/pnv.c | 30 ++++++ hw/ppc/pnv_homer.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 303 insertions(+) create mode 100644 hw/ppc/pnv_homer.c (limited to 'hw') diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs index 2c4e1c8..580bb4f 100644 --- a/hw/ppc/Makefile.objs +++ b/hw/ppc/Makefile.objs @@ -9,6 +9,7 @@ obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o # IBM PowerNV obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o +obj-$(CONFIG_POWERNV) += pnv_homer.o ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o endif diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 09d2d7c..77a86c6 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -860,6 +860,11 @@ static void pnv_chip_power8_instance_init(Object *obj) TYPE_PNV8_OCC, &error_abort, NULL); object_property_add_const_link(OBJECT(&chip8->occ), "psi", OBJECT(&chip8->psi), &error_abort); + + object_initialize_child(obj, "homer", &chip8->homer, sizeof(chip8->homer), + TYPE_PNV8_HOMER, &error_abort, NULL); + object_property_add_const_link(OBJECT(&chip8->homer), "chip", obj, + &error_abort); } static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp) @@ -957,6 +962,16 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) /* OCC SRAM model */ memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip), &chip8->occ.sram_regs); + + /* HOMER */ + object_property_set_bool(OBJECT(&chip8->homer), true, "realized", + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip), + &chip8->homer.regs); } static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data) @@ -1039,6 +1054,11 @@ static void pnv_chip_power9_instance_init(Object *obj) TYPE_PNV9_OCC, &error_abort, NULL); object_property_add_const_link(OBJECT(&chip9->occ), "psi", OBJECT(&chip9->psi), &error_abort); + + object_initialize_child(obj, "homer", &chip9->homer, sizeof(chip9->homer), + TYPE_PNV9_HOMER, &error_abort, NULL); + object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj, + &error_abort); } static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp) @@ -1149,6 +1169,16 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp) /* OCC SRAM model */ memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip), &chip9->occ.sram_regs); + + /* HOMER */ + object_property_set_bool(OBJECT(&chip9->homer), true, "realized", + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip), + &chip9->homer.regs); } static void pnv_chip_power9_class_init(ObjectClass *klass, void *data) diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c new file mode 100644 index 0000000..cc881a3 --- /dev/null +++ b/hw/ppc/pnv_homer.c @@ -0,0 +1,272 @@ +/* + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers + * + * Copyright (c) 2019, IBM Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "exec/hwaddr.h" +#include "exec/memory.h" +#include "sysemu/cpus.h" +#include "hw/qdev-core.h" +#include "hw/ppc/pnv.h" +#include "hw/ppc/pnv_homer.h" + + +static bool core_max_array(PnvHomer *homer, hwaddr addr) +{ + int i; + PnvHomerClass *hmrc = PNV_HOMER_GET_CLASS(homer); + + for (i = 0; i <= homer->chip->nr_cores; i++) { + if (addr == (hmrc->core_max_base + i)) { + return true; + } + } + return false; +} + +/* P8 Pstate table */ + +#define PNV8_OCC_PSTATE_VERSION 0x1f8001 +#define PNV8_OCC_PSTATE_MIN 0x1f8003 +#define PNV8_OCC_PSTATE_VALID 0x1f8000 +#define PNV8_OCC_PSTATE_THROTTLE 0x1f8002 +#define PNV8_OCC_PSTATE_NOM 0x1f8004 +#define PNV8_OCC_PSTATE_TURBO 0x1f8005 +#define PNV8_OCC_PSTATE_ULTRA_TURBO 0x1f8006 +#define PNV8_OCC_PSTATE_DATA 0x1f8008 +#define PNV8_OCC_PSTATE_ID_ZERO 0x1f8010 +#define PNV8_OCC_PSTATE_ID_ONE 0x1f8018 +#define PNV8_OCC_PSTATE_ID_TWO 0x1f8020 +#define PNV8_OCC_VDD_VOLTAGE_IDENTIFIER 0x1f8012 +#define PNV8_OCC_VCS_VOLTAGE_IDENTIFIER 0x1f8013 +#define PNV8_OCC_PSTATE_ZERO_FREQUENCY 0x1f8014 +#define PNV8_OCC_PSTATE_ONE_FREQUENCY 0x1f801c +#define PNV8_OCC_PSTATE_TWO_FREQUENCY 0x1f8024 +#define PNV8_CORE_MAX_BASE 0x1f8810 + + +static uint64_t pnv_power8_homer_read(void *opaque, hwaddr addr, + unsigned size) +{ + PnvHomer *homer = PNV_HOMER(opaque); + + switch (addr) { + case PNV8_OCC_PSTATE_VERSION: + case PNV8_OCC_PSTATE_MIN: + case PNV8_OCC_PSTATE_ID_ZERO: + return 0; + case PNV8_OCC_PSTATE_VALID: + case PNV8_OCC_PSTATE_THROTTLE: + case PNV8_OCC_PSTATE_NOM: + case PNV8_OCC_PSTATE_TURBO: + case PNV8_OCC_PSTATE_ID_ONE: + case PNV8_OCC_VDD_VOLTAGE_IDENTIFIER: + case PNV8_OCC_VCS_VOLTAGE_IDENTIFIER: + return 1; + case PNV8_OCC_PSTATE_ULTRA_TURBO: + case PNV8_OCC_PSTATE_ID_TWO: + return 2; + case PNV8_OCC_PSTATE_DATA: + return 0x1000000000000000; + /* P8 frequency for 0, 1, and 2 pstates */ + case PNV8_OCC_PSTATE_ZERO_FREQUENCY: + case PNV8_OCC_PSTATE_ONE_FREQUENCY: + case PNV8_OCC_PSTATE_TWO_FREQUENCY: + return 3000; + } + /* pstate table core max array */ + if (core_max_array(homer, addr)) { + return 1; + } + return 0; +} + +static void pnv_power8_homer_write(void *opaque, hwaddr addr, + uint64_t val, unsigned size) +{ + /* callback function defined to homer write */ + return; +} + +static const MemoryRegionOps pnv_power8_homer_ops = { + .read = pnv_power8_homer_read, + .write = pnv_power8_homer_write, + .valid.min_access_size = 1, + .valid.max_access_size = 8, + .impl.min_access_size = 1, + .impl.max_access_size = 8, + .endianness = DEVICE_BIG_ENDIAN, +}; + +static void pnv_homer_power8_class_init(ObjectClass *klass, void *data) +{ + PnvHomerClass *homer = PNV_HOMER_CLASS(klass); + + homer->homer_size = PNV_HOMER_SIZE; + homer->homer_ops = &pnv_power8_homer_ops; + homer->core_max_base = PNV8_CORE_MAX_BASE; +} + +static const TypeInfo pnv_homer_power8_type_info = { + .name = TYPE_PNV8_HOMER, + .parent = TYPE_PNV_HOMER, + .instance_size = sizeof(PnvHomer), + .class_init = pnv_homer_power8_class_init, +}; + +/* P9 Pstate table */ + +#define PNV9_OCC_PSTATE_ID_ZERO 0xe2018 +#define PNV9_OCC_PSTATE_ID_ONE 0xe2020 +#define PNV9_OCC_PSTATE_ID_TWO 0xe2028 +#define PNV9_OCC_PSTATE_DATA 0xe2000 +#define PNV9_OCC_PSTATE_DATA_AREA 0xe2008 +#define PNV9_OCC_PSTATE_MIN 0xe2003 +#define PNV9_OCC_PSTATE_NOM 0xe2004 +#define PNV9_OCC_PSTATE_TURBO 0xe2005 +#define PNV9_OCC_PSTATE_ULTRA_TURBO 0xe2818 +#define PNV9_OCC_MAX_PSTATE_ULTRA_TURBO 0xe2006 +#define PNV9_OCC_PSTATE_MAJOR_VERSION 0xe2001 +#define PNV9_OCC_OPAL_RUNTIME_DATA 0xe2b85 +#define PNV9_CHIP_HOMER_IMAGE_POINTER 0x200008 +#define PNV9_CHIP_HOMER_BASE 0x0 +#define PNV9_OCC_PSTATE_ZERO_FREQUENCY 0xe201c +#define PNV9_OCC_PSTATE_ONE_FREQUENCY 0xe2024 +#define PNV9_OCC_PSTATE_TWO_FREQUENCY 0xe202c +#define PNV9_OCC_ROLE_MASTER_OR_SLAVE 0xe2002 +#define PNV9_CORE_MAX_BASE 0xe2819 + + +static uint64_t pnv_power9_homer_read(void *opaque, hwaddr addr, + unsigned size) +{ + PnvHomer *homer = PNV_HOMER(opaque); + + switch (addr) { + case PNV9_OCC_MAX_PSTATE_ULTRA_TURBO: + case PNV9_OCC_PSTATE_ID_ZERO: + return 0; + case PNV9_OCC_PSTATE_DATA: + case PNV9_OCC_ROLE_MASTER_OR_SLAVE: + case PNV9_OCC_PSTATE_NOM: + case PNV9_OCC_PSTATE_TURBO: + case PNV9_OCC_PSTATE_ID_ONE: + case PNV9_OCC_PSTATE_ULTRA_TURBO: + case PNV9_OCC_OPAL_RUNTIME_DATA: + return 1; + case PNV9_OCC_PSTATE_MIN: + case PNV9_OCC_PSTATE_ID_TWO: + return 2; + + /* 3000 khz frequency for 0, 1, and 2 pstates */ + case PNV9_OCC_PSTATE_ZERO_FREQUENCY: + case PNV9_OCC_PSTATE_ONE_FREQUENCY: + case PNV9_OCC_PSTATE_TWO_FREQUENCY: + return 3000; + case PNV9_OCC_PSTATE_MAJOR_VERSION: + return 0x90; + case PNV9_CHIP_HOMER_BASE: + case PNV9_OCC_PSTATE_DATA_AREA: + case PNV9_CHIP_HOMER_IMAGE_POINTER: + return 0x1000000000000000; + } + /* pstate table core max array */ + if (core_max_array(homer, addr)) { + return 1; + } + return 0; +} + +static void pnv_power9_homer_write(void *opaque, hwaddr addr, + uint64_t val, unsigned size) +{ + /* callback function defined to homer write */ + return; +} + +static const MemoryRegionOps pnv_power9_homer_ops = { + .read = pnv_power9_homer_read, + .write = pnv_power9_homer_write, + .valid.min_access_size = 1, + .valid.max_access_size = 8, + .impl.min_access_size = 1, + .impl.max_access_size = 8, + .endianness = DEVICE_BIG_ENDIAN, +}; + +static void pnv_homer_power9_class_init(ObjectClass *klass, void *data) +{ + PnvHomerClass *homer = PNV_HOMER_CLASS(klass); + + homer->homer_size = PNV9_HOMER_SIZE; + homer->homer_ops = &pnv_power9_homer_ops; + homer->core_max_base = PNV9_CORE_MAX_BASE; +} + +static const TypeInfo pnv_homer_power9_type_info = { + .name = TYPE_PNV9_HOMER, + .parent = TYPE_PNV_HOMER, + .instance_size = sizeof(PnvHomer), + .class_init = pnv_homer_power9_class_init, +}; + +static void pnv_homer_realize(DeviceState *dev, Error **errp) +{ + PnvHomer *homer = PNV_HOMER(dev); + PnvHomerClass *hmrc = PNV_HOMER_GET_CLASS(homer); + Object *obj; + Error *local_err = NULL; + + obj = object_property_get_link(OBJECT(dev), "chip", &local_err); + if (!obj) { + error_propagate(errp, local_err); + error_prepend(errp, "required link 'chip' not found: "); + return; + } + homer->chip = PNV_CHIP(obj); + /* homer region */ + memory_region_init_io(&homer->regs, OBJECT(dev), + hmrc->homer_ops, homer, "homer-main-memory", + hmrc->homer_size); +} + +static void pnv_homer_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = pnv_homer_realize; + dc->desc = "PowerNV HOMER Memory"; +} + +static const TypeInfo pnv_homer_type_info = { + .name = TYPE_PNV_HOMER, + .parent = TYPE_DEVICE, + .instance_size = sizeof(PnvHomer), + .class_init = pnv_homer_class_init, + .class_size = sizeof(PnvHomerClass), + .abstract = true, +}; + +static void pnv_homer_register_types(void) +{ + type_register_static(&pnv_homer_type_info); + type_register_static(&pnv_homer_power8_type_info); + type_register_static(&pnv_homer_power9_type_info); +} + +type_init(pnv_homer_register_types); -- cgit v1.1 From 4a99d40551d265e56584bfa6657d9a549e729127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Wed, 11 Sep 2019 15:39:36 +0200 Subject: spapr/irq: Introduce an ics_irq_free() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It will help us to discard interrupt numbers which have not been claimed in the next patch. Signed-off-by: Cédric Le Goater Message-Id: <20190911133937.2716-2-clg@kaod.org> Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_irq.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 06fe243..d8f46b6 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -114,9 +114,6 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, xics_spapr_init(spapr); } -#define ICS_IRQ_FREE(ics, srcno) \ - (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK))) - static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { @@ -129,7 +126,7 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, return -1; } - if (!ICS_IRQ_FREE(ics, irq - ics->offset)) { + if (!ics_irq_free(ics, irq - ics->offset)) { error_setg(errp, "IRQ %d is not free", irq); return -1; } @@ -147,7 +144,7 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num) if (ics_valid_irq(ics, irq)) { trace_spapr_irq_free(0, irq, num); for (i = srcno; i < srcno + num; ++i) { - if (ICS_IRQ_FREE(ics, i)) { + if (ics_irq_free(ics, i)) { trace_spapr_irq_free_warn(0, i); } memset(&ics->irqs[i], 0, sizeof(ICSIRQState)); @@ -767,7 +764,7 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) return -1; } for (i = first; i < first + num; ++i) { - if (!ICS_IRQ_FREE(ics, i)) { + if (!ics_irq_free(ics, i)) { break; } } -- cgit v1.1 From 4c3539d491026a0cc68e3b886f16cb7f57efd46b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Wed, 11 Sep 2019 15:39:37 +0200 Subject: spapr/irq: Only claim VALID interrupts at the KVM level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A typical pseries VM with 16 vCPUs, one disk, one network adapater uses less than 100 interrupts but the whole IRQ number space of the QEMU machine is allocated at reset time and it is 8K wide. This is wasting a considerable amount of interrupt numbers in the global IRQ space which has 1M interrupts per socket on a POWER9. To optimise the HW resources, only request at the KVM level interrupts which have been claimed by the guest. This will help to increase the maximum number of VMs per system and also help supporting nested guests using the XIVE interrupt mode. Signed-off-by: Cédric Le Goater Message-Id: <20190911133937.2716-3-clg@kaod.org> Signed-off-by: Greg Kurz Message-Id: <156942766014.1274533.10792048853177121231.stgit@bahia.lan> [dwg: Folded in fix up from Greg Kurz] Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 40 +++++++++++++++++++++++++++++++++++++--- hw/intc/xics_kvm.c | 8 ++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 17af4d1..2006f96 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -255,11 +255,16 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp) static void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp) { + SpaprXive *xive = SPAPR_XIVE(xsrc->xive); int i; for (i = 0; i < xsrc->nr_irqs; i++) { Error *local_err = NULL; + if (!xive_eas_is_valid(&xive->eat[i])) { + continue; + } + kvmppc_xive_source_reset_one(xsrc, i, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -328,11 +333,18 @@ uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset, static void kvmppc_xive_source_get_state(XiveSource *xsrc) { + SpaprXive *xive = SPAPR_XIVE(xsrc->xive); int i; for (i = 0; i < xsrc->nr_irqs; i++) { + uint8_t pq; + + if (!xive_eas_is_valid(&xive->eat[i])) { + continue; + } + /* Perform a load without side effect to retrieve the PQ bits */ - uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET); + pq = xive_esb_read(xsrc, i, XIVE_ESB_GET); /* and save PQ locally */ xive_source_esb_set(xsrc, i, pq); @@ -521,9 +533,14 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running, */ if (running) { for (i = 0; i < xsrc->nr_irqs; i++) { - uint8_t pq = xive_source_esb_get(xsrc, i); + uint8_t pq; uint8_t old_pq; + if (!xive_eas_is_valid(&xive->eat[i])) { + continue; + } + + pq = xive_source_esb_get(xsrc, i); old_pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8)); /* @@ -545,7 +562,13 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running, * migration is in progress. */ for (i = 0; i < xsrc->nr_irqs; i++) { - uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET); + uint8_t pq; + + if (!xive_eas_is_valid(&xive->eat[i])) { + continue; + } + + pq = xive_esb_read(xsrc, i, XIVE_ESB_GET); /* * PQ is set to PENDING to possibly catch a triggered @@ -655,6 +678,17 @@ int kvmppc_xive_post_load(SpaprXive *xive, int version_id) continue; } + /* + * We can only restore the source config if the source has been + * previously set in KVM. Since we don't do that for all interrupts + * at reset time anymore, let's do it now. + */ + kvmppc_xive_source_reset_one(&xive->source, i, &local_err); + if (local_err) { + error_report_err(local_err); + return -1; + } + kvmppc_xive_set_source_config(xive, i, &xive->eat[i], &local_err); if (local_err) { error_report_err(local_err); diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index a4d2e87..ba90d6d 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -190,6 +190,10 @@ void ics_get_kvm_state(ICSState *ics) for (i = 0; i < ics->nr_irqs; i++) { ICSIRQState *irq = &ics->irqs[i]; + if (ics_irq_free(ics, i)) { + continue; + } + kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES, i + ics->offset, &state, false, &error_fatal); @@ -301,6 +305,10 @@ int ics_set_kvm_state(ICSState *ics, Error **errp) Error *local_err = NULL; int ret; + if (ics_irq_free(ics, i)) { + continue; + } + ret = ics_set_kvm_state_one(ics, i, &local_err); if (ret < 0) { error_propagate(errp, local_err); -- cgit v1.1 From daa36379ce1a0a683562d40ca20f9b722ef595e1 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 28 Aug 2019 13:59:27 +1000 Subject: spapr: Simplify handling of pre ISA 3.0 guest workaround handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Certain old guest versions don't understand the radix MMU introduced with POWER ISA 3.0, but incorrectly select it if presented with the option at CAS time. We workaround this in qemu by explicitly excluding the radix (and other ISA 3.0 linked) options if the guest doesn't explicitly note support for ISA 3.0. This is handled by the 'cas_legacy_guest_workaround' flag, which is pretty vague. Rename it to 'cas_pre_isa3_guest' to be clearer about what it's for. In addition, we unnecessarily call spapr_populate_pa_features() with different options when initially constructing the device tree and when adjusting it at CAS time. At the initial construct time cas_pre_isa3_guest is already false, so we can still use the flag, rather than explicitly overriding it to be false at the callsite. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Reviewed-by: Alexey Kardashevskiy --- hw/ppc/spapr.c | 10 ++++------ hw/ppc/spapr_hcall.c | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2725b13..b906ac6 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -220,8 +220,7 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu) /* Populate the "ibm,pa-features" property */ static void spapr_populate_pa_features(SpaprMachineState *spapr, PowerPCCPU *cpu, - void *fdt, int offset, - bool legacy_guest) + void *fdt, int offset) { uint8_t pa_features_206[] = { 6, 0, 0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 }; @@ -287,7 +286,7 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr, if ((spapr_get_cap(spapr, SPAPR_CAP_HTM) != 0) && pa_size > 24) { pa_features[24] |= 0x80; /* Transactional memory support */ } - if (legacy_guest && pa_size > 40) { + if (spapr->cas_pre_isa3_guest && pa_size > 40) { /* Workaround for broken kernels that attempt (guest) radix * mode when they can't handle it, if they see the radix bit set * in pa-features. So hide it from them. */ @@ -350,8 +349,7 @@ static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr) return ret; } - spapr_populate_pa_features(spapr, cpu, fdt, offset, - spapr->cas_legacy_guest_workaround); + spapr_populate_pa_features(spapr, cpu, fdt, offset); } return ret; } @@ -553,7 +551,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, page_sizes_prop, page_sizes_prop_size))); } - spapr_populate_pa_features(spapr, cpu, fdt, offset, false); + spapr_populate_pa_features(spapr, cpu, fdt, offset); _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id", cs->cpu_index / vcpus_per_socket))); diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 23e4bdb..3d3a671 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1765,8 +1765,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, exit(EXIT_FAILURE); } } - spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest, - OV1_PPC_3_00); + spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00); spapr_ovec_cleanup(ov1_guest); if (!spapr->cas_reboot) { /* If spapr_machine_reset() did not set up a HPT but one is necessary -- cgit v1.1 From db5127b28ae1113bbc10f843d1cd219dc90a52d1 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 6 Sep 2019 14:48:28 +1000 Subject: spapr: Move handling of special NVLink numa node from reset to init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The number of NUMA nodes in the system is fixed from the command line. Therefore, there's no need to recalculate it at reset time, and we can determine the special gpu_numa_id value used for NVLink2 devices at init time. This simplifies the reset path a bit which will make further improvements easier. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Tested-by: Alexey Kardashevskiy Reviewed-by: Alexey Kardashevskiy --- hw/ppc/spapr.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index b906ac6..7c3a443 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1739,16 +1739,6 @@ static void spapr_machine_reset(MachineState *machine) spapr_setup_hpt_and_vrma(spapr); } - /* - * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node. - * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is - * called from vPHB reset handler so we initialize the counter here. - * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM - * must be equally distant from any other node. - * The final value of spapr->gpu_numa_id is going to be written to - * max-associativity-domains in spapr_build_fdt(). - */ - spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes); qemu_devices_reset(); /* @@ -2887,6 +2877,17 @@ static void spapr_machine_init(MachineState *machine) } + /* + * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node. + * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is + * called from vPHB reset handler so we initialize the counter here. + * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM + * must be equally distant from any other node. + * The final value of spapr->gpu_numa_id is going to be written to + * max-associativity-domains in spapr_build_fdt(). + */ + spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes); + if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) && ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0, spapr->max_compat_pvr)) { -- cgit v1.1 From f767b1ac5766fcc48cc3939eeb7db7b95b98408b Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Fri, 23 Aug 2019 12:39:57 +1000 Subject: spapr: Fixes a leak in CAS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a missing g_free(fdt) if the resulting tree is bigger than the space allocated by SLOF. Signed-off-by: Alexey Kardashevskiy Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 7c3a443..c69c034 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1026,6 +1026,7 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr, _FDT((fdt_pack(fdt))); if (fdt_totalsize(fdt) + sizeof(hdr) > size) { + g_free(fdt); trace_spapr_cas_failed(size); return -1; } -- cgit v1.1 From 3a17e38f6eac0028f2e4ca052455b38a4048c85d Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Tue, 3 Sep 2019 12:34:34 +1000 Subject: spapr: Skip leading zeroes from memory@ DT node names The device tree build by QEMU at the machine reset time is used by SLOF to build its internal device tree but the node names are not preserved exactly so when QEMU provides a device tree update in response to H_CAS, it might become tricky to match a node from the update blob to the actual node in SLOF. This removed leading zeroes from "memory@" nodes and makes the DTC checker happy. Signed-off-by: Alexey Kardashevskiy Signed-off-by: David Gibson Reviewed-by: Greg Kurz --- hw/ppc/spapr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index c69c034..42a5b8d 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -388,7 +388,7 @@ static int spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start, mem_reg_property[0] = cpu_to_be64(start); mem_reg_property[1] = cpu_to_be64(size); - sprintf(mem_name, "memory@" TARGET_FMT_lx, start); + sprintf(mem_name, "memory@%" HWADDR_PRIx, start); off = fdt_add_subnode(fdt, 0, mem_name); _FDT(off); _FDT((fdt_setprop_string(fdt, off, "device_type", "memory"))); -- cgit v1.1 From 5ced78955fe3f74002ad27676cf7c65cc89d6660 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Tue, 3 Sep 2019 12:35:33 +1000 Subject: spapr: Do not put empty properties for -kernel/-initrd/-append We are going to use spapr_build_fdt() for the boot time FDT and as an update for SLOF during handling of H_CAS. SLOF will apply all properties from the QEMU's FDT which is usually ok unless there are properties changed by grub or guest kernel. The properties are: bootargs, linux,initrd-start, linux,initrd-end, linux,stdout-path, linux,rtas-base, linux,rtas-entry. Resetting those during CAS will most likely cause grub failure. Don't create such properties if we're booting without "-kernel" and "-initrd" so they won't get included into the DT update blob and therefore the guest is more likely to boot successfully. Signed-off-by: Alexey Kardashevskiy [dwg: Tweaked commit message based on Greg Kurz's input] Signed-off-by: David Gibson --- hw/ppc/spapr.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 42a5b8d..f1c57c2 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1179,11 +1179,16 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt) _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen")); - _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline)); - _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start", - spapr->initrd_base)); - _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end", - spapr->initrd_base + spapr->initrd_size)); + if (machine->kernel_cmdline && machine->kernel_cmdline[0]) { + _FDT(fdt_setprop_string(fdt, chosen, "bootargs", + machine->kernel_cmdline)); + } + if (spapr->initrd_size) { + _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start", + spapr->initrd_base)); + _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end", + spapr->initrd_base + spapr->initrd_size)); + } if (spapr->kernel_size) { uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR), -- cgit v1.1 From 744a928ccee92482385f58e835108aa6ebd54ecb Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Tue, 16 Jul 2019 15:27:43 +1000 Subject: spapr: Stop providing RTAS blob SLOF implements one itself so let's remove it from QEMU. It is one less image and simpler setup as the RTAS blob never stays in its initial place anyway as the guest OS always decides where to put it. Signed-off-by: Alexey Kardashevskiy Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr.c | 32 ++------------------------------ hw/ppc/spapr_rtas.c | 41 ----------------------------------------- 2 files changed, 2 insertions(+), 71 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f1c57c2..3742a8c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -96,7 +96,6 @@ * We load our kernel at 4M, leaving space for SLOF initial image */ #define FDT_MAX_SIZE 0x100000 -#define RTAS_MAX_SIZE 0x10000 #define RTAS_MAX_ADDR 0x80000000 /* RTAS must stay below that */ #define FW_MAX_SIZE 0x400000 #define FW_FILE_NAME "slof.bin" @@ -1723,8 +1722,7 @@ static void spapr_machine_reset(MachineState *machine) { SpaprMachineState *spapr = SPAPR_MACHINE(machine); PowerPCCPU *first_ppc_cpu; - uint32_t rtas_limit; - hwaddr rtas_addr, fdt_addr; + hwaddr fdt_addr; void *fdt; int rc; @@ -1788,14 +1786,10 @@ static void spapr_machine_reset(MachineState *machine) * or just below 2GB, whichever is lower, so that it can be * processed with 32-bit real mode code if necessary */ - rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR); - rtas_addr = rtas_limit - RTAS_MAX_SIZE; - fdt_addr = rtas_addr - FDT_MAX_SIZE; + fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE; fdt = spapr_build_fdt(spapr); - spapr_load_rtas(spapr, fdt, rtas_addr); - rc = fdt_pack(fdt); /* Should only fail if we've built a corrupted tree */ @@ -2955,28 +2949,6 @@ static void spapr_machine_init(MachineState *machine) spapr_create_lmb_dr_connectors(spapr); } - filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin"); - if (!filename) { - error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin"); - exit(1); - } - spapr->rtas_size = get_image_size(filename); - if (spapr->rtas_size < 0) { - error_report("Could not get size of LPAR rtas '%s'", filename); - exit(1); - } - spapr->rtas_blob = g_malloc(spapr->rtas_size); - if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) { - error_report("Could not load LPAR rtas '%s'", filename); - exit(1); - } - if (spapr->rtas_size > RTAS_MAX_SIZE) { - error_report("RTAS too big ! 0x%zx bytes (max is 0x%x)", - (size_t)spapr->rtas_size, RTAS_MAX_SIZE); - exit(1); - } - g_free(filename); - /* Set up RTAS event infrastructure */ spapr_events_init(spapr); diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index bee3835..8d8d8cd 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -477,47 +477,6 @@ void spapr_dt_rtas_tokens(void *fdt, int rtas) } } -void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr) -{ - int rtas_node; - int ret; - - /* Copy RTAS blob into guest RAM */ - cpu_physical_memory_write(addr, spapr->rtas_blob, spapr->rtas_size); - - ret = fdt_add_mem_rsv(fdt, addr, spapr->rtas_size); - if (ret < 0) { - error_report("Couldn't add RTAS reserve entry: %s", - fdt_strerror(ret)); - exit(1); - } - - /* Update the device tree with the blob's location */ - rtas_node = fdt_path_offset(fdt, "/rtas"); - assert(rtas_node >= 0); - - ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-base", addr); - if (ret < 0) { - error_report("Couldn't add linux,rtas-base property: %s", - fdt_strerror(ret)); - exit(1); - } - - ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-entry", addr); - if (ret < 0) { - error_report("Couldn't add linux,rtas-entry property: %s", - fdt_strerror(ret)); - exit(1); - } - - ret = fdt_setprop_cell(fdt, rtas_node, "rtas-size", spapr->rtas_size); - if (ret < 0) { - error_report("Couldn't add rtas-size property: %s", - fdt_strerror(ret)); - exit(1); - } -} - static void core_rtas_register_types(void) { spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character", -- cgit v1.1 From c4ec08ab70bab90685d1443d6da47293e3aa312a Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Fri, 27 Sep 2019 12:26:51 +1000 Subject: spapr-pci: Stop providing assigned-addresses QEMU does not allocate PCI resources (BARs) in any case - coldplug devices are configured by the firmware and hotplug devices rely on the guest system to do the assignment via the PCI rescan mechanism. Also in order to create non empty "assigned-addresses", the device has to be enabled (i.e. PCI_COMMAND needs the MMIO bit set) first as otherwise io_regions[i].addr are -1, and devices are not enabled at this point. This removes "assigned-addresses" and leaves it to those who actually do resource allocation. Signed-off-by: Alexey Kardashevskiy Message-Id: <20190927022651.71642-1-aik@ozlabs.ru> Signed-off-by: David Gibson --- hw/ppc/spapr_pci.c | 42 +++++++----------------------------------- 1 file changed, 7 insertions(+), 35 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 7b71ad7..c1c9634 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -835,7 +835,7 @@ static char *spapr_phb_get_loc_code(SpaprPhbState *sphb, PCIDevice *pdev) #define b_fff(x) b_x((x), 8, 3) /* function number */ #define b_rrrrrrrr(x) b_x((x), 0, 8) /* register number */ -/* for 'reg'/'assigned-addresses' OF properties */ +/* for 'reg' OF properties */ #define RESOURCE_CELLS_SIZE 2 #define RESOURCE_CELLS_ADDRESS 3 @@ -849,17 +849,14 @@ typedef struct ResourceFields { typedef struct ResourceProps { ResourceFields reg[8]; - ResourceFields assigned[7]; uint32_t reg_len; - uint32_t assigned_len; } ResourceProps; -/* fill in the 'reg'/'assigned-resources' OF properties for +/* fill in the 'reg' OF properties for * a PCI device. 'reg' describes resource requirements for a - * device's IO/MEM regions, 'assigned-addresses' describes the - * actual resource assignments. + * device's IO/MEM regions. * - * the properties are arrays of ('phys-addr', 'size') pairs describing + * the property is an array of ('phys-addr', 'size') pairs describing * the addressable regions of the PCI device, where 'phys-addr' is a * RESOURCE_CELLS_ADDRESS-tuple of 32-bit integers corresponding to * (phys.hi, phys.mid, phys.lo), and 'size' is a @@ -888,18 +885,7 @@ typedef struct ResourceProps { * phys.mid and phys.lo correspond respectively to the hi/lo portions * of the actual address of the region. * - * how the phys-addr/size values are used differ slightly between - * 'reg' and 'assigned-addresses' properties. namely, 'reg' has - * an additional description for the config space region of the - * device, and in the case of QEMU has n=0 and phys.mid=phys.lo=0 - * to describe the region as relocatable, with an address-mapping - * that corresponds directly to the PHB's address space for the - * resource. 'assigned-addresses' always has n=1 set with an absolute - * address assigned for the resource. in general, 'assigned-addresses' - * won't be populated, since addresses for PCI devices are generally - * unmapped initially and left to the guest to assign. - * - * note also that addresses defined in these properties are, at least + * note also that addresses defined in this property are, at least * for PAPR guests, relative to the PHBs IO/MEM windows, and * correspond directly to the addresses in the BARs. * @@ -913,8 +899,8 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) uint32_t dev_id = (b_bbbbbbbb(bus_num) | b_ddddd(PCI_SLOT(d->devfn)) | b_fff(PCI_FUNC(d->devfn))); - ResourceFields *reg, *assigned; - int i, reg_idx = 0, assigned_idx = 0; + ResourceFields *reg; + int i, reg_idx = 0; /* config space region */ reg = &rp->reg[reg_idx++]; @@ -943,21 +929,9 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) reg->phys_lo = 0; reg->size_hi = cpu_to_be32(d->io_regions[i].size >> 32); reg->size_lo = cpu_to_be32(d->io_regions[i].size); - - if (d->io_regions[i].addr == PCI_BAR_UNMAPPED) { - continue; - } - - assigned = &rp->assigned[assigned_idx++]; - assigned->phys_hi = cpu_to_be32(be32_to_cpu(reg->phys_hi) | b_n(1)); - assigned->phys_mid = cpu_to_be32(d->io_regions[i].addr >> 32); - assigned->phys_lo = cpu_to_be32(d->io_regions[i].addr); - assigned->size_hi = reg->size_hi; - assigned->size_lo = reg->size_lo; } rp->reg_len = reg_idx * sizeof(ResourceFields); - rp->assigned_len = assigned_idx * sizeof(ResourceFields); } typedef struct PCIClass PCIClass; @@ -1471,8 +1445,6 @@ static int spapr_dt_pci_device(SpaprPhbState *sphb, PCIDevice *dev, populate_resource_props(dev, &rp); _FDT(fdt_setprop(fdt, offset, "reg", (uint8_t *)rp.reg, rp.reg_len)); - _FDT(fdt_setprop(fdt, offset, "assigned-addresses", - (uint8_t *)rp.assigned, rp.assigned_len)); if (sphb->pcie_ecs && pci_is_express(dev)) { _FDT(fdt_setprop_cell(fdt, offset, "ibm,pci-config-space-type", 0x1)); -- cgit v1.1 From e68cd0cb5cf49d334abe17231a1d2c28b846afa2 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Mon, 2 Sep 2019 15:41:16 +1000 Subject: spapr: Render full FDT on ibm,client-architecture-support The ibm,client-architecture-support call is a way for the guest to negotiate capabilities with a hypervisor. It is implemented as: - the guest calls SLOF via client interface; - SLOF calls QEMU (H_CAS hypercall) with an options vector from the guest; - QEMU returns a device tree diff (which uses FDT format with an additional header before it); - SLOF walks through the partial diff tree and updates its internal tree with the values from the diff. This changes QEMU to simply re-render the entire tree and send it as an update. SLOF can handle this already mostly, [1] is needed before this can be applied. This stores the resulting tree in the spapr machine to have the latest valid FDT copy possible (this should not matter much as H_UPDATE_DT happens right after that but nevertheless). The benefit is reduced code size as there is no need for another set of DT rendering helpers such as spapr_fixup_cpu_dt(). The downside is that the updates are bigger now (as they include all nodes and properties) but the difference on a '-smp 256,threads=1' system before/after is 2.35s vs. 2.5s. [1] https://patchwork.ozlabs.org/patch/1152915/ Signed-off-by: Alexey Kardashevskiy Signed-off-by: David Gibson --- hw/ppc/spapr.c | 88 ++++++---------------------------------------------------- 1 file changed, 9 insertions(+), 79 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 3742a8c..43920c1 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -295,64 +295,6 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr, _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size))); } -static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr) -{ - MachineState *ms = MACHINE(spapr); - int ret = 0, offset, cpus_offset; - CPUState *cs; - char cpu_model[32]; - uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)}; - - CPU_FOREACH(cs) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - DeviceClass *dc = DEVICE_GET_CLASS(cs); - int index = spapr_get_vcpu_id(cpu); - int compat_smt = MIN(ms->smp.threads, ppc_compat_max_vthreads(cpu)); - - if (!spapr_is_thread0_in_vcore(spapr, cpu)) { - continue; - } - - snprintf(cpu_model, 32, "%s@%x", dc->fw_name, index); - - cpus_offset = fdt_path_offset(fdt, "/cpus"); - if (cpus_offset < 0) { - cpus_offset = fdt_add_subnode(fdt, 0, "cpus"); - if (cpus_offset < 0) { - return cpus_offset; - } - } - offset = fdt_subnode_offset(fdt, cpus_offset, cpu_model); - if (offset < 0) { - offset = fdt_add_subnode(fdt, cpus_offset, cpu_model); - if (offset < 0) { - return offset; - } - } - - ret = fdt_setprop(fdt, offset, "ibm,pft-size", - pft_size_prop, sizeof(pft_size_prop)); - if (ret < 0) { - return ret; - } - - if (ms->numa_state->num_nodes > 1) { - ret = spapr_fixup_cpu_numa_dt(fdt, offset, cpu); - if (ret < 0) { - return ret; - } - } - - ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt); - if (ret < 0) { - return ret; - } - - spapr_populate_pa_features(spapr, cpu, fdt, offset); - } - return ret; -} - static hwaddr spapr_node0_size(MachineState *machine) { if (machine->numa_state->num_nodes) { @@ -983,11 +925,13 @@ static bool spapr_hotplugged_dev_before_cas(void) return false; } +static void *spapr_build_fdt(SpaprMachineState *spapr); + int spapr_h_cas_compose_response(SpaprMachineState *spapr, target_ulong addr, target_ulong size, SpaprOptionVector *ov5_updates) { - void *fdt, *fdt_skel; + void *fdt; SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 }; if (spapr_hotplugged_dev_before_cas()) { @@ -1003,25 +947,7 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr, size -= sizeof(hdr); - /* Create skeleton */ - fdt_skel = g_malloc0(size); - _FDT((fdt_create(fdt_skel, size))); - _FDT((fdt_finish_reservemap(fdt_skel))); - _FDT((fdt_begin_node(fdt_skel, ""))); - _FDT((fdt_end_node(fdt_skel))); - _FDT((fdt_finish(fdt_skel))); - fdt = g_malloc0(size); - _FDT((fdt_open_into(fdt_skel, fdt, size))); - g_free(fdt_skel); - - /* Fixup cpu nodes */ - _FDT((spapr_fixup_cpu_dt(fdt, spapr))); - - if (spapr_dt_cas_updates(spapr, fdt, ov5_updates)) { - return -1; - } - - /* Pack resulting tree */ + fdt = spapr_build_fdt(spapr); _FDT((fdt_pack(fdt))); if (fdt_totalsize(fdt) + sizeof(hdr) > size) { @@ -1033,7 +959,11 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr, cpu_physical_memory_write(addr, &hdr, sizeof(hdr)); cpu_physical_memory_write(addr + sizeof(hdr), fdt, fdt_totalsize(fdt)); trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr)); - g_free(fdt); + + g_free(spapr->fdt_blob); + spapr->fdt_size = fdt_totalsize(fdt); + spapr->fdt_initial_size = spapr->fdt_size; + spapr->fdt_blob = fdt; return 0; } -- cgit v1.1 From 627fa61746f70f7c799f08e9048bb6a482402138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 1 Oct 2019 10:57:22 +0200 Subject: spapr/xive: skip partially initialized vCPUs in presenter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When vCPUs are hotplugged, they are added to the QEMU CPU list before being fully realized. This can crash the XIVE presenter because the 'tctx' pointer is not necessarily initialized when looking for a matching target. These vCPUs are not valid targets for the presenter. Skip them. Signed-off-by: Cédric Le Goater Message-Id: <20191001085722.32755-1-clg@kaod.org> Signed-off-by: David Gibson Reviewed-by: Greg Kurz --- hw/intc/xive.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'hw') diff --git a/hw/intc/xive.c b/hw/intc/xive.c index b741721..29df06d 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1397,6 +1397,14 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, int ring; /* + * Skip partially initialized vCPUs. This can happen when + * vCPUs are hotplugged. + */ + if (!tctx) { + continue; + } + + /* * HW checks that the CPU is enabled in the Physical Thread * Enable Register (PTER). */ -- cgit v1.1 From d5803c7319f5cc79afda066e2c8b9c61436b43df Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 13:56:47 +1000 Subject: xics: Eliminate 'reject', 'resend' and 'eoi' class hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently ics_reject(), ics_resend() and ics_eoi() indirect through class methods. But there's only one implementation of each method, the one in TYPE_ICS_SIMPLE. TYPE_ICS_BASE has no implementation, but it's never instantiated, and has no other subtypes. So clean up by eliminating the method and just having ics_reject(), ics_resend() and ics_eoi() contain the logic directly. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/trace-events | 4 ++-- hw/intc/xics.c | 54 +++++++++++++--------------------------------------- 2 files changed, 15 insertions(+), 43 deletions(-) (limited to 'hw') diff --git a/hw/intc/trace-events b/hw/intc/trace-events index 719f46b..fdc716c 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -70,8 +70,8 @@ xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq 0x%x] xics_masked_pending(void) "set_irq_msi: masked pending" xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq 0x%x]" xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq 0x%x [src %d] server 0x%x prio 0x%x" -xics_ics_simple_reject(int nr, int srcno) "reject irq 0x%x [src %d]" -xics_ics_simple_eoi(int nr) "ics_eoi: irq 0x%x" +xics_ics_reject(int nr, int srcno) "reject irq 0x%x [src %d]" +xics_ics_eoi(int nr) "ics_eoi: irq 0x%x" # s390_flic_kvm.c flic_create_device(int err) "flic: create device failed %d" diff --git a/hw/intc/xics.c b/hw/intc/xics.c index b2fca29..93139b0 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -98,32 +98,8 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon) #define XISR(icp) (((icp)->xirr) & XISR_MASK) #define CPPR(icp) (((icp)->xirr) >> 24) -static void ics_reject(ICSState *ics, uint32_t nr) -{ - ICSStateClass *k = ICS_BASE_GET_CLASS(ics); - - if (k->reject) { - k->reject(ics, nr); - } -} - -void ics_resend(ICSState *ics) -{ - ICSStateClass *k = ICS_BASE_GET_CLASS(ics); - - if (k->resend) { - k->resend(ics); - } -} - -static void ics_eoi(ICSState *ics, int nr) -{ - ICSStateClass *k = ICS_BASE_GET_CLASS(ics); - - if (k->eoi) { - k->eoi(ics, nr); - } -} +static void ics_reject(ICSState *ics, uint32_t nr); +static void ics_eoi(ICSState *ics, uint32_t nr); static void icp_check_ipi(ICPState *icp) { @@ -427,7 +403,7 @@ Object *icp_create(Object *cpu, const char *type, XICSFabric *xi, Error **errp) /* * ICS: Source layer */ -static void ics_simple_resend_msi(ICSState *ics, int srcno) +static void ics_resend_msi(ICSState *ics, int srcno) { ICSIRQState *irq = ics->irqs + srcno; @@ -440,7 +416,7 @@ static void ics_simple_resend_msi(ICSState *ics, int srcno) } } -static void ics_simple_resend_lsi(ICSState *ics, int srcno) +static void ics_resend_lsi(ICSState *ics, int srcno) { ICSIRQState *irq = ics->irqs + srcno; @@ -478,7 +454,7 @@ static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val) } else { irq->status &= ~XICS_STATUS_ASSERTED; } - ics_simple_resend_lsi(ics, srcno); + ics_resend_lsi(ics, srcno); } void ics_simple_set_irq(void *opaque, int srcno, int val) @@ -512,7 +488,7 @@ static void ics_simple_write_xive_msi(ICSState *ics, int srcno) static void ics_simple_write_xive_lsi(ICSState *ics, int srcno) { - ics_simple_resend_lsi(ics, srcno); + ics_resend_lsi(ics, srcno); } void ics_simple_write_xive(ICSState *ics, int srcno, int server, @@ -534,11 +510,11 @@ void ics_simple_write_xive(ICSState *ics, int srcno, int server, } } -static void ics_simple_reject(ICSState *ics, uint32_t nr) +static void ics_reject(ICSState *ics, uint32_t nr) { ICSIRQState *irq = ics->irqs + nr - ics->offset; - trace_xics_ics_simple_reject(nr, nr - ics->offset); + trace_xics_ics_reject(nr, nr - ics->offset); if (irq->flags & XICS_FLAGS_IRQ_MSI) { irq->status |= XICS_STATUS_REJECTED; } else if (irq->flags & XICS_FLAGS_IRQ_LSI) { @@ -546,26 +522,26 @@ static void ics_simple_reject(ICSState *ics, uint32_t nr) } } -static void ics_simple_resend(ICSState *ics) +void ics_resend(ICSState *ics) { int i; for (i = 0; i < ics->nr_irqs; i++) { /* FIXME: filter by server#? */ if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) { - ics_simple_resend_lsi(ics, i); + ics_resend_lsi(ics, i); } else { - ics_simple_resend_msi(ics, i); + ics_resend_msi(ics, i); } } } -static void ics_simple_eoi(ICSState *ics, uint32_t nr) +static void ics_eoi(ICSState *ics, uint32_t nr) { int srcno = nr - ics->offset; ICSIRQState *irq = ics->irqs + srcno; - trace_xics_ics_simple_eoi(nr); + trace_xics_ics_eoi(nr); if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { irq->status &= ~XICS_STATUS_SENT; @@ -617,10 +593,6 @@ static void ics_simple_class_init(ObjectClass *klass, void *data) &isc->parent_realize); device_class_set_parent_reset(dc, ics_simple_reset, &isc->parent_reset); - - isc->reject = ics_simple_reject; - isc->resend = ics_simple_resend; - isc->eoi = ics_simple_eoi; } static const TypeInfo ics_simple_info = { -- cgit v1.1 From 28976c99cfa59e1880a86a59af20099eba62f5e7 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 14:13:39 +1000 Subject: xics: Rename misleading ics_simple_*() functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are a number of ics_simple_*() functions that aren't actually specific to TYPE_XICS_SIMPLE at all, and are equally valid on TYPE_XICS_BASE. Rename them to ics_*() accordingly. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/trace-events | 6 +++--- hw/intc/xics.c | 29 ++++++++++++++--------------- hw/intc/xics_spapr.c | 12 ++++++------ hw/ppc/pnv_psi.c | 4 ++-- hw/ppc/spapr_irq.c | 2 +- 5 files changed, 26 insertions(+), 27 deletions(-) (limited to 'hw') diff --git a/hw/intc/trace-events b/hw/intc/trace-events index fdc716c..527c3f7 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -66,10 +66,10 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) "icp_accept: XIRR 0x%"PRIx xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server %d given XIRR 0x%"PRIx32" new XIRR 0x%"PRIx32 xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver irq 0x%"PRIx32" priority 0x%x" xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new XIRR=0x%x new pending priority=0x%x" -xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq 0x%x]" +xics_ics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq 0x%x]" xics_masked_pending(void) "set_irq_msi: masked pending" -xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq 0x%x]" -xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq 0x%x [src %d] server 0x%x prio 0x%x" +xics_ics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq 0x%x]" +xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq 0x%x [src %d] server 0x%x prio 0x%x" xics_ics_reject(int nr, int srcno) "reject irq 0x%x [src %d]" xics_ics_eoi(int nr) "ics_eoi: irq 0x%x" diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 93139b0..310dc72 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -428,11 +428,11 @@ static void ics_resend_lsi(ICSState *ics, int srcno) } } -static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val) +static void ics_set_irq_msi(ICSState *ics, int srcno, int val) { ICSIRQState *irq = ics->irqs + srcno; - trace_xics_ics_simple_set_irq_msi(srcno, srcno + ics->offset); + trace_xics_ics_set_irq_msi(srcno, srcno + ics->offset); if (val) { if (irq->priority == 0xff) { @@ -444,11 +444,11 @@ static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val) } } -static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val) +static void ics_set_irq_lsi(ICSState *ics, int srcno, int val) { ICSIRQState *irq = ics->irqs + srcno; - trace_xics_ics_simple_set_irq_lsi(srcno, srcno + ics->offset); + trace_xics_ics_set_irq_lsi(srcno, srcno + ics->offset); if (val) { irq->status |= XICS_STATUS_ASSERTED; } else { @@ -457,7 +457,7 @@ static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val) ics_resend_lsi(ics, srcno); } -void ics_simple_set_irq(void *opaque, int srcno, int val) +void ics_set_irq(void *opaque, int srcno, int val) { ICSState *ics = (ICSState *)opaque; @@ -467,13 +467,13 @@ void ics_simple_set_irq(void *opaque, int srcno, int val) } if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { - ics_simple_set_irq_lsi(ics, srcno, val); + ics_set_irq_lsi(ics, srcno, val); } else { - ics_simple_set_irq_msi(ics, srcno, val); + ics_set_irq_msi(ics, srcno, val); } } -static void ics_simple_write_xive_msi(ICSState *ics, int srcno) +static void ics_write_xive_msi(ICSState *ics, int srcno) { ICSIRQState *irq = ics->irqs + srcno; @@ -486,13 +486,13 @@ static void ics_simple_write_xive_msi(ICSState *ics, int srcno) icp_irq(ics, irq->server, srcno + ics->offset, irq->priority); } -static void ics_simple_write_xive_lsi(ICSState *ics, int srcno) +static void ics_write_xive_lsi(ICSState *ics, int srcno) { ics_resend_lsi(ics, srcno); } -void ics_simple_write_xive(ICSState *ics, int srcno, int server, - uint8_t priority, uint8_t saved_priority) +void ics_write_xive(ICSState *ics, int srcno, int server, + uint8_t priority, uint8_t saved_priority) { ICSIRQState *irq = ics->irqs + srcno; @@ -500,13 +500,12 @@ void ics_simple_write_xive(ICSState *ics, int srcno, int server, irq->priority = priority; irq->saved_priority = saved_priority; - trace_xics_ics_simple_write_xive(ics->offset + srcno, srcno, server, - priority); + trace_xics_ics_write_xive(ics->offset + srcno, srcno, server, priority); if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { - ics_simple_write_xive_lsi(ics, srcno); + ics_write_xive_lsi(ics, srcno); } else { - ics_simple_write_xive_msi(ics, srcno); + ics_write_xive_msi(ics, srcno); } } diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 6577be0..3e94448 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -179,7 +179,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, } srcno = nr - ics->offset; - ics_simple_write_xive(ics, srcno, server, priority, priority); + ics_write_xive(ics, srcno, server, priority, priority); rtas_st(rets, 0, RTAS_OUT_SUCCESS); } @@ -243,8 +243,8 @@ static void rtas_int_off(PowerPCCPU *cpu, SpaprMachineState *spapr, } srcno = nr - ics->offset; - ics_simple_write_xive(ics, srcno, ics->irqs[srcno].server, 0xff, - ics->irqs[srcno].priority); + ics_write_xive(ics, srcno, ics->irqs[srcno].server, 0xff, + ics->irqs[srcno].priority); rtas_st(rets, 0, RTAS_OUT_SUCCESS); } @@ -276,9 +276,9 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, } srcno = nr - ics->offset; - ics_simple_write_xive(ics, srcno, ics->irqs[srcno].server, - ics->irqs[srcno].saved_priority, - ics->irqs[srcno].saved_priority); + ics_write_xive(ics, srcno, ics->irqs[srcno].server, + ics->irqs[srcno].saved_priority, + ics->irqs[srcno].saved_priority); rtas_st(rets, 0, RTAS_OUT_SUCCESS); } diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c index 88ba8e7..8ea81e9 100644 --- a/hw/ppc/pnv_psi.c +++ b/hw/ppc/pnv_psi.c @@ -311,7 +311,7 @@ static void pnv_psi_set_xivr(PnvPsi *psi, uint32_t reg, uint64_t val) * do for now but a more accurate implementation would instead * use a fixed server/prio and a remapper of the generated irq. */ - ics_simple_write_xive(ics, src, server, prio, prio); + ics_write_xive(ics, src, server, prio, prio); } static uint64_t pnv_psi_reg_read(PnvPsi *psi, uint32_t offset, bool mmio) @@ -514,7 +514,7 @@ static void pnv_psi_power8_realize(DeviceState *dev, Error **errp) ics_set_irq_type(ics, i, true); } - psi->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs); + psi->qirqs = qemu_allocate_irqs(ics_set_irq, ics, ics->nr_irqs); /* XSCOM region for PSI registers */ pnv_xscom_region_init(&psi->xscom_regs, OBJECT(dev), &pnv_psi_xscom_ops, diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index d8f46b6..ac189c5 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -210,7 +210,7 @@ static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val) { SpaprMachineState *spapr = opaque; - ics_simple_set_irq(spapr->ics, srcno, val); + ics_set_irq(spapr->ics, srcno, val); } static void spapr_irq_reset_xics(SpaprMachineState *spapr, Error **errp) -- cgit v1.1 From da2ef5b2f24be70d4fa0b05fd6799031cd3a456e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 14:19:22 +1000 Subject: xics: Eliminate reset hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently TYPE_XICS_BASE and TYPE_XICS_SIMPLE have their own reset methods, using the standard technique for having the subtype call the supertype's methods before doing its own thing. But TYPE_XICS_SIMPLE is the only subtype of TYPE_XICS_BASE ever instantiated, so there's no point having the split here. Merge them together into just an ics_reset() function. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/xics.c | 57 ++++++++++++++++++++++++--------------------------------- 1 file changed, 24 insertions(+), 33 deletions(-) (limited to 'hw') diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 310dc72..82e6f09 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -547,11 +547,28 @@ static void ics_eoi(ICSState *ics, uint32_t nr) } } -static void ics_simple_reset(DeviceState *dev) +static void ics_reset_irq(ICSIRQState *irq) { - ICSStateClass *icsc = ICS_BASE_GET_CLASS(dev); + irq->priority = 0xff; + irq->saved_priority = 0xff; +} - icsc->parent_reset(dev); +static void ics_reset(DeviceState *dev) +{ + ICSState *ics = ICS_BASE(dev); + int i; + uint8_t flags[ics->nr_irqs]; + + for (i = 0; i < ics->nr_irqs; i++) { + flags[i] = ics->irqs[i].flags; + } + + memset(ics->irqs, 0, sizeof(ICSIRQState) * ics->nr_irqs); + + for (i = 0; i < ics->nr_irqs; i++) { + ics_reset_irq(ics->irqs + i); + ics->irqs[i].flags = flags[i]; + } if (kvm_irqchip_in_kernel()) { Error *local_err = NULL; @@ -563,9 +580,9 @@ static void ics_simple_reset(DeviceState *dev) } } -static void ics_simple_reset_handler(void *dev) +static void ics_reset_handler(void *dev) { - ics_simple_reset(dev); + ics_reset(dev); } static void ics_simple_realize(DeviceState *dev, Error **errp) @@ -580,7 +597,7 @@ static void ics_simple_realize(DeviceState *dev, Error **errp) return; } - qemu_register_reset(ics_simple_reset_handler, ics); + qemu_register_reset(ics_reset_handler, ics); } static void ics_simple_class_init(ObjectClass *klass, void *data) @@ -590,8 +607,6 @@ static void ics_simple_class_init(ObjectClass *klass, void *data) device_class_set_parent_realize(dc, ics_simple_realize, &isc->parent_realize); - device_class_set_parent_reset(dc, ics_simple_reset, - &isc->parent_reset); } static const TypeInfo ics_simple_info = { @@ -602,30 +617,6 @@ static const TypeInfo ics_simple_info = { .class_size = sizeof(ICSStateClass), }; -static void ics_reset_irq(ICSIRQState *irq) -{ - irq->priority = 0xff; - irq->saved_priority = 0xff; -} - -static void ics_base_reset(DeviceState *dev) -{ - ICSState *ics = ICS_BASE(dev); - int i; - uint8_t flags[ics->nr_irqs]; - - for (i = 0; i < ics->nr_irqs; i++) { - flags[i] = ics->irqs[i].flags; - } - - memset(ics->irqs, 0, sizeof(ICSIRQState) * ics->nr_irqs); - - for (i = 0; i < ics->nr_irqs; i++) { - ics_reset_irq(ics->irqs + i); - ics->irqs[i].flags = flags[i]; - } -} - static void ics_base_realize(DeviceState *dev, Error **errp) { ICSState *ics = ICS_BASE(dev); @@ -726,7 +717,7 @@ static void ics_base_class_init(ObjectClass *klass, void *data) dc->realize = ics_base_realize; dc->props = ics_base_properties; - dc->reset = ics_base_reset; + dc->reset = ics_reset; dc->vmsd = &vmstate_ics_base; } -- cgit v1.1 From 642e92719e2790dfa0e12be1cfd822a0ff2322aa Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 15:29:25 +1000 Subject: xics: Merge TYPE_ICS_BASE and TYPE_ICS_SIMPLE classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TYPE_ICS_SIMPLE is the only subtype of TYPE_ICS_BASE that's ever instantiated. The existence of different classes is mostly a hang over from when we (misguidedly) had separate subtypes for the KVM and non-KVM version of the device. There could be some call for an abstract base type for ICS variants that use a different representation of their state (PowerNV PHB3 might want this). The current split isn't really in the right place for that though. If we need this in future, we can re-implement it more in line with what we actually need. So, collapse the two classes together into just TYPE_ICS. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/xics.c | 86 +++++++++++++++++------------------------------------- hw/ppc/pnv_psi.c | 2 +- hw/ppc/spapr_irq.c | 4 +-- 3 files changed, 30 insertions(+), 62 deletions(-) (limited to 'hw') diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 82e6f09..dfe7dbd 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -555,7 +555,7 @@ static void ics_reset_irq(ICSIRQState *irq) static void ics_reset(DeviceState *dev) { - ICSState *ics = ICS_BASE(dev); + ICSState *ics = ICS(dev); int i; uint8_t flags[ics->nr_irqs]; @@ -573,7 +573,7 @@ static void ics_reset(DeviceState *dev) if (kvm_irqchip_in_kernel()) { Error *local_err = NULL; - ics_set_kvm_state(ICS_BASE(dev), &local_err); + ics_set_kvm_state(ICS(dev), &local_err); if (local_err) { error_report_err(local_err); } @@ -585,47 +585,15 @@ static void ics_reset_handler(void *dev) ics_reset(dev); } -static void ics_simple_realize(DeviceState *dev, Error **errp) +static void ics_realize(DeviceState *dev, Error **errp) { - ICSState *ics = ICS_SIMPLE(dev); - ICSStateClass *icsc = ICS_BASE_GET_CLASS(ics); + ICSState *ics = ICS(dev); Error *local_err = NULL; - - icsc->parent_realize(dev, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - qemu_register_reset(ics_reset_handler, ics); -} - -static void ics_simple_class_init(ObjectClass *klass, void *data) -{ - DeviceClass *dc = DEVICE_CLASS(klass); - ICSStateClass *isc = ICS_BASE_CLASS(klass); - - device_class_set_parent_realize(dc, ics_simple_realize, - &isc->parent_realize); -} - -static const TypeInfo ics_simple_info = { - .name = TYPE_ICS_SIMPLE, - .parent = TYPE_ICS_BASE, - .instance_size = sizeof(ICSState), - .class_init = ics_simple_class_init, - .class_size = sizeof(ICSStateClass), -}; - -static void ics_base_realize(DeviceState *dev, Error **errp) -{ - ICSState *ics = ICS_BASE(dev); Object *obj; - Error *err = NULL; - obj = object_property_get_link(OBJECT(dev), ICS_PROP_XICS, &err); + obj = object_property_get_link(OBJECT(dev), ICS_PROP_XICS, &local_err); if (!obj) { - error_propagate_prepend(errp, err, + error_propagate_prepend(errp, local_err, "required link '" ICS_PROP_XICS "' not found: "); return; @@ -637,16 +605,18 @@ static void ics_base_realize(DeviceState *dev, Error **errp) return; } ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState)); + + qemu_register_reset(ics_reset_handler, ics); } -static void ics_base_instance_init(Object *obj) +static void ics_instance_init(Object *obj) { - ICSState *ics = ICS_BASE(obj); + ICSState *ics = ICS(obj); ics->offset = XICS_IRQ_BASE; } -static int ics_base_pre_save(void *opaque) +static int ics_pre_save(void *opaque) { ICSState *ics = opaque; @@ -657,7 +627,7 @@ static int ics_base_pre_save(void *opaque) return 0; } -static int ics_base_post_load(void *opaque, int version_id) +static int ics_post_load(void *opaque, int version_id) { ICSState *ics = opaque; @@ -675,7 +645,7 @@ static int ics_base_post_load(void *opaque, int version_id) return 0; } -static const VMStateDescription vmstate_ics_base_irq = { +static const VMStateDescription vmstate_ics_irq = { .name = "ics/irq", .version_id = 2, .minimum_version_id = 1, @@ -689,45 +659,44 @@ static const VMStateDescription vmstate_ics_base_irq = { }, }; -static const VMStateDescription vmstate_ics_base = { +static const VMStateDescription vmstate_ics = { .name = "ics", .version_id = 1, .minimum_version_id = 1, - .pre_save = ics_base_pre_save, - .post_load = ics_base_post_load, + .pre_save = ics_pre_save, + .post_load = ics_post_load, .fields = (VMStateField[]) { /* Sanity check */ VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL), VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs, - vmstate_ics_base_irq, + vmstate_ics_irq, ICSIRQState), VMSTATE_END_OF_LIST() }, }; -static Property ics_base_properties[] = { +static Property ics_properties[] = { DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0), DEFINE_PROP_END_OF_LIST(), }; -static void ics_base_class_init(ObjectClass *klass, void *data) +static void ics_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - dc->realize = ics_base_realize; - dc->props = ics_base_properties; + dc->realize = ics_realize; + dc->props = ics_properties; dc->reset = ics_reset; - dc->vmsd = &vmstate_ics_base; + dc->vmsd = &vmstate_ics; } -static const TypeInfo ics_base_info = { - .name = TYPE_ICS_BASE, +static const TypeInfo ics_info = { + .name = TYPE_ICS, .parent = TYPE_DEVICE, - .abstract = true, .instance_size = sizeof(ICSState), - .instance_init = ics_base_instance_init, - .class_init = ics_base_class_init, + .instance_init = ics_instance_init, + .class_init = ics_class_init, .class_size = sizeof(ICSStateClass), }; @@ -767,8 +736,7 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool lsi) static void xics_register_types(void) { - type_register_static(&ics_simple_info); - type_register_static(&ics_base_info); + type_register_static(&ics_info); type_register_static(&icp_info); type_register_static(&xics_fabric_info); } diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c index 8ea81e9..a997f16 100644 --- a/hw/ppc/pnv_psi.c +++ b/hw/ppc/pnv_psi.c @@ -469,7 +469,7 @@ static void pnv_psi_power8_instance_init(Object *obj) Pnv8Psi *psi8 = PNV8_PSI(obj); object_initialize_child(obj, "ics-psi", &psi8->ics, sizeof(psi8->ics), - TYPE_ICS_SIMPLE, &error_abort, NULL); + TYPE_ICS, &error_abort, NULL); } static const uint8_t irq_to_xivr[] = { diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index ac189c5..6c45d2a 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, Object *obj; Error *local_err = NULL; - obj = object_new(TYPE_ICS_SIMPLE); + obj = object_new(TYPE_ICS); object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), &error_fatal); @@ -109,7 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, return; } - spapr->ics = ICS_BASE(obj); + spapr->ics = ICS(obj); xics_spapr_init(spapr); } -- cgit v1.1 From 9db8c551c98c285bad2ed19ce28a721d33c20d2f Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 15:51:55 +1000 Subject: xics: Create sPAPR specific ICS subtype MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We create a subtype of TYPE_ICS specifically for sPAPR. For now all this does is move the setup of the PAPR specific hcalls and RTAS calls to the realize() function for this, rather than requiring the PAPR code to explicitly call xics_spapr_init(). In future it will have some more function. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- hw/ppc/spapr_irq.c | 6 ++---- 2 files changed, 35 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 3e94448..e6dd004 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, rtas_st(rets, 0, RTAS_OUT_SUCCESS); } -void xics_spapr_init(SpaprMachineState *spapr) +static void ics_spapr_realize(DeviceState *dev, Error **errp) { + ICSState *ics = ICS_SPAPR(dev); + ICSStateClass *icsc = ICS_GET_CLASS(ics); + Error *local_err = NULL; + + icsc->parent_realize(dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); } + +static void ics_spapr_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + ICSStateClass *isc = ICS_CLASS(klass); + + device_class_set_parent_realize(dc, ics_spapr_realize, + &isc->parent_realize); +} + +static const TypeInfo ics_spapr_info = { + .name = TYPE_ICS_SPAPR, + .parent = TYPE_ICS, + .class_init = ics_spapr_class_init, +}; + +static void xics_spapr_register_types(void) +{ + type_register_static(&ics_spapr_info); +} + +type_init(xics_spapr_register_types) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 6c45d2a..8c26fa2 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, Object *obj; Error *local_err = NULL; - obj = object_new(TYPE_ICS); + obj = object_new(TYPE_ICS_SPAPR); object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), &error_fatal); @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, return; } - spapr->ics = ICS(obj); - - xics_spapr_init(spapr); + spapr->ics = ICS_SPAPR(obj); } static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, -- cgit v1.1 From 258aa5ce1c62bf9cf28dc344f52e13ea3a0a38cd Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 23 Sep 2019 15:43:58 +1000 Subject: spapr: Fold spapr_phb_lsi_qirq() into its single caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No point having a two-line helper that's used exactly once, and not likely to be used anywhere else in future. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé --- hw/ppc/spapr_pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index c1c9634..01ff41d 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -721,9 +721,10 @@ static void pci_spapr_set_irq(void *opaque, int irq_num, int level) * corresponding qemu_irq. */ SpaprPhbState *phb = opaque; + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); trace_spapr_pci_lsi_set(phb->dtbusname, irq_num, phb->lsi_table[irq_num].irq); - qemu_set_irq(spapr_phb_lsi_qirq(phb, irq_num), level); + qemu_set_irq(spapr_qirq(spapr, phb->lsi_table[irq_num].irq), level); } static PCIINTxRoute spapr_route_intx_pin_to_irq(void *opaque, int pin) -- cgit v1.1 From 7678b74a94a70186da3b3971d7ae92ca1a81c969 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 23 Sep 2019 15:50:09 +1000 Subject: spapr: Replace spapr_vio_qirq() helper with spapr_vio_irq_pulse() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every caller of spapr_vio_qirq() immediately calls qemu_irq_pulse() with the result, so we might as well just fold that into the helper. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé --- hw/char/spapr_vty.c | 3 +-- hw/net/spapr_llan.c | 3 +-- hw/ppc/spapr_vio.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c index 087c93e..8f4d9fe 100644 --- a/hw/char/spapr_vty.c +++ b/hw/char/spapr_vty.c @@ -5,7 +5,6 @@ #include "cpu.h" #include "migration/vmstate.h" #include "chardev/char-fe.h" -#include "hw/irq.h" #include "hw/ppc/spapr.h" #include "hw/ppc/spapr_vio.h" #include "hw/qdev-properties.h" @@ -37,7 +36,7 @@ static void vty_receive(void *opaque, const uint8_t *buf, int size) if ((dev->in == dev->out) && size) { /* toggle line to simulate edge interrupt */ - qemu_irq_pulse(spapr_vio_qirq(&dev->sdev)); + spapr_vio_irq_pulse(&dev->sdev); } for (i = 0; i < size; i++) { if (dev->in - dev->out >= VTERM_BUFSIZE) { diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c index 701e6e1..3d96884 100644 --- a/hw/net/spapr_llan.c +++ b/hw/net/spapr_llan.c @@ -27,7 +27,6 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "hw/irq.h" #include "qemu/log.h" #include "qemu/module.h" #include "net/net.h" @@ -267,7 +266,7 @@ static ssize_t spapr_vlan_receive(NetClientState *nc, const uint8_t *buf, } if (sdev->signal_state & 1) { - qemu_irq_pulse(spapr_vio_qirq(sdev)); + spapr_vio_irq_pulse(sdev); } return size; diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index 0803649..554de99 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -23,7 +23,6 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "qapi/visitor.h" -#include "hw/irq.h" #include "qemu/log.h" #include "hw/loader.h" #include "elf.h" @@ -294,7 +293,7 @@ int spapr_vio_send_crq(SpaprVioDevice *dev, uint8_t *crq) dev->crq.qnext = (dev->crq.qnext + 16) % dev->crq.qsize; if (dev->signal_state & 1) { - qemu_irq_pulse(spapr_vio_qirq(dev)); + spapr_vio_irq_pulse(dev); } return 0; -- cgit v1.1 From ad8de98636e7cadeb1be4efa997cfe2a60bd5c30 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 10:53:50 +1000 Subject: spapr: Clarify and fix handling of nr_irqs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the XICS and XIVE interrupt backends have a "nr-irqs" property, but it means slightly different things. For XICS (or, strictly, the ICS) it indicates the number of "real" external IRQs. Those start at XICS_IRQ_BASE (0x1000) and don't include the special IPI vector. For XIVE, however, it includes the whole IRQ space, including XIVE's many IPI vectors. The spapr code currently doesn't handle this sensibly, with the nr_irqs value in SpaprIrq having different meanings depending on the backend. We fix this by renaming nr_irqs to nr_xirqs and making it always indicate just the number of external irqs, adjusting the value we pass to XIVE accordingly. We also move to using common constants in most of the irq configurations, to make it clearer that the IRQ space looks the same to the guest (and emulated devices), even if the backend is different. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr_irq.c | 53 +++++++++++++++++++---------------------------------- 1 file changed, 19 insertions(+), 34 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 8c26fa2..3207b6b 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -92,7 +92,7 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr, * XICS IRQ backend. */ -static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, +static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_xirqs, Error **errp) { Object *obj; @@ -102,7 +102,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), &error_fatal); - object_property_set_int(obj, nr_irqs, "nr-irqs", &error_fatal); + object_property_set_int(obj, nr_xirqs, "nr-irqs", &error_fatal); object_property_set_bool(obj, true, "realized", &local_err); if (local_err) { error_propagate(errp, local_err); @@ -234,13 +234,9 @@ static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp) } } -#define SPAPR_IRQ_XICS_NR_IRQS 0x1000 -#define SPAPR_IRQ_XICS_NR_MSIS \ - (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI) - SpaprIrq spapr_irq_xics = { - .nr_irqs = SPAPR_IRQ_XICS_NR_IRQS, - .nr_msis = SPAPR_IRQ_XICS_NR_MSIS, + .nr_xirqs = SPAPR_NR_XIRQS, + .nr_msis = SPAPR_NR_MSIS, .ov5 = SPAPR_OV5_XIVE_LEGACY, .init = spapr_irq_init_xics, @@ -260,7 +256,7 @@ SpaprIrq spapr_irq_xics = { /* * XIVE IRQ backend. */ -static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_irqs, +static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_xirqs, Error **errp) { uint32_t nr_servers = spapr_max_server_number(spapr); @@ -268,7 +264,7 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_irqs, int i; dev = qdev_create(NULL, TYPE_SPAPR_XIVE); - qdev_prop_set_uint32(dev, "nr-irqs", nr_irqs); + qdev_prop_set_uint32(dev, "nr-irqs", nr_xirqs + SPAPR_XIRQ_BASE); /* * 8 XIVE END structures per CPU. One for each available priority */ @@ -308,7 +304,7 @@ static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq) { SpaprXive *xive = spapr->xive; - if (irq >= xive->nr_irqs) { + if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) { return NULL; } @@ -404,17 +400,9 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp) } } -/* - * XIVE uses the full IRQ number space. Set it to 8K to be compatible - * with XICS. - */ - -#define SPAPR_IRQ_XIVE_NR_IRQS 0x2000 -#define SPAPR_IRQ_XIVE_NR_MSIS (SPAPR_IRQ_XIVE_NR_IRQS - SPAPR_IRQ_MSI) - SpaprIrq spapr_irq_xive = { - .nr_irqs = SPAPR_IRQ_XIVE_NR_IRQS, - .nr_msis = SPAPR_IRQ_XIVE_NR_MSIS, + .nr_xirqs = SPAPR_NR_XIRQS, + .nr_msis = SPAPR_NR_MSIS, .ov5 = SPAPR_OV5_XIVE_EXPLOIT, .init = spapr_irq_init_xive, @@ -450,18 +438,18 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) &spapr_irq_xive : &spapr_irq_xics; } -static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_irqs, +static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_xirqs, Error **errp) { Error *local_err = NULL; - spapr_irq_xics.init(spapr, spapr_irq_xics.nr_irqs, &local_err); + spapr_irq_xics.init(spapr, spapr_irq_xics.nr_xirqs, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - spapr_irq_xive.init(spapr, spapr_irq_xive.nr_irqs, &local_err); + spapr_irq_xive.init(spapr, spapr_irq_xive.nr_xirqs, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -586,12 +574,9 @@ static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr) /* * Define values in sync with the XIVE and XICS backend */ -#define SPAPR_IRQ_DUAL_NR_IRQS 0x2000 -#define SPAPR_IRQ_DUAL_NR_MSIS (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI) - SpaprIrq spapr_irq_dual = { - .nr_irqs = SPAPR_IRQ_DUAL_NR_IRQS, - .nr_msis = SPAPR_IRQ_DUAL_NR_MSIS, + .nr_xirqs = SPAPR_NR_XIRQS, + .nr_msis = SPAPR_NR_MSIS, .ov5 = SPAPR_OV5_XIVE_BOTH, .init = spapr_irq_init_dual, @@ -693,10 +678,10 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) spapr_irq_msi_init(spapr, spapr->irq->nr_msis); } - spapr->irq->init(spapr, spapr->irq->nr_irqs, errp); + spapr->irq->init(spapr, spapr->irq->nr_xirqs, errp); spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr, - spapr->irq->nr_irqs); + spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); } int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) @@ -804,11 +789,11 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) return first + ics->offset; } -#define SPAPR_IRQ_XICS_LEGACY_NR_IRQS 0x400 +#define SPAPR_IRQ_XICS_LEGACY_NR_XIRQS 0x400 SpaprIrq spapr_irq_xics_legacy = { - .nr_irqs = SPAPR_IRQ_XICS_LEGACY_NR_IRQS, - .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_IRQS, + .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, + .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, .ov5 = SPAPR_OV5_XIVE_LEGACY, .init = spapr_irq_init_xics, -- cgit v1.1 From fe9b61b2468a6de170ae0e9afe92fa1daa7ab48b Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 11:34:12 +1000 Subject: spapr: Eliminate nr_irqs parameter to SpaprIrq::init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only reason this parameter was needed was to work around the inconsistent meaning of nr_irqs between xics and xive. Now that we've fixed that, we can consistently use the number directly in the SpaprIrq configuration. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/spapr_irq.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 3207b6b..cded3a0 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -92,8 +92,7 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr, * XICS IRQ backend. */ -static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_xirqs, - Error **errp) +static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) { Object *obj; Error *local_err = NULL; @@ -102,7 +101,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_xirqs, object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), &error_fatal); - object_property_set_int(obj, nr_xirqs, "nr-irqs", &error_fatal); + object_property_set_int(obj, spapr->irq->nr_xirqs, + "nr-irqs", &error_fatal); object_property_set_bool(obj, true, "realized", &local_err); if (local_err) { error_propagate(errp, local_err); @@ -256,15 +256,15 @@ SpaprIrq spapr_irq_xics = { /* * XIVE IRQ backend. */ -static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_xirqs, - Error **errp) +static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) { uint32_t nr_servers = spapr_max_server_number(spapr); DeviceState *dev; int i; dev = qdev_create(NULL, TYPE_SPAPR_XIVE); - qdev_prop_set_uint32(dev, "nr-irqs", nr_xirqs + SPAPR_XIRQ_BASE); + qdev_prop_set_uint32(dev, "nr-irqs", + spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); /* * 8 XIVE END structures per CPU. One for each available priority */ @@ -438,18 +438,17 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) &spapr_irq_xive : &spapr_irq_xics; } -static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_xirqs, - Error **errp) +static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) { Error *local_err = NULL; - spapr_irq_xics.init(spapr, spapr_irq_xics.nr_xirqs, &local_err); + spapr_irq_xics.init(spapr, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - spapr_irq_xive.init(spapr, spapr_irq_xive.nr_xirqs, &local_err); + spapr_irq_xive.init(spapr, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -678,7 +677,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) spapr_irq_msi_init(spapr, spapr->irq->nr_msis); } - spapr->irq->init(spapr, spapr->irq->nr_xirqs, errp); + spapr->irq->init(spapr, errp); spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr, spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); -- cgit v1.1 From 9f53c0db19605f76324fb09af23d30e181a06211 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 11:12:19 +1000 Subject: spapr: Fix indexing of XICS irqs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spapr global irq numbers are different from the source numbers on the ICS when using XICS - they're offset by XICS_IRQ_BASE (0x1000). But spapr_irq_set_irq_xics() was passing through the global irq number to the ICS code unmodified. We only got away with this because of a counteracting bug - we were incorrectly adjusting the qemu_irq we returned for a requested global irq number. That approach mostly worked but is very confusing, incorrectly relies on the way the qemu_irq array is allocated, and undermines the intention of having the global array of qemu_irqs for spapr have a consistent meaning regardless of irq backend. So, fix both set_irq and qemu_irq indexing. We rename some parameters at the same time to make it clear that they are referring to spapr global irq numbers. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/spapr_irq.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index cded3a0..8f79aa8 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -153,10 +153,9 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num) static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq) { ICSState *ics = spapr->ics; - uint32_t srcno = irq - ics->offset; if (ics_valid_irq(ics, irq)) { - return spapr->qirqs[srcno]; + return spapr->qirqs[irq]; } return NULL; @@ -204,9 +203,10 @@ static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) return 0; } -static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val) +static void spapr_irq_set_irq_xics(void *opaque, int irq, int val) { SpaprMachineState *spapr = opaque; + uint32_t srcno = irq - spapr->ics->offset; ics_set_irq(spapr->ics, srcno, val); } @@ -377,14 +377,14 @@ static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) spapr_xive_mmio_set_enabled(spapr->xive, true); } -static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val) +static void spapr_irq_set_irq_xive(void *opaque, int irq, int val) { SpaprMachineState *spapr = opaque; if (kvm_irqchip_in_kernel()) { - kvmppc_xive_source_set_irq(&spapr->xive->source, srcno, val); + kvmppc_xive_source_set_irq(&spapr->xive->source, irq, val); } else { - xive_source_set_irq(&spapr->xive->source, srcno, val); + xive_source_set_irq(&spapr->xive->source, irq, val); } } @@ -558,11 +558,11 @@ static void spapr_irq_reset_dual(SpaprMachineState *spapr, Error **errp) spapr_irq_current(spapr)->reset(spapr, errp); } -static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val) +static void spapr_irq_set_irq_dual(void *opaque, int irq, int val) { SpaprMachineState *spapr = opaque; - spapr_irq_current(spapr)->set_irq(spapr, srcno, val); + spapr_irq_current(spapr)->set_irq(spapr, irq, val); } static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr) -- cgit v1.1 From af1861511d3853664e5362ea3d2eb14b1f8d05b4 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 23 Sep 2019 16:18:28 +1000 Subject: spapr: Simplify spapr_qirq() handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently spapr_qirq(), whic is used to find the qemu_irq for an spapr global irq number, redirects through the SpaprIrq::qirq method. But the array of qemu_irqs is allocated in the PAPR layer, not the backends, and so the method implementations all return the same thing, just differing in the preliminary checks they make. So, we can remove the method, and just implement spapr_qirq() directly, including all the relevant checks in one place. We change all those checks into assert()s as well, since a failure here indicates an error in the calling code. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé --- hw/ppc/spapr_irq.c | 54 +++++++++++++++++++----------------------------------- 1 file changed, 19 insertions(+), 35 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 8f79aa8..8f17907 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -150,17 +150,6 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num) } } -static qemu_irq spapr_qirq_xics(SpaprMachineState *spapr, int irq) -{ - ICSState *ics = spapr->ics; - - if (ics_valid_irq(ics, irq)) { - return spapr->qirqs[irq]; - } - - return NULL; -} - static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon) { CPUState *cs; @@ -242,7 +231,6 @@ SpaprIrq spapr_irq_xics = { .init = spapr_irq_init_xics, .claim = spapr_irq_claim_xics, .free = spapr_irq_free_xics, - .qirq = spapr_qirq_xics, .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, .cpu_intc_create = spapr_irq_cpu_intc_create_xics, @@ -300,20 +288,6 @@ static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq, int num) } } -static qemu_irq spapr_qirq_xive(SpaprMachineState *spapr, int irq) -{ - SpaprXive *xive = spapr->xive; - - if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) { - return NULL; - } - - /* The sPAPR machine/device should have claimed the IRQ before */ - assert(xive_eas_is_valid(&xive->eat[irq])); - - return spapr->qirqs[irq]; -} - static void spapr_irq_print_info_xive(SpaprMachineState *spapr, Monitor *mon) { @@ -408,7 +382,6 @@ SpaprIrq spapr_irq_xive = { .init = spapr_irq_init_xive, .claim = spapr_irq_claim_xive, .free = spapr_irq_free_xive, - .qirq = spapr_qirq_xive, .print_info = spapr_irq_print_info_xive, .dt_populate = spapr_dt_xive, .cpu_intc_create = spapr_irq_cpu_intc_create_xive, @@ -482,11 +455,6 @@ static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq, int num) spapr_irq_xive.free(spapr, irq, num); } -static qemu_irq spapr_qirq_dual(SpaprMachineState *spapr, int irq) -{ - return spapr_irq_current(spapr)->qirq(spapr, irq); -} - static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon) { spapr_irq_current(spapr)->print_info(spapr, mon); @@ -581,7 +549,6 @@ SpaprIrq spapr_irq_dual = { .init = spapr_irq_init_dual, .claim = spapr_irq_claim_dual, .free = spapr_irq_free_dual, - .qirq = spapr_qirq_dual, .print_info = spapr_irq_print_info_dual, .dt_populate = spapr_irq_dt_populate_dual, .cpu_intc_create = spapr_irq_cpu_intc_create_dual, @@ -695,7 +662,25 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) { - return spapr->irq->qirq(spapr, irq); + /* + * This interface is basically for VIO and PHB devices to find the + * right qemu_irq to manipulate, so we only allow access to the + * external irqs for now. Currently anything which needs to + * access the IPIs most naturally gets there via the guest side + * interfaces, we can change this if we need to in future. + */ + assert(irq >= SPAPR_XIRQ_BASE); + assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); + + if (spapr->ics) { + assert(ics_valid_irq(spapr->ics, irq)); + } + if (spapr->xive) { + assert(irq < spapr->xive->nr_irqs); + assert(xive_eas_is_valid(&spapr->xive->eat[irq])); + } + + return spapr->qirqs[irq]; } int spapr_irq_post_load(SpaprMachineState *spapr, int version_id) @@ -798,7 +783,6 @@ SpaprIrq spapr_irq_xics_legacy = { .init = spapr_irq_init_xics, .claim = spapr_irq_claim_xics, .free = spapr_irq_free_xics, - .qirq = spapr_qirq_xics, .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, .cpu_intc_create = spapr_irq_cpu_intc_create_xics, -- cgit v1.1 From 14789694cd1ba57b1d548451b8b7630e888d679f Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 23 Sep 2019 15:19:28 +1000 Subject: spapr: Eliminate SpaprIrq:get_nodename method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This method is used to determine the name of the irq backend's node in the device tree, so that we can find its phandle (after SLOF may have modified it from the phandle we initially gave it). But, in the two cases the only difference between the node name is the presence of a unit address. Searching for a node name without considering unit address is standard practice for the device tree, and fdt_subnode_offset() will do exactly that, making this method unecessary. While we're there, remove the XICS_NODENAME define. The name "interrupt-controller" is required by PAPR (and IEEE1275), and a bunch of places assume it already. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Greg Kurz --- hw/intc/xics_spapr.c | 2 +- hw/ppc/spapr_irq.c | 25 +++---------------------- 2 files changed, 4 insertions(+), 23 deletions(-) (limited to 'hw') diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index e6dd004..6e5eb24 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -316,7 +316,7 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, }; int node; - _FDT(node = fdt_add_subnode(fdt, 0, XICS_NODENAME)); + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); _FDT(fdt_setprop_string(fdt, node, "device_type", "PowerPC-External-Interrupt-Presentation")); diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 8f17907..ec2229d 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -211,11 +211,6 @@ static void spapr_irq_reset_xics(SpaprMachineState *spapr, Error **errp) } } -static const char *spapr_irq_get_nodename_xics(SpaprMachineState *spapr) -{ - return XICS_NODENAME; -} - static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp) { if (kvm_enabled()) { @@ -237,7 +232,6 @@ SpaprIrq spapr_irq_xics = { .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, .set_irq = spapr_irq_set_irq_xics, - .get_nodename = spapr_irq_get_nodename_xics, .init_kvm = spapr_irq_init_kvm_xics, }; @@ -362,11 +356,6 @@ static void spapr_irq_set_irq_xive(void *opaque, int irq, int val) } } -static const char *spapr_irq_get_nodename_xive(SpaprMachineState *spapr) -{ - return spapr->xive->nodename; -} - static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp) { if (kvm_enabled()) { @@ -388,7 +377,6 @@ SpaprIrq spapr_irq_xive = { .post_load = spapr_irq_post_load_xive, .reset = spapr_irq_reset_xive, .set_irq = spapr_irq_set_irq_xive, - .get_nodename = spapr_irq_get_nodename_xive, .init_kvm = spapr_irq_init_kvm_xive, }; @@ -533,11 +521,6 @@ static void spapr_irq_set_irq_dual(void *opaque, int irq, int val) spapr_irq_current(spapr)->set_irq(spapr, irq, val); } -static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr) -{ - return spapr_irq_current(spapr)->get_nodename(spapr); -} - /* * Define values in sync with the XIVE and XICS backend */ @@ -555,7 +538,6 @@ SpaprIrq spapr_irq_dual = { .post_load = spapr_irq_post_load_dual, .reset = spapr_irq_reset_dual, .set_irq = spapr_irq_set_irq_dual, - .get_nodename = spapr_irq_get_nodename_dual, .init_kvm = NULL, /* should not be used */ }; @@ -699,13 +681,13 @@ void spapr_irq_reset(SpaprMachineState *spapr, Error **errp) int spapr_irq_get_phandle(SpaprMachineState *spapr, void *fdt, Error **errp) { - const char *nodename = spapr->irq->get_nodename(spapr); + const char *nodename = "interrupt-controller"; int offset, phandle; offset = fdt_subnode_offset(fdt, 0, nodename); if (offset < 0) { - error_setg(errp, "Can't find node \"%s\": %s", nodename, - fdt_strerror(offset)); + error_setg(errp, "Can't find node \"%s\": %s", + nodename, fdt_strerror(offset)); return -1; } @@ -789,6 +771,5 @@ SpaprIrq spapr_irq_xics_legacy = { .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, .set_irq = spapr_irq_set_irq_xics, - .get_nodename = spapr_irq_get_nodename_xics, .init_kvm = spapr_irq_init_kvm_xics, }; -- cgit v1.1 From 85d0425652a5117164d716a284df5aa22ddf44fe Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 25 Sep 2019 00:05:03 +1000 Subject: spapr: Remove unhelpful tracepoints from spapr_irq_free_xics() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These traces contain some useless information (the always-0 source#) and have no equivalents for XIVE mode. For now just remove them, and we can put back something more sensible if and when we need it. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé --- hw/ppc/spapr_irq.c | 4 ---- hw/ppc/trace-events | 4 ---- 2 files changed, 8 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index ec2229d..9919910 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -140,11 +140,7 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num) int i; if (ics_valid_irq(ics, irq)) { - trace_spapr_irq_free(0, irq, num); for (i = srcno; i < srcno + num; ++i) { - if (ics_irq_free(ics, i)) { - trace_spapr_irq_free_warn(0, i); - } memset(&ics->irqs[i], 0, sizeof(ICSIRQState)); } } diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events index 96dad76..9ea620f 100644 --- a/hw/ppc/trace-events +++ b/hw/ppc/trace-events @@ -13,10 +13,6 @@ spapr_pci_msi_retry(unsigned config_addr, unsigned req_num, unsigned max_irqs) " spapr_cas_failed(unsigned long n) "DT diff buffer is too small: %ld bytes" spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes" -# spapr_irq.c -spapr_irq_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs" -spapr_irq_free_warn(int src, int irq) "Source#%d, irq %d is already free" - # spapr_hcall.c spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=0x%x, explicit_match=%u, new=0x%x" spapr_h_resize_hpt_prepare(uint64_t flags, uint64_t shift) "flags=0x%"PRIx64", shift=%"PRIu64 -- cgit v1.1 From f233cee97bfbab24517171ef5a56d8a54d8a96ef Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 25 Sep 2019 00:12:21 +1000 Subject: spapr: Handle freeing of multiple irqs in frontend only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spapr_irq_free() can be used to free multiple irqs at once. That's useful for its callers, but there's no need to make the individual backend hooks handle this. We can loop across the irqs in spapr_irq_free() itself and have the hooks just do one at time. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr_irq.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 9919910..d2ac35b 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -133,16 +133,13 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, return 0; } -static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq, int num) +static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) { ICSState *ics = spapr->ics; uint32_t srcno = irq - ics->offset; - int i; if (ics_valid_irq(ics, irq)) { - for (i = srcno; i < srcno + num; ++i) { - memset(&ics->irqs[i], 0, sizeof(ICSIRQState)); - } + memset(&ics->irqs[srcno], 0, sizeof(ICSIRQState)); } } @@ -269,13 +266,9 @@ static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, return 0; } -static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq, int num) +static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) { - int i; - - for (i = irq; i < irq + num; ++i) { - spapr_xive_irq_free(spapr->xive, i); - } + spapr_xive_irq_free(spapr->xive, irq); } static void spapr_irq_print_info_xive(SpaprMachineState *spapr, @@ -433,10 +426,10 @@ static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, return ret; } -static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq, int num) +static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) { - spapr_irq_xics.free(spapr, irq, num); - spapr_irq_xive.free(spapr, irq, num); + spapr_irq_xics.free(spapr, irq); + spapr_irq_xive.free(spapr, irq); } static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon) @@ -635,7 +628,11 @@ int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) { - spapr->irq->free(spapr, irq, num); + int i; + + for (i = irq; i < (irq + num); i++) { + spapr->irq->free(spapr, i); + } } qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) -- cgit v1.1 From 580dde5e4a4597be26cb948a711727c2a406f158 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 25 Sep 2019 13:49:59 +1000 Subject: spapr, xics, xive: Better use of assert()s on irq claim/free paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The irq claim and free paths for both XICS and XIVE check for some validity conditions. Some of these represent genuine runtime failures, however others - particularly checking that the basic irq number is in a sane range - could only fail in the case of bugs in the callin code. Therefore use assert()s instead of runtime failures for those. In addition the non backend-specific part of the claim/free paths should only be used for PAPR external irqs, that is in the range SPAPR_XIRQ_BASE to the maximum irq number. Put assert()s for that into the top level dispatchers as well. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/spapr_xive.c | 8 ++------ hw/ppc/spapr_irq.c | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 14 deletions(-) (limited to 'hw') diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index c1c9719..47b5ec0 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -532,9 +532,7 @@ bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi) { XiveSource *xsrc = &xive->source; - if (lisn >= xive->nr_irqs) { - return false; - } + assert(lisn < xive->nr_irqs); /* * Set default values when allocating an IRQ number @@ -559,9 +557,7 @@ bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi) bool spapr_xive_irq_free(SpaprXive *xive, uint32_t lisn) { - if (lisn >= xive->nr_irqs) { - return false; - } + assert(lisn < xive->nr_irqs); xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID); return true; diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index d2ac35b..025c802 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -118,11 +118,7 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, ICSState *ics = spapr->ics; assert(ics); - - if (!ics_valid_irq(ics, irq)) { - error_setg(errp, "IRQ %d is invalid", irq); - return -1; - } + assert(ics_valid_irq(ics, irq)); if (!ics_irq_free(ics, irq - ics->offset)) { error_setg(errp, "IRQ %d is not free", irq); @@ -138,9 +134,9 @@ static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) ICSState *ics = spapr->ics; uint32_t srcno = irq - ics->offset; - if (ics_valid_irq(ics, irq)) { - memset(&ics->irqs[srcno], 0, sizeof(ICSIRQState)); - } + assert(ics_valid_irq(ics, irq)); + + memset(&ics->irqs[srcno], 0, sizeof(ICSIRQState)); } static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon) @@ -623,6 +619,9 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { + assert(irq >= SPAPR_XIRQ_BASE); + assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); + return spapr->irq->claim(spapr, irq, lsi, errp); } @@ -630,6 +629,9 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) { int i; + assert(irq >= SPAPR_XIRQ_BASE); + assert((irq + num) <= (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); + for (i = irq; i < (irq + num); i++) { spapr->irq->free(spapr, i); } -- cgit v1.1 From e594c2ad1c3207ff308449203fd5abc002ac89c9 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 25 Sep 2019 13:24:14 +1000 Subject: xive: Improve irq claim/free path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spapr_xive_irq_claim() returns a bool to indicate if it succeeded. But most of the callers and one callee use int return values and/or an Error * with more information instead. In any case, ints are a more common idiom for success/failure states than bools (one never knows what sense they'll be in). So instead change to an int return value to indicate presence of error + an Error * to describe the details through that call chain. It also didn't actually check if the irq was already claimed, which is one of the primary purposes of the claim path, so do that. spapr_xive_irq_free() also returned a bool... which no callers checked and was always true, so just drop it. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/intc/spapr_xive.c | 20 +++++++++----------- hw/intc/spapr_xive_kvm.c | 8 ++++---- hw/ppc/spapr_irq.c | 11 +++++------ 3 files changed, 18 insertions(+), 21 deletions(-) (limited to 'hw') diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 47b5ec0..04879ab 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -528,12 +528,17 @@ static void spapr_xive_register_types(void) type_init(spapr_xive_register_types) -bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi) +int spapr_xive_irq_claim(SpaprXive *xive, int lisn, bool lsi, Error **errp) { XiveSource *xsrc = &xive->source; assert(lisn < xive->nr_irqs); + if (xive_eas_is_valid(&xive->eat[lisn])) { + error_setg(errp, "IRQ %d is not free", lisn); + return -EBUSY; + } + /* * Set default values when allocating an IRQ number */ @@ -543,24 +548,17 @@ bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi) } if (kvm_irqchip_in_kernel()) { - Error *local_err = NULL; - - kvmppc_xive_source_reset_one(xsrc, lisn, &local_err); - if (local_err) { - error_report_err(local_err); - return false; - } + return kvmppc_xive_source_reset_one(xsrc, lisn, errp); } - return true; + return 0; } -bool spapr_xive_irq_free(SpaprXive *xive, uint32_t lisn) +void spapr_xive_irq_free(SpaprXive *xive, int lisn) { assert(lisn < xive->nr_irqs); xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID); - return true; } /* diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 2006f96..51b334b 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -232,14 +232,14 @@ void kvmppc_xive_sync_source(SpaprXive *xive, uint32_t lisn, Error **errp) * only need to inform the KVM XIVE device about their type: LSI or * MSI. */ -void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp) +int kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp) { SpaprXive *xive = SPAPR_XIVE(xsrc->xive); uint64_t state = 0; /* The KVM XIVE device is not in use */ if (xive->fd == -1) { - return; + return -ENODEV; } if (xive_source_irq_is_lsi(xsrc, srcno)) { @@ -249,8 +249,8 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp) } } - kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state, - true, errp); + return kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state, + true, errp); } static void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 025c802..516bf00 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -246,7 +246,10 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) /* Enable the CPU IPIs */ for (i = 0; i < nr_servers; ++i) { - spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, false); + if (spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, + false, errp) < 0) { + return; + } } spapr_xive_hcall_init(spapr); @@ -255,11 +258,7 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { - if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { - error_setg(errp, "IRQ %d is invalid", irq); - return -1; - } - return 0; + return spapr_xive_irq_claim(spapr->xive, irq, lsi, errp); } static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) -- cgit v1.1 From ca62823b79443e3f498c6e6b9fea5f8bbe61033e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 25 Sep 2019 15:12:07 +1000 Subject: spapr: Use less cryptic representation of which irq backends are supported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SpaprIrq::ov5 stores the value for a particular byte in PAPR option vector 5 which indicates whether XICS, XIVE or both interrupt controllers are available. As usual for PAPR, the encoding is kind of overly complicated and confusing (though to be fair there are some backwards compat things it has to handle). But to make our internal code clearer, have SpaprIrq encode more directly which backends are available as two booleans, and derive the OV5 value from that at the point we need it. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/spapr.c | 15 ++++++++++++--- hw/ppc/spapr_hcall.c | 6 +++--- hw/ppc/spapr_irq.c | 12 ++++++++---- 3 files changed, 23 insertions(+), 10 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 43920c1..514a17a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1066,19 +1066,28 @@ static void spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt, PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu); char val[2 * 4] = { - 23, spapr->irq->ov5, /* Xive mode. */ + 23, 0x00, /* XICS / XIVE mode */ 24, 0x00, /* Hash/Radix, filled in below. */ 25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */ 26, 0x40, /* Radix options: GTSE == yes. */ }; + if (spapr->irq->xics && spapr->irq->xive) { + val[1] = SPAPR_OV5_XIVE_BOTH; + } else if (spapr->irq->xive) { + val[1] = SPAPR_OV5_XIVE_EXPLOIT; + } else { + assert(spapr->irq->xics); + val[1] = SPAPR_OV5_XIVE_LEGACY; + } + if (!ppc_check_compat(first_ppc_cpu, CPU_POWERPC_LOGICAL_3_00, 0, first_ppc_cpu->compat_pvr)) { /* * If we're in a pre POWER9 compat mode then the guest should * do hash and use the legacy interrupt mode */ - val[1] = 0x00; /* XICS */ + val[1] = SPAPR_OV5_XIVE_LEGACY; /* XICS */ val[3] = 0x00; /* Hash */ } else if (kvm_enabled()) { if (kvmppc_has_cap_mmu_radix() && kvmppc_has_cap_mmu_hash_v3()) { @@ -2767,7 +2776,7 @@ static void spapr_machine_init(MachineState *machine) spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2); /* advertise XIVE on POWER9 machines */ - if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) { + if (spapr->irq->xive) { spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT); } diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 3d3a671..140f05c 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1784,13 +1784,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, * terminate the boot. */ if (guest_xive) { - if (spapr->irq->ov5 == SPAPR_OV5_XIVE_LEGACY) { + if (!spapr->irq->xive) { error_report( "Guest requested unavailable interrupt mode (XIVE), try the ic-mode=xive or ic-mode=dual machine property"); exit(EXIT_FAILURE); } } else { - if (spapr->irq->ov5 == SPAPR_OV5_XIVE_EXPLOIT) { + if (!spapr->irq->xics) { error_report( "Guest requested unavailable interrupt mode (XICS), either don't set the ic-mode machine property or try ic-mode=xics or ic-mode=dual"); exit(EXIT_FAILURE); @@ -1804,7 +1804,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, */ if (!spapr->cas_reboot) { spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT) - && spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH; + && spapr->irq->xics && spapr->irq->xive; } spapr_ovec_cleanup(ov5_updates); diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 516bf00..3ac67ba 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -210,7 +210,8 @@ static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp) SpaprIrq spapr_irq_xics = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, - .ov5 = SPAPR_OV5_XIVE_LEGACY, + .xics = true, + .xive = false, .init = spapr_irq_init_xics, .claim = spapr_irq_claim_xics, @@ -350,7 +351,8 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp) SpaprIrq spapr_irq_xive = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, - .ov5 = SPAPR_OV5_XIVE_EXPLOIT, + .xics = false, + .xive = true, .init = spapr_irq_init_xive, .claim = spapr_irq_claim_xive, @@ -511,7 +513,8 @@ static void spapr_irq_set_irq_dual(void *opaque, int irq, int val) SpaprIrq spapr_irq_dual = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, - .ov5 = SPAPR_OV5_XIVE_BOTH, + .xics = true, + .xive = true, .init = spapr_irq_init_dual, .claim = spapr_irq_claim_dual, @@ -754,7 +757,8 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) SpaprIrq spapr_irq_xics_legacy = { .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, - .ov5 = SPAPR_OV5_XIVE_LEGACY, + .xics = true, + .xive = false, .init = spapr_irq_init_xics, .claim = spapr_irq_claim_xics, -- cgit v1.1 From 0a3fd3df6f6a9b318909ab8400172d2d5a42abf9 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 30 Sep 2019 12:20:47 +1000 Subject: spapr: Add return value to spapr_irq_check() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicitly return success or failure, rather than just relying on the Error ** parameter. This makes handling it less verbose in the caller. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/spapr_irq.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 3ac67ba..0413fbd 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -529,7 +529,7 @@ SpaprIrq spapr_irq_dual = { }; -static void spapr_irq_check(SpaprMachineState *spapr, Error **errp) +static int spapr_irq_check(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -545,7 +545,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp) */ if (spapr->irq == &spapr_irq_dual) { spapr->irq = &spapr_irq_xics; - return; + return 0; } /* @@ -565,7 +565,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp) */ if (spapr->irq == &spapr_irq_xive) { error_setg(errp, "XIVE-only machines require a POWER9 CPU"); - return; + return -1; } } @@ -579,8 +579,10 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp) machine_kernel_irqchip_required(machine) && xics_kvm_has_broken_disconnect(spapr)) { error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on"); - return; + return -1; } + + return 0; } /* @@ -589,7 +591,6 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp) void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); - Error *local_err = NULL; if (machine_kernel_irqchip_split(machine)) { error_setg(errp, "kernel_irqchip split mode not supported on pseries"); @@ -602,9 +603,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) return; } - spapr_irq_check(spapr, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (spapr_irq_check(spapr, errp) < 0) { return; } -- cgit v1.1 From f478d9af213f169449f7aaba7257aba1a4032e1e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 30 Sep 2019 12:24:43 +1000 Subject: spapr: Eliminate SpaprIrq::init hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This method is used to set up the interrupt backends for the current configuration. However, this means some confusing redirection between the "dual" mode init and the init hooks for xics only and xive only modes. Since we now have simple flags indicating whether XICS and/or XIVE are supported, it's easier to just open code each initialization directly in spapr_irq_init(). This will also make some future cleanups simpler. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/spapr_irq.c | 130 +++++++++++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 69 deletions(-) (limited to 'hw') diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 0413fbd..457eabe 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -92,26 +92,6 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr, * XICS IRQ backend. */ -static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) -{ - Object *obj; - Error *local_err = NULL; - - obj = object_new(TYPE_ICS_SPAPR); - object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); - object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), - &error_fatal); - object_property_set_int(obj, spapr->irq->nr_xirqs, - "nr-irqs", &error_fatal); - object_property_set_bool(obj, true, "realized", &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - spapr->ics = ICS_SPAPR(obj); -} - static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { @@ -213,7 +193,6 @@ SpaprIrq spapr_irq_xics = { .xics = true, .xive = false, - .init = spapr_irq_init_xics, .claim = spapr_irq_claim_xics, .free = spapr_irq_free_xics, .print_info = spapr_irq_print_info_xics, @@ -228,33 +207,6 @@ SpaprIrq spapr_irq_xics = { /* * XIVE IRQ backend. */ -static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) -{ - uint32_t nr_servers = spapr_max_server_number(spapr); - DeviceState *dev; - int i; - - dev = qdev_create(NULL, TYPE_SPAPR_XIVE); - qdev_prop_set_uint32(dev, "nr-irqs", - spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); - /* - * 8 XIVE END structures per CPU. One for each available priority - */ - qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3); - qdev_init_nofail(dev); - - spapr->xive = SPAPR_XIVE(dev); - - /* Enable the CPU IPIs */ - for (i = 0; i < nr_servers; ++i) { - if (spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, - false, errp) < 0) { - return; - } - } - - spapr_xive_hcall_init(spapr); -} static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) @@ -354,7 +306,6 @@ SpaprIrq spapr_irq_xive = { .xics = false, .xive = true, - .init = spapr_irq_init_xive, .claim = spapr_irq_claim_xive, .free = spapr_irq_free_xive, .print_info = spapr_irq_print_info_xive, @@ -385,23 +336,6 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) &spapr_irq_xive : &spapr_irq_xics; } -static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) -{ - Error *local_err = NULL; - - spapr_irq_xics.init(spapr, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - spapr_irq_xive.init(spapr, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } -} - static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { @@ -516,7 +450,6 @@ SpaprIrq spapr_irq_dual = { .xics = true, .xive = true, - .init = spapr_irq_init_dual, .claim = spapr_irq_claim_dual, .free = spapr_irq_free_dual, .print_info = spapr_irq_print_info_dual, @@ -612,7 +545,67 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) spapr_irq_msi_init(spapr, spapr->irq->nr_msis); } - spapr->irq->init(spapr, errp); + if (spapr->irq->xics) { + Error *local_err = NULL; + Object *obj; + + obj = object_new(TYPE_ICS_SPAPR); + object_property_add_child(OBJECT(spapr), "ics", obj, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + object_property_set_int(obj, spapr->irq->nr_xirqs, "nr-irqs", + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + object_property_set_bool(obj, true, "realized", &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + spapr->ics = ICS_SPAPR(obj); + } + + if (spapr->irq->xive) { + uint32_t nr_servers = spapr_max_server_number(spapr); + DeviceState *dev; + int i; + + dev = qdev_create(NULL, TYPE_SPAPR_XIVE); + qdev_prop_set_uint32(dev, "nr-irqs", + spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); + /* + * 8 XIVE END structures per CPU. One for each available + * priority + */ + qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3); + qdev_init_nofail(dev); + + spapr->xive = SPAPR_XIVE(dev); + + /* Enable the CPU IPIs */ + for (i = 0; i < nr_servers; ++i) { + if (spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, + false, errp) < 0) { + return; + } + } + + spapr_xive_hcall_init(spapr); + } spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr, spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); @@ -759,7 +752,6 @@ SpaprIrq spapr_irq_xics_legacy = { .xics = true, .xive = false, - .init = spapr_irq_init_xics, .claim = spapr_irq_claim_xics, .free = spapr_irq_free_xics, .print_info = spapr_irq_print_info_xics, -- cgit v1.1 From 1aba8716c8335e88b8c358002a6e1ac89f7dd258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Thu, 3 Oct 2019 16:36:17 +0200 Subject: ppc/pnv: Remove the XICSFabric Interface from the POWER9 machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POWER8 PowerNV machine needs to implement a XICSFabric interface as this is the POWER8 interrupt controller model. But the POWER9 machine uselessly inherits of XICSFabric from the common PowerNV machine definition. Open code machine definitions to have a better control on the different interfaces each machine should define. Fixes: f30c843ced50 ("ppc/pnv: Introduce PowerNV machines with fixed CPU models") Signed-off-by: Cédric Le Goater Message-Id: <20191003143617.21682-1-clg@kaod.org> Signed-off-by: David Gibson --- hw/ppc/pnv.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) (limited to 'hw') diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 77a86c6..7cf64b6 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -1485,23 +1485,21 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) .parent = TYPE_PNV9_CHIP, \ } -#define DEFINE_PNV_MACHINE_TYPE(cpu, class_initfn) \ - { \ - .name = MACHINE_TYPE_NAME(cpu), \ - .parent = TYPE_PNV_MACHINE, \ - .instance_size = sizeof(PnvMachineState), \ - .instance_init = pnv_machine_instance_init, \ - .class_init = class_initfn, \ - .interfaces = (InterfaceInfo[]) { \ - { TYPE_XICS_FABRIC }, \ - { TYPE_INTERRUPT_STATS_PROVIDER }, \ - { }, \ - }, \ - } - static const TypeInfo types[] = { - DEFINE_PNV_MACHINE_TYPE("powernv8", pnv_machine_power8_class_init), - DEFINE_PNV_MACHINE_TYPE("powernv9", pnv_machine_power9_class_init), + { + .name = MACHINE_TYPE_NAME("powernv9"), + .parent = TYPE_PNV_MACHINE, + .class_init = pnv_machine_power9_class_init, + }, + { + .name = MACHINE_TYPE_NAME("powernv8"), + .parent = TYPE_PNV_MACHINE, + .class_init = pnv_machine_power8_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_XICS_FABRIC }, + { }, + }, + }, { .name = TYPE_PNV_MACHINE, .parent = TYPE_MACHINE, @@ -1510,7 +1508,6 @@ static const TypeInfo types[] = { .instance_init = pnv_machine_instance_init, .class_init = pnv_machine_class_init, .interfaces = (InterfaceInfo[]) { - { TYPE_XICS_FABRIC }, { TYPE_INTERRUPT_STATS_PROVIDER }, { }, }, -- cgit v1.1