Hi all,
I've spent the holiday break reviving the Nested Virt KVM/arm64
implementation[1] and allowing it to work on the Apple M2 SoC. The
amusing part is that it actually works!
However, the way the vgic is implemented on this HW is still at odds
with the rest of the architecture, and requires some hacks, some of
which are independent of the actual NV code. This is what this series
is about.
The first patch places M2 on the naughty list of broken SEIS
implementations, just like the M1 before it. The second patch allows
a vgic MI to be registered, even if this MI cannot be masked (we
disable it at the source anyway). The last patch hacks the AIC driver
to actually register the vgic MI with KVM.
I plan to take the first patch as a fix for 6.2, while the rest can be
deferred to 6.3.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-6.2-WIP
Marc Zyngier (3):
KVM: arm64: vgic: Add Apple M2 cpus to the list of broken SEIS
implementations
KVM: arm64: vgic: Allow registration of a non-maskable maintenance
interrupt
irqchip/apple-aic: Register vgic maintenance interrupt with KVM
arch/arm64/include/asm/cputype.h | 4 +++
arch/arm64/kvm/vgic/vgic-init.c | 2 +-
arch/arm64/kvm/vgic/vgic-v3.c | 3 +-
drivers/irqchip/irq-apple-aic.c | 55 ++++++++++++++++++++++++--------
4 files changed, 49 insertions(+), 15 deletions(-)
--
2.34.1
I really hoped that Apple had fixed their not-quite-a-vgic implementation
when moving from M1 to M2. Alas, it seems they didn't, and running
a buggy EFI version results in the vgic generating SErrors outside
of the guest and taking the host down.
Apply the same workaround as for M1. Yes, this is all a bit crap.
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm64/include/asm/cputype.h | 4 ++++
arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 4e8b66c74ea2..683ca3af4084 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -124,6 +124,8 @@
#define APPLE_CPU_PART_M1_FIRESTORM_PRO 0x025
#define APPLE_CPU_PART_M1_ICESTORM_MAX 0x028
#define APPLE_CPU_PART_M1_FIRESTORM_MAX 0x029
+#define APPLE_CPU_PART_M2_BLIZZARD 0x032
+#define APPLE_CPU_PART_M2_AVALANCHE 0x033
#define AMPERE_CPU_PART_AMPERE1 0xAC3
@@ -177,6 +179,8 @@
#define MIDR_APPLE_M1_FIRESTORM_PRO MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_FIRESTORM_PRO)
#define MIDR_APPLE_M1_ICESTORM_MAX MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_ICESTORM_MAX)
#define MIDR_APPLE_M1_FIRESTORM_MAX MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_FIRESTORM_MAX)
+#define MIDR_APPLE_M2_BLIZZARD MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M2_BLIZZARD)
+#define MIDR_APPLE_M2_AVALANCHE MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M2_AVALANCHE)
#define MIDR_AMPERE1 MIDR_CPU_MODEL(ARM_CPU_IMP_AMPERE, AMPERE_CPU_PART_AMPERE1)
/* Fujitsu Erratum 010001 affects A64FX 1.0 and 1.1, (v0r0 and v1r0) */
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 826ff6f2a4e7..c6442b08fe80 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -615,7 +615,8 @@ static const struct midr_range broken_seis[] = {
MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM_PRO),
MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM_PRO),
MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM_MAX),
- MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM_MAX),
+ MIDR_ALL_VERSIONS(MIDR_APPLE_M2_BLIZZARD),
+ MIDR_ALL_VERSIONS(MIDR_APPLE_M2_AVALANCHE),
{},
};
--
2.34.1
Our Apple M1/M2 friends do have a per-CPU maintenance interrupt,
but no mask to make use of it in the standard Linux framework.
Given that KVM directly drives the *source* of the interrupt and
leaves the GIC interrupt always enabled, there is no harm in tolerating
such a setup. It will become useful once we enable NV on M2 HW.
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm64/kvm/vgic/vgic-init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index f6d4f4052555..e61d9ca01768 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -572,7 +572,7 @@ int kvm_vgic_hyp_init(void)
if (ret)
return ret;
- if (!has_mask)
+ if (!has_mask && !kvm_vgic_global_state.maint_irq)
return 0;
ret = request_percpu_irq(kvm_vgic_global_state.maint_irq,
--
2.34.1
In order to deliver vgic maintenance interrupts that Nested Virt
requires, hook it into the FIQ space, even if it is delivered
as an IRQ (we don't distinguish between the two anyway).
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-apple-aic.c | 55 +++++++++++++++++++++++++--------
1 file changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index ae3437f03e6c..09fd52d91e45 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -210,7 +210,6 @@
FIELD_PREP(AIC_EVENT_NUM, x))
#define AIC_HWIRQ_IRQ(x) FIELD_GET(AIC_EVENT_NUM, x)
#define AIC_HWIRQ_DIE(x) FIELD_GET(AIC_EVENT_DIE, x)
-#define AIC_NR_FIQ 6
#define AIC_NR_SWIPI 32
/*
@@ -222,11 +221,18 @@
* running at EL2 (with VHE). When the kernel is running at EL1, the
* mapping differs and aic_irq_domain_translate() performs the remapping.
*/
-
-#define AIC_TMR_EL0_PHYS AIC_TMR_HV_PHYS
-#define AIC_TMR_EL0_VIRT AIC_TMR_HV_VIRT
-#define AIC_TMR_EL02_PHYS AIC_TMR_GUEST_PHYS
-#define AIC_TMR_EL02_VIRT AIC_TMR_GUEST_VIRT
+enum fiq_hwirq {
+ /* Must be ordered as in apple-aic.h */
+ AIC_TMR_EL0_PHYS = AIC_TMR_HV_PHYS,
+ AIC_TMR_EL0_VIRT = AIC_TMR_HV_VIRT,
+ AIC_TMR_EL02_PHYS = AIC_TMR_GUEST_PHYS,
+ AIC_TMR_EL02_VIRT = AIC_TMR_GUEST_VIRT,
+ AIC_CPU_PMU_Effi = AIC_CPU_PMU_E,
+ AIC_CPU_PMU_Perf = AIC_CPU_PMU_P,
+ /* No need for this to be discovered from DT */
+ AIC_VGIC_MI,
+ AIC_NR_FIQ
+};
static DEFINE_STATIC_KEY_TRUE(use_fast_ipi);
@@ -384,14 +390,20 @@ static void __exception_irq_entry aic_handle_irq(struct pt_regs *regs)
/*
* vGIC maintenance interrupts end up here too, so we need to check
- * for them separately. This should never trigger if KVM is working
- * properly, because it will have already taken care of clearing it
- * on guest exit before this handler runs.
+ * for them separately. It should however only trigger when NV is
+ * in use, and be cleared when coming back from the handler.
*/
- if (is_kernel_in_hyp_mode() && (read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EN) &&
- read_sysreg_s(SYS_ICH_MISR_EL2) != 0) {
- pr_err_ratelimited("vGIC IRQ fired and not handled by KVM, disabling.\n");
- sysreg_clear_set_s(SYS_ICH_HCR_EL2, ICH_HCR_EN, 0);
+ if (is_kernel_in_hyp_mode() &&
+ (read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EN) &&
+ read_sysreg_s(SYS_ICH_MISR_EL2) != 0) {
+ generic_handle_domain_irq(aic_irqc->hw_domain,
+ AIC_FIQ_HWIRQ(AIC_VGIC_MI));
+
+ if (unlikely((read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EN) &&
+ read_sysreg_s(SYS_ICH_MISR_EL2))) {
+ pr_err_ratelimited("vGIC IRQ fired and not handled by KVM, disabling.\n");
+ sysreg_clear_set_s(SYS_ICH_HCR_EL2, ICH_HCR_EN, 0);
+ }
}
}
@@ -1178,6 +1190,23 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
"irqchip/apple-aic/ipi:starting",
aic_init_cpu, NULL);
+ if (is_kernel_in_hyp_mode()) {
+ struct irq_fwspec mi = {
+ .fwnode = of_node_to_fwnode(node),
+ .param_count = 3,
+ .param = {
+ [0] = AIC_FIQ, /* This is a lie */
+ [1] = AIC_VGIC_MI,
+ [2] = IRQ_TYPE_LEVEL_HIGH,
+ },
+ };
+
+ vgic_info.maint_irq = irq_domain_alloc_irqs(irqc->hw_domain,
+ 1, NUMA_NO_NODE,
+ &mi);
+ WARN_ON(!vgic_info.maint_irq);
+ }
+
vgic_set_kvm_info(&vgic_info);
pr_info("Initialized with %d/%d IRQs * %d/%d die(s), %d FIQs, %d vIPIs",
--
2.34.1
Hi Marc,
I have only been lurking on the kvmarm mailing list for a little bit and
it has mainly just been reading patches/review to get more familiar with
various virtualization concepts so I apologize if the following review
is rather shallow but...
On Tue, Jan 03, 2023 at 09:50:20AM +0000, Marc Zyngier wrote:
> I really hoped that Apple had fixed their not-quite-a-vgic implementation
> when moving from M1 to M2. Alas, it seems they didn't, and running
> a buggy EFI version results in the vgic generating SErrors outside
> of the guest and taking the host down.
>
> Apply the same workaround as for M1. Yes, this is all a bit crap.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> arch/arm64/include/asm/cputype.h | 4 ++++
> arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 4e8b66c74ea2..683ca3af4084 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -124,6 +124,8 @@
> #define APPLE_CPU_PART_M1_FIRESTORM_PRO 0x025
> #define APPLE_CPU_PART_M1_ICESTORM_MAX 0x028
> #define APPLE_CPU_PART_M1_FIRESTORM_MAX 0x029
> +#define APPLE_CPU_PART_M2_BLIZZARD 0x032
> +#define APPLE_CPU_PART_M2_AVALANCHE 0x033
>
> #define AMPERE_CPU_PART_AMPERE1 0xAC3
>
> @@ -177,6 +179,8 @@
> #define MIDR_APPLE_M1_FIRESTORM_PRO MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_FIRESTORM_PRO)
> #define MIDR_APPLE_M1_ICESTORM_MAX MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_ICESTORM_MAX)
> #define MIDR_APPLE_M1_FIRESTORM_MAX MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_FIRESTORM_MAX)
> +#define MIDR_APPLE_M2_BLIZZARD MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M2_BLIZZARD)
> +#define MIDR_APPLE_M2_AVALANCHE MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M2_AVALANCHE)
> #define MIDR_AMPERE1 MIDR_CPU_MODEL(ARM_CPU_IMP_AMPERE, AMPERE_CPU_PART_AMPERE1)
>
> /* Fujitsu Erratum 010001 affects A64FX 1.0 and 1.1, (v0r0 and v1r0) */
> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 826ff6f2a4e7..c6442b08fe80 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -615,7 +615,8 @@ static const struct midr_range broken_seis[] = {
> MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM_PRO),
> MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM_PRO),
> MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM_MAX),
> - MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM_MAX),
The commit message makes no note of this removal, was it intentional?
MIDR_APPLE_M1_FIRESTORM_MAX is only used here so I assume it is not.
> + MIDR_ALL_VERSIONS(MIDR_APPLE_M2_BLIZZARD),
> + MIDR_ALL_VERSIONS(MIDR_APPLE_M2_AVALANCHE),
> {},
> };
>
> --
> 2.34.1
>
Cheers,
Nathan
On Tue, 3 Jan 2023 09:50:19 +0000, Marc Zyngier wrote:
> I've spent the holiday break reviving the Nested Virt KVM/arm64
> implementation[1] and allowing it to work on the Apple M2 SoC. The
> amusing part is that it actually works!
>
> However, the way the vgic is implemented on this HW is still at odds
> with the rest of the architecture, and requires some hacks, some of
> which are independent of the actual NV code. This is what this series
> is about.
>
> [...]
Applied to fixes, thanks!
[1/3] KVM: arm64: vgic: Add Apple M2 cpus to the list of broken SEIS implementations
commit: 4f6202c9fb51cc6a2593ad37d8ddff136b7acef2
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
Hi Nathan,
On Tue, 03 Jan 2023 17:59:12 +0000,
Nathan Chancellor <[email protected]> wrote:
>
> Hi Marc,
>
> I have only been lurking on the kvmarm mailing list for a little bit and
> it has mainly just been reading patches/review to get more familiar with
> various virtualization concepts so I apologize if the following review
> is rather shallow but...
No need to apologise. Any extra pair of eye is welcome, specially when
the idiot behind the keyboard writes stuff like the patch below...
>
> On Tue, Jan 03, 2023 at 09:50:20AM +0000, Marc Zyngier wrote:
> > I really hoped that Apple had fixed their not-quite-a-vgic implementation
> > when moving from M1 to M2. Alas, it seems they didn't, and running
> > a buggy EFI version results in the vgic generating SErrors outside
> > of the guest and taking the host down.
> >
> > Apply the same workaround as for M1. Yes, this is all a bit crap.
> >
> > Signed-off-by: Marc Zyngier <[email protected]>
> > ---
> > arch/arm64/include/asm/cputype.h | 4 ++++
> > arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> > index 4e8b66c74ea2..683ca3af4084 100644
> > --- a/arch/arm64/include/asm/cputype.h
> > +++ b/arch/arm64/include/asm/cputype.h
> > @@ -124,6 +124,8 @@
> > #define APPLE_CPU_PART_M1_FIRESTORM_PRO 0x025
> > #define APPLE_CPU_PART_M1_ICESTORM_MAX 0x028
> > #define APPLE_CPU_PART_M1_FIRESTORM_MAX 0x029
> > +#define APPLE_CPU_PART_M2_BLIZZARD 0x032
> > +#define APPLE_CPU_PART_M2_AVALANCHE 0x033
> >
> > #define AMPERE_CPU_PART_AMPERE1 0xAC3
> >
> > @@ -177,6 +179,8 @@
> > #define MIDR_APPLE_M1_FIRESTORM_PRO MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_FIRESTORM_PRO)
> > #define MIDR_APPLE_M1_ICESTORM_MAX MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_ICESTORM_MAX)
> > #define MIDR_APPLE_M1_FIRESTORM_MAX MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M1_FIRESTORM_MAX)
> > +#define MIDR_APPLE_M2_BLIZZARD MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M2_BLIZZARD)
> > +#define MIDR_APPLE_M2_AVALANCHE MIDR_CPU_MODEL(ARM_CPU_IMP_APPLE, APPLE_CPU_PART_M2_AVALANCHE)
> > #define MIDR_AMPERE1 MIDR_CPU_MODEL(ARM_CPU_IMP_AMPERE, AMPERE_CPU_PART_AMPERE1)
> >
> > /* Fujitsu Erratum 010001 affects A64FX 1.0 and 1.1, (v0r0 and v1r0) */
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index 826ff6f2a4e7..c6442b08fe80 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -615,7 +615,8 @@ static const struct midr_range broken_seis[] = {
> > MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM_PRO),
> > MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM_PRO),
> > MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM_MAX),
> > - MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM_MAX),
>
> The commit message makes no note of this removal, was it intentional?
> MIDR_APPLE_M1_FIRESTORM_MAX is only used here so I assume it is not.
Absolutely not intentional :-/ Thanks a lot for spotting this!
I'll fix this immediately (good thing I didn't send the fixes PR!).
Thanks again,
M.
--
Without deviation from the norm, progress is not possible.
On Tue, 3 Jan 2023 09:50:19 +0000, Marc Zyngier wrote:
> I've spent the holiday break reviving the Nested Virt KVM/arm64
> implementation[1] and allowing it to work on the Apple M2 SoC. The
> amusing part is that it actually works!
>
> However, the way the vgic is implemented on this HW is still at odds
> with the rest of the architecture, and requires some hacks, some of
> which are independent of the actual NV code. This is what this series
> is about.
>
> [...]
Applied to kvmarm/next, thanks!
[2/3] KVM: arm64: vgic: Allow registration of a non-maskable maintenance interrupt
https://git.kernel.org/kvmarm/kvmarm/c/43c5c868bddc
[3/3] irqchip/apple-aic: Register vgic maintenance interrupt with KVM
https://git.kernel.org/kvmarm/kvmarm/c/13aad0c00bb1
--
Best,
Oliver