lkml.org 
[lkml]   [2020]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 2/2] KVM: SVM: Use a separate vmcb for the nested L2 guest
Date
On 11/10/20 20:48, Cathy Avery wrote:
> @@ -432,6 +432,16 @@ int enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
> int ret;
>
> svm->nested.vmcb = vmcb_gpa;
> +
> + WARN_ON(svm->vmcb == svm->nested.vmcb02);
> +
> + svm->nested.vmcb02->control = svm->vmcb01->control;

This assignment of the control area should be in
nested_prepare_vmcb_control, which is already filling in most of
vmcb02->control.

Right now, we save a copy_vmcb_control-area in nested_svm_vmexit so it
evens out.

Later, it should be possible to remove most of the assignments from
nested_prepare_vmcb_control.

> + svm->nested.vmcb02->save.cr4 = svm->vmcb01->save.cr4;

I cannot understand this statement.

> + nested_svm_vmloadsave(svm->vmcb01, svm->nested.vmcb02);

This is because the vmsave just after the vmexit has moved the
vmloadsave registers from vmcb12 to vmcb01, but the next vmload will use
vmcb02.

> + svm->vmcb = svm->nested.vmcb02;
> + svm->vmcb_pa = svm->nested.vmcb02_pa;
> load_nested_vmcb_control(svm, &nested_vmcb->control);
> nested_prepare_vmcb_save(svm, nested_vmcb);
> nested_prepare_vmcb_control(svm);


> @@ -628,8 +620,10 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> nested_vmcb->control.pause_filter_thresh =
> svm->vmcb->control.pause_filter_thresh;
>
> - /* Restore the original control entries */
> - copy_vmcb_control_area(&vmcb->control, &hsave->control);
> + nested_svm_vmloadsave(svm->nested.vmcb02, svm->vmcb01);

Same here: the next vmentry's vmload will move the vmloadsave registers
from vmcb01 to vmcb12, but for now they are in vmcb02.

It's 16+16 memory-to-memory u64 copies. They mostly even out with the
14+14 copies that we don't have to do anymore on registers handled by
VMRUN (es/cs/ss/ds/gdt/idt/rsp/rax---they don't have to be stashed away
in hsave anymore). Also, we are able to reuse nested_svm_vmloadsave,
which makes it overall a fair tradeoff... but it would have been worth a
comment or two. :)

> + svm->nested.vmcb02->control = svm->vmcb01->control;
> + svm->nested.vmcb02->save = svm->vmcb01->save;
> + svm->vmcb01->save = save;

I would have moved these after the comment, matching the existing
copy_vmcb_control_area and save-area assignment.

Also, the first save-area assignment should be (because the WARN_ON
below must be removed)

svm->nested.vmcb02->save = svm->vmcb->save;

or

if (svm->vmcb == svm->vmcb01)
svm->nested.vmcb02->save = svm->vmcb01->save;

I have applied the patch and fixed the conflicts, so when I have some
time I will play a bit more with it and/or pass the updated version back
to you.

In the meanwhile, perhaps you could write a new selftests testcase that
tries to do KVM_SET_NESTED_STATE while in L2. It can be a simplified
version of state-test that invokes KVM_GET_NESTED_STATE +
KVM_SET_NESTED_STATE when it gets back to L0.

Paolo

\
 
 \ /
  Last update: 2020-11-13 18:59    [W:0.126 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site