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

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

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