Introduce a `gicc_accessible` flag to indicate whether it is safe to access the
GICv3 CPU interface. This flag will determine the availability of the GICv3 CPU
interface based on whether the associated QOM vCPUs are enabled or disabled.
Additionally, implement checks throughout the GICv3 codebase to ensure that any
updates or accesses to GICC registers (e.g., `ICC_*_EL1`) occur only when the
`gicc_accessible` flag is set. This ensures that operations such as KVM VGIC
GICC register fetches or modifications are performed only for GICv3 CPU
interfaces that are valid and associated with active QOM vCPUs.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
cpu-common.c | 11 +++++++++
hw/arm/virt.c | 1 +
hw/intc/arm_gicv3_common.c | 14 ++++++++---
hw/intc/arm_gicv3_cpuif.c | 8 +++++++
hw/intc/arm_gicv3_kvm.c | 23 ++++++++++++++++++-
include/hw/core/cpu.h | 12 ++++++++++
include/hw/intc/arm_gicv3_common.h | 37 ++++++++++++++++++++++++++++++
7 files changed, 102 insertions(+), 4 deletions(-)
diff --git a/cpu-common.c b/cpu-common.c
index 6b262233a3..b96eb4ca55 100644
--- a/cpu-common.c
+++ b/cpu-common.c
@@ -24,6 +24,7 @@
#include "sysemu/cpus.h"
#include "qemu/lockable.h"
#include "trace/trace-root.h"
+#include "hw/boards.h"
QemuMutex qemu_cpu_list_lock;
static QemuCond exclusive_cond;
@@ -108,6 +109,16 @@ void cpu_list_remove(CPUState *cpu)
cpu_list_generation_id++;
}
+CPUState *qemu_get_possible_cpu(int index)
+{
+ MachineState *ms = MACHINE(qdev_get_machine());
+ const CPUArchIdList *possible_cpus = ms->possible_cpus;
+
+ assert((index >= 0) && (index < possible_cpus->len));
+
+ return CPU(possible_cpus->cpus[index].cpu);
+}
+
CPUState *qemu_get_cpu(int index)
{
CPUState *cpu;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b286afc62c..00fd65b4e1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -795,6 +795,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
vms->gic = qdev_new(gictype);
qdev_prop_set_uint32(vms->gic, "revision", revision);
qdev_prop_set_uint32(vms->gic, "num-cpu", max_cpus);
+ qdev_prop_set_uint32(vms->gic, "num-smp-cpu", smp_cpus);
/* Note that the num-irq property counts both internal and external
* interrupts; there are always 32 of the former (mandated by GIC spec).
*/
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index bd50a1b079..218ced1d9f 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -436,13 +436,20 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
s->cpu = g_new0(GICv3CPUState, s->num_cpu);
for (i = 0; i < s->num_cpu; i++) {
- CPUState *cpu = qemu_get_cpu(i);
+ CPUState *cpu = qemu_get_possible_cpu(i);
+ bool gicc_accessible = (i < s->num_smp_cpu);
uint64_t cpu_affid;
- s->cpu[i].cpu = cpu;
+ /*
+ * Accordingly, set the QOM `GICv3CPUState` as either accessible or
+ * inaccessible based on the `CPUState` of the associated QOM vCPU.
+ */
+ gicv3_set_cpustate(&s->cpu[i], cpu, gicc_accessible);
+
s->cpu[i].gic = s;
/* Store GICv3CPUState in CPUARMState gicv3state pointer */
- gicv3_set_gicv3state(cpu, &s->cpu[i]);
+ if (gicc_accessible)
+ gicv3_set_gicv3state(cpu, &s->cpu[i]);
/* Pre-construct the GICR_TYPER:
* For our implementation:
@@ -607,6 +614,7 @@ static void arm_gic_common_linux_init(ARMLinuxBootIf *obj,
static Property arm_gicv3_common_properties[] = {
DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
+ DEFINE_PROP_UINT32("num-smp-cpu", GICv3State, num_smp_cpu, 1),
DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index bdb13b00e9..151f957d42 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -1052,6 +1052,10 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
ARMCPU *cpu = ARM_CPU(cs->cpu);
CPUARMState *env = &cpu->env;
+ if (!gicv3_cpu_accessible(cs)) {
+ return;
+ }
+
g_assert(bql_locked());
trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
@@ -2036,6 +2040,10 @@ static void icc_generate_sgi(CPUARMState *env, GICv3CPUState *cs,
for (i = 0; i < s->num_cpu; i++) {
GICv3CPUState *ocs = &s->cpu[i];
+ if (!gicv3_cpu_accessible(ocs)) {
+ continue;
+ }
+
if (irm) {
/* IRM == 1 : route to all CPUs except self */
if (cs == ocs) {
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 9ea6b8e218..7e741ace50 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -24,6 +24,7 @@
#include "hw/intc/arm_gicv3_common.h"
#include "qemu/error-report.h"
#include "qemu/module.h"
+#include "sysemu/cpus.h"
#include "sysemu/kvm.h"
#include "sysemu/runstate.h"
#include "kvm_arm.h"
@@ -458,6 +459,16 @@ static void kvm_arm_gicv3_put(GICv3State *s)
GICv3CPUState *c = &s->cpu[ncpu];
int num_pri_bits;
+ /*
+ * We must ensure that we do not attempt to access or update KVM GICC
+ * registers if their corresponding QOM `GICv3CPUState` is marked as
+ * 'inaccessible', either because their corresponding QOM vCPU objects
+ * do not exist or are disabled due to hot-unplug action.
+ */
+ if (!gicv3_cpu_accessible(c)) {
+ continue;
+ }
+
kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, true);
kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
&c->icc_ctlr_el1[GICV3_NS], true);
@@ -616,6 +627,14 @@ static void kvm_arm_gicv3_get(GICv3State *s)
GICv3CPUState *c = &s->cpu[ncpu];
int num_pri_bits;
+ /*
+ * don't attempt to access KVM VGIC for the disabled vCPUs where
+ * GICv3CPUState is inaccessible.
+ */
+ if (!gicv3_cpu_accessible(c)) {
+ continue;
+ }
+
kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, false);
kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
&c->icc_ctlr_el1[GICV3_NS], false);
@@ -815,7 +834,9 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
for (i = 0; i < s->num_cpu; i++) {
ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
- define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
+ if (gicv3_cpu_accessible(&s->cpu[i])) {
+ define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
+ }
}
/* Try to create the device via the device control API */
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 4e0cb325a0..0be1984698 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -950,6 +950,18 @@ static inline bool cpu_in_exclusive_context(const CPUState *cpu)
*/
CPUState *qemu_get_cpu(int index);
+/**
+ * qemu_get_possible_cpu:
+ * @index: The CPUState@cpu_index value of the CPU to obtain.
+ * Input index MUST be in range [0, Max Possible CPUs)
+ *
+ * If CPUState object exists,then it gets a CPU matching
+ * @index in the possible CPU array.
+ *
+ * Returns: The possible CPU or %NULL if CPU does not exist.
+ */
+CPUState *qemu_get_possible_cpu(int index);
+
/**
* cpu_exists:
* @id: Guest-exposed CPU ID to lookup.
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index cd09bee3bc..4383a4ce08 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -190,6 +190,7 @@ struct GICv3CPUState {
uint64_t icc_apr[3][4];
uint64_t icc_igrpen[3];
uint64_t icc_ctlr_el3;
+ bool gicc_accessible;
/* Virtualization control interface */
uint64_t ich_apr[3][4]; /* ich_apr[GICV3_G1][x] never used */
@@ -255,6 +256,7 @@ struct GICv3State {
uint32_t nb_redist_regions; /* number of redist regions */
uint32_t num_cpu;
+ uint32_t num_smp_cpu;
uint32_t num_irq;
uint32_t revision;
bool lpi_enable;
@@ -353,4 +355,39 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
*/
const char *gicv3_class_name(void);
+/**
+ * gicv3_cpu_accessible
+ *
+ * The `GICv3CPUState` can become inaccessible if the associated `CPUState` is
+ * either unavailable or in a disabled state. This state is independent of the
+ * KVM VGIC and is not compliant with ARM CPU architecture (i.e. there is no
+ * way we can explicitly enable/disable ARM GIC CPU interface). This change
+ * is specific to QOM only.
+ *
+ * Returns: True if accessible otherwise False
+ */
+static inline bool gicv3_cpu_accessible(GICv3CPUState *gicc)
+{
+ assert(gicc);
+ return gicc->gicc_accessible;
+}
+
+/**
+ * gicv3_set_cpustate
+ *
+ * Sets `GICv3CPUState` and the associated `CPUState` as accessible and
+ * available for use
+ */
+static inline void gicv3_set_cpustate(GICv3CPUState *s,
+ CPUState *cpu,
+ bool gicc_accessible)
+{
+ if (gicc_accessible) {
+ s->cpu = cpu;
+ s->gicc_accessible = true;
+ } else {
+ s->cpu = NULL;
+ s->gicc_accessible = false;
+ }
+}
#endif
--
2.34.1