On Mon, Jun 14, 2010, Avi Kivity wrote about "Re: [PATCH 16/24] Implement VMLAUNCH and VMRESUME":
> >+ vmx_set_cr0(vcpu,
> >+ (get_shadow_vmcs(vcpu)->guest_cr0&
> >+ ~get_shadow_vmcs(vcpu)->cr0_guest_host_mask) |
> >+ (get_shadow_vmcs(vcpu)->cr0_read_shadow&
> >+ get_shadow_vmcs(vcpu)->cr0_guest_host_mask));
> >+
> >+ /* However, vmx_set_cr0 incorrectly enforces KVM's relationship
> >between
> >+ * GUEST_CR0 and CR0_READ_SHADOW, e.g., that the former is the same
> >as
> >+ * the latter with with TS added if !fpu_active. We need to take the
> >+ * actual GUEST_CR0 that L1 wanted, just with added TS if !fpu_active
> >+ * like KVM wants (for the "lazy fpu" feature, to avoid the costly
> >+ * restoration of fpu registers until the FPU is really used).
> >+ */
> >+ vmcs_writel(GUEST_CR0, get_shadow_vmcs(vcpu)->guest_cr0 |
> >+ (vcpu->fpu_active ? 0 : X86_CR0_TS));
> >
>
> Please update vmx_set_cr0() instead.
How would you like that I do that?
I could split vmx_set_cr0(vcpu, cr0) into a __vmx_set_cr0(vcpu, cr0, hw_cr0)
and vmx_set_cr0 that calls it. Is this what you had in mind? Won't it be
a little ugly? I agree, though, that it will avoid the vmwriting GUEST_CR0
twice in the nested case.
> >+ /* we have to set the X86_CR0_PG bit of the cached cr0, because
> >+ * kvm_mmu_reset_context enables paging only if X86_CR0_PG is set in
> >+ * CR0 (we need the paging so that KVM treat this guest as a paging
> >+ * guest so we can easly forward page faults to L1.)
> >+ */
> >+ vcpu->arch.cr0 |= X86_CR0_PG;
> >
>
> Since this version doesn't support unrestricted nested guests, cr0.pg
> will be already set or we will have failed vmentry.
I believe without this "hack", things didn't work properly during boot of
L2, when cr0_read_shadow.pg was not yet set. I think PG is set in guest_cr0,
but not in cr0_read_shadow, which is what vcpu->arch.cr0 caches.
> >+ if (enable_ept&& !nested_cpu_has_vmx_ept(vcpu)) {
> >
>
> We don't support nested ept yet, yes?
Right. It seems like this (and a couple of other places) were left from
our internal codebase (which did have nested ept). I'll clean it up.
--
Nadav Har'El | Monday, Sep 20 2010, 12 Tishri 5771
nyh@xxxxxxxxxxxxxxxxxxx |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |The two rules for success are: 1. Never
http://nadav.harel.org.il |tell them everything you know.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
