From fa416ae6157a933ad3f7106090684759baaaf3c9 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Mon, 11 Nov 2024 16:37:49 +1000 Subject: [PATCH 1/6] target/ppc: Fix non-maskable interrupt while halted The ppc (pnv and spapr) NMI injection code does not go through the asynchronous interrupt path and set a bit in env->pending_interrupts and raise an interrupt request that the cpu_exec() loop can see. Instead it injects the exception directly into registers. This can lead to cpu_exec() missing that the thread has work to do, if a NMI is injected while it was idle. Fix this by clearing halted when injecting the interrupt. Probably NMI injection should be reworked to use the interrupt request interface, but this seems to work as a minimal fix. Fixes: 3431648272d3 ("spapr: Add support for new NMI interface") Reviewed-by: Glenn Miles Signed-off-by: Nicholas Piggin --- target/ppc/excp_helper.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 70daa5076a..9f811af0a4 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -2495,10 +2495,16 @@ static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt) } } +/* + * system reset is not delivered via normal irq method, so have to set + * halted = 0 to resume CPU running if it was halted. Possibly we should + * move it over to using PPC_INTERRUPT_RESET rather than async_run_on_cpu. + */ void ppc_cpu_do_system_reset(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); + cs->halted = 0; powerpc_excp(cpu, POWERPC_EXCP_RESET); } @@ -2520,6 +2526,7 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector) /* Anything for nested required here? MSR[HV] bit? */ + cs->halted = 0; powerpc_set_excp_state(cpu, vector, msr); } From 96746f7a95a6e32d6578d417ae41dc24c564fafa Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Mon, 11 Nov 2024 15:23:29 +1000 Subject: [PATCH 2/6] ppc/pnv: Fix direct controls quiesce powernv CPUs have a set of control registers that can stop, start, and do other things to control a thread's execution. Using this interface to stop a thread puts it into a particular state that can be queried, and is distinguishable from other things that might stop the CPU (e.g., going idle, or being debugged via gdb, or stopped by the monitor). Add a new flag that can speficially distinguish this state where it is stopped with control registers. This solves some hangs when rebooting powernv machines when skiboot is modified to allow QEMU to use the CPU control facility (that uses controls to bring all secondaries to a known state). Fixes: c8891955086 ("ppc/pnv: Implement POWER10 PC xscom registers for direct controls") Reviewed-by: Glenn Miles Signed-off-by: Nicholas Piggin --- hw/ppc/pnv_core.c | 9 +++++++-- target/ppc/cpu.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index a30693990b..cbfac49862 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -217,8 +217,8 @@ static uint64_t pnv_core_power10_xscom_read(void *opaque, hwaddr addr, case PNV10_XSCOM_EC_CORE_RAS_STATUS: for (i = 0; i < nr_threads; i++) { PowerPCCPU *cpu = pc->threads[i]; - CPUState *cs = CPU(cpu); - if (cs->stopped) { + CPUPPCState *env = &cpu->env; + if (env->quiesced) { val |= PPC_BIT(0 + 8 * i) | PPC_BIT(1 + 8 * i); } } @@ -244,20 +244,25 @@ static void pnv_core_power10_xscom_write(void *opaque, hwaddr addr, for (i = 0; i < nr_threads; i++) { PowerPCCPU *cpu = pc->threads[i]; CPUState *cs = CPU(cpu); + CPUPPCState *env = &cpu->env; if (val & PPC_BIT(7 + 8 * i)) { /* stop */ val &= ~PPC_BIT(7 + 8 * i); cpu_pause(cs); + env->quiesced = true; } if (val & PPC_BIT(6 + 8 * i)) { /* start */ val &= ~PPC_BIT(6 + 8 * i); + env->quiesced = false; cpu_resume(cs); } if (val & PPC_BIT(4 + 8 * i)) { /* sreset */ val &= ~PPC_BIT(4 + 8 * i); + env->quiesced = false; pnv_cpu_do_nmi_resume(cs); } if (val & PPC_BIT(3 + 8 * i)) { /* clear maint */ + env->quiesced = false; /* * Hardware has very particular cases for where clear maint * must be used and where start must be used to resume a diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 945af07a64..0b4f1013b8 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1355,6 +1355,7 @@ struct CPUArchState { * special way (such as routing some resume causes to 0x100, i.e. sreset). */ bool resume_as_sreset; + bool quiesced; #endif /* These resources are used only in TCG */ From 2fc0a78a57731fda50d5b01e16fd68681900f709 Mon Sep 17 00:00:00 2001 From: Glenn Miles Date: Thu, 14 Nov 2024 15:21:19 -0600 Subject: [PATCH 3/6] target/ppc: Fix THREAD_SIBLING_FOREACH for multi-socket The THREAD_SIBLING_FOREACH macro wasn't excluding threads from other chips. Add chip_index field to the thread state and add a check for the new field in the macro. Fixes: b769d4c8f4c6 ("target/ppc: Add initial flags and helpers for SMT support") Signed-off-by: Glenn Miles [npiggin: set chip_index for spapr too] Reviewed-by: Nicholas Piggin Signed-off-by: Nicholas Piggin --- hw/ppc/pnv_core.c | 2 ++ hw/ppc/spapr_cpu_core.c | 1 + target/ppc/cpu.h | 7 +++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index cbfac49862..e6b02294b1 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -322,6 +322,8 @@ static void pnv_core_cpu_realize(PnvCore *pc, PowerPCCPU *cpu, Error **errp, pir_spr->default_value = pir; tir_spr->default_value = tir; + env->chip_index = pc->chip->chip_id; + if (pc->big_core) { /* 2 "small cores" get the same core index for SMT operations */ env->core_index = core_hwid >> 1; diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index ada439e831..135f86a622 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -313,6 +313,7 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp) return NULL; } + env->chip_index = sc->node_id; env->core_index = cc->core_id; cpu->node_id = sc->node_id; diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 0b4f1013b8..2ffac2ed03 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1253,6 +1253,7 @@ struct CPUArchState { /* For SMT processors */ bool has_smt_siblings; int core_index; + int chip_index; #if !defined(CONFIG_USER_ONLY) /* MMU context, only relevant for full system emulation */ @@ -1412,8 +1413,10 @@ struct CPUArchState { #define THREAD_SIBLING_FOREACH(cs, cs_sibling) \ CPU_FOREACH(cs_sibling) \ - if (POWERPC_CPU(cs)->env.core_index == \ - POWERPC_CPU(cs_sibling)->env.core_index) + if ((POWERPC_CPU(cs)->env.chip_index == \ + POWERPC_CPU(cs_sibling)->env.chip_index) && \ + (POWERPC_CPU(cs)->env.core_index == \ + POWERPC_CPU(cs_sibling)->env.core_index)) #define SET_FIT_PERIOD(a_, b_, c_, d_) \ do { \ From 5e39814916c91485073da38f457a43bf317c1f08 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Thu, 29 Aug 2024 16:26:43 +1000 Subject: [PATCH 4/6] ppc/pnv: Add xscom- prefix to pervasive-control region name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By convention, xscom regions get a xscom- prefix. Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet model") Reviewed-by: Glenn Miles Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Nicholas Piggin --- hw/ppc/pnv_nest_pervasive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/pnv_nest_pervasive.c b/hw/ppc/pnv_nest_pervasive.c index 77476753a4..780fa69dde 100644 --- a/hw/ppc/pnv_nest_pervasive.c +++ b/hw/ppc/pnv_nest_pervasive.c @@ -177,7 +177,7 @@ static void pnv_nest_pervasive_realize(DeviceState *dev, Error **errp) pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr, OBJECT(nest_pervasive), &pnv_nest_pervasive_control_xscom_ops, - nest_pervasive, "pervasive-control", + nest_pervasive, "xscom-pervasive-control", PNV10_XSCOM_CHIPLET_CTRL_REGS_SIZE); } From e8185fdc63e1db1efba695aae568fae8a075a815 Mon Sep 17 00:00:00 2001 From: Harsh Prateek Bora Date: Mon, 25 Nov 2024 16:43:34 +1000 Subject: [PATCH 5/6] ppc/spapr: fix drc index mismatch for partially enabled vcpus In case when vcpus are explicitly enabled/disabled in a non-consecutive order within a libvirt xml, it results in a drc index mismatch during vcpu hotplug later because the existing logic uses vcpu id to derive the corresponding drc index which is not correct. Use env->core_index to derive a vcpu's drc index as appropriate to fix this issue. For ex, for the given libvirt xml config: We see below error on guest console with "virsh setvcpus 5" : pseries-hotplug-cpu: CPU with drc index 10000002 already exists This patch fixes the issue by using correct drc index for explicitly enabled vcpus during init. Reported-by: Anushree Mathur Tested-by: Anushree Mathur Signed-off-by: Harsh Prateek Bora Reviewed-by: Nicholas Piggin Signed-off-by: Nicholas Piggin --- hw/ppc/spapr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 5c02037c56..0d4efaa0c0 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -701,7 +701,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset, uint32_t radix_AP_encodings[PPC_PAGE_SIZES_MAX_SZ]; int i; - drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index); + drc = spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, env->core_index); if (drc) { drc_index = spapr_drc_index(drc); _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index))); From 0805136a44d39adc2467f23ac3c65e680e45d0a2 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Mon, 25 Nov 2024 15:48:45 +1000 Subject: [PATCH 6/6] hw/ppc/pegasos2: Fix IRQ routing from pci.0 The MV64361 has two PCI buses one of which is used for AGP on PegasosII. So far we only emulated the PCI bus on pci.1 but some graphics cards are only recognised by some guests when connected to pci.0 corresponding to the AGP port. So far the interrupts were not routed from pci.0 so this patch fixes that allowing the use of both PCI buses. On real board only INTA and INTB are connected for AGP but to avoid surprises we connect all 4 PCI interrupt lines so pci.0 can be used for all PCI cards as well. Signed-off-by: BALATON Zoltan Reviewed-by: Nicholas Piggin Signed-off-by: Nicholas Piggin --- hw/pci-host/mv64361.c | 1 + hw/ppc/pegasos2.c | 30 +++++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c index 1036d8600d..421c287eb0 100644 --- a/hw/pci-host/mv64361.c +++ b/hw/pci-host/mv64361.c @@ -95,6 +95,7 @@ static void mv64361_pcihost_realize(DeviceState *dev, Error **errp) &s->mem, &s->io, 0, 4, TYPE_PCI_BUS); g_free(name); pci_create_simple(h->bus, 0, TYPE_MV64361_PCI_BRIDGE); + qdev_init_gpio_out(dev, s->irq, ARRAY_SIZE(s->irq)); } static Property mv64361_pcihost_props[] = { diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c index 8ff4a00c34..16abeaac82 100644 --- a/hw/ppc/pegasos2.c +++ b/hw/ppc/pegasos2.c @@ -14,6 +14,7 @@ #include "hw/sysbus.h" #include "hw/pci/pci_host.h" #include "hw/irq.h" +#include "hw/or-irq.h" #include "hw/pci-host/mv64361.h" #include "hw/isa/vt82c686.h" #include "hw/ide/pci.h" @@ -73,8 +74,11 @@ OBJECT_DECLARE_TYPE(Pegasos2MachineState, MachineClass, PEGASOS2_MACHINE) struct Pegasos2MachineState { MachineState parent_obj; + PowerPCCPU *cpu; DeviceState *mv; + IRQState pci_irqs[PCI_NUM_PINS]; + OrIRQState orirq[PCI_NUM_PINS]; qemu_irq mv_pirq[PCI_NUM_PINS]; qemu_irq via_pirq[PCI_NUM_PINS]; Vof *vof; @@ -177,7 +181,6 @@ static void pegasos2_init(MachineState *machine) pm->mv_pirq[i] = qdev_get_gpio_in_named(pm->mv, "gpp", 12 + i); } pci_bus = mv64361_get_pci_bus(pm->mv, 1); - pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS); /* VIA VT8231 South Bridge (multifunction PCI device) */ via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA)); @@ -209,6 +212,31 @@ static void pegasos2_init(MachineState *machine) /* other PC hardware */ pci_vga_init(pci_bus); + /* PCI interrupt routing: lines from pci.0 and pci.1 are ORed */ + for (int h = 0; h < 2; h++) { + DeviceState *pd; + g_autofree const char *pn = g_strdup_printf("pcihost%d", h); + + pd = DEVICE(object_resolve_path_component(OBJECT(pm->mv), pn)); + assert(pd); + for (i = 0; i < PCI_NUM_PINS; i++) { + OrIRQState *ori = &pm->orirq[i]; + + if (h == 0) { + g_autofree const char *n = g_strdup_printf("pci-orirq[%d]", i); + + object_initialize_child_with_props(OBJECT(pm), n, + ori, sizeof(*ori), + TYPE_OR_IRQ, &error_fatal, + "num-lines", "2", NULL); + qdev_realize(DEVICE(ori), NULL, &error_fatal); + qemu_init_irq(&pm->pci_irqs[i], pegasos2_pci_irq, pm, i); + qdev_connect_gpio_out(DEVICE(ori), 0, &pm->pci_irqs[i]); + } + qdev_connect_gpio_out(pd, i, qdev_get_gpio_in(DEVICE(ori), h)); + } + } + if (machine->kernel_filename) { sz = load_elf(machine->kernel_filename, NULL, NULL, NULL, &pm->kernel_entry, &pm->kernel_addr, NULL, NULL, 1,