From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1947135AbdEZHSh (ORCPT ); Fri, 26 May 2017 03:18:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44272 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S970690AbdEZHSU (ORCPT ); Fri, 26 May 2017 03:18:20 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C1A3F4E341 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pbonzini@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com C1A3F4E341 Subject: Re: [PATCH] KVM: x86: dynamically allocate large struct in em_fxrstor To: Nick Desaulniers Cc: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20170524062433.20680-1-nick.desaulniers@gmail.com> <20170524141957.GA8174@potion> <20170525013653.dvxpczik77l6ogp7@lostoracle.net> <52f0d4a8-aacf-dc64-8117-2ba33bbfd928@redhat.com> <20170526041327.ttnlnwyuv4bbxdxx@lostoracle.net> From: Paolo Bonzini Message-ID: Date: Fri, 26 May 2017 09:18:12 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <20170526041327.ttnlnwyuv4bbxdxx@lostoracle.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 26 May 2017 07:18:17 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26/05/2017 06:13, Nick Desaulniers wrote: > On Thu, May 25, 2017 at 04:07:08PM +0200, Paolo Bonzini wrote: >> I think we should do the fixup backwards. >> >> That is: >> >> - first do get_fpu >> >> - if the fixup is necessary, i.e. ctxt->mode < X86EMUL_MODE_PROT64, do >> fxsave into &fxstate. >> >> - then do segmented_read_std with the correct size, which is >> - offsetof(struct fxregs_state, xmm_space[16]), i.e. 416 >> if ctxt->mode == X86EMUL_MODE_PROT64 >> - offsetof(struct fxregs_state, xmm_space[8]), i.e. 288 >> if ctxt->mode < X86EMUL_MODE_PROT64 and CR4.OSFXSR=1 >> - offsetof(struct fxregs_state, xmm_space[0]), i.e. 160 >> if ctxt->mode < X86EMUL_MODE_PROT64 and CR4.OSFXSR=0 > > but we still want to do a segmented_read_std with size 512 otherwise, > correct? No, 512 is never necessary. ctxt->mode is never > X86EMUL_MODE_PROT64 (see the definition of enum x86emul_mode in arch/x86/include/asm/kvm_emulate.h. >> - then check fx_state.mxcsr >> >> - then do fxrstor > > This sounds like we conditionally do the fxsave, but then always do the > fxrstor. Is that ok? I guess the original code kind of does that as > well. Correct. They idea is that fxrstor on Linux always accesses 416 bytes, but we may have to restore only the first part for accurate emulation. The fxsave retrieves the current state so that we can leave it unmodified when we do fxrstor. >> - finally do put_fpu > > Sounds straight forward. I can see how fxsave and CR4.OSFXSR are > accessed in fxstor_fixup. Is it ok to skip those memcpy's that would > otherwise occur when calling fxrstor_fixup() (which after these changes, > we would not be)? Yes, the memcpys are replaced with a shorter segmented_read_std. Thanks, Paolo