[PATCH v5 02/13] accel, hw/arm, include/system/hvf: infrastructure changes for HVF vGIC

Mohamed Mediouni posted 13 patches 3 months, 2 weeks ago
Maintainers: Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Shannon Zhao <shannon.zhaosl@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Alexander Graf <agraf@csgraf.de>
There is a newer version of this series
[PATCH v5 02/13] accel, hw/arm, include/system/hvf: infrastructure changes for HVF vGIC
Posted by Mohamed Mediouni 3 months, 2 weeks ago
Misc changes needed for HVF vGIC enablement.

Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
---
 accel/hvf/hvf-all.c        | 44 ++++++++++++++++++++++++++++++++++++++
 accel/stubs/hvf-stub.c     |  1 +
 hw/arm/virt.c              | 16 +++++++++-----
 hw/intc/arm_gicv3_common.c |  3 +++
 include/system/hvf.h       |  3 +++
 system/vl.c                |  2 ++
 6 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
index 0a4b498e83..5af76ba7a6 100644
--- a/accel/hvf/hvf-all.c
+++ b/accel/hvf/hvf-all.c
@@ -10,6 +10,8 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qapi/qapi-visit-common.h"
 #include "accel/accel-ops.h"
 #include "system/address-spaces.h"
 #include "system/memory.h"
@@ -20,6 +22,7 @@
 #include "trace.h"
 
 bool hvf_allowed;
+bool hvf_kernel_irqchip;
 
 struct mac_slot {
     int present;
@@ -290,6 +293,41 @@ static int hvf_gdbstub_sstep_flags(AccelState *as)
     return SSTEP_ENABLE | SSTEP_NOIRQ;
 }
 
+static void hvf_set_kernel_irqchip(Object *obj, Visitor *v,
+                                   const char *name, void *opaque,
+                                   Error **errp)
+{
+#ifdef __aarch64__
+    OnOffSplit mode;
+    if (!visit_type_OnOffSplit(v, name, &mode, errp)) {
+        return;
+    }
+
+    switch (mode) {
+    case ON_OFF_SPLIT_ON:
+        hvf_kernel_irqchip = true;
+        break;
+
+    case ON_OFF_SPLIT_OFF:
+        hvf_kernel_irqchip = false;
+        break;
+
+    case ON_OFF_SPLIT_SPLIT:
+        error_setg(errp, "HVF: split irqchip is not supported on Arm.");
+        break;
+
+    default:
+        /*
+         * The value was checked in visit_type_OnOffSplit() above. If
+         * we get here, then something is wrong in QEMU.
+         */
+        abort();
+    }
+#else
+    error_setg(errp, "HVF: setting irqchip configuration not supported on x86_64.");
+#endif
+}
+
 static void hvf_accel_class_init(ObjectClass *oc, const void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
@@ -297,6 +335,12 @@ static void hvf_accel_class_init(ObjectClass *oc, const void *data)
     ac->init_machine = hvf_accel_init;
     ac->allowed = &hvf_allowed;
     ac->gdbstub_supported_sstep_flags = hvf_gdbstub_sstep_flags;
+    hvf_kernel_irqchip = true;
+    object_class_property_add(oc, "kernel-irqchip", "on|off|split",
+        NULL, hvf_set_kernel_irqchip,
+        NULL, NULL);
+    object_class_property_set_description(oc, "kernel-irqchip",
+        "Configure HVF irqchip");
 }
 
 static const TypeInfo hvf_accel_type = {
diff --git a/accel/stubs/hvf-stub.c b/accel/stubs/hvf-stub.c
index 42eadc5ca9..6bd08759ba 100644
--- a/accel/stubs/hvf-stub.c
+++ b/accel/stubs/hvf-stub.c
@@ -10,3 +10,4 @@
 #include "system/hvf.h"
 
 bool hvf_allowed;
+bool hvf_kernel_irqchip;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ef6be3660f..7da1176cda 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -830,7 +830,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
      * interrupts; there are always 32 of the former (mandated by GIC spec).
      */
     qdev_prop_set_uint32(vms->gic, "num-irq", NUM_IRQS + 32);
-    if (!kvm_irqchip_in_kernel()) {
+    if (!kvm_irqchip_in_kernel() && !hvf_irqchip_in_kernel()) {
         qdev_prop_set_bit(vms->gic, "has-security-extensions", vms->secure);
     }
 
@@ -853,8 +853,9 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
         qdev_prop_set_array(vms->gic, "redist-region-count",
                             redist_region_count);
 
-        if (!kvm_irqchip_in_kernel()) {
-            if (vms->tcg_its) {
+        if (!kvm_irqchip_in_kernel() &&
+         !(hvf_enabled() && hvf_irqchip_in_kernel())) {
+            if (vms->its && vms->tcg_its) {
                 object_property_set_link(OBJECT(vms->gic), "sysmem",
                                          OBJECT(mem), &error_fatal);
                 qdev_prop_set_bit(vms->gic, "has-lpi", true);
@@ -864,7 +865,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
                                  ARCH_GIC_MAINT_IRQ);
         }
     } else {
-        if (!kvm_irqchip_in_kernel()) {
+        if (!kvm_irqchip_in_kernel() && !hvf_irqchip_in_kernel()) {
             qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
                               vms->virt);
         }
@@ -2058,7 +2059,12 @@ static void finalize_gic_version(VirtMachineState *vms)
         /* KVM w/o kernel irqchip can only deal with GICv2 */
         gics_supported |= VIRT_GIC_VERSION_2_MASK;
         accel_name = "KVM with kernel-irqchip=off";
-    } else if (tcg_enabled() || hvf_enabled() || qtest_enabled())  {
+    } else if (hvf_enabled()) {
+        if (!hvf_irqchip_in_kernel()) {
+            gics_supported |= VIRT_GIC_VERSION_2_MASK;
+        }
+        gics_supported |= VIRT_GIC_VERSION_3_MASK;
+    } else if (tcg_enabled() || qtest_enabled()) {
         gics_supported |= VIRT_GIC_VERSION_2_MASK;
         if (module_object_class_by_name("arm-gicv3")) {
             gics_supported |= VIRT_GIC_VERSION_3_MASK;
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index e438d8c042..b8eee27260 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -32,6 +32,7 @@
 #include "gicv3_internal.h"
 #include "hw/arm/linux-boot-if.h"
 #include "system/kvm.h"
+#include "system/hvf.h"
 
 
 static void gicv3_gicd_no_migration_shift_bug_post_load(GICv3State *cs)
@@ -662,6 +663,8 @@ const char *gicv3_class_name(void)
 {
     if (kvm_irqchip_in_kernel()) {
         return "kvm-arm-gicv3";
+    } else if (hvf_enabled() && hvf_irqchip_in_kernel()) {
+        return "hvf-arm-gicv3";
     } else {
         if (kvm_enabled()) {
             error_report("Userspace GICv3 is not supported with KVM");
diff --git a/include/system/hvf.h b/include/system/hvf.h
index d3dcf088b3..dc8da85979 100644
--- a/include/system/hvf.h
+++ b/include/system/hvf.h
@@ -26,8 +26,11 @@
 #ifdef CONFIG_HVF_IS_POSSIBLE
 extern bool hvf_allowed;
 #define hvf_enabled() (hvf_allowed)
+extern bool hvf_kernel_irqchip;
+#define hvf_irqchip_in_kernel()  (hvf_kernel_irqchip)
 #else /* !CONFIG_HVF_IS_POSSIBLE */
 #define hvf_enabled() 0
+#define hvf_irqchip_in_kernel() 0
 #endif /* !CONFIG_HVF_IS_POSSIBLE */
 
 #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
diff --git a/system/vl.c b/system/vl.c
index 3b7057e6c6..1c072d15a4 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1773,6 +1773,8 @@ static void qemu_apply_legacy_machine_options(QDict *qdict)
                                    false);
         object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip", value,
                                    false);
+        object_register_sugar_prop(ACCEL_CLASS_NAME("hvf"), "kernel-irqchip", value,
+                                   false);
         qdict_del(qdict, "kernel-irqchip");
     }
 
-- 
2.39.5 (Apple Git-154)
Re: [PATCH v5 02/13] accel, hw/arm, include/system/hvf: infrastructure changes for HVF vGIC
Posted by Philippe Mathieu-Daudé 3 months, 1 week ago
On 28/7/25 15:41, Mohamed Mediouni wrote:
> Misc changes needed for HVF vGIC enablement.
> 
> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
> ---
>   accel/hvf/hvf-all.c        | 44 ++++++++++++++++++++++++++++++++++++++
>   accel/stubs/hvf-stub.c     |  1 +
>   hw/arm/virt.c              | 16 +++++++++-----
>   hw/intc/arm_gicv3_common.c |  3 +++
>   include/system/hvf.h       |  3 +++
>   system/vl.c                |  2 ++
>   6 files changed, 64 insertions(+), 5 deletions(-)
> 
> diff --git a/accel/hvf/hvf-all.c b/accel/hvf/hvf-all.c
> index 0a4b498e83..5af76ba7a6 100644
> --- a/accel/hvf/hvf-all.c
> +++ b/accel/hvf/hvf-all.c
> @@ -10,6 +10,8 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-visit-common.h"
>   #include "accel/accel-ops.h"
>   #include "system/address-spaces.h"
>   #include "system/memory.h"
> @@ -20,6 +22,7 @@
>   #include "trace.h"
>   
>   bool hvf_allowed;
> +bool hvf_kernel_irqchip;

OK.

>   
>   struct mac_slot {
>       int present;
> @@ -290,6 +293,41 @@ static int hvf_gdbstub_sstep_flags(AccelState *as)
>       return SSTEP_ENABLE | SSTEP_NOIRQ;
>   }
>   
> +static void hvf_set_kernel_irqchip(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +#ifdef __aarch64__

We try to avoid such #ifdef'ry...

> +    OnOffSplit mode;
> +    if (!visit_type_OnOffSplit(v, name, &mode, errp)) {
> +        return;
> +    }
> +
> +    switch (mode) {
> +    case ON_OFF_SPLIT_ON:
> +        hvf_kernel_irqchip = true;
> +        break;
> +
> +    case ON_OFF_SPLIT_OFF:
> +        hvf_kernel_irqchip = false;
> +        break;
> +
> +    case ON_OFF_SPLIT_SPLIT:
> +        error_setg(errp, "HVF: split irqchip is not supported on Arm.");
> +        break;
> +
> +    default:
> +        /*
> +         * The value was checked in visit_type_OnOffSplit() above. If
> +         * we get here, then something is wrong in QEMU.
> +         */
> +        abort();
> +    }
> +#else
> +    error_setg(errp, "HVF: setting irqchip configuration not supported on x86_64.");
> +#endif
> +}
> +
>   static void hvf_accel_class_init(ObjectClass *oc, const void *data)
>   {
>       AccelClass *ac = ACCEL_CLASS(oc);
> @@ -297,6 +335,12 @@ static void hvf_accel_class_init(ObjectClass *oc, const void *data)
>       ac->init_machine = hvf_accel_init;
>       ac->allowed = &hvf_allowed;
>       ac->gdbstub_supported_sstep_flags = hvf_gdbstub_sstep_flags;
... better add hvf_arch_register_class_properties(), empty stub
on x86 and filled in aarch64.

> +    hvf_kernel_irqchip = true;
> +    object_class_property_add(oc, "kernel-irqchip", "on|off|split",
> +        NULL, hvf_set_kernel_irqchip,
> +        NULL, NULL);
> +    object_class_property_set_description(oc, "kernel-irqchip",
> +        "Configure HVF irqchip");
>   }


> diff --git a/system/vl.c b/system/vl.c
> index 3b7057e6c6..1c072d15a4 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1773,6 +1773,8 @@ static void qemu_apply_legacy_machine_options(QDict *qdict)
>                                      false);
>           object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip", value,
>                                      false);
> +        object_register_sugar_prop(ACCEL_CLASS_NAME("hvf"), "kernel-irqchip", value,
> +                                   false);

Discussing about this object_register_sugar_prop() call on IRC, Paolo
said:

  > It's okay the property already exists for KVM so it's okay to add
  > the synonym for HVF as well
  > Maybe make sure it's not registered for x86?

>           qdict_del(qdict, "kernel-irqchip");
>       }
>
Re: [PATCH v5 02/13] accel, hw/arm, include/system/hvf: infrastructure changes for HVF vGIC
Posted by Mohamed Mediouni 3 months, 1 week ago

> On 6. Aug 2025, at 13:32, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> ... better add hvf_arch_register_class_properties(), empty stub
> on x86 and filled in aarch64.
Would it be fine to have the setting still without it doing anything right now?

(For reference, Apple added APIC APIs in macOS 12 on x86 - which we might possibly want to add later or not…)

Or might as well implement support for the HVF APIC in the same series (?)