[PATCH 1/3] KVM: arm64: vgic: Add Apple M2 cpus to the list of broken SEIS implementations

Marc Zyngier posted 3 patches 2 years, 8 months ago
[PATCH 1/3] KVM: arm64: vgic: Add Apple M2 cpus to the list of broken SEIS implementations
Posted by Marc Zyngier 2 years, 8 months ago
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 <maz@kernel.org>
---
 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
Re: [PATCH 1/3] KVM: arm64: vgic: Add Apple M2 cpus to the list of broken SEIS implementations
Posted by Nathan Chancellor 2 years, 8 months ago
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 <maz@kernel.org>
> ---
>  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
Re: [PATCH 1/3] KVM: arm64: vgic: Add Apple M2 cpus to the list of broken SEIS implementations
Posted by Marc Zyngier 2 years, 8 months ago
Hi Nathan,

On Tue, 03 Jan 2023 17:59:12 +0000,
Nathan Chancellor <nathan@kernel.org> 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 <maz@kernel.org>
> > ---
> >  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.