From: Wanpeng Li <wanpeng.li@linux.intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>, Jan Kiszka <jan.kiszka@siemens.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
Gleb Natapov <gleb@kernel.org>, Bandan Das <bsd@redhat.com>,
Zhang Yang <yang.z.zhang@intel.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: nVMX: nested TPR shadow/threshold emulation
Date: Thu, 31 Jul 2014 16:03:54 +0800 [thread overview]
Message-ID: <20140731080353.GA3033@kernel> (raw)
In-Reply-To: <53D90D5A.3060404@redhat.com>
Hi Paolo,
On Wed, Jul 30, 2014 at 05:20:58PM +0200, Paolo Bonzini wrote:
>Il 30/07/2014 14:04, Wanpeng Li ha scritto:
>> @@ -7962,14 +7965,14 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>> if (!vmx->rdtscp_enabled)
>> exec_control &= ~SECONDARY_EXEC_RDTSCP;
>> /* Take the following fields only from vmcs12 */
>> - exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>> - SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>> + exec_control &= ~(SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>> SECONDARY_EXEC_APIC_REGISTER_VIRT);
>
>This change is wrong. You don't have to take L0's "virtualize APIC
>accesses" setting into account, because while running L2 you cannot
>modify L1's CR8 (only the virtual nested one).
>
Agreed.
>> +
>> + virtual_apic_page = nested_get_page(vcpu,
>> + vmcs12->virtual_apic_page_addr);
>> + if (vmcs_read64(VIRTUAL_APIC_PAGE_ADDR) !=
>> + page_to_phys(virtual_apic_page))
>> + vmcs_write64(VIRTUAL_APIC_PAGE_ADDR,
>> + page_to_phys(virtual_apic_page));
>> + nested_release_page(virtual_apic_page);
>> +
>
>You cannot release this page here. You need to the exactly the same
>thing that is done for apic_access_page.
>
Agreed.
>One thing:
>
>> + if (nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
>> + vmcs_write32(TPR_THRESHOLD, vmcs12->tpr_threshold);
>
>I think you can just do this write unconditionally, since most
>hypervisors will enable this. Also, you probably can add the tpr
What will happen if a hypervisor doesn't enable it? I make it more
cleaner in version two.
>threshold field to the read-write fields for shadow VMCS.
>
Agreed.
Regards,
Wanpeng Li
>Paolo
>--
>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
next prev parent reply other threads:[~2014-07-31 8:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-30 12:04 [PATCH] KVM: nVMX: nested TPR shadow/threshold emulation Wanpeng Li
2014-07-30 15:20 ` Paolo Bonzini
2014-07-31 8:03 ` Wanpeng Li [this message]
2014-07-31 9:25 ` Paolo Bonzini
2014-08-01 0:57 ` Zhang, Yang Z
2014-08-01 6:35 ` Paolo Bonzini
2014-08-01 6:44 ` Zhang, Yang Z
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140731080353.GA3033@kernel \
--to=wanpeng.li@linux.intel.com \
--cc=bsd@redhat.com \
--cc=gleb@kernel.org \
--cc=jan.kiszka@siemens.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=yang.z.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).