KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Tao Xu <tao3.xu@intel.com>
To: "Radim Krčmář" <rkrcmar@redhat.com>,
	"Xiaoyao Li" <xiaoyao.li@linux.intel.com>
Cc: pbonzini@redhat.com, corbet@lwn.net, tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
	sean.j.christopherson@intel.com, fenghua.yu@intel.com,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	jingqi.liu@intel.com
Subject: Re: [PATCH RESEND v3 2/3] KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL
Date: Tue, 18 Jun 2019 10:40:18 +0800
Message-ID: <afc6182b-8e78-5032-c579-35bf2732b740@intel.com> (raw)
In-Reply-To: <20190617155038.GA13955@flask>

On 6/17/2019 11:50 PM, Radim Krčmář wrote:
> 2019-06-17 14:31+0800, Xiaoyao Li:
>> On 6/17/2019 11:32 AM, Xiaoyao Li wrote:
>>> On 6/16/2019 5:55 PM, Tao Xu wrote:
>>>> +    if (vmx->msr_ia32_umwait_control != host_umwait_control)
>>>> +        add_atomic_switch_msr(vmx, MSR_IA32_UMWAIT_CONTROL,
>>>> +                      vmx->msr_ia32_umwait_control,
>>>> +                      host_umwait_control, false);
>>>
>>> The bit 1 is reserved, at least, we need to do below to ensure not
>>> modifying the reserved bit:
>>>
>>>       guest_val = (vmx->msr_ia32_umwait_control & ~BIT_ULL(1)) |
>>>               (host_val & BIT_ULL(1))
>>>
>>
>> I find a better solution to ensure reserved bit 1 not being modified in
>> vmx_set_msr() as below:
>>
>> 	if((data ^ umwait_control_cached) & BIT_ULL(1))
>> 		return 1;
> 
> We could just be checking
> 
> 	if (data & BIT_ULL(1))
> 
> because the guest cannot change its visible reserved value and KVM
> currently initializes the value to 0.
> 
> The arch/x86/kernel/cpu/umwait.c series assumes that the reserved bit
> is 0 (hopefully deliberately) and I would do the same in KVM as it
> simplifies the logic.  (We don't have to even think about migrations
> between machines with a different reserved value and making it play
> nicely with possible future implementations of that bit.)
> 
> Thanks.
> 
Thank you Radim and xiaoyao's review, I will improve it in the next 
version. Xiaoyao's suggestion remind me another thing. And I am 
wondering if we need to initialize the value of MSR_IA32_UMWAIT_CONTROL 
in KVM to 0x186a0(umwait_control = 100000, as host does).

Although the guest with new kernel(has umwait host patch)can initialize 
the value to 0x186a0. But there is a case that a guest with a old kernel 
and the host with the new kernel and has the cpuid of WAITPKG. Because 
the msr value is 0, the guest umwait will have no max time by default.

  parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-16  9:55 [PATCH RESEND v3 0/3] KVM: x86: Enable user wait instructions Tao Xu
2019-06-16  9:55 ` [PATCH RESEND v3 1/3] KVM: x86: add support for " Tao Xu
2019-06-16  9:55 ` [PATCH RESEND v3 2/3] KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL Tao Xu
2019-06-17  3:32   ` Xiaoyao Li
2019-06-17  6:31     ` Xiaoyao Li
2019-06-17 15:50       ` Radim Krčmář
2019-06-17 15:54         ` Xiaoyao Li
2019-06-18  2:40         ` Tao Xu [this message]
2019-06-18  3:04     ` Tao Xu
2019-06-16  9:55 ` [PATCH RESEND v3 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE Tao Xu

Reply instructions:

You may reply publically 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=afc6182b-8e78-5032-c579-35bf2732b740@intel.com \
    --to=tao3.xu@intel.com \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=jingqi.liu@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=tglx@linutronix.de \
    --cc=xiaoyao.li@linux.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

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox