Re: [PATCH v5] KVM: x86: avoid large stack allocations in em_fxrstor

[Paolo Bonzini]

> +             size = offsetof(struct fxregs_state, xmm_space[16]);

This still has the same issue (it should be multiplied by 4).  Here's my
take on it; I checked the compiled code and it's pretty good too (the
compiler knows to do the fxsave if and only if ctxt->mode <
X86EMUL_MODE_PROT64, because that's when the size is smaller):

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 55470ad07c2a..b76f19d2684d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3944,10 +3944,19 @@ static int check_fxsr(struct x86_emulate_ctxt *ctxt)
  * Hardware doesn't save and restore XMM 0-7 without CR4.OSFXSR, but does save
  * and restore MXCSR.
  */
-static size_t xmm_offset(struct x86_emulate_ctxt *ctxt)
+static size_t __fxstate_size(int nregs)
 {
-       bool b = ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR;
-       return offsetof(struct fxregs_state, xmm_space[b ? 8 * 16 / 4 : 0]);
+       return offsetof(struct fxregs_state, xmm_space[0]) + nregs * 16;
+}
+
+static inline size_t fxstate_size(struct x86_emulate_ctxt *ctxt)
+{
+       bool cr4_osfxsr;
+       if (ctxt->mode == X86EMUL_MODE_PROT64)
+               return __fxstate_size(16);
+
+       cr4_osfxsr = ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR;
+       return __fxstate_size(cr4_osfxsr ? 8 : 0);
 }
 
 /*
@@ -3987,7 +3996,7 @@ static int em_fxsave(struct x86_emulate_ctxt *ctxt)
                return rc;
 
        return segmented_write_std(ctxt, ctxt->memop.addr.mem, &fx_state,
-               xmm_offset(ctxt));
+                                  fxstate_size(ctxt));
 }
 
 static int em_fxrstor(struct x86_emulate_ctxt *ctxt)
@@ -4002,13 +4011,11 @@ static int em_fxrstor(struct x86_emulate_ctxt *ctxt)
 
        ctxt->ops->get_fpu(ctxt);
 
-       if (ctxt->mode < X86EMUL_MODE_PROT64) {
+       size = fxstate_size(ctxt);
+       if (size < __fxstate_size(16)) {
                rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state));
                if (rc != X86EMUL_CONTINUE)
                        goto out;
-               size = xmm_offset(ctxt);
-       } else {
-               size = offsetof(struct fxregs_state, xmm_space[16]);
        }
 
        rc = segmented_read_std(ctxt, ctxt->memop.addr.mem, &fx_state, size);

Thanks Nick for the patches and Radim for the reviews!

Paolo