From: Quentin Perret <qperret@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>,
qemu-devel@nongnu.org, kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
Date: Fri, 22 Nov 2019 15:38:36 +0000 [thread overview]
Message-ID: <20191122153836.GA222628@google.com> (raw)
In-Reply-To: <20191122135833.28953-1-maz@kernel.org>
On Friday 22 Nov 2019 at 13:58:33 (+0000), Marc Zyngier wrote:
> The ARMv8 ARM states when executing at EL2, EL3 or Secure EL1,
> ISR_EL1 shows the pending status of the physical IRQ, FIQ, or
> SError interrupts.
>
> Unfortunately, QEMU's implementation only considers the HCR_EL2
> bits, and ignores the current exception level. This means a hypervisor
> trying to look at its own interrupt state actually sees the guest
> state, which is unexpected and breaks KVM as of Linux 5.3.
>
> Instead, check for the running EL and return the physical bits
> if not running in a virtualized context.
>
> Fixes: 636540e9c40b
> Reported-by: Quentin Perret <qperret@google.com>
And FWIW, Tested-by: Quentin Perret <qperret@google.com>
Thanks Marc :)
Quentin
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> target/arm/helper.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index a089fb5a69..027fffbff6 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1934,8 +1934,11 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> CPUState *cs = env_cpu(env);
> uint64_t hcr_el2 = arm_hcr_el2_eff(env);
> uint64_t ret = 0;
> + bool allow_virt = (arm_current_el(env) == 1 &&
> + (!arm_is_secure_below_el3(env) ||
> + (env->cp15.scr_el3 & SCR_EEL2)));
>
> - if (hcr_el2 & HCR_IMO) {
> + if (allow_virt && (hcr_el2 & HCR_IMO)) {
> if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
> ret |= CPSR_I;
> }
> @@ -1945,7 +1948,7 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> }
> }
>
> - if (hcr_el2 & HCR_FMO) {
> + if (allow_virt && (hcr_el2 & HCR_FMO)) {
> if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
> ret |= CPSR_F;
> }
> --
> 2.17.1
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: Quentin Perret <qperret@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: qemu-devel@nongnu.org, kvmarm@lists.cs.columbia.edu,
Will Deacon <will@kernel.org>,
Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
Date: Fri, 22 Nov 2019 15:38:36 +0000 [thread overview]
Message-ID: <20191122153836.GA222628@google.com> (raw)
In-Reply-To: <20191122135833.28953-1-maz@kernel.org>
On Friday 22 Nov 2019 at 13:58:33 (+0000), Marc Zyngier wrote:
> The ARMv8 ARM states when executing at EL2, EL3 or Secure EL1,
> ISR_EL1 shows the pending status of the physical IRQ, FIQ, or
> SError interrupts.
>
> Unfortunately, QEMU's implementation only considers the HCR_EL2
> bits, and ignores the current exception level. This means a hypervisor
> trying to look at its own interrupt state actually sees the guest
> state, which is unexpected and breaks KVM as of Linux 5.3.
>
> Instead, check for the running EL and return the physical bits
> if not running in a virtualized context.
>
> Fixes: 636540e9c40b
> Reported-by: Quentin Perret <qperret@google.com>
And FWIW, Tested-by: Quentin Perret <qperret@google.com>
Thanks Marc :)
Quentin
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> target/arm/helper.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index a089fb5a69..027fffbff6 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1934,8 +1934,11 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> CPUState *cs = env_cpu(env);
> uint64_t hcr_el2 = arm_hcr_el2_eff(env);
> uint64_t ret = 0;
> + bool allow_virt = (arm_current_el(env) == 1 &&
> + (!arm_is_secure_below_el3(env) ||
> + (env->cp15.scr_el3 & SCR_EEL2)));
>
> - if (hcr_el2 & HCR_IMO) {
> + if (allow_virt && (hcr_el2 & HCR_IMO)) {
> if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
> ret |= CPSR_I;
> }
> @@ -1945,7 +1948,7 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> }
> }
>
> - if (hcr_el2 & HCR_FMO) {
> + if (allow_virt && (hcr_el2 & HCR_FMO)) {
> if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
> ret |= CPSR_F;
> }
> --
> 2.17.1
>
next prev parent reply other threads:[~2019-11-22 15:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-22 13:58 [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2 Marc Zyngier
2019-11-22 13:58 ` Marc Zyngier
2019-11-22 14:16 ` Peter Maydell
2019-11-22 14:16 ` Peter Maydell
2019-11-22 15:34 ` Philippe Mathieu-Daudé
2019-11-22 15:34 ` Philippe Mathieu-Daudé
2019-11-22 16:17 ` Richard Henderson
2019-11-22 16:17 ` Richard Henderson
2019-11-22 14:24 ` Edgar E. Iglesias
2019-11-22 14:24 ` Edgar E. Iglesias
2019-11-22 15:38 ` Quentin Perret [this message]
2019-11-22 15:38 ` Quentin Perret
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=20191122153836.GA222628@google.com \
--to=qperret@google.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=maz@kernel.org \
--cc=qemu-devel@nongnu.org \
--cc=will@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.