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

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

> +		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