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

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

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