Hi Paolo,
On Wed, Feb 04, 2015 at 05:06:37PM +0000, James Hogan wrote:
> 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)
Please don't apply this patch yet. lose_fpu() uses function symbols
which aren't exported for modules to use yet, so that'll need fixing
first or KVM won't build as a module.
Thanks
James
> 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@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Cc: Sanjay Lal <sanjayl@xxxxxxxxxxx>
> Cc: Gleb Natapov <gleb@xxxxxxxxxx>
> Cc: kvm@xxxxxxxxxxxxxxx
> Cc: linux-mips@xxxxxxxxxxxxxx
> Cc: <stable@xxxxxxxxxxxxxxx> # v3.10+: 044f0f03eca0: MIPS: KVM: Deliver guest interrupts
> Cc: <stable@xxxxxxxxxxxxxxx> # v3.10+
> ---
> arch/mips/kvm/locore.S | 2 +-
> arch/mips/kvm/mips.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/kvm/locore.S b/arch/mips/kvm/locore.S
> index d7279c03c517..4a68b176d6e4 100644
> --- a/arch/mips/kvm/locore.S
> +++ b/arch/mips/kvm/locore.S
> @@ -434,7 +434,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
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index dd133ccecec4..270bbd41769e 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/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>
> @@ -379,6 +380,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> vcpu->mmio_needed = 0;
> }
>
> + lose_fpu(1);
> +
> local_irq_disable();
> /* Check if we have any exceptions/interrupts pending */
> kvm_mips_deliver_interrupts(vcpu,
> @@ -987,9 +990,6 @@ static 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);
>
> --
> 2.0.5
>
Attachment:
signature.asc
Description: Digital signature