Xen devel - Mar 2007 - [PATCH] Proper use of VMX execution controls MSR.

Better use of VMX execution controls MSR.

Signed-off-by: Xin Li<xin.b.li@intel.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 28/3/07 16:51, "Li, Xin B" <xin.b.li@intel.com> wrote:
> Better use of VMX execution controls MSR.
> 
> Signed-off-by: Xin Li<xin.b.li@intel.com>
Is this actually to fix a problem with a future processor?

This whole bit-forcing thing seems extremely odd to me. We set the controls
that Xen currently needs to do its job as a VMM properly -- we can''t
just
clear some of those controls because the processor says to do so. So I think
our current treatment of the MSR high bits is appropriate (if it tells us to
zero one of the control bits that we make use of, we are in trouble -- we
have a processor that isn''t backwards compatible!).

I also feel uneasy about setting extra bits (as specified by the MSR low
bits), but I reason that if we are told to set bits of flags which are
currently architecturally-undefined then it is reasonable to let the
processor tell us what to do with them. Which is why I do respect the MSR
low bits.

Why did Intel ever choose this insane scheme? Would it have been so hard to
have defined bitmaps with set-to-enable semantics, and always require zero
for reserved bits? Actually I suppose you do have set-to-enable semantics
(otherwise my current asymmetric treatment of MSR high and low words would
not make sense). But all this messing with setting vs. clearing reserved
bits seems pretty stupid.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

The patch conforms to the spec strictly, but I also think the spec is really
odd...
Seems Keir hasn''t checked it in.

 -- Dexuan
>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
>Sent: 2007年3月29日 2:02
>To: Li, Xin B; xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] [PATCH] Proper use of VMX execution controls MSR.
>
>On 28/3/07 16:51, "Li, Xin B" <xin.b.li@intel.com> wrote:
>
>> Better use of VMX execution controls MSR.
>>
>> Signed-off-by: Xin Li<xin.b.li@intel.com>
>
>Is this actually to fix a problem with a future processor?
>
>This whole bit-forcing thing seems extremely odd to me. We set the controls
>that Xen currently needs to do its job as a VMM properly -- we
can''t just
>clear some of those controls because the processor says to do so. So I think
>our current treatment of the MSR high bits is appropriate (if it tells us to
>zero one of the control bits that we make use of, we are in trouble -- we
>have a processor that isn''t backwards compatible!).
>
>I also feel uneasy about setting extra bits (as specified by the MSR low
>bits), but I reason that if we are told to set bits of flags which are
>currently architecturally-undefined then it is reasonable to let the
>processor tell us what to do with them. Which is why I do respect the MSR
>low bits.
>
>Why did Intel ever choose this insane scheme? Would it have been so hard to
>have defined bitmaps with set-to-enable semantics, and always require zero
>for reserved bits? Actually I suppose you do have set-to-enable semantics
>(otherwise my current asymmetric treatment of MSR high and low words would
>not make sense). But all this messing with setting vs. clearing reserved
>bits seems pretty stupid.
>
> -- Keir
>
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser wrote:
> On 28/3/07 16:51, "Li, Xin B" <xin.b.li@intel.com> wrote:
> 
>> Better use of VMX execution controls MSR.
>> 
>> Signed-off-by: Xin Li<xin.b.li@intel.com>
> 
> Is this actually to fix a problem with a future processor?
> 
> This whole bit-forcing thing seems extremely odd to me. We set the
> controls that Xen currently needs to do its job as a VMM properly --
> we can''t just clear some of those controls because the processor
says
> to do so. So I think our current treatment of the MSR high bits is
> appropriate (if it tells us to zero one of the control bits that we
> make use of, we are in trouble -- we have a processor that isn''t
> backwards compatible!). 
> 
Right. BUG_ON() is correct because the processor does not meet the
programmer''s assumption. 
> I also feel uneasy about setting extra bits (as specified by the MSR
> low bits), but I reason that if we are told to set bits of flags
> which are currently architecturally-undefined then it is reasonable
> to let the processor tell us what to do with them. Which is why I do
> respect the MSR low bits.
> 
This is okay because newer processors simply provide more settings, i.e
1 => 0 or 1. The code usually is written with the setting = 1. Some VMM
may use the setting 0 for new processors if it can benefit from that.
> 
>  -- Keir
Jun
---
Intel Open Source Technology Center

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>On 28/3/07 16:51, "Li, Xin B" <xin.b.li@intel.com> wrote:
>
>> Better use of VMX execution controls MSR.
>> 
>> Signed-off-by: Xin Li<xin.b.li@intel.com>
>
>Is this actually to fix a problem with a future processor?
No, this is just for better coding for hardware features that may not be
available
on all processors.
>This whole bit-forcing thing seems extremely odd to me. We set the
controls
>that Xen currently needs to do its job as a VMM properly -- we
can''t
just
>clear some of those controls because the processor says to do so. So I
think
>our current treatment of the MSR high bits is appropriate (if it tells
us to
>zero one of the control bits that we make use of, we are in trouble --
we
>have a processor that isn''t backwards compatible!).
I also think its appropriate.
>I also feel uneasy about setting extra bits (as specified by the MSR
low bits),
>but I reason that if we are told to set bits of flags which are
currently 
>architecturally-undefined then it is reasonable to let the processor
tell us what
>to do with them. Which is why I do respect the MSR low bits.
Yes, we must repect MSR low bits.
>
>Why did Intel ever choose this insane scheme? Would it have been so
hard to
>have defined bitmaps with set-to-enable semantics, and always require
zero
>for reserved bits? Actually I suppose you do have set-to-enable
semantics
>(otherwise my current asymmetric treatment of MSR high and low words
would
>not make sense). But all this messing with setting vs. clearing
reserved
>bits seems pretty stupid.
Yeah, I think the basic logic is still set-to-enable semantics here.

I made this patch when we add VMX MSR bitmap support, which is used to
avoid 
FS_BASE and GS_BASE MSR accesses VMExits in 64 bit guests. But some
early
precessors may not support this feature, so the MSR reports 0 in high
bits, meaning we
must set the bit to zero in VMCS.  So a straightforward way is to check
if the feature
is availale and then set to VMCS accordingly. But we have lots of
hardware features,
with new features being added to hareware, and I really don''t like to
add the almost
same code just for another new feature detection.  The idea here is to
use the control MSR
to adjust the input controls we _hardcoded_ in Xen, for the features
we''ve supported in Xen,
we always set to 1 in the input control, like in
MONITOR_CPU_BASED_EXEC_CONTROLS,
then on processors supporting this feature, We get 1, while on
processors not supporting this
feature, we get 0.  This adjusted output control value can be also used
to choose code
path on different processors, and the attached patch can be an example.
-Xin


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Okay, I see. Well, that makes sense but I think we should have two masks for
each control word -- the bits we would like to set, and the bits we must be
able to set. Also your previous patch added an unnecessary AND with
''msr_hi
XOR msr_lo''.

This is pretty trivial to fix up. I''ll do a patch.

 -- Keir

On 29/3/07 07:22, "Li, Xin B" <xin.b.li@intel.com> wrote:
> But we have lots of
> hardware features,
> with new features being added to hareware, and I really don''t like
to
> add the almost
> same code just for another new feature detection.  The idea here is to
> use the control MSR
> to adjust the input controls we _hardcoded_ in Xen, for the features
> we''ve supported in Xen,
> we always set to 1 in the input control, like in
> MONITOR_CPU_BASED_EXEC_CONTROLS,
> then on processors supporting this feature, We get 1, while on
> processors not supporting this
> feature, we get 0.  This adjusted output control value can be also used
> to choose code
> path on different processors, and the attached patch can be an example.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>Okay, I see. Well, that makes sense but I think we should have two
masks for
>each control word -- the bits we would like to set, and the bits we
must be
>able to set. Also your previous patch added an unnecessary AND 
>with ''msr_hi XOR msr_lo''.
No, it''s necessary. According to Intel Spec:
If the bit in msr_lo is 1, then the control bit written to VMCS must be
1
If the bit in msr_hi is 0, then the control bit written to VMCS must be
0 

The msr_lo and msr_hi combinations are:
1) L = 0 and H = 0: the adjusted control bit is 0, no matter what''s the
input control value, that''s why we need AND msr_hi.
2) L = 0 and H = 1: the adjusted control bit can be 0 or 1, depends on
if we support it.
3) L = 1 and H = 0: INVALID combination!!!
4) L = 1 and H = 1: the adjusted control bit is 1, no matter what''s the
input control value, that''s why we need OR msr_lo.

Specially for case 2, 
if we have the feature support in Xen, we should set the input control
bit, and expect the adjusted value to be 1;
if we don''t have the feature support in Xen, we should clear the input
control bit, and expect adjusted value to be 0;
That''s meaning the adjusted output is determined by the input control.
So the expression should be "AND (msr_hi XOR msr_lo)".

-Xin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 29/3/07 09:37, "Li, Xin B" <xin.b.li@intel.com> wrote:
> if we have the feature support in Xen, we should set the input control
> bit, and expect the adjusted value to be 1;
> if we don''t have the feature support in Xen, we should clear the
input
> control bit, and expect adjusted value to be 0;
> That''s meaning the adjusted output is determined by the input
control.
> So the expression should be "AND (msr_hi XOR msr_lo)".
The subsequent AND with msr_hi and the OR with msr_lo has exactly the same
effect. So the AND (msr_hi XOR msr_lo) is not incorrect, but it is
redundant.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel