WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel][PATCH 02/02] VMX: Add HVM RDTSCP support

To: Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: RE: [Xen-devel][PATCH 02/02] VMX: Add HVM RDTSCP support
From: "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>
Date: Sat, 12 Dec 2009 08:23:33 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: Keir, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Fraser <keir.fraser@xxxxxxxxxxxxx>
Delivery-date: Fri, 11 Dec 2009 16:24:03 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <c87ca38b-8acd-4e0c-a559-45dcd44ed141@default>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <EADF0A36011179459010BDF5142A457501D13FE832@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <c87ca38b-8acd-4e0c-a559-45dcd44ed141@default>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acp6wGb184ibvFCeT3Gu8dJF0RKVQAAAEGqw
Thread-topic: [Xen-devel][PATCH 02/02] VMX: Add HVM RDTSCP support
Whether a system has rdtscp support is indicated by
the cpuid. Management tool or system admin should
use CPUIDdetermine whether the migration is allowed. 
I think besides RDTSCP, we already have such cases.

Thanks!
Dongxiao

Dan Magenheimer wrote:
> One other concern with this patch... is it possible
> for an HVM to migrate from a machine that has rdtscp
> support to a machine that does not?  If so, the
> migration could result in a nasty crash unless you
> also have code that emulates rdtscp if it is not
> present.  Some of the code exists for this already
> but I think you need to intercept illegal instruction
> traps, filter out the traps caused by rdtscp,
> and inject the rest.  (This is done already for
> PV domains.)
> 
>> -----Original Message-----
>> From: Xu, Dongxiao [mailto:dongxiao.xu@xxxxxxxxx]
>> Sent: Friday, December 11, 2009 4:33 PM
>> To: Jan Beulich
>> Cc: Dan Magenheimer; xen-devel@xxxxxxxxxxxxxxxxxxx; Keir Fraser
>> Subject: RE: [Xen-devel][PATCH 02/02] VMX: Add HVM RDTSCP support
>> 
>> 
>> Hi, Jan,
>>      There are two kinds of conditions for adding new MSRs,
>> let's call it CONDITION 1
>> and CONDITION 2.
>> 1. Developer adding new MSR both in msr_index[] and the enum.
>> 2. Developer adding new MSR *ONLY* in enum. (Like TSC_AUX MSR)
>> 
>> My original thought is that, since both msr_index and enum is seldom
>> modified, so I add that BUILD_BUG_ON(MSR_INDEX_SIZE != 3) to
>> reminder the developer that currently we only save three MSRs for
>> host state. However, as you said, 
>> he need to adjust the
>> BUILD_BUG_ON Condition each time for "CONDITION 1". But if he only
>> adds MSR into enum (CONDITION 2, also like the TSC_AUX MSR), then he
>> is not needed to modify the BUILD_BUG_ON Condition.
>> 
>> I understand what you mean now. This is to reminder developer that,
>> if he want to add new MSR both into the enum and msr_index[], it
>> should be 
>> placed in the end of msr_index[],
>> but BEFORE VMX_INDEX_MSR_TSC_AUX in enum. Then he is not
>> needed to modify he
>> following condition check.
>> "ARRAY_SIZE(msr_index) == VMX_INDEX_MSR_TSC_AUX &&
>> VMX_INDEX_MSR_TSC_AUX == VMX_MSR_COUNT - 1".
>> But it he ONLY wants to add the MSR in enum (CONDITION 2,
>> also like the TSC_AUX MSR),
>> the modifcation to BUILD_BUG_ON condition is needed:
>> "ARRAY_SIZE(msr_index) == VMX_INDEX_MSR_TSC_AUX &&
>> VMX_INDEX_MSR_TSC_AUX == VMX_MSR_COUNT - 2".
>> 
>> Both approaches have the effect to reminder that, MSRs in
>> enum is not the same as MSRs
>> in msr_index[]. Anyway I attached the patch according to your
>> suggestion. 
>> 
>> Jan Beulich wrote:
>>>>>> "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx> 11.12.09 17:22 >>>
>>>>    Here is the updated one, I add the BUILD_BUG_ON() and some
>>>> comments. 
>>> 
>>> I still don't think that's what we want: It's not a bug if
>>> MSR_INDEX_SIZE is not 3. A BUILD_BUG_ON() should check things that
>>> ought to be guaranteed, but shouldn't require adjustment each time
>>> some table gets expanded/shrunk. What you want to minimally check is
>>> ARRAY_SIZE(msr_index) == VMX_INDEX_MSR_TSC_AUX &&
>>> VMX_INDEX_MSR_TSC_AUX == VMX_MSR_COUNT - 1.
>>> 
>>> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel