From: Haibo Xu <haibo.xu@linaro.org>
Up to now virt support on guest has been only supported with TCG.
Now it becomes feasible to use it with KVM acceleration.
Also check only in-kernel GICv3 is used along with KVM EL2.
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v6 -> v7:
- rebase on top of "hw/arm/virt: Make EL2 accelerator check an
accept-list". I dared to keep Richard's R-b though.
v2 -> v3:
- check gic version/in-kernel implementation when kvm el2 is set (Peter)
v1 -> v2:
- fixed test ordering: virt && ((kvm && !kvm_el2) || hvf) [Richard]
- tweeked the commit title & message
---
hw/arm/virt.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 550a272fbb..1c0a2c43c4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -792,6 +792,13 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
default:
g_assert_not_reached();
}
+
+ if (kvm_enabled() && vms->virt &&
+ (revision != 3 || !kvm_irqchip_in_kernel())) {
+ error_report("KVM EL2 only is supported with in-kernel GICv3");
+ exit(1);
+ }
+
vms->gic = qdev_new(gictype);
qdev_prop_set_uint32(vms->gic, "revision", revision);
qdev_prop_set_uint32(vms->gic, "num-cpu", smp_cpus);
@@ -2211,7 +2218,8 @@ static void machvirt_init(MachineState *machine)
exit(1);
}
- if (vms->virt && !tcg_enabled() && !qtest_enabled()) {
+ if (vms->virt && !(kvm_enabled() && kvm_arm_el2_supported()) &&
+ !tcg_enabled() && !qtest_enabled()) {
error_report("mach-virt: %s does not support providing "
"Virtualization extensions to the guest CPU",
current_accel_name());
--
2.49.0On Wed, 2 Jul 2025 at 17:31, Eric Auger <eric.auger@redhat.com> wrote:
>
> From: Haibo Xu <haibo.xu@linaro.org>
>
> Up to now virt support on guest has been only supported with TCG.
> Now it becomes feasible to use it with KVM acceleration.
>
> Also check only in-kernel GICv3 is used along with KVM EL2.
>
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> ---
> v6 -> v7:
> - rebase on top of "hw/arm/virt: Make EL2 accelerator check an
> accept-list". I dared to keep Richard's R-b though.
>
> v2 -> v3:
> - check gic version/in-kernel implementation when kvm el2 is set (Peter)
>
> v1 -> v2:
> - fixed test ordering: virt && ((kvm && !kvm_el2) || hvf) [Richard]
> - tweeked the commit title & message
> ---
> hw/arm/virt.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 550a272fbb..1c0a2c43c4 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -792,6 +792,13 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
> default:
> g_assert_not_reached();
> }
> +
> + if (kvm_enabled() && vms->virt &&
> + (revision != 3 || !kvm_irqchip_in_kernel())) {
> + error_report("KVM EL2 only is supported with in-kernel GICv3");
"is only supported"
> + exit(1);
> + }
> +
> vms->gic = qdev_new(gictype);
> qdev_prop_set_uint32(vms->gic, "revision", revision);
> qdev_prop_set_uint32(vms->gic, "num-cpu", smp_cpus);
> @@ -2211,7 +2218,8 @@ static void machvirt_init(MachineState *machine)
> exit(1);
> }
>
> - if (vms->virt && !tcg_enabled() && !qtest_enabled()) {
> + if (vms->virt && !(kvm_enabled() && kvm_arm_el2_supported()) &&
> + !tcg_enabled() && !qtest_enabled()) {
> error_report("mach-virt: %s does not support providing "
> "Virtualization extensions to the guest CPU",
> current_accel_name());
Have you tested doing a VM migration of a KVM with EL2 setup?
I suppose the system registers probably generally Just Work
via the sysreg GET/SET_ONE_REG API, but won't the in-kernel
GICv3 have extra state that we need to migrate in
hw/intc/arm_gicv3_kvm.c ?
thanks
-- PMM
Hi Peter, Marc,
On 7/4/25 2:22 PM, Peter Maydell wrote:
> On Wed, 2 Jul 2025 at 17:31, Eric Auger <eric.auger@redhat.com> wrote:
>> From: Haibo Xu <haibo.xu@linaro.org>
>>
>> Up to now virt support on guest has been only supported with TCG.
>> Now it becomes feasible to use it with KVM acceleration.
>>
>> Also check only in-kernel GICv3 is used along with KVM EL2.
>>
>> 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>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> ---
>> v6 -> v7:
>> - rebase on top of "hw/arm/virt: Make EL2 accelerator check an
>> accept-list". I dared to keep Richard's R-b though.
>>
>> v2 -> v3:
>> - check gic version/in-kernel implementation when kvm el2 is set (Peter)
>>
>> v1 -> v2:
>> - fixed test ordering: virt && ((kvm && !kvm_el2) || hvf) [Richard]
>> - tweeked the commit title & message
>> ---
>> hw/arm/virt.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 550a272fbb..1c0a2c43c4 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -792,6 +792,13 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
>> default:
>> g_assert_not_reached();
>> }
>> +
>> + if (kvm_enabled() && vms->virt &&
>> + (revision != 3 || !kvm_irqchip_in_kernel())) {
>> + error_report("KVM EL2 only is supported with in-kernel GICv3");
> "is only supported"
OK
>
>> + exit(1);
>> + }
>> +
>> vms->gic = qdev_new(gictype);
>> qdev_prop_set_uint32(vms->gic, "revision", revision);
>> qdev_prop_set_uint32(vms->gic, "num-cpu", smp_cpus);
>> @@ -2211,7 +2218,8 @@ static void machvirt_init(MachineState *machine)
>> exit(1);
>> }
>>
>> - if (vms->virt && !tcg_enabled() && !qtest_enabled()) {
>> + if (vms->virt && !(kvm_enabled() && kvm_arm_el2_supported()) &&
>> + !tcg_enabled() && !qtest_enabled()) {
>> error_report("mach-virt: %s does not support providing "
>> "Virtualization extensions to the guest CPU",
>> current_accel_name());
> Have you tested doing a VM migration of a KVM with EL2 setup?
Yes I did. Marc fixed some save/restore bugs in the past and I was able
to migrate after this fix.
Now I would lie if I'd tell you it works well. It is extremely slow to
converge. Usually you get rcu stalls in the L2 guest on source too.
> I suppose the system registers probably generally Just Work
> via the sysreg GET/SET_ONE_REG API, but won't the in-kernel
> GICv3 have extra state that we need to migrate in
> hw/intc/arm_gicv3_kvm.c ?
Do you see some specific registers/resources that would need attention?
Otherwise I can put a migration blocker until we spend more time
analyzing migration slowness
What do you think?
Eric
>
> thanks
> -- PMM
>
On Mon, 7 Jul 2025 at 10:30, Eric Auger <eric.auger@redhat.com> wrote: > > Hi Peter, Marc, > > On 7/4/25 2:22 PM, Peter Maydell wrote: > > I suppose the system registers probably generally Just Work > > via the sysreg GET/SET_ONE_REG API, but won't the in-kernel > > GICv3 have extra state that we need to migrate in > > hw/intc/arm_gicv3_kvm.c ? > Do you see some specific registers/resources that would need attention? All the EL2-only-accessible GIC registers: ICH_AP*R<n>_EL2, ICH_EISR_EL2, ICH_ELRSR_EL2, ICH_HCR_EL2, ICH_LR<n>_EL2, etc etc. These all need to be exposed via KVM_DEV_ARM_VGIC_GRP_CPU_SYSREG and hw/intc/arm_gicv3_kvm.c needs code to be able to save and restore them into the GIC data structures (and we need to make sure the kernel isn't accidentally exposing them as CPU registers via the GET/SET_ONE_REG API, I think, in whatever way we do that for the existing EL1 GIC cpuif registers). I don't see any of the EL2 sysregs listed in the kernel's gic_v3_icc_reg_descs[], which looks like it's what drives the handling of KVM_DEV_ARM_VGIC_GRP_CPU_SYSREG. I'm ok with just putting in a migration blocker for the moment, especially if this needs kernel-side changes. thanks -- PMM
On Mon, 07 Jul 2025 10:53:38 +0100,
Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 7 Jul 2025 at 10:30, Eric Auger <eric.auger@redhat.com> wrote:
> >
> > Hi Peter, Marc,
> >
> > On 7/4/25 2:22 PM, Peter Maydell wrote:
> > > I suppose the system registers probably generally Just Work
> > > via the sysreg GET/SET_ONE_REG API, but won't the in-kernel
> > > GICv3 have extra state that we need to migrate in
> > > hw/intc/arm_gicv3_kvm.c ?
> > Do you see some specific registers/resources that would need attention?
>
> All the EL2-only-accessible GIC registers: ICH_AP*R<n>_EL2,
> ICH_EISR_EL2, ICH_ELRSR_EL2, ICH_HCR_EL2, ICH_LR<n>_EL2,
> etc etc.
It isn't obvious to me why we'd expose any of the status registers to
userspace. ICH_{EISR,ELRSR,MISR}_EL2 are entirely synthetic, and are
directly generated by the emulation from the rest of the state.
Save/restoring this state would make things pointlessly complicated,
unless I'm missing something obvious.
> These all need to be exposed via KVM_DEV_ARM_VGIC_GRP_CPU_SYSREG
> and hw/intc/arm_gicv3_kvm.c needs code to be able to save and
> restore them into the GIC data structures (and we need to make
> sure the kernel isn't accidentally exposing them as CPU registers
> via the GET/SET_ONE_REG API, I think, in whatever way we do
> that for the existing EL1 GIC cpuif registers).
>
> I don't see any of the EL2 sysregs listed in the kernel's
> gic_v3_icc_reg_descs[], which looks like it's what drives
> the handling of KVM_DEV_ARM_VGIC_GRP_CPU_SYSREG.
Hmmm... This is indeed pretty inconsistent. I'll have a look at it.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Mon, 7 Jul 2025 at 15:32, Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 07 Jul 2025 10:53:38 +0100,
> Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 7 Jul 2025 at 10:30, Eric Auger <eric.auger@redhat.com> wrote:
> > >
> > > Hi Peter, Marc,
> > >
> > > On 7/4/25 2:22 PM, Peter Maydell wrote:
> > > > I suppose the system registers probably generally Just Work
> > > > via the sysreg GET/SET_ONE_REG API, but won't the in-kernel
> > > > GICv3 have extra state that we need to migrate in
> > > > hw/intc/arm_gicv3_kvm.c ?
> > > Do you see some specific registers/resources that would need attention?
> >
> > All the EL2-only-accessible GIC registers: ICH_AP*R<n>_EL2,
> > ICH_EISR_EL2, ICH_ELRSR_EL2, ICH_HCR_EL2, ICH_LR<n>_EL2,
> > etc etc.
>
> It isn't obvious to me why we'd expose any of the status registers to
> userspace. ICH_{EISR,ELRSR,MISR}_EL2 are entirely synthetic, and are
> directly generated by the emulation from the rest of the state.
>
> Save/restoring this state would make things pointlessly complicated,
> unless I'm missing something obvious.
Good point: we don't need to expose those, only the actual
state-holding registers. I was rather mindlessly going through
and listing all the ICH_ registers out of the spec...
So should the list be:
ICH_AP*R<n>_EL2, ICH_HCR_EL2, ICH_HR<n>_EL2, ICH_VMCR_EL2
?
(Those are the ones the TCG GICv3 has state for in its data
structures.)
How about ICH_VTR_EL2 ? I guess we should handle it the same
way we do other existing ID registers (migrate it
and write value back to kernel so it can validate that
the destination hardware supports the same config) ?
Speaking of GIC ID registers, we never updated QEMU to
handle the GICv4.1 GICD_TYPER2 register, so we don't try
to send and sanity check that on migration at the moment.
thanks
-- PMM
On Mon, 07 Jul 2025 15:46:04 +0100,
Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 7 Jul 2025 at 15:32, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Mon, 07 Jul 2025 10:53:38 +0100,
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Mon, 7 Jul 2025 at 10:30, Eric Auger <eric.auger@redhat.com> wrote:
> > > >
> > > > Hi Peter, Marc,
> > > >
> > > > On 7/4/25 2:22 PM, Peter Maydell wrote:
> > > > > I suppose the system registers probably generally Just Work
> > > > > via the sysreg GET/SET_ONE_REG API, but won't the in-kernel
> > > > > GICv3 have extra state that we need to migrate in
> > > > > hw/intc/arm_gicv3_kvm.c ?
> > > > Do you see some specific registers/resources that would need attention?
> > >
> > > All the EL2-only-accessible GIC registers: ICH_AP*R<n>_EL2,
> > > ICH_EISR_EL2, ICH_ELRSR_EL2, ICH_HCR_EL2, ICH_LR<n>_EL2,
> > > etc etc.
> >
> > It isn't obvious to me why we'd expose any of the status registers to
> > userspace. ICH_{EISR,ELRSR,MISR}_EL2 are entirely synthetic, and are
> > directly generated by the emulation from the rest of the state.
> >
> > Save/restoring this state would make things pointlessly complicated,
> > unless I'm missing something obvious.
>
> Good point: we don't need to expose those, only the actual
> state-holding registers. I was rather mindlessly going through
> and listing all the ICH_ registers out of the spec...
>
> So should the list be:
> ICH_AP*R<n>_EL2, ICH_HCR_EL2, ICH_HR<n>_EL2, ICH_VMCR_EL2
> ?
> (Those are the ones the TCG GICv3 has state for in its data
> structures.)
Yup, this matches my own expectations.
> How about ICH_VTR_EL2 ? I guess we should handle it the same
> way we do other existing ID registers (migrate it
> and write value back to kernel so it can validate that
> the destination hardware supports the same config) ?
Indeed. Specially as this contains the number of LRs, which could
otherwise lead to UNDEF exceptions if migrating in the wrong
direction.
What of ICC_SRE_EL2? We have it as RAO/WI for all the non-RES0
fields. It could be saved/restored as well for the same purposes...
> Speaking of GIC ID registers, we never updated QEMU to
> handle the GICv4.1 GICD_TYPER2 register, so we don't try
> to send and sanity check that on migration at the moment.
KVM only exposes a virtual GICv3 to the guest, even in the host can do
GICv4.0+. The only nit is the "nASSGIcap" bit, which is always
advertised to the guest when the host if v4.1-capable. We have a patch
series in progress to address this and make it controllable.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Mon, 7 Jul 2025 at 16:44, Marc Zyngier <maz@kernel.org> wrote: > > On Mon, 07 Jul 2025 15:46:04 +0100, > Peter Maydell <peter.maydell@linaro.org> wrote: > > Speaking of GIC ID registers, we never updated QEMU to > > handle the GICv4.1 GICD_TYPER2 register, so we don't try > > to send and sanity check that on migration at the moment. > > KVM only exposes a virtual GICv3 to the guest, even in the host can do > GICv4.0+. The only nit is the "nASSGIcap" bit, which is always > advertised to the guest when the host if v4.1-capable. We have a patch > series in progress to address this and make it controllable. Isn't that out-of-spec? If the GIC the guest sees is a GICv3 then GICD_TYPER2 should be RES0... Anyway, I have some lightly tested QEMU patches that migrate the GICD_TYPER2 value, which I'll send out in a moment. -- PMM
Hi Peter, On 7/7/25 11:53 AM, Peter Maydell wrote: > On Mon, 7 Jul 2025 at 10:30, Eric Auger <eric.auger@redhat.com> wrote: >> Hi Peter, Marc, >> >> On 7/4/25 2:22 PM, Peter Maydell wrote: >>> I suppose the system registers probably generally Just Work >>> via the sysreg GET/SET_ONE_REG API, but won't the in-kernel >>> GICv3 have extra state that we need to migrate in >>> hw/intc/arm_gicv3_kvm.c ? >> Do you see some specific registers/resources that would need attention? > All the EL2-only-accessible GIC registers: ICH_AP*R<n>_EL2, > ICH_EISR_EL2, ICH_ELRSR_EL2, ICH_HCR_EL2, ICH_LR<n>_EL2, > etc etc. > > These all need to be exposed via KVM_DEV_ARM_VGIC_GRP_CPU_SYSREG > and hw/intc/arm_gicv3_kvm.c needs code to be able to save and > restore them into the GIC data structures (and we need to make > sure the kernel isn't accidentally exposing them as CPU registers > via the GET/SET_ONE_REG API, I think, in whatever way we do > that for the existing EL1 GIC cpuif registers). > > I don't see any of the EL2 sysregs listed in the kernel's > gic_v3_icc_reg_descs[], which looks like it's what drives > the handling of KVM_DEV_ARM_VGIC_GRP_CPU_SYSREG. OK thank you for the insights. > > I'm ok with just putting in a migration blocker for the moment, > especially if this needs kernel-side changes. OK I will do that until we get this fixed Thanks! Eric > > thanks > -- PMM >
© 2016 - 2025 Red Hat, Inc.