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
|
Home