From: Ard Biesheuvel <ardb@kernel.org>
To: Marc Zyngier <maz@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>,
James Morse <james.morse@arm.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Android Kernel Team <kernel-team@android.com>,
kvmarm <kvmarm@lists.cs.columbia.edu>,
Julien Thierry <julien.thierry.kdev@gmail.com>
Subject: Re: [PATCH 2/2] KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3 compatibility
Date: Fri, 8 Jan 2021 19:19:47 +0100 [thread overview]
Message-ID: <CAMj1kXFoB_qNH-7pvhYzwSaXC0tV3d427qfKAUmj3rqiU=t_JQ@mail.gmail.com> (raw)
In-Reply-To: <72a1bea27a29d35413c193f81b7ba170@kernel.org>
On Fri, 8 Jan 2021 at 19:13, Marc Zyngier <maz@kernel.org> wrote:
>
> On 2021-01-08 17:59, Ard Biesheuvel wrote:
> > On Fri, 8 Jan 2021 at 18:12, Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> It looks like we have broken firmware out there that wrongly
> >> advertises
> >> a GICv2 compatibility interface, despite the CPUs not being able to
> >> deal
> >> with it.
> >>
> >> To work around this, check that the CPU initialising KVM is actually
> >> able
> >> to switch to MMIO instead of system registers, and use that as a
> >> precondition to enable GICv2 compatibility in KVM.
> >>
> >> Note that the detection happens on a single CPU. If the firmware is
> >> lying *and* that the CPUs are asymetric, all hope is lost anyway.
> >>
> >> Reported-by: Shameerali Kolothum Thodi
> >> <shameerali.kolothum.thodi@huawei.com>
> >> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> ---
> >> arch/arm64/kvm/hyp/vgic-v3-sr.c | 34
> >> +++++++++++++++++++++++++++++++--
> >> arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++--
> >> 2 files changed, 38 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> >> b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> >> index 005daa0c9dd7..d504499ab917 100644
> >> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> >> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> >> @@ -408,11 +408,41 @@ void __vgic_v3_init_lrs(void)
> >> /*
> >> * Return the GIC CPU configuration:
> >> * - [31:0] ICH_VTR_EL2
> >> - * - [63:32] RES0
> >> + * - [62:32] RES0
> >> + * - [63] MMIO (GICv2) capable
> >> */
> >> u64 __vgic_v3_get_gic_config(void)
> >> {
> >> - return read_gicreg(ICH_VTR_EL2);
> >> + u64 sre = read_gicreg(ICC_SRE_EL1);
> >> + unsigned long flags = 0;
> >> + bool v2_capable;
> >> +
> >> + /*
> >> + * To check whether we have a MMIO-based (GICv2 compatible)
> >> + * CPU interface, we need to disable the system register
> >> + * view. To do that safely, we have to prevent any interrupt
> >> + * from firing (which would be deadly).
> >> + *
> >> + * Note that this only makes sense on VHE, as interrupts are
> >> + * already masked for nVHE as part of the exception entry to
> >> + * EL2.
> >> + */
> >> + if (has_vhe())
> >> + flags = local_daif_save();
> >> +
> >> + write_gicreg(0, ICC_SRE_EL1);
> >> + isb();
> >> +
> >> + v2_capable = !(read_gicreg(ICC_SRE_EL1) & ICC_SRE_EL1_SRE);
> >> +
> >> + write_gicreg(sre, ICC_SRE_EL1);
> >> + isb();
> >> +
> >> + if (has_vhe())
> >> + local_daif_restore(flags);
> >> +
> >> + return (read_gicreg(ICH_VTR_EL2) |
> >> + v2_capable ? (1ULL << 63) : 0);
> >> }
> >>
> >
> > Is it necessary to perform this check unconditionally? We only care
> > about this if the firmware claims v2 compat support.
>
> Indeed. But this is done exactly once per boot, and I see it as
> a way to extract the CPU configuration more than anything else.
>
> Extracting it *only* when we have some v2 compat info would mean
> sharing that information with EL2 (in the nVHE case), and it felt
> more hassle than it is worth.
>
> Do you foresee any issue with this, other than the whole thing
> being disgusting (which I wilfully admit)?
>
No I don't think it's a problem per se. Just a bit disappointing that
every system will be burdened with this for as long as the last v2
compat capable system is still being supported.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-08 18:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-08 17:12 [PATCH 0/2] KVM: arm64: Work around firmware wongly advertising GICv2 compatibility Marc Zyngier
2021-01-08 17:12 ` [PATCH 1/2] KVM: arm64: Rename __vgic_v3_get_ich_vtr_el2() to __vgic_v3_get_gic_config() Marc Zyngier
2021-01-08 17:12 ` [PATCH 2/2] KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3 compatibility Marc Zyngier
2021-01-08 17:59 ` Ard Biesheuvel
2021-01-08 18:12 ` Marc Zyngier
2021-01-08 18:19 ` Ard Biesheuvel [this message]
2021-01-11 12:21 ` Shameerali Kolothum Thodi
2021-01-11 13:20 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMj1kXFoB_qNH-7pvhYzwSaXC0tV3d427qfKAUmj3rqiU=t_JQ@mail.gmail.com' \
--to=ardb@kernel.org \
--cc=james.morse@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).