Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
On 11/22/19 2:16 PM, Peter Maydell wrote: > RTH: vaguely wondering if this might be related to the > bug you ran into trying to test your VHE emulation > patchset... Thanks for the thought. It might be related, but it isn't the final cause: the inner guest does not yet succeed including this patch. r~ ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
On Friday 22 Nov 2019 at 13:58:33 (+), 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 And FWIW, Tested-by: Quentin Perret Thanks Marc :) Quentin > Signed-off-by: Marc Zyngier > --- > 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
Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
On 11/22/19 3:16 PM, Peter Maydell wrote: On Fri, 22 Nov 2019 at 13:59, 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 Signed-off-by: Marc Zyngier Congratulations on your first QEMU patch :-) :)) ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
On Fri, Nov 22, 2019 at 01:58:33PM +, 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 > Signed-off-by: Marc Zyngier Looks good to me: Reviewed-by: Edgar E. Iglesias > --- > 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
Re: [PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
On Fri, 22 Nov 2019 at 13:59, 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 > Signed-off-by: Marc Zyngier Congratulations on your first QEMU patch :-) I've applied this to target-arm.next and will get it into rc3 ("fixes running newer kernels" seems like an rc-ish kind of bug). RTH: vaguely wondering if this might be related to the bug you ran into trying to test your VHE emulation patchset... thanks -- PMM ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH] target/arm: Fix ISR_EL1 tracking when executing at EL2
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 Signed-off-by: Marc Zyngier --- 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