diff mbox

[v3,5/6] KVM: nVMX: Update mmu.base_role.nxe after EFER loading on VM-entry/exit

Message ID 08db7d166ab05ce133300ba069c0728f5c97120b.1375971992.git.jan.kiszka@siemens.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka Aug. 8, 2013, 2:26 p.m. UTC
This job is normally performed by the architectural EFER set service
which we cannot use as it prevents transitions that are valid when
switching between L1 and L2. So open-code the update of base_role.nxe
after changing EFER on VM-entry and exit.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kvm/vmx.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

Gleb Natapov Sept. 3, 2013, 8:39 a.m. UTC | #1
On Thu, Aug 08, 2013 at 04:26:32PM +0200, Jan Kiszka wrote:
> This job is normally performed by the architectural EFER set service
> which we cannot use as it prevents transitions that are valid when
> switching between L1 and L2. So open-code the update of base_role.nxe
> after changing EFER on VM-entry and exit.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/x86/kvm/vmx.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 9b0510b..7cc208a 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7735,6 +7735,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
>  	/* Note: modifies VM_ENTRY/EXIT_CONTROLS and GUEST/HOST_IA32_EFER */
>  	vmx_set_efer(vcpu, vcpu->arch.efer);
> +	vcpu->arch.mmu.base_role.nxe =
> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
>  
kvm_mmu_reset_context() called later in this function does that.

>  	if (nested_cpu_has_ept(vmcs12)) {
>  		kvm_mmu_unload(vcpu);
> @@ -8179,6 +8181,8 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
>  	else
>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
>  	vmx_set_efer(vcpu, vcpu->arch.efer);
> +	vcpu->arch.mmu.base_role.nxe =
> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
>
Same here.
  
>  	kvm_register_write(vcpu, VCPU_REGS_RSP, vmcs12->host_rsp);
>  	kvm_register_write(vcpu, VCPU_REGS_RIP, vmcs12->host_rip);
> -- 
> 1.7.3.4

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka Sept. 3, 2013, 8:51 a.m. UTC | #2
On 2013-09-03 10:39, Gleb Natapov wrote:
> On Thu, Aug 08, 2013 at 04:26:32PM +0200, Jan Kiszka wrote:
>> This job is normally performed by the architectural EFER set service
>> which we cannot use as it prevents transitions that are valid when
>> switching between L1 and L2. So open-code the update of base_role.nxe
>> after changing EFER on VM-entry and exit.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  arch/x86/kvm/vmx.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 9b0510b..7cc208a 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -7735,6 +7735,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
>>  	/* Note: modifies VM_ENTRY/EXIT_CONTROLS and GUEST/HOST_IA32_EFER */
>>  	vmx_set_efer(vcpu, vcpu->arch.efer);
>> +	vcpu->arch.mmu.base_role.nxe =
>> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
>>  
> kvm_mmu_reset_context() called later in this function does that.

For all relevant scenarios? The code is pretty convoluted, so this is
non-obvious for me: kvm_mmu_reset_context calls init_kvm_mmu which may
call init_kvm_softmmu that does kvm_init_shadow_mmu with nxe
initialization. But is that path always taken?

Jan

> 
>>  	if (nested_cpu_has_ept(vmcs12)) {
>>  		kvm_mmu_unload(vcpu);
>> @@ -8179,6 +8181,8 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
>>  	else
>>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
>>  	vmx_set_efer(vcpu, vcpu->arch.efer);
>> +	vcpu->arch.mmu.base_role.nxe =
>> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
>>
> Same here.
>   
>>  	kvm_register_write(vcpu, VCPU_REGS_RSP, vmcs12->host_rsp);
>>  	kvm_register_write(vcpu, VCPU_REGS_RIP, vmcs12->host_rip);
>> -- 
>> 1.7.3.4
> 
> --
> 			Gleb.
>
Gleb Natapov Sept. 3, 2013, 9:04 a.m. UTC | #3
On Tue, Sep 03, 2013 at 10:51:31AM +0200, Jan Kiszka wrote:
> On 2013-09-03 10:39, Gleb Natapov wrote:
> > On Thu, Aug 08, 2013 at 04:26:32PM +0200, Jan Kiszka wrote:
> >> This job is normally performed by the architectural EFER set service
> >> which we cannot use as it prevents transitions that are valid when
> >> switching between L1 and L2. So open-code the update of base_role.nxe
> >> after changing EFER on VM-entry and exit.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  arch/x86/kvm/vmx.c |    4 ++++
> >>  1 files changed, 4 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> index 9b0510b..7cc208a 100644
> >> --- a/arch/x86/kvm/vmx.c
> >> +++ b/arch/x86/kvm/vmx.c
> >> @@ -7735,6 +7735,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> >>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
> >>  	/* Note: modifies VM_ENTRY/EXIT_CONTROLS and GUEST/HOST_IA32_EFER */
> >>  	vmx_set_efer(vcpu, vcpu->arch.efer);
> >> +	vcpu->arch.mmu.base_role.nxe =
> >> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
> >>  
> > kvm_mmu_reset_context() called later in this function does that.
> 
> For all relevant scenarios? The code is pretty convoluted, so this is
> non-obvious for me: kvm_mmu_reset_context calls init_kvm_mmu which may
> call init_kvm_softmmu that does kvm_init_shadow_mmu with nxe
> initialization. But is that path always taken?
> 
What scenarios are relevant here? kvm_mmu_reset_context() is what
kvm_set_efer() would have called anyway. Since you use !enable_ept
here I assume that relevant scenario is shadow on shadow and in this
case it is easy to see that kvm_mmu_reset_context() will call
kvm_init_shadow_mmu() eventually. For all other scenarios guest nx
setting is irrelevant.

> Jan
> 
> > 
> >>  	if (nested_cpu_has_ept(vmcs12)) {
> >>  		kvm_mmu_unload(vcpu);
> >> @@ -8179,6 +8181,8 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
> >>  	else
> >>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
> >>  	vmx_set_efer(vcpu, vcpu->arch.efer);
> >> +	vcpu->arch.mmu.base_role.nxe =
> >> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
> >>
> > Same here.
> >   
> >>  	kvm_register_write(vcpu, VCPU_REGS_RSP, vmcs12->host_rsp);
> >>  	kvm_register_write(vcpu, VCPU_REGS_RIP, vmcs12->host_rip);
> >> -- 
> >> 1.7.3.4
> > 
> > --
> > 			Gleb.
> > 
> 
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE
> Corporate Competence Center Embedded Linux

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka Sept. 3, 2013, 9:32 a.m. UTC | #4
On 2013-09-03 11:04, Gleb Natapov wrote:
> On Tue, Sep 03, 2013 at 10:51:31AM +0200, Jan Kiszka wrote:
>> On 2013-09-03 10:39, Gleb Natapov wrote:
>>> On Thu, Aug 08, 2013 at 04:26:32PM +0200, Jan Kiszka wrote:
>>>> This job is normally performed by the architectural EFER set service
>>>> which we cannot use as it prevents transitions that are valid when
>>>> switching between L1 and L2. So open-code the update of base_role.nxe
>>>> after changing EFER on VM-entry and exit.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  arch/x86/kvm/vmx.c |    4 ++++
>>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>> index 9b0510b..7cc208a 100644
>>>> --- a/arch/x86/kvm/vmx.c
>>>> +++ b/arch/x86/kvm/vmx.c
>>>> @@ -7735,6 +7735,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>>>>  		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
>>>>  	/* Note: modifies VM_ENTRY/EXIT_CONTROLS and GUEST/HOST_IA32_EFER */
>>>>  	vmx_set_efer(vcpu, vcpu->arch.efer);
>>>> +	vcpu->arch.mmu.base_role.nxe =
>>>> +		(vcpu->arch.efer & EFER_NX) && !enable_ept;
>>>>  
>>> kvm_mmu_reset_context() called later in this function does that.
>>
>> For all relevant scenarios? The code is pretty convoluted, so this is
>> non-obvious for me: kvm_mmu_reset_context calls init_kvm_mmu which may
>> call init_kvm_softmmu that does kvm_init_shadow_mmu with nxe
>> initialization. But is that path always taken?
>>
> What scenarios are relevant here? kvm_mmu_reset_context() is what
> kvm_set_efer() would have called anyway. Since you use !enable_ept
> here I assume that relevant scenario is shadow on shadow and in this
> case it is easy to see that kvm_mmu_reset_context() will call
> kvm_init_shadow_mmu() eventually. For all other scenarios guest nx
> setting is irrelevant.

Indeed... Ah! I missed that 2c9afa52 changed the picture. OK, another
obsolete one.

Jan
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9b0510b..7cc208a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7735,6 +7735,8 @@  static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
 	/* Note: modifies VM_ENTRY/EXIT_CONTROLS and GUEST/HOST_IA32_EFER */
 	vmx_set_efer(vcpu, vcpu->arch.efer);
+	vcpu->arch.mmu.base_role.nxe =
+		(vcpu->arch.efer & EFER_NX) && !enable_ept;
 
 	if (nested_cpu_has_ept(vmcs12)) {
 		kvm_mmu_unload(vcpu);
@@ -8179,6 +8181,8 @@  static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	else
 		vcpu->arch.efer &= ~(EFER_LMA | EFER_LME);
 	vmx_set_efer(vcpu, vcpu->arch.efer);
+	vcpu->arch.mmu.base_role.nxe =
+		(vcpu->arch.efer & EFER_NX) && !enable_ept;
 
 	kvm_register_write(vcpu, VCPU_REGS_RSP, vmcs12->host_rsp);
 	kvm_register_write(vcpu, VCPU_REGS_RIP, vmcs12->host_rip);