[PATCH 0/5] KVM: arm64: Assorted vgic-v3 fixes

[PATCH 0/5] KVM: arm64: Assorted vgic-v3 fixes

[Marc Zyngier]

Here's a bunch of vgic-v3 fixes I have been sitting on for some
time. None of them are critical, though some are rather entertaining.

The first one is a leftover from the initial Apple-M1 enablement,
which doesn't advertise the GIC support via ID_AA64PFR0_EL1 (which is
expected, as it only has half a GIC...). We address it by forcefully
advertising the feature if the guest has a GICv3.

The second patch is really fun, and shows how things can go wrong when
they are badly specified. The gist of it is that on systems that
advertise ICH_VTR_EL2.SEIS, we need to fallback to the full GICv3
cpuif emulation. The third patch is a direct consequence of the
previous one.

The last two patches are more of a harmless oddity: virtual LPIs
happen to have an active state buried into the pseudocode (and only
there). Fun!  Nothing goes wrong with that, but we can perform a minor
optimisation, and we need to align the emulation to match the
pseudocode.

All of this is only targeting 5.16, and I don't plan to backport any
of it.

Marc Zyngier (5):
  KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3
  KVM: arm64: Work around GICv3 locally generated SErrors
  KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS
  KVM: arm64: vgic-v3: Don't propagate LPI active state from LRs into
    the distributor
  KVM: arm64: vgic-v3: Align emulated cpuif LPI state machine with the
    pseudocode

 arch/arm64/kvm/hyp/vgic-v3-sr.c | 22 ++++++++--------------
 arch/arm64/kvm/sys_regs.c       |  5 +++++
 arch/arm64/kvm/vgic/vgic-v3.c   | 11 ++++++++++-
 3 files changed, 23 insertions(+), 15 deletions(-)

-- 
2.30.2

[PATCH 0/5] KVM: arm64: Assorted vgic-v3 fixes

[Marc Zyngier]

Here's a bunch of vgic-v3 fixes I have been sitting on for some
time. None of them are critical, though some are rather entertaining.

The first one is a leftover from the initial Apple-M1 enablement,
which doesn't advertise the GIC support via ID_AA64PFR0_EL1 (which is
expected, as it only has half a GIC...). We address it by forcefully
advertising the feature if the guest has a GICv3.

The second patch is really fun, and shows how things can go wrong when
they are badly specified. The gist of it is that on systems that
advertise ICH_VTR_EL2.SEIS, we need to fallback to the full GICv3
cpuif emulation. The third patch is a direct consequence of the
previous one.

The last two patches are more of a harmless oddity: virtual LPIs
happen to have an active state buried into the pseudocode (and only
there). Fun!  Nothing goes wrong with that, but we can perform a minor
optimisation, and we need to align the emulation to match the
pseudocode.

All of this is only targeting 5.16, and I don't plan to backport any
of it.

Marc Zyngier (5):
  KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3
  KVM: arm64: Work around GICv3 locally generated SErrors
  KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS
  KVM: arm64: vgic-v3: Don't propagate LPI active state from LRs into
    the distributor
  KVM: arm64: vgic-v3: Align emulated cpuif LPI state machine with the
    pseudocode

 arch/arm64/kvm/hyp/vgic-v3-sr.c | 22 ++++++++--------------
 arch/arm64/kvm/sys_regs.c       |  5 +++++
 arch/arm64/kvm/vgic/vgic-v3.c   | 11 ++++++++++-
 3 files changed, 23 insertions(+), 15 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/5] KVM: arm64: Assorted vgic-v3 fixes

[Marc Zyngier]

Here's a bunch of vgic-v3 fixes I have been sitting on for some
time. None of them are critical, though some are rather entertaining.

The first one is a leftover from the initial Apple-M1 enablement,
which doesn't advertise the GIC support via ID_AA64PFR0_EL1 (which is
expected, as it only has half a GIC...). We address it by forcefully
advertising the feature if the guest has a GICv3.

The second patch is really fun, and shows how things can go wrong when
they are badly specified. The gist of it is that on systems that
advertise ICH_VTR_EL2.SEIS, we need to fallback to the full GICv3
cpuif emulation. The third patch is a direct consequence of the
previous one.

The last two patches are more of a harmless oddity: virtual LPIs
happen to have an active state buried into the pseudocode (and only
there). Fun!  Nothing goes wrong with that, but we can perform a minor
optimisation, and we need to align the emulation to match the
pseudocode.

All of this is only targeting 5.16, and I don't plan to backport any
of it.

Marc Zyngier (5):
  KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3
  KVM: arm64: Work around GICv3 locally generated SErrors
  KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS
  KVM: arm64: vgic-v3: Don't propagate LPI active state from LRs into
    the distributor
  KVM: arm64: vgic-v3: Align emulated cpuif LPI state machine with the
    pseudocode

 arch/arm64/kvm/hyp/vgic-v3-sr.c | 22 ++++++++--------------
 arch/arm64/kvm/sys_regs.c       |  5 +++++
 arch/arm64/kvm/vgic/vgic-v3.c   | 11 ++++++++++-
 3 files changed, 23 insertions(+), 15 deletions(-)

-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Marc Zyngier]

Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
visible on the host, even if we were running a GICv2-enabled VM
on a GICv3+compat host.

That's fine, but we also now have the case of a host that does not
expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
confusing. Thank you M1.

Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
when a GICv3 is exposed to the guest. This also hides a GICv4.1
CPU interface from the guest which has no business knowing about
the v4.1 extension.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d46e185f31e..0e8fc29df19c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1075,6 +1075,11 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
+		if (irqchip_in_kernel(vcpu->kvm) &&
+		    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
+			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC);
+			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1);
+		}
 		break;
 	case SYS_ID_AA64PFR1_EL1:
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);
-- 
2.30.2

[PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Marc Zyngier]

Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
visible on the host, even if we were running a GICv2-enabled VM
on a GICv3+compat host.

That's fine, but we also now have the case of a host that does not
expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
confusing. Thank you M1.

Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
when a GICv3 is exposed to the guest. This also hides a GICv4.1
CPU interface from the guest which has no business knowing about
the v4.1 extension.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d46e185f31e..0e8fc29df19c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1075,6 +1075,11 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
+		if (irqchip_in_kernel(vcpu->kvm) &&
+		    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
+			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC);
+			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1);
+		}
 		break;
 	case SYS_ID_AA64PFR1_EL1:
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Marc Zyngier]

Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
visible on the host, even if we were running a GICv2-enabled VM
on a GICv3+compat host.

That's fine, but we also now have the case of a host that does not
expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
confusing. Thank you M1.

Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
when a GICv3 is exposed to the guest. This also hides a GICv4.1
CPU interface from the guest which has no business knowing about
the v4.1 extension.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d46e185f31e..0e8fc29df19c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1075,6 +1075,11 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
+		if (irqchip_in_kernel(vcpu->kvm) &&
+		    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
+			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC);
+			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1);
+		}
 		break;
 	case SYS_ID_AA64PFR1_EL1:
 		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);
-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
> visible on the host, even if we were running a GICv2-enabled VM
> on a GICv3+compat host.
>
> That's fine, but we also now have the case of a host that does not
> expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
> confusing. Thank you M1.
>
> Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
> when a GICv3 is exposed to the guest. This also hides a GICv4.1
> CPU interface from the guest which has no business knowing about
> the v4.1 extension.

Had a look at the gic-v3 driver, and as far as I can tell it does not check that a
GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't get this wrong, then this
patch is to ensure architectural compliance for a guest even if the hardware is
not necessarily compliant, right?

GICv4.1 is an extension to GICv4 (which itself was an extension to GICv3) to add
support for virtualization features (virtual SGIs), so I don't see any harm in
hiding it from the guest, since the guest cannot virtual SGIs.

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1d46e185f31e..0e8fc29df19c 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1075,6 +1075,11 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
>  		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
>  		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
> +		if (irqchip_in_kernel(vcpu->kvm) &&
> +		    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> +			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC);
> +			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1);
> +		}
>  		break;
>  	case SYS_ID_AA64PFR1_EL1:
>  		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
> visible on the host, even if we were running a GICv2-enabled VM
> on a GICv3+compat host.
>
> That's fine, but we also now have the case of a host that does not
> expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
> confusing. Thank you M1.
>
> Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
> when a GICv3 is exposed to the guest. This also hides a GICv4.1
> CPU interface from the guest which has no business knowing about
> the v4.1 extension.

Had a look at the gic-v3 driver, and as far as I can tell it does not check that a
GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't get this wrong, then this
patch is to ensure architectural compliance for a guest even if the hardware is
not necessarily compliant, right?

GICv4.1 is an extension to GICv4 (which itself was an extension to GICv3) to add
support for virtualization features (virtual SGIs), so I don't see any harm in
hiding it from the guest, since the guest cannot virtual SGIs.

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1d46e185f31e..0e8fc29df19c 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1075,6 +1075,11 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
>  		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
>  		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
> +		if (irqchip_in_kernel(vcpu->kvm) &&
> +		    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> +			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC);
> +			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1);
> +		}
>  		break;
>  	case SYS_ID_AA64PFR1_EL1:
>  		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
> visible on the host, even if we were running a GICv2-enabled VM
> on a GICv3+compat host.
>
> That's fine, but we also now have the case of a host that does not
> expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
> confusing. Thank you M1.
>
> Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
> when a GICv3 is exposed to the guest. This also hides a GICv4.1
> CPU interface from the guest which has no business knowing about
> the v4.1 extension.

Had a look at the gic-v3 driver, and as far as I can tell it does not check that a
GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't get this wrong, then this
patch is to ensure architectural compliance for a guest even if the hardware is
not necessarily compliant, right?

GICv4.1 is an extension to GICv4 (which itself was an extension to GICv3) to add
support for virtualization features (virtual SGIs), so I don't see any harm in
hiding it from the guest, since the guest cannot virtual SGIs.

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1d46e185f31e..0e8fc29df19c 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1075,6 +1075,11 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
>  		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3);
>  		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
> +		if (irqchip_in_kernel(vcpu->kvm) &&
> +		    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> +			val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_GIC);
> +			val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_GIC), 1);
> +		}
>  		break;
>  	case SYS_ID_AA64PFR1_EL1:
>  		val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_MTE);
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Marc Zyngier]

Hi Alex,

On Wed, 29 Sep 2021 16:29:09 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 9/24/21 09:25, Marc Zyngier wrote:
> > Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
> > visible on the host, even if we were running a GICv2-enabled VM
> > on a GICv3+compat host.
> >
> > That's fine, but we also now have the case of a host that does not
> > expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
> > confusing. Thank you M1.
> >
> > Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
> > when a GICv3 is exposed to the guest. This also hides a GICv4.1
> > CPU interface from the guest which has no business knowing about
> > the v4.1 extension.
> 
> Had a look at the gic-v3 driver, and as far as I can tell it does
> not check that a GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't
> get this wrong, then this patch is to ensure architectural
> compliance for a guest even if the hardware is not necessarily
> compliant, right?

Indeed. Not having this made some of my own tests fail on M1 as they
rely on ID_AA64PFR0_EL1.GIC being correct. I also pondered setting it
to 0 when emulating a GICv2, but that'd be a change in behaviour, and
I want to think a bit more about the effects of that.

> 
> GICv4.1 is an extension to GICv4 (which itself was an extension to
> GICv3) to add support for virtualization features (virtual SGIs), so
> I don't see any harm in hiding it from the guest, since the guest
> cannot virtual SGIs.

Indeed. The guest already has another way to look into this by
checking whether the distributor allows active-less SGIs.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Marc Zyngier]

Hi Alex,

On Wed, 29 Sep 2021 16:29:09 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 9/24/21 09:25, Marc Zyngier wrote:
> > Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
> > visible on the host, even if we were running a GICv2-enabled VM
> > on a GICv3+compat host.
> >
> > That's fine, but we also now have the case of a host that does not
> > expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
> > confusing. Thank you M1.
> >
> > Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
> > when a GICv3 is exposed to the guest. This also hides a GICv4.1
> > CPU interface from the guest which has no business knowing about
> > the v4.1 extension.
> 
> Had a look at the gic-v3 driver, and as far as I can tell it does
> not check that a GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't
> get this wrong, then this patch is to ensure architectural
> compliance for a guest even if the hardware is not necessarily
> compliant, right?

Indeed. Not having this made some of my own tests fail on M1 as they
rely on ID_AA64PFR0_EL1.GIC being correct. I also pondered setting it
to 0 when emulating a GICv2, but that'd be a change in behaviour, and
I want to think a bit more about the effects of that.

> 
> GICv4.1 is an extension to GICv4 (which itself was an extension to
> GICv3) to add support for virtualization features (virtual SGIs), so
> I don't see any harm in hiding it from the guest, since the guest
> cannot virtual SGIs.

Indeed. The guest already has another way to look into this by
checking whether the distributor allows active-less SGIs.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Marc Zyngier]

Hi Alex,

On Wed, 29 Sep 2021 16:29:09 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 9/24/21 09:25, Marc Zyngier wrote:
> > Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
> > visible on the host, even if we were running a GICv2-enabled VM
> > on a GICv3+compat host.
> >
> > That's fine, but we also now have the case of a host that does not
> > expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
> > confusing. Thank you M1.
> >
> > Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
> > when a GICv3 is exposed to the guest. This also hides a GICv4.1
> > CPU interface from the guest which has no business knowing about
> > the v4.1 extension.
> 
> Had a look at the gic-v3 driver, and as far as I can tell it does
> not check that a GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't
> get this wrong, then this patch is to ensure architectural
> compliance for a guest even if the hardware is not necessarily
> compliant, right?

Indeed. Not having this made some of my own tests fail on M1 as they
rely on ID_AA64PFR0_EL1.GIC being correct. I also pondered setting it
to 0 when emulating a GICv2, but that'd be a change in behaviour, and
I want to think a bit more about the effects of that.

> 
> GICv4.1 is an extension to GICv4 (which itself was an extension to
> GICv3) to add support for virtualization features (virtual SGIs), so
> I don't see any harm in hiding it from the guest, since the guest
> cannot virtual SGIs.

Indeed. The guest already has another way to look into this by
checking whether the distributor allows active-less SGIs.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Alexandru Elisei]

Hi Marc,

On 9/29/21 17:04, Marc Zyngier wrote:
> Hi Alex,
>
> On Wed, 29 Sep 2021 16:29:09 +0100,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>> Hi Marc,
>>
>> On 9/24/21 09:25, Marc Zyngier wrote:
>>> Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
>>> visible on the host, even if we were running a GICv2-enabled VM
>>> on a GICv3+compat host.
>>>
>>> That's fine, but we also now have the case of a host that does not
>>> expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
>>> confusing. Thank you M1.
>>>
>>> Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
>>> when a GICv3 is exposed to the guest. This also hides a GICv4.1
>>> CPU interface from the guest which has no business knowing about
>>> the v4.1 extension.
>> Had a look at the gic-v3 driver, and as far as I can tell it does
>> not check that a GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't
>> get this wrong, then this patch is to ensure architectural
>> compliance for a guest even if the hardware is not necessarily
>> compliant, right?
> Indeed. Not having this made some of my own tests fail on M1 as they
> rely on ID_AA64PFR0_EL1.GIC being correct. I also pondered setting it
> to 0 when emulating a GICv2, but that'd be a change in behaviour, and
> I want to think a bit more about the effects of that.
>
>> GICv4.1 is an extension to GICv4 (which itself was an extension to
>> GICv3) to add support for virtualization features (virtual SGIs), so
>> I don't see any harm in hiding it from the guest, since the guest
>> cannot virtual SGIs.
> Indeed. The guest already has another way to look into this by
> checking whether the distributor allows active-less SGIs.

Thank you for the clarification, the patch looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Thanks,
>
> 	M.
>

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Alexandru Elisei]

Hi Marc,

On 9/29/21 17:04, Marc Zyngier wrote:
> Hi Alex,
>
> On Wed, 29 Sep 2021 16:29:09 +0100,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>> Hi Marc,
>>
>> On 9/24/21 09:25, Marc Zyngier wrote:
>>> Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
>>> visible on the host, even if we were running a GICv2-enabled VM
>>> on a GICv3+compat host.
>>>
>>> That's fine, but we also now have the case of a host that does not
>>> expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
>>> confusing. Thank you M1.
>>>
>>> Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
>>> when a GICv3 is exposed to the guest. This also hides a GICv4.1
>>> CPU interface from the guest which has no business knowing about
>>> the v4.1 extension.
>> Had a look at the gic-v3 driver, and as far as I can tell it does
>> not check that a GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't
>> get this wrong, then this patch is to ensure architectural
>> compliance for a guest even if the hardware is not necessarily
>> compliant, right?
> Indeed. Not having this made some of my own tests fail on M1 as they
> rely on ID_AA64PFR0_EL1.GIC being correct. I also pondered setting it
> to 0 when emulating a GICv2, but that'd be a change in behaviour, and
> I want to think a bit more about the effects of that.
>
>> GICv4.1 is an extension to GICv4 (which itself was an extension to
>> GICv3) to add support for virtualization features (virtual SGIs), so
>> I don't see any harm in hiding it from the guest, since the guest
>> cannot virtual SGIs.
> Indeed. The guest already has another way to look into this by
> checking whether the distributor allows active-less SGIs.

Thank you for the clarification, the patch looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Thanks,
>
> 	M.
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] KVM: arm64: Force ID_AA64PFR0_EL1.GIC=1 when exposing a virtual GICv3

[Alexandru Elisei]

Hi Marc,

On 9/29/21 17:04, Marc Zyngier wrote:
> Hi Alex,
>
> On Wed, 29 Sep 2021 16:29:09 +0100,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>> Hi Marc,
>>
>> On 9/24/21 09:25, Marc Zyngier wrote:
>>> Until now, we always let ID_AA64PFR0_EL1.GIC reflect the value
>>> visible on the host, even if we were running a GICv2-enabled VM
>>> on a GICv3+compat host.
>>>
>>> That's fine, but we also now have the case of a host that does not
>>> expose ID_AA64PFR0_EL1.GIC==1 despite having a vGIC. Yes, this is
>>> confusing. Thank you M1.
>>>
>>> Let's go back to first principles and expose ID_AA64PFR0_EL1.GIC=1
>>> when a GICv3 is exposed to the guest. This also hides a GICv4.1
>>> CPU interface from the guest which has no business knowing about
>>> the v4.1 extension.
>> Had a look at the gic-v3 driver, and as far as I can tell it does
>> not check that a GICv3 is advertised in ID_AA64PFR0_EL1. If I didn't
>> get this wrong, then this patch is to ensure architectural
>> compliance for a guest even if the hardware is not necessarily
>> compliant, right?
> Indeed. Not having this made some of my own tests fail on M1 as they
> rely on ID_AA64PFR0_EL1.GIC being correct. I also pondered setting it
> to 0 when emulating a GICv2, but that'd be a change in behaviour, and
> I want to think a bit more about the effects of that.
>
>> GICv4.1 is an extension to GICv4 (which itself was an extension to
>> GICv3) to add support for virtualization features (virtual SGIs), so
>> I don't see any harm in hiding it from the guest, since the guest
>> cannot virtual SGIs.
> Indeed. The guest already has another way to look into this by
> checking whether the distributor allows active-less SGIs.

Thank you for the clarification, the patch looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Thanks,
>
> 	M.
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Marc Zyngier]

The infamous M1 has a feature nobody else ever implemented,
in the form of the "GIC locally generated SError interrupts",
also known as SEIS for short.

These SErrors are generated when a guest does something that violates
the GIC state machine. It would have been simpler to just *ignore*
the damned thing, but that's not what this HW does. Oh well.

This part of of the architecture is also amazingly under-specified.
There is a whole 10 lines that describe the feature in a spec that
is 930 pages long, and some of these lines are factually wrong.
Oh, and it is deprecated, so the insentive to clarify it is low.

Now, the spec says that this should be a *virtual* SError when
HCR_EL2.AMO is set. As it turns out, that's not always the case
on this CPU, and the SError sometimes fires on the host as a
physical SError. Goodbye, cruel world. This clearly is a HW bug,
and it means that a guest can easily take the host down, on demand.

Thankfully, we have seen systems that were just as broken in the
past, and we have the perfect vaccine for it.

Apple M1, please meet the Cavium ThunderX workaround. All your
GIC accesses will be trapped, sanitised, and emulated. Only the
signalling aspect of the HW will be used. It won't be super speedy,
but it will at least be safe. You're most welcome.

Given that this has only ever been seen on this single implementation,
that the spec is unclear at best and that we cannot trust it to ever
be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
being set.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 21a6207fb2ee..ae59e2580bf5 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -671,6 +671,14 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
 		group1_trap = true;
 	}
 
+	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
+		kvm_info("GICv3 with locally generated SEI\n");
+
+		group0_trap = true;
+		group1_trap = true;
+		common_trap = true;
+	}
+
 	if (group0_trap || group1_trap || common_trap) {
 		kvm_info("GICv3 sysreg trapping enabled ([%s%s%s], reduced performance)\n",
 			 group0_trap ? "G0" : "",
-- 
2.30.2

[PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Marc Zyngier]

The infamous M1 has a feature nobody else ever implemented,
in the form of the "GIC locally generated SError interrupts",
also known as SEIS for short.

These SErrors are generated when a guest does something that violates
the GIC state machine. It would have been simpler to just *ignore*
the damned thing, but that's not what this HW does. Oh well.

This part of of the architecture is also amazingly under-specified.
There is a whole 10 lines that describe the feature in a spec that
is 930 pages long, and some of these lines are factually wrong.
Oh, and it is deprecated, so the insentive to clarify it is low.

Now, the spec says that this should be a *virtual* SError when
HCR_EL2.AMO is set. As it turns out, that's not always the case
on this CPU, and the SError sometimes fires on the host as a
physical SError. Goodbye, cruel world. This clearly is a HW bug,
and it means that a guest can easily take the host down, on demand.

Thankfully, we have seen systems that were just as broken in the
past, and we have the perfect vaccine for it.

Apple M1, please meet the Cavium ThunderX workaround. All your
GIC accesses will be trapped, sanitised, and emulated. Only the
signalling aspect of the HW will be used. It won't be super speedy,
but it will at least be safe. You're most welcome.

Given that this has only ever been seen on this single implementation,
that the spec is unclear at best and that we cannot trust it to ever
be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
being set.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 21a6207fb2ee..ae59e2580bf5 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -671,6 +671,14 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
 		group1_trap = true;
 	}
 
+	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
+		kvm_info("GICv3 with locally generated SEI\n");
+
+		group0_trap = true;
+		group1_trap = true;
+		common_trap = true;
+	}
+
 	if (group0_trap || group1_trap || common_trap) {
 		kvm_info("GICv3 sysreg trapping enabled ([%s%s%s], reduced performance)\n",
 			 group0_trap ? "G0" : "",
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Marc Zyngier]

The infamous M1 has a feature nobody else ever implemented,
in the form of the "GIC locally generated SError interrupts",
also known as SEIS for short.

These SErrors are generated when a guest does something that violates
the GIC state machine. It would have been simpler to just *ignore*
the damned thing, but that's not what this HW does. Oh well.

This part of of the architecture is also amazingly under-specified.
There is a whole 10 lines that describe the feature in a spec that
is 930 pages long, and some of these lines are factually wrong.
Oh, and it is deprecated, so the insentive to clarify it is low.

Now, the spec says that this should be a *virtual* SError when
HCR_EL2.AMO is set. As it turns out, that's not always the case
on this CPU, and the SError sometimes fires on the host as a
physical SError. Goodbye, cruel world. This clearly is a HW bug,
and it means that a guest can easily take the host down, on demand.

Thankfully, we have seen systems that were just as broken in the
past, and we have the perfect vaccine for it.

Apple M1, please meet the Cavium ThunderX workaround. All your
GIC accesses will be trapped, sanitised, and emulated. Only the
signalling aspect of the HW will be used. It won't be super speedy,
but it will at least be safe. You're most welcome.

Given that this has only ever been seen on this single implementation,
that the spec is unclear at best and that we cannot trust it to ever
be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
being set.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 21a6207fb2ee..ae59e2580bf5 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -671,6 +671,14 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
 		group1_trap = true;
 	}
 
+	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
+		kvm_info("GICv3 with locally generated SEI\n");
+
+		group0_trap = true;
+		group1_trap = true;
+		common_trap = true;
+	}
+
 	if (group0_trap || group1_trap || common_trap) {
 		kvm_info("GICv3 sysreg trapping enabled ([%s%s%s], reduced performance)\n",
 			 group0_trap ? "G0" : "",
-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Joey Gouly]

Hi Marc,

On Fri, Sep 24, 2021 at 09:25:39AM +0100, Marc Zyngier wrote:
> The infamous M1 has a feature nobody else ever implemented,
> in the form of the "GIC locally generated SError interrupts",
> also known as SEIS for short.
> 
> These SErrors are generated when a guest does something that violates
> the GIC state machine. It would have been simpler to just *ignore*
> the damned thing, but that's not what this HW does. Oh well.
> 
> This part of of the architecture is also amazingly under-specified.
> There is a whole 10 lines that describe the feature in a spec that
> is 930 pages long, and some of these lines are factually wrong.
> Oh, and it is deprecated, so the insentive to clarify it is low.
> 
> Now, the spec says that this should be a *virtual* SError when
> HCR_EL2.AMO is set. As it turns out, that's not always the case
> on this CPU, and the SError sometimes fires on the host as a
> physical SError. Goodbye, cruel world. This clearly is a HW bug,
> and it means that a guest can easily take the host down, on demand.
> 
> Thankfully, we have seen systems that were just as broken in the
> past, and we have the perfect vaccine for it.
> 
> Apple M1, please meet the Cavium ThunderX workaround. All your
> GIC accesses will be trapped, sanitised, and emulated. Only the
> signalling aspect of the HW will be used. It won't be super speedy,
> but it will at least be safe. You're most welcome.
> 
> Given that this has only ever been seen on this single implementation,
> that the spec is unclear at best and that we cannot trust it to ever
> be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> being set.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

[...]

I reproduced this issue on my M1 by using kvmtool and EDKII [1], and
have confirmed that this fixes it.

Tested-by: Joey Gouly <joey.gouly@arm.com>

Thanks,
Joey

[1] It is fixed in EDKII now, but I reverted Ard's EDKII commit locally:
  a82bad9730178a1e3a67c9bfc83412b87a8ad734

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Joey Gouly]

Hi Marc,

On Fri, Sep 24, 2021 at 09:25:39AM +0100, Marc Zyngier wrote:
> The infamous M1 has a feature nobody else ever implemented,
> in the form of the "GIC locally generated SError interrupts",
> also known as SEIS for short.
> 
> These SErrors are generated when a guest does something that violates
> the GIC state machine. It would have been simpler to just *ignore*
> the damned thing, but that's not what this HW does. Oh well.
> 
> This part of of the architecture is also amazingly under-specified.
> There is a whole 10 lines that describe the feature in a spec that
> is 930 pages long, and some of these lines are factually wrong.
> Oh, and it is deprecated, so the insentive to clarify it is low.
> 
> Now, the spec says that this should be a *virtual* SError when
> HCR_EL2.AMO is set. As it turns out, that's not always the case
> on this CPU, and the SError sometimes fires on the host as a
> physical SError. Goodbye, cruel world. This clearly is a HW bug,
> and it means that a guest can easily take the host down, on demand.
> 
> Thankfully, we have seen systems that were just as broken in the
> past, and we have the perfect vaccine for it.
> 
> Apple M1, please meet the Cavium ThunderX workaround. All your
> GIC accesses will be trapped, sanitised, and emulated. Only the
> signalling aspect of the HW will be used. It won't be super speedy,
> but it will at least be safe. You're most welcome.
> 
> Given that this has only ever been seen on this single implementation,
> that the spec is unclear at best and that we cannot trust it to ever
> be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> being set.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

[...]

I reproduced this issue on my M1 by using kvmtool and EDKII [1], and
have confirmed that this fixes it.

Tested-by: Joey Gouly <joey.gouly@arm.com>

Thanks,
Joey

[1] It is fixed in EDKII now, but I reverted Ard's EDKII commit locally:
  a82bad9730178a1e3a67c9bfc83412b87a8ad734
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Joey Gouly]

Hi Marc,

On Fri, Sep 24, 2021 at 09:25:39AM +0100, Marc Zyngier wrote:
> The infamous M1 has a feature nobody else ever implemented,
> in the form of the "GIC locally generated SError interrupts",
> also known as SEIS for short.
> 
> These SErrors are generated when a guest does something that violates
> the GIC state machine. It would have been simpler to just *ignore*
> the damned thing, but that's not what this HW does. Oh well.
> 
> This part of of the architecture is also amazingly under-specified.
> There is a whole 10 lines that describe the feature in a spec that
> is 930 pages long, and some of these lines are factually wrong.
> Oh, and it is deprecated, so the insentive to clarify it is low.
> 
> Now, the spec says that this should be a *virtual* SError when
> HCR_EL2.AMO is set. As it turns out, that's not always the case
> on this CPU, and the SError sometimes fires on the host as a
> physical SError. Goodbye, cruel world. This clearly is a HW bug,
> and it means that a guest can easily take the host down, on demand.
> 
> Thankfully, we have seen systems that were just as broken in the
> past, and we have the perfect vaccine for it.
> 
> Apple M1, please meet the Cavium ThunderX workaround. All your
> GIC accesses will be trapped, sanitised, and emulated. Only the
> signalling aspect of the HW will be used. It won't be super speedy,
> but it will at least be safe. You're most welcome.
> 
> Given that this has only ever been seen on this single implementation,
> that the spec is unclear at best and that we cannot trust it to ever
> be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> being set.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

[...]

I reproduced this issue on my M1 by using kvmtool and EDKII [1], and
have confirmed that this fixes it.

Tested-by: Joey Gouly <joey.gouly@arm.com>

Thanks,
Joey

[1] It is fixed in EDKII now, but I reverted Ard's EDKII commit locally:
  a82bad9730178a1e3a67c9bfc83412b87a8ad734

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> The infamous M1 has a feature nobody else ever implemented,
> in the form of the "GIC locally generated SError interrupts",
> also known as SEIS for short.
>
> These SErrors are generated when a guest does something that violates
> the GIC state machine. It would have been simpler to just *ignore*
> the damned thing, but that's not what this HW does. Oh well.
>
> This part of of the architecture is also amazingly under-specified.
> There is a whole 10 lines that describe the feature in a spec that
> is 930 pages long, and some of these lines are factually wrong.
> Oh, and it is deprecated, so the insentive to clarify it is low.
>
> Now, the spec says that this should be a *virtual* SError when
> HCR_EL2.AMO is set. As it turns out, that's not always the case
> on this CPU, and the SError sometimes fires on the host as a
> physical SError. Goodbye, cruel world. This clearly is a HW bug,
> and it means that a guest can easily take the host down, on demand.
>
> Thankfully, we have seen systems that were just as broken in the
> past, and we have the perfect vaccine for it.
>
> Apple M1, please meet the Cavium ThunderX workaround. All your
> GIC accesses will be trapped, sanitised, and emulated. Only the
> signalling aspect of the HW will be used. It won't be super speedy,
> but it will at least be safe. You're most welcome.
>
> Given that this has only ever been seen on this single implementation,
> that the spec is unclear at best and that we cannot trust it to ever
> be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> being set.

I grepped for system error in Arm IHI 0069F, and turns out there's a number of
ways to make the GIC generate one:

- When programming the ITS

- On a write to ICC_DIR_EL1 (or the corresponding virtual CPU interface register)
with split priority drop/interrupt deactivation is not enabled.

- On a write to GICV_AEOIR or GICC_DIR.

ITS and the legacy GICv2 interface is memory mapped, so I am going to trust that
KVM emulates that correctly and avoids putting the GIC into a state that triggers
the SErrors.

The CPU interface registers are accessed directly by the guest, then changing that
to trap-and-emulate looks like the only way to avoid the guest from crashing the
host with an SError.

As for making the trap-and-emulate depend on the ICH_VTR_EL2.SEIS being set, that
sounds reasonable to me, considering that there were no reports so far of this
being implemented. And if it turns out that there are device which implement GIC
generated SErrors *correctly* and the trap-and-emulate cost is too much, then we
can always get an errata number from Apple and have the trapping depend on that,
right?

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 21a6207fb2ee..ae59e2580bf5 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -671,6 +671,14 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>  		group1_trap = true;
>  	}
>  
> +	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
> +		kvm_info("GICv3 with locally generated SEI\n");
> +
> +		group0_trap = true;
> +		group1_trap = true;
> +		common_trap = true;
> +	}
> +
>  	if (group0_trap || group1_trap || common_trap) {
>  		kvm_info("GICv3 sysreg trapping enabled ([%s%s%s], reduced performance)\n",
>  			 group0_trap ? "G0" : "",

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> The infamous M1 has a feature nobody else ever implemented,
> in the form of the "GIC locally generated SError interrupts",
> also known as SEIS for short.
>
> These SErrors are generated when a guest does something that violates
> the GIC state machine. It would have been simpler to just *ignore*
> the damned thing, but that's not what this HW does. Oh well.
>
> This part of of the architecture is also amazingly under-specified.
> There is a whole 10 lines that describe the feature in a spec that
> is 930 pages long, and some of these lines are factually wrong.
> Oh, and it is deprecated, so the insentive to clarify it is low.
>
> Now, the spec says that this should be a *virtual* SError when
> HCR_EL2.AMO is set. As it turns out, that's not always the case
> on this CPU, and the SError sometimes fires on the host as a
> physical SError. Goodbye, cruel world. This clearly is a HW bug,
> and it means that a guest can easily take the host down, on demand.
>
> Thankfully, we have seen systems that were just as broken in the
> past, and we have the perfect vaccine for it.
>
> Apple M1, please meet the Cavium ThunderX workaround. All your
> GIC accesses will be trapped, sanitised, and emulated. Only the
> signalling aspect of the HW will be used. It won't be super speedy,
> but it will at least be safe. You're most welcome.
>
> Given that this has only ever been seen on this single implementation,
> that the spec is unclear at best and that we cannot trust it to ever
> be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> being set.

I grepped for system error in Arm IHI 0069F, and turns out there's a number of
ways to make the GIC generate one:

- When programming the ITS

- On a write to ICC_DIR_EL1 (or the corresponding virtual CPU interface register)
with split priority drop/interrupt deactivation is not enabled.

- On a write to GICV_AEOIR or GICC_DIR.

ITS and the legacy GICv2 interface is memory mapped, so I am going to trust that
KVM emulates that correctly and avoids putting the GIC into a state that triggers
the SErrors.

The CPU interface registers are accessed directly by the guest, then changing that
to trap-and-emulate looks like the only way to avoid the guest from crashing the
host with an SError.

As for making the trap-and-emulate depend on the ICH_VTR_EL2.SEIS being set, that
sounds reasonable to me, considering that there were no reports so far of this
being implemented. And if it turns out that there are device which implement GIC
generated SErrors *correctly* and the trap-and-emulate cost is too much, then we
can always get an errata number from Apple and have the trapping depend on that,
right?

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 21a6207fb2ee..ae59e2580bf5 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -671,6 +671,14 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>  		group1_trap = true;
>  	}
>  
> +	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
> +		kvm_info("GICv3 with locally generated SEI\n");
> +
> +		group0_trap = true;
> +		group1_trap = true;
> +		common_trap = true;
> +	}
> +
>  	if (group0_trap || group1_trap || common_trap) {
>  		kvm_info("GICv3 sysreg trapping enabled ([%s%s%s], reduced performance)\n",
>  			 group0_trap ? "G0" : "",

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> The infamous M1 has a feature nobody else ever implemented,
> in the form of the "GIC locally generated SError interrupts",
> also known as SEIS for short.
>
> These SErrors are generated when a guest does something that violates
> the GIC state machine. It would have been simpler to just *ignore*
> the damned thing, but that's not what this HW does. Oh well.
>
> This part of of the architecture is also amazingly under-specified.
> There is a whole 10 lines that describe the feature in a spec that
> is 930 pages long, and some of these lines are factually wrong.
> Oh, and it is deprecated, so the insentive to clarify it is low.
>
> Now, the spec says that this should be a *virtual* SError when
> HCR_EL2.AMO is set. As it turns out, that's not always the case
> on this CPU, and the SError sometimes fires on the host as a
> physical SError. Goodbye, cruel world. This clearly is a HW bug,
> and it means that a guest can easily take the host down, on demand.
>
> Thankfully, we have seen systems that were just as broken in the
> past, and we have the perfect vaccine for it.
>
> Apple M1, please meet the Cavium ThunderX workaround. All your
> GIC accesses will be trapped, sanitised, and emulated. Only the
> signalling aspect of the HW will be used. It won't be super speedy,
> but it will at least be safe. You're most welcome.
>
> Given that this has only ever been seen on this single implementation,
> that the spec is unclear at best and that we cannot trust it to ever
> be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> being set.

I grepped for system error in Arm IHI 0069F, and turns out there's a number of
ways to make the GIC generate one:

- When programming the ITS

- On a write to ICC_DIR_EL1 (or the corresponding virtual CPU interface register)
with split priority drop/interrupt deactivation is not enabled.

- On a write to GICV_AEOIR or GICC_DIR.

ITS and the legacy GICv2 interface is memory mapped, so I am going to trust that
KVM emulates that correctly and avoids putting the GIC into a state that triggers
the SErrors.

The CPU interface registers are accessed directly by the guest, then changing that
to trap-and-emulate looks like the only way to avoid the guest from crashing the
host with an SError.

As for making the trap-and-emulate depend on the ICH_VTR_EL2.SEIS being set, that
sounds reasonable to me, considering that there were no reports so far of this
being implemented. And if it turns out that there are device which implement GIC
generated SErrors *correctly* and the trap-and-emulate cost is too much, then we
can always get an errata number from Apple and have the trapping depend on that,
right?

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 21a6207fb2ee..ae59e2580bf5 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -671,6 +671,14 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>  		group1_trap = true;
>  	}
>  
> +	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
> +		kvm_info("GICv3 with locally generated SEI\n");
> +
> +		group0_trap = true;
> +		group1_trap = true;
> +		common_trap = true;
> +	}
> +
>  	if (group0_trap || group1_trap || common_trap) {
>  		kvm_info("GICv3 sysreg trapping enabled ([%s%s%s], reduced performance)\n",
>  			 group0_trap ? "G0" : "",
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Marc Zyngier]

Hi Alex,

On Mon, 04 Oct 2021 12:23:41 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 9/24/21 09:25, Marc Zyngier wrote:
> > The infamous M1 has a feature nobody else ever implemented,
> > in the form of the "GIC locally generated SError interrupts",
> > also known as SEIS for short.
> >
> > These SErrors are generated when a guest does something that violates
> > the GIC state machine. It would have been simpler to just *ignore*
> > the damned thing, but that's not what this HW does. Oh well.
> >
> > This part of of the architecture is also amazingly under-specified.
> > There is a whole 10 lines that describe the feature in a spec that
> > is 930 pages long, and some of these lines are factually wrong.
> > Oh, and it is deprecated, so the insentive to clarify it is low.
> >
> > Now, the spec says that this should be a *virtual* SError when
> > HCR_EL2.AMO is set. As it turns out, that's not always the case
> > on this CPU, and the SError sometimes fires on the host as a
> > physical SError. Goodbye, cruel world. This clearly is a HW bug,
> > and it means that a guest can easily take the host down, on demand.
> >
> > Thankfully, we have seen systems that were just as broken in the
> > past, and we have the perfect vaccine for it.
> >
> > Apple M1, please meet the Cavium ThunderX workaround. All your
> > GIC accesses will be trapped, sanitised, and emulated. Only the
> > signalling aspect of the HW will be used. It won't be super speedy,
> > but it will at least be safe. You're most welcome.
> >
> > Given that this has only ever been seen on this single implementation,
> > that the spec is unclear at best and that we cannot trust it to ever
> > be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> > being set.
> 
> I grepped for system error in Arm IHI 0069F, and turns out there's a number of
> ways to make the GIC generate one:
> 
> - When programming the ITS
> 
> - On a write to ICC_DIR_EL1 (or the corresponding virtual CPU interface register)
> with split priority drop/interrupt deactivation is not enabled.
> 
> - On a write to GICV_AEOIR or GICC_DIR.
> 
> ITS and the legacy GICv2 interface is memory mapped, so I am going
> to trust that KVM emulates that correctly and avoids putting the GIC
> into a state that triggers the SErrors.

And to be clear, if the host kernel was doing the wrong thing, it
would take a *physical* SError. And on the M1, it really doesn't
matter as there is no physical GIC.

> The CPU interface registers are accessed directly by the guest, then
> changing that to trap-and-emulate looks like the only way to avoid
> the guest from crashing the host with an SError.
> 
> As for making the trap-and-emulate depend on the ICH_VTR_EL2.SEIS
> being set, that sounds reasonable to me, considering that there were
> no reports so far of this being implemented. And if it turns out
> that there are device which implement GIC generated SErrors
> *correctly* and the trap-and-emulate cost is too much, then we can
> always get an errata number from Apple and have the trapping depend
> on that, right?

I have very little hope that we can get Apple to give us anything
here. The CPU doesn't even advertise that it has a vGIC, so we're in
uncharted territories. But we could definitely key that on the MIDR.

> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks!

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Marc Zyngier]

Hi Alex,

On Mon, 04 Oct 2021 12:23:41 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 9/24/21 09:25, Marc Zyngier wrote:
> > The infamous M1 has a feature nobody else ever implemented,
> > in the form of the "GIC locally generated SError interrupts",
> > also known as SEIS for short.
> >
> > These SErrors are generated when a guest does something that violates
> > the GIC state machine. It would have been simpler to just *ignore*
> > the damned thing, but that's not what this HW does. Oh well.
> >
> > This part of of the architecture is also amazingly under-specified.
> > There is a whole 10 lines that describe the feature in a spec that
> > is 930 pages long, and some of these lines are factually wrong.
> > Oh, and it is deprecated, so the insentive to clarify it is low.
> >
> > Now, the spec says that this should be a *virtual* SError when
> > HCR_EL2.AMO is set. As it turns out, that's not always the case
> > on this CPU, and the SError sometimes fires on the host as a
> > physical SError. Goodbye, cruel world. This clearly is a HW bug,
> > and it means that a guest can easily take the host down, on demand.
> >
> > Thankfully, we have seen systems that were just as broken in the
> > past, and we have the perfect vaccine for it.
> >
> > Apple M1, please meet the Cavium ThunderX workaround. All your
> > GIC accesses will be trapped, sanitised, and emulated. Only the
> > signalling aspect of the HW will be used. It won't be super speedy,
> > but it will at least be safe. You're most welcome.
> >
> > Given that this has only ever been seen on this single implementation,
> > that the spec is unclear at best and that we cannot trust it to ever
> > be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> > being set.
> 
> I grepped for system error in Arm IHI 0069F, and turns out there's a number of
> ways to make the GIC generate one:
> 
> - When programming the ITS
> 
> - On a write to ICC_DIR_EL1 (or the corresponding virtual CPU interface register)
> with split priority drop/interrupt deactivation is not enabled.
> 
> - On a write to GICV_AEOIR or GICC_DIR.
> 
> ITS and the legacy GICv2 interface is memory mapped, so I am going
> to trust that KVM emulates that correctly and avoids putting the GIC
> into a state that triggers the SErrors.

And to be clear, if the host kernel was doing the wrong thing, it
would take a *physical* SError. And on the M1, it really doesn't
matter as there is no physical GIC.

> The CPU interface registers are accessed directly by the guest, then
> changing that to trap-and-emulate looks like the only way to avoid
> the guest from crashing the host with an SError.
> 
> As for making the trap-and-emulate depend on the ICH_VTR_EL2.SEIS
> being set, that sounds reasonable to me, considering that there were
> no reports so far of this being implemented. And if it turns out
> that there are device which implement GIC generated SErrors
> *correctly* and the trap-and-emulate cost is too much, then we can
> always get an errata number from Apple and have the trapping depend
> on that, right?

I have very little hope that we can get Apple to give us anything
here. The CPU doesn't even advertise that it has a vGIC, so we're in
uncharted territories. But we could definitely key that on the MIDR.

> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks!

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 2/5] KVM: arm64: Work around GICv3 locally generated SErrors

[Marc Zyngier]

Hi Alex,

On Mon, 04 Oct 2021 12:23:41 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On 9/24/21 09:25, Marc Zyngier wrote:
> > The infamous M1 has a feature nobody else ever implemented,
> > in the form of the "GIC locally generated SError interrupts",
> > also known as SEIS for short.
> >
> > These SErrors are generated when a guest does something that violates
> > the GIC state machine. It would have been simpler to just *ignore*
> > the damned thing, but that's not what this HW does. Oh well.
> >
> > This part of of the architecture is also amazingly under-specified.
> > There is a whole 10 lines that describe the feature in a spec that
> > is 930 pages long, and some of these lines are factually wrong.
> > Oh, and it is deprecated, so the insentive to clarify it is low.
> >
> > Now, the spec says that this should be a *virtual* SError when
> > HCR_EL2.AMO is set. As it turns out, that's not always the case
> > on this CPU, and the SError sometimes fires on the host as a
> > physical SError. Goodbye, cruel world. This clearly is a HW bug,
> > and it means that a guest can easily take the host down, on demand.
> >
> > Thankfully, we have seen systems that were just as broken in the
> > past, and we have the perfect vaccine for it.
> >
> > Apple M1, please meet the Cavium ThunderX workaround. All your
> > GIC accesses will be trapped, sanitised, and emulated. Only the
> > signalling aspect of the HW will be used. It won't be super speedy,
> > but it will at least be safe. You're most welcome.
> >
> > Given that this has only ever been seen on this single implementation,
> > that the spec is unclear at best and that we cannot trust it to ever
> > be implemented correctly, gate the workaround solely on ICH_VTR_EL2.SEIS
> > being set.
> 
> I grepped for system error in Arm IHI 0069F, and turns out there's a number of
> ways to make the GIC generate one:
> 
> - When programming the ITS
> 
> - On a write to ICC_DIR_EL1 (or the corresponding virtual CPU interface register)
> with split priority drop/interrupt deactivation is not enabled.
> 
> - On a write to GICV_AEOIR or GICC_DIR.
> 
> ITS and the legacy GICv2 interface is memory mapped, so I am going
> to trust that KVM emulates that correctly and avoids putting the GIC
> into a state that triggers the SErrors.

And to be clear, if the host kernel was doing the wrong thing, it
would take a *physical* SError. And on the M1, it really doesn't
matter as there is no physical GIC.

> The CPU interface registers are accessed directly by the guest, then
> changing that to trap-and-emulate looks like the only way to avoid
> the guest from crashing the host with an SError.
> 
> As for making the trap-and-emulate depend on the ICH_VTR_EL2.SEIS
> being set, that sounds reasonable to me, considering that there were
> no reports so far of this being implemented. And if it turns out
> that there are device which implement GIC generated SErrors
> *correctly* and the trap-and-emulate cost is too much, then we can
> always get an errata number from Apple and have the trapping depend
> on that, right?

I have very little hope that we can get Apple to give us anything
here. The CPU doesn't even advertise that it has a vGIC, so we're in
uncharted territories. But we could definitely key that on the MIDR.

> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks!

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS

[Marc Zyngier]

Since we are trapping all sysreg accesses when ICH_VTR_EL2.SEIS
is set, and that we never deliver an SError when emulating
any of the GICv3 sysregs, don't advertise ICC_CTLR_EL1.SEIS.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 39f8f7f9227c..b3b50de496a3 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -987,8 +987,6 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
 	/* IDbits */
 	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
-	/* SEIS */
-	val |= ((vtr >> 22) & 1) << ICC_CTLR_EL1_SEIS_SHIFT;
 	/* A3V */
 	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
 	/* EOImode */
-- 
2.30.2

[PATCH 3/5] KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS

[Marc Zyngier]

Since we are trapping all sysreg accesses when ICH_VTR_EL2.SEIS
is set, and that we never deliver an SError when emulating
any of the GICv3 sysregs, don't advertise ICC_CTLR_EL1.SEIS.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 39f8f7f9227c..b3b50de496a3 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -987,8 +987,6 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
 	/* IDbits */
 	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
-	/* SEIS */
-	val |= ((vtr >> 22) & 1) << ICC_CTLR_EL1_SEIS_SHIFT;
 	/* A3V */
 	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
 	/* EOImode */
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS

[Marc Zyngier]

Since we are trapping all sysreg accesses when ICH_VTR_EL2.SEIS
is set, and that we never deliver an SError when emulating
any of the GICv3 sysregs, don't advertise ICC_CTLR_EL1.SEIS.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 39f8f7f9227c..b3b50de496a3 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -987,8 +987,6 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
 	/* IDbits */
 	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
-	/* SEIS */
-	val |= ((vtr >> 22) & 1) << ICC_CTLR_EL1_SEIS_SHIFT;
 	/* A3V */
 	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
 	/* EOImode */
-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 3/5] KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> Since we are trapping all sysreg accesses when ICH_VTR_EL2.SEIS
> is set, and that we never deliver an SError when emulating
> any of the GICv3 sysregs, don't advertise ICC_CTLR_EL1.SEIS.

Makes sense, we don't emulate it, so don't advertise it. Checked
__vgic_v3_write_ctlr(), and we only allow the guest to modify EOI mode and which
register is responsible for determining the binary point for the interrupt priority.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index 39f8f7f9227c..b3b50de496a3 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -987,8 +987,6 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
>  	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
>  	/* IDbits */
>  	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
> -	/* SEIS */
> -	val |= ((vtr >> 22) & 1) << ICC_CTLR_EL1_SEIS_SHIFT;
>  	/* A3V */
>  	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
>  	/* EOImode */

Re: [PATCH 3/5] KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> Since we are trapping all sysreg accesses when ICH_VTR_EL2.SEIS
> is set, and that we never deliver an SError when emulating
> any of the GICv3 sysregs, don't advertise ICC_CTLR_EL1.SEIS.

Makes sense, we don't emulate it, so don't advertise it. Checked
__vgic_v3_write_ctlr(), and we only allow the guest to modify EOI mode and which
register is responsible for determining the binary point for the interrupt priority.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index 39f8f7f9227c..b3b50de496a3 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -987,8 +987,6 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
>  	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
>  	/* IDbits */
>  	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
> -	/* SEIS */
> -	val |= ((vtr >> 22) & 1) << ICC_CTLR_EL1_SEIS_SHIFT;
>  	/* A3V */
>  	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
>  	/* EOImode */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] KVM: arm64: vgic-v3: Don't advertise ICC_CTLR_EL1.SEIS

[Alexandru Elisei]

Hi Marc,

On 9/24/21 09:25, Marc Zyngier wrote:
> Since we are trapping all sysreg accesses when ICH_VTR_EL2.SEIS
> is set, and that we never deliver an SError when emulating
> any of the GICv3 sysregs, don't advertise ICC_CTLR_EL1.SEIS.

Makes sense, we don't emulate it, so don't advertise it. Checked
__vgic_v3_write_ctlr(), and we only allow the guest to modify EOI mode and which
register is responsible for determining the binary point for the interrupt priority.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index 39f8f7f9227c..b3b50de496a3 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -987,8 +987,6 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
>  	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
>  	/* IDbits */
>  	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
> -	/* SEIS */
> -	val |= ((vtr >> 22) & 1) << ICC_CTLR_EL1_SEIS_SHIFT;
>  	/* A3V */
>  	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
>  	/* EOImode */
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 4/5] KVM: arm64: vgic-v3: Don't propagate LPI active state from LRs into the distributor

[Marc Zyngier]

Christoffer reported that while LPIs to not have an active state,
the pseudocode for the vGIC clearly indicates that LPIs injected
in the LRs do transition via an active state just like any other
interrupt, and that it is only at the priority drop stage that
the active state gets cleared. This is probably done for the sake
of simplicity in the HW, and to trip every single SW developer.

So as it turns out, we can observe an active LPI if the guest
exits between the read of IAR and the write to EOI. This isn't a
big deal and nothing breaks (the active LPI is made inactive on
the next EOI).

However, this active LPI will occupy a LR at the next entry, which
is pointless. We could instead ignore this active state and keep
the distributor blissfully unaware of this oddity. Just do that.

Reported-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index ae59e2580bf5..d281c6a533ee 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -69,9 +69,10 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 
 		raw_spin_lock(&irq->irq_lock);
 
-		/* Always preserve the active bit, note deactivation */
+		/* Preserve the active bit for non-LPI, note deactivation */
 		deactivated = irq->active && !(val & ICH_LR_ACTIVE_BIT);
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
+		irq->active &= irq->intid <= VGIC_MAX_SPI;
 
 		if (irq->active && is_v2_sgi)
 			irq->active_source = cpuid;
-- 
2.30.2

[PATCH 4/5] KVM: arm64: vgic-v3: Don't propagate LPI active state from LRs into the distributor

[Marc Zyngier]

Christoffer reported that while LPIs to not have an active state,
the pseudocode for the vGIC clearly indicates that LPIs injected
in the LRs do transition via an active state just like any other
interrupt, and that it is only at the priority drop stage that
the active state gets cleared. This is probably done for the sake
of simplicity in the HW, and to trip every single SW developer.

So as it turns out, we can observe an active LPI if the guest
exits between the read of IAR and the write to EOI. This isn't a
big deal and nothing breaks (the active LPI is made inactive on
the next EOI).

However, this active LPI will occupy a LR at the next entry, which
is pointless. We could instead ignore this active state and keep
the distributor blissfully unaware of this oddity. Just do that.

Reported-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index ae59e2580bf5..d281c6a533ee 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -69,9 +69,10 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 
 		raw_spin_lock(&irq->irq_lock);
 
-		/* Always preserve the active bit, note deactivation */
+		/* Preserve the active bit for non-LPI, note deactivation */
 		deactivated = irq->active && !(val & ICH_LR_ACTIVE_BIT);
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
+		irq->active &= irq->intid <= VGIC_MAX_SPI;
 
 		if (irq->active && is_v2_sgi)
 			irq->active_source = cpuid;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] KVM: arm64: vgic-v3: Don't propagate LPI active state from LRs into the distributor

[Marc Zyngier]

Christoffer reported that while LPIs to not have an active state,
the pseudocode for the vGIC clearly indicates that LPIs injected
in the LRs do transition via an active state just like any other
interrupt, and that it is only at the priority drop stage that
the active state gets cleared. This is probably done for the sake
of simplicity in the HW, and to trip every single SW developer.

So as it turns out, we can observe an active LPI if the guest
exits between the read of IAR and the write to EOI. This isn't a
big deal and nothing breaks (the active LPI is made inactive on
the next EOI).

However, this active LPI will occupy a LR at the next entry, which
is pointless. We could instead ignore this active state and keep
the distributor blissfully unaware of this oddity. Just do that.

Reported-by: Christoffer Dall <christoffer.dall@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index ae59e2580bf5..d281c6a533ee 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -69,9 +69,10 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 
 		raw_spin_lock(&irq->irq_lock);
 
-		/* Always preserve the active bit, note deactivation */
+		/* Preserve the active bit for non-LPI, note deactivation */
 		deactivated = irq->active && !(val & ICH_LR_ACTIVE_BIT);
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
+		irq->active &= irq->intid <= VGIC_MAX_SPI;
 
 		if (irq->active && is_v2_sgi)
 			irq->active_source = cpuid;
-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 5/5] KVM: arm64: vgic-v3: Align emulated cpuif LPI state machine with the pseudocode

[Marc Zyngier]

Having realised that a virtual LPI does transition through an active
state that does not exist on bare metal, align the CPU interface
emulation with the behaviour specified in the architecture pseudocode.

The LPIs now transition to active on IAR read, and to inactive on
EOI write. Special care is taken not to increment the EOIcount for
an LPI that isn't present in the LRs.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index b3b50de496a3..20db2f281cf2 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -695,9 +695,7 @@ static void __vgic_v3_read_iar(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 		goto spurious;
 
 	lr_val &= ~ICH_LR_STATE;
-	/* No active state for LPIs */
-	if ((lr_val & ICH_LR_VIRTUAL_ID_MASK) <= VGIC_MAX_SPI)
-		lr_val |= ICH_LR_ACTIVE_BIT;
+	lr_val |= ICH_LR_ACTIVE_BIT;
 	__gic_v3_set_lr(lr_val, lr);
 	__vgic_v3_set_active_priority(lr_prio, vmcr, grp);
 	vcpu_set_reg(vcpu, rt, lr_val & ICH_LR_VIRTUAL_ID_MASK);
@@ -764,20 +762,18 @@ static void __vgic_v3_write_eoir(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	/* Drop priority in any case */
 	act_prio = __vgic_v3_clear_highest_active_priority();
 
-	/* If EOIing an LPI, no deactivate to be performed */
-	if (vid >= VGIC_MIN_LPI)
-		return;
-
-	/* EOImode == 1, nothing to be done here */
-	if (vmcr & ICH_VMCR_EOIM_MASK)
-		return;
-
 	lr = __vgic_v3_find_active_lr(vcpu, vid, &lr_val);
 	if (lr == -1) {
-		__vgic_v3_bump_eoicount();
+		/* Do not bump EOIcount for LPIs that aren't in the LRs */
+		if (!(vid >= VGIC_MIN_LPI))
+			__vgic_v3_bump_eoicount();
 		return;
 	}
 
+	/* EOImode == 1 and not an LPI, nothing to be done here */
+	if ((vmcr & ICH_VMCR_EOIM_MASK) && !(vid >= VGIC_MIN_LPI))
+		return;
+
 	lr_prio = (lr_val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
 
 	/* If priorities or group do not match, the guest has fscked-up. */
-- 
2.30.2

[PATCH 5/5] KVM: arm64: vgic-v3: Align emulated cpuif LPI state machine with the pseudocode

[Marc Zyngier]

Having realised that a virtual LPI does transition through an active
state that does not exist on bare metal, align the CPU interface
emulation with the behaviour specified in the architecture pseudocode.

The LPIs now transition to active on IAR read, and to inactive on
EOI write. Special care is taken not to increment the EOIcount for
an LPI that isn't present in the LRs.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index b3b50de496a3..20db2f281cf2 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -695,9 +695,7 @@ static void __vgic_v3_read_iar(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 		goto spurious;
 
 	lr_val &= ~ICH_LR_STATE;
-	/* No active state for LPIs */
-	if ((lr_val & ICH_LR_VIRTUAL_ID_MASK) <= VGIC_MAX_SPI)
-		lr_val |= ICH_LR_ACTIVE_BIT;
+	lr_val |= ICH_LR_ACTIVE_BIT;
 	__gic_v3_set_lr(lr_val, lr);
 	__vgic_v3_set_active_priority(lr_prio, vmcr, grp);
 	vcpu_set_reg(vcpu, rt, lr_val & ICH_LR_VIRTUAL_ID_MASK);
@@ -764,20 +762,18 @@ static void __vgic_v3_write_eoir(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	/* Drop priority in any case */
 	act_prio = __vgic_v3_clear_highest_active_priority();
 
-	/* If EOIing an LPI, no deactivate to be performed */
-	if (vid >= VGIC_MIN_LPI)
-		return;
-
-	/* EOImode == 1, nothing to be done here */
-	if (vmcr & ICH_VMCR_EOIM_MASK)
-		return;
-
 	lr = __vgic_v3_find_active_lr(vcpu, vid, &lr_val);
 	if (lr == -1) {
-		__vgic_v3_bump_eoicount();
+		/* Do not bump EOIcount for LPIs that aren't in the LRs */
+		if (!(vid >= VGIC_MIN_LPI))
+			__vgic_v3_bump_eoicount();
 		return;
 	}
 
+	/* EOImode == 1 and not an LPI, nothing to be done here */
+	if ((vmcr & ICH_VMCR_EOIM_MASK) && !(vid >= VGIC_MIN_LPI))
+		return;
+
 	lr_prio = (lr_val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
 
 	/* If priorities or group do not match, the guest has fscked-up. */
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] KVM: arm64: vgic-v3: Align emulated cpuif LPI state machine with the pseudocode

[Marc Zyngier]

Having realised that a virtual LPI does transition through an active
state that does not exist on bare metal, align the CPU interface
emulation with the behaviour specified in the architecture pseudocode.

The LPIs now transition to active on IAR read, and to inactive on
EOI write. Special care is taken not to increment the EOIcount for
an LPI that isn't present in the LRs.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index b3b50de496a3..20db2f281cf2 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -695,9 +695,7 @@ static void __vgic_v3_read_iar(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 		goto spurious;
 
 	lr_val &= ~ICH_LR_STATE;
-	/* No active state for LPIs */
-	if ((lr_val & ICH_LR_VIRTUAL_ID_MASK) <= VGIC_MAX_SPI)
-		lr_val |= ICH_LR_ACTIVE_BIT;
+	lr_val |= ICH_LR_ACTIVE_BIT;
 	__gic_v3_set_lr(lr_val, lr);
 	__vgic_v3_set_active_priority(lr_prio, vmcr, grp);
 	vcpu_set_reg(vcpu, rt, lr_val & ICH_LR_VIRTUAL_ID_MASK);
@@ -764,20 +762,18 @@ static void __vgic_v3_write_eoir(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	/* Drop priority in any case */
 	act_prio = __vgic_v3_clear_highest_active_priority();
 
-	/* If EOIing an LPI, no deactivate to be performed */
-	if (vid >= VGIC_MIN_LPI)
-		return;
-
-	/* EOImode == 1, nothing to be done here */
-	if (vmcr & ICH_VMCR_EOIM_MASK)
-		return;
-
 	lr = __vgic_v3_find_active_lr(vcpu, vid, &lr_val);
 	if (lr == -1) {
-		__vgic_v3_bump_eoicount();
+		/* Do not bump EOIcount for LPIs that aren't in the LRs */
+		if (!(vid >= VGIC_MIN_LPI))
+			__vgic_v3_bump_eoicount();
 		return;
 	}
 
+	/* EOImode == 1 and not an LPI, nothing to be done here */
+	if ((vmcr & ICH_VMCR_EOIM_MASK) && !(vid >= VGIC_MIN_LPI))
+		return;
+
 	lr_prio = (lr_val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
 
 	/* If priorities or group do not match, the guest has fscked-up. */
-- 
2.30.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm