From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751111AbdEZENl (ORCPT <rfc822;w@1wt.eu>);
        Fri, 26 May 2017 00:13:41 -0400
Received: from mail-pf0-f178.google.com ([209.85.192.178]:34585 "EHLO
        mail-pf0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752759AbdEZENb (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 26 May 2017 00:13:31 -0400
Date: Thu, 25 May 2017 21:13:27 -0700
From: Nick Desaulniers <nick.desaulniers@gmail.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: dynamically allocate large struct in em_fxrstor
Message-ID: <20170526041327.ttnlnwyuv4bbxdxx@lostoracle.net>
References: <20170524062433.20680-1-nick.desaulniers@gmail.com>
 <20170524141957.GA8174@potion>
 <20170525013653.dvxpczik77l6ogp7@lostoracle.net>
 <52f0d4a8-aacf-dc64-8117-2ba33bbfd928@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <52f0d4a8-aacf-dc64-8117-2ba33bbfd928@redhat.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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?

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

> - 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)?