linux-kernel - Re: [PATCH] KVM: VMX: simplify vmx_prepare_switch_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190607174753.GH9083@linux.intel.com>
Date:   Fri, 7 Jun 2019 10:47:53 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH] KVM: VMX: simplify vmx_prepare_switch_to_{guest,host}

On Fri, Jun 07, 2019 at 10:37:10AM -0700, Sean Christopherson wrote:
> On Fri, Jun 07, 2019 at 07:08:21PM +0200, Paolo Bonzini wrote:
> > vmx->loaded_cpu_state can only be NULL or equal to vmx->loaded_vmcs,
> > so change it to a bool.  Because the direction of the bool is
> > now the opposite of vmx->guest_msrs_dirty, change the direction of
> > vmx->guest_msrs_dirty so that they match.
> > 
> > Finally, do not imply that MSRs have to be reloaded when
> > vmx->guest_sregs_loaded is false; instead, set vmx->guest_msrs_loaded
> > to false explicitly in vmx_prepare_switch_to_host.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> 
> ...
> 
> > @@ -1165,13 +1163,15 @@ static void vmx_prepare_switch_to_host(struct vcpu_vmx *vmx)
> >  	wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
> >  #endif
> >  	load_fixmap_gdt(raw_smp_processor_id());
> > +	vmx->guest_sregs_loaded = false;
> > +	vmx->guest_msrs_loaded = false;
> >  }
> >  
> >  #ifdef CONFIG_X86_64
> >  static u64 vmx_read_guest_kernel_gs_base(struct vcpu_vmx *vmx)
> >  {
> >  	preempt_disable();
> > -	if (vmx->loaded_cpu_state)
> > +	if (vmx->guest_sregs_loaded)
> >  		rdmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);
> 
> This is the hiccup with naming it sregs_loaded.  The split bools is also
> kinda wonky since the 32->64 case is a one-off scenario.  I think a
> cleaner solution would be to remove guest_msrs_dirty and refresh the MSRs
> directly from setup_msrs().  Then loaded_cpu_state -> loaded_guest_state
> can be a straight conversion from loaded_vmcs -> bool.  I'll send patches.

Actually, would it be easier on your end if I do a v2 of the series that
would introduce vmx_sync_vmcs_host_state(), and splice these patch into it?