[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 2/2] Xen: Fix VMCS setting for x2APIC mode guest while enabling APICV



>>> On 29.01.13 at 16:14, "Li, Jiongxi" <jiongxi.li@xxxxxxxxx> wrote:
>> > +            set_bit(msr, msr_bitmap + 0x000/BYTES_PER_LONG); /*
>> read-low */
>> > +        if (type & MSR_TYPE_W)
>> > +            set_bit(msr, msr_bitmap + 0x800/BYTES_PER_LONG); /*
>> write-low */
>> > +    }
>> > +    else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
>> > +    {
>> > +        msr &= 0x1fff;
>> > +        if (type & MSR_TYPE_R)
>> > +            set_bit(msr, msr_bitmap + 0x400/BYTES_PER_LONG); /*
>> read-high */
>> > +        if (type & MSR_TYPE_W)
>> > +            set_bit(msr, msr_bitmap + 0xc00/BYTES_PER_LONG); /*
>> write-high */
>> 
>> I believe that while the corresponding disable function is fine in
>> this regard, here you need to do something in a final "else":
>> A disable not having any effect is fine (we'll still get the
>> intercept), but an enable not having any effect is a problem. So
>> I'd suggest adding a one-time warning and/or ASSERT(0) there.
> 
> A final "else" means out of the ranges 00000000H - 00001FFFH and
> C0000000H - C0001FFFH. According to SDM, RDMSR and WRMSR
> out of the ranges will cause a VM exit, it is just what we want for
> "enable intercept" function, right?. So is it necessary to handle 
> "else" case here?

Then maybe it doesn't make that much sense. I just wondered
how one would get notified of having tried to pass an out of
range MSR index to one of these functions. Right now, you'd
have to go hunt for this when - at least in a debug build - you
could be pointed right at the wrong invocation by a stack trace.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.