[PATCH 3.14 28/73] MIPS: KVM: Deliver guest interrupts after local_irq_disable()

[PATCH 3.14 28/73] MIPS: KVM: Deliver guest interrupts after local_irq_disable()

[Greg Kroah-Hartman]

3.14-stable review patch.  If anyone has any objections, please let me know.

------------------

From: James Hogan <james.hogan@imgtec.com>

commit 044f0f03eca0110e1835b2ea038a484b93950328 upstream.

When about to run the guest, deliver guest interrupts after disabling
host interrupts. This should prevent an hrtimer interrupt from being
handled after delivering guest interrupts, and therefore not delivering
the guest timer interrupt until after the next guest exit.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/mips/kvm/kvm_mips.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -418,11 +418,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
 		vcpu->mmio_needed = 0;
 	}
 
+	local_irq_disable();
 	/* Check if we have any exceptions/interrupts pending */
 	kvm_mips_deliver_interrupts(vcpu,
 				    kvm_read_c0_guest_cause(vcpu->arch.cop0));
 
-	local_irq_disable();
 	kvm_guest_enter();
 
 	r = __kvm_mips_vcpu_run(run, vcpu);

[PATCH 3.14 58/73] KVM: MIPS: Dont leak FPU/DSP to guest

[Greg Kroah-Hartman]

3.14-stable review patch.  If anyone has any objections, please let me know.

------------------

From: James Hogan <james.hogan@imgtec.com>

commit f798217dfd038af981a18bbe4bc57027a08bb182 upstream.

The FPU and DSP are enabled via the CP0 Status CU1 and MX bits by
kvm_mips_set_c0_status() on a guest exit, presumably in case there is
active state that needs saving if pre-emption occurs. However neither of
these bits are cleared again when returning to the guest.

This effectively gives the guest access to the FPU/DSP hardware after
the first guest exit even though it is not aware of its presence,
allowing FP instructions in guest user code to intermittently actually
execute instead of trapping into the guest OS for emulation. It will
then read & manipulate the hardware FP registers which technically
belong to the user process (e.g. QEMU), or are stale from another user
process. It can also crash the guest OS by causing an FP exception, for
which a guest exception handler won't have been registered.

First lets save and disable the FPU (and MSA) state with lose_fpu(1)
before entering the guest. This simplifies the problem, especially for
when guest FPU/MSA support is added in the future, and prevents FR=1 FPU
state being live when the FR bit gets cleared for the guest, which
according to the architecture causes the contents of the FPU and vector
registers to become UNPREDICTABLE.

We can then safely remove the enabling of the FPU in
kvm_mips_set_c0_status(), since there should never be any active FPU or
MSA state to save at pre-emption, which should plug the FPU leak.

DSP state is always live rather than being lazily restored, so for that
it is simpler to just clear the MX bit again when re-entering the guest.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Sanjay Lal <sanjayl@kymasys.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> # v3.10+: 044f0f03eca0: MIPS: KVM: Deliver guest interrupts
Cc: <stable@vger.kernel.org> # v3.10+
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
This should apply to stable trees 3.12 and 3.14, but not 3.10. The files
had been renamed since v3.14 so it cherry-picked cleanly but the patch
didn't apply cleanly. I've also added a reference to the "MIPS: Export
FP functions used by lose_fpu(1) for KVM" commit which is itself marked
for stable, but is needed to avoid a build failure when KVM=m.
---
 arch/mips/kvm/kvm_locore.S |    2 +-
 arch/mips/kvm/kvm_mips.c   |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

--- a/arch/mips/kvm/kvm_locore.S
+++ b/arch/mips/kvm/kvm_locore.S
@@ -428,7 +428,7 @@ __kvm_mips_return_to_guest:
 	/* Setup status register for running guest in UM */
 	.set	at
 	or	v1, v1, (ST0_EXL | KSU_USER | ST0_IE)
-	and	v1, v1, ~ST0_CU0
+	and	v1, v1, ~(ST0_CU0 | ST0_MX)
 	.set	noat
 	mtc0	v1, CP0_STATUS
 	ehb
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -15,6 +15,7 @@
 #include <linux/vmalloc.h>
 #include <linux/fs.h>
 #include <linux/bootmem.h>
+#include <asm/fpu.h>
 #include <asm/page.h>
 #include <asm/cacheflush.h>
 #include <asm/mmu_context.h>
@@ -418,6 +419,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
 		vcpu->mmio_needed = 0;
 	}
 
+	lose_fpu(1);
+
 	local_irq_disable();
 	/* Check if we have any exceptions/interrupts pending */
 	kvm_mips_deliver_interrupts(vcpu,
@@ -1021,9 +1024,6 @@ void kvm_mips_set_c0_status(void)
 {
 	uint32_t status = read_c0_status();
 
-	if (cpu_has_fpu)
-		status |= (ST0_CU1);
-
 	if (cpu_has_dsp)
 		status |= (ST0_MX);
 

Re: [PATCH 3.14 58/73] KVM: MIPS: Dont leak FPU/DSP to guest

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 4605 bytes --]

Hi Greg,

On Tue, Mar 03, 2015 at 10:13:26PM -0800, Greg Kroah-Hartman wrote:
> 3.14-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: James Hogan <james.hogan@imgtec.com>
> 
> commit f798217dfd038af981a18bbe4bc57027a08bb182 upstream.
> 
> The FPU and DSP are enabled via the CP0 Status CU1 and MX bits by
> kvm_mips_set_c0_status() on a guest exit, presumably in case there is
> active state that needs saving if pre-emption occurs. However neither of
> these bits are cleared again when returning to the guest.
> 
> This effectively gives the guest access to the FPU/DSP hardware after
> the first guest exit even though it is not aware of its presence,
> allowing FP instructions in guest user code to intermittently actually
> execute instead of trapping into the guest OS for emulation. It will
> then read & manipulate the hardware FP registers which technically
> belong to the user process (e.g. QEMU), or are stale from another user
> process. It can also crash the guest OS by causing an FP exception, for
> which a guest exception handler won't have been registered.
> 
> First lets save and disable the FPU (and MSA) state with lose_fpu(1)
> before entering the guest. This simplifies the problem, especially for
> when guest FPU/MSA support is added in the future, and prevents FR=1 FPU
> state being live when the FR bit gets cleared for the guest, which
> according to the architecture causes the contents of the FPU and vector
> registers to become UNPREDICTABLE.
> 
> We can then safely remove the enabling of the FPU in
> kvm_mips_set_c0_status(), since there should never be any active FPU or
> MSA state to save at pre-emption, which should plug the FPU leak.
> 
> DSP state is always live rather than being lazily restored, so for that
> it is simpler to just clear the MX bit again when re-entering the guest.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: <stable@vger.kernel.org> # v3.10+: 044f0f03eca0: MIPS: KVM: Deliver guest interrupts

The original 3.10 and 3.12/3.14 backports had this added:
Cc: <stable@vger.kernel.org> # v3.10+: 3ce465e04bfd: MIPS: Export FP functions used by lose_fpu(1) for KVM                                         
Which I can't see included in the v3.10 stable queue or branch. It fixes
a build error with MIPS malta_kvm_defconfig (MIPS=y, KVM=m) after this
patch is applied.

Same applies to the 3.14 queue too I think.

Cheers
James

> Cc: <stable@vger.kernel.org> # v3.10+
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> This should apply to stable trees 3.12 and 3.14, but not 3.10. The files
> had been renamed since v3.14 so it cherry-picked cleanly but the patch
> didn't apply cleanly. I've also added a reference to the "MIPS: Export
> FP functions used by lose_fpu(1) for KVM" commit which is itself marked
> for stable, but is needed to avoid a build failure when KVM=m.
> ---
>  arch/mips/kvm/kvm_locore.S |    2 +-
>  arch/mips/kvm/kvm_mips.c   |    6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> --- a/arch/mips/kvm/kvm_locore.S
> +++ b/arch/mips/kvm/kvm_locore.S
> @@ -428,7 +428,7 @@ __kvm_mips_return_to_guest:
>  	/* Setup status register for running guest in UM */
>  	.set	at
>  	or	v1, v1, (ST0_EXL | KSU_USER | ST0_IE)
> -	and	v1, v1, ~ST0_CU0
> +	and	v1, v1, ~(ST0_CU0 | ST0_MX)
>  	.set	noat
>  	mtc0	v1, CP0_STATUS
>  	ehb
> --- a/arch/mips/kvm/kvm_mips.c
> +++ b/arch/mips/kvm/kvm_mips.c
> @@ -15,6 +15,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/fs.h>
>  #include <linux/bootmem.h>
> +#include <asm/fpu.h>
>  #include <asm/page.h>
>  #include <asm/cacheflush.h>
>  #include <asm/mmu_context.h>
> @@ -418,6 +419,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
>  		vcpu->mmio_needed = 0;
>  	}
>  
> +	lose_fpu(1);
> +
>  	local_irq_disable();
>  	/* Check if we have any exceptions/interrupts pending */
>  	kvm_mips_deliver_interrupts(vcpu,
> @@ -1021,9 +1024,6 @@ void kvm_mips_set_c0_status(void)
>  {
>  	uint32_t status = read_c0_status();
>  
> -	if (cpu_has_fpu)
> -		status |= (ST0_CU1);
> -
>  	if (cpu_has_dsp)
>  		status |= (ST0_MX);
>  
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 3.14 58/73] KVM: MIPS: Dont leak FPU/DSP to guest

[Greg Kroah-Hartman]

On Wed, Mar 04, 2015 at 08:10:40AM +0000, James Hogan wrote:
> Hi Greg,
> 
> On Tue, Mar 03, 2015 at 10:13:26PM -0800, Greg Kroah-Hartman wrote:
> > 3.14-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: James Hogan <james.hogan@imgtec.com>
> > 
> > commit f798217dfd038af981a18bbe4bc57027a08bb182 upstream.
> > 
> > The FPU and DSP are enabled via the CP0 Status CU1 and MX bits by
> > kvm_mips_set_c0_status() on a guest exit, presumably in case there is
> > active state that needs saving if pre-emption occurs. However neither of
> > these bits are cleared again when returning to the guest.
> > 
> > This effectively gives the guest access to the FPU/DSP hardware after
> > the first guest exit even though it is not aware of its presence,
> > allowing FP instructions in guest user code to intermittently actually
> > execute instead of trapping into the guest OS for emulation. It will
> > then read & manipulate the hardware FP registers which technically
> > belong to the user process (e.g. QEMU), or are stale from another user
> > process. It can also crash the guest OS by causing an FP exception, for
> > which a guest exception handler won't have been registered.
> > 
> > First lets save and disable the FPU (and MSA) state with lose_fpu(1)
> > before entering the guest. This simplifies the problem, especially for
> > when guest FPU/MSA support is added in the future, and prevents FR=1 FPU
> > state being live when the FR bit gets cleared for the guest, which
> > according to the architecture causes the contents of the FPU and vector
> > registers to become UNPREDICTABLE.
> > 
> > We can then safely remove the enabling of the FPU in
> > kvm_mips_set_c0_status(), since there should never be any active FPU or
> > MSA state to save at pre-emption, which should plug the FPU leak.
> > 
> > DSP state is always live rather than being lazily restored, so for that
> > it is simpler to just clear the MX bit again when re-entering the guest.
> > 
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Sanjay Lal <sanjayl@kymasys.com>
> > Cc: Gleb Natapov <gleb@kernel.org>
> > Cc: kvm@vger.kernel.org
> > Cc: linux-mips@linux-mips.org
> > Cc: <stable@vger.kernel.org> # v3.10+: 044f0f03eca0: MIPS: KVM: Deliver guest interrupts
> 
> The original 3.10 and 3.12/3.14 backports had this added:
> Cc: <stable@vger.kernel.org> # v3.10+: 3ce465e04bfd: MIPS: Export FP functions used by lose_fpu(1) for KVM                                         
> Which I can't see included in the v3.10 stable queue or branch. It fixes
> a build error with MIPS malta_kvm_defconfig (MIPS=y, KVM=m) after this
> patch is applied.
> 
> Same applies to the 3.14 queue too I think.

Odd, I remember having problems in this area and thought I had queued
this up.  It's now applied to both trees, thanks.

greg k-h