| Message ID | 20240209160039.677865-3-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ARM Nested Virt Support | expand |
On Fri, 9 Feb 2024 at 16:00, Eric Auger <eric.auger@redhat.com> wrote: > > From: Haibo Xu <haibo.xu@linaro.org> > > Allow virt arm machine to set the intid for the KVM GIC maintenance > interrupt. > > Signed-off-by: Haibo Xu <haibo.xu@linaro.org> > Signed-off-by: Miguel Luis <miguel.luis@oracle.com> > Signed-off-by: Eric Auger <eric.auger@redhat.com> > > --- > v1 -> v2: > - [Miguel] replaced the has_virt_extensions by the maintenance irq > intid property. [Eric] restored kvm_device_check_attr and > kvm_device_access standard usage and conditionally call those > if the prop is set This seems reasonable, but it's not the same way we opted to handle telling the kernel the IRQ number for the PMU interrupt (where we use kvm_arm_pmu_set_irq()). I guess we have to do it this way because it's a device attr so we need to set it in gic realize, though? By the way, does the kernel automatically complain and fail if we try to enable nested-virt with a GICv2 or with a userspace GIC, or do we need to catch and produce error messages for those (invalid) combinations ourselves? thanks -- PMM
Hi Peter, On 3/5/24 17:46, Peter Maydell wrote: > On Fri, 9 Feb 2024 at 16:00, Eric Auger <eric.auger@redhat.com> wrote: >> From: Haibo Xu <haibo.xu@linaro.org> >> >> Allow virt arm machine to set the intid for the KVM GIC maintenance >> interrupt. >> >> Signed-off-by: Haibo Xu <haibo.xu@linaro.org> >> Signed-off-by: Miguel Luis <miguel.luis@oracle.com> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> >> --- >> v1 -> v2: >> - [Miguel] replaced the has_virt_extensions by the maintenance irq >> intid property. [Eric] restored kvm_device_check_attr and >> kvm_device_access standard usage and conditionally call those >> if the prop is set Please forgive me for the delay > This seems reasonable, but it's not the same way we opted to > handle telling the kernel the IRQ number for the PMU interrupt > (where we use kvm_arm_pmu_set_irq()). I guess we have to do > it this way because it's a device attr so we need to set it > in gic realize, though? This cannot follow the same pattern as the kvm_arm_pmu_set_irq() because the maintenance irq must be set between before the GICv3 KVM device creation and the KVM_DEV_ARM_VGIC_CTRL_INIT. The GICv3 realize function calls both so I cannot set the maintenance after the realize. It would fail with -EBUSY. Hope this helps. Thanks Eric > > By the way, does the kernel automatically complain and fail > if we try to enable nested-virt with a GICv2 or with a > userspace GIC, or do we need to catch and produce error > messages for those (invalid) combinations ourselves? > > thanks > -- PMM >
Hi Peter, On 3/5/24 17:46, Peter Maydell wrote: > On Fri, 9 Feb 2024 at 16:00, Eric Auger <eric.auger@redhat.com> wrote: >> From: Haibo Xu <haibo.xu@linaro.org> >> >> Allow virt arm machine to set the intid for the KVM GIC maintenance >> interrupt. >> >> Signed-off-by: Haibo Xu <haibo.xu@linaro.org> >> Signed-off-by: Miguel Luis <miguel.luis@oracle.com> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> >> --- >> v1 -> v2: >> - [Miguel] replaced the has_virt_extensions by the maintenance irq >> intid property. [Eric] restored kvm_device_check_attr and >> kvm_device_access standard usage and conditionally call those >> if the prop is set > This seems reasonable, but it's not the same way we opted to > handle telling the kernel the IRQ number for the PMU interrupt > (where we use kvm_arm_pmu_set_irq()). I guess we have to do > it this way because it's a device attr so we need to set it > in gic realize, though? > > By the way, does the kernel automatically complain and fail > if we try to enable nested-virt with a GICv2 or with a > userspace GIC, or do we need to catch and produce error > messages for those (invalid) combinations ourselves? I don't think there is any check of that kind in Marc's series yet. This may be added if GICv2 KVM device is created while kvm_mode is set to KVM_MODE_NV. Wrt userspace irqchip compat, KVM_CAP_ARM_USER_IRQ extension may not be exposed in case of nested virt. Marc, is that something you would like to integrate into the kernel series? Thanks Eric > > thanks > -- PMM >
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 368c2a415a..5214aca898 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -750,6 +750,9 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem) OBJECT(mem), &error_fatal); qdev_prop_set_bit(vms->gic, "has-lpi", true); } + } else { + qdev_prop_set_uint32(vms->gic, "maintenance-interrupt-id", + ARCH_GIC_MAINT_IRQ); } } else { if (!kvm_irqchip_in_kernel()) { diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index cb55c72681..df056dc35c 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -564,6 +564,7 @@ static Property arm_gicv3_common_properties[] = { DEFINE_PROP_UINT32("revision", GICv3State, revision, 3), DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0), DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0), + DEFINE_PROP_UINT32("maintenance-interrupt-id", GICv3State, maint_irq, 0), /* * Compatibility property: force 8 bits of physical priority, even * if the CPU being emulated should have fewer. diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 77eb37e131..23fad60515 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -22,6 +22,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "hw/intc/arm_gicv3_common.h" +#include "hw/arm/virt.h" #include "qemu/error-report.h" #include "qemu/module.h" #include "sysemu/kvm.h" @@ -820,6 +821,26 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) return; } + if (s->maint_irq) { + int ret; + + ret = kvm_device_check_attr(s->dev_fd, + KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, 0); + if (!ret) { + error_setg_errno(errp, errno, + "VGICv3 setting maintenance IRQ is not " + "supported by this host kernel"); + return; + } + + ret = kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, 0, + &s->maint_irq, true, errp); + if (ret) { + error_setg_errno(errp, errno, "Failed to set VGIC maintenance IRQ"); + return; + } + } + multiple_redist_region_allowed = kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION); diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index 4e2fb518e7..4ff421a165 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -246,6 +246,7 @@ struct GICv3State { uint32_t num_cpu; uint32_t num_irq; uint32_t revision; + uint32_t maint_irq; bool lpi_enable; bool security_extn; bool force_8bit_prio;