|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 4/6] hvm/mtrr: copy hardware state for Dom0
>>> On 15.05.18 at 16:36, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -185,6 +185,30 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v)
> ((uint64_t)PAT_TYPE_UC_MINUS << 48) | /* PAT6: UC- */
> ((uint64_t)PAT_TYPE_UNCACHABLE << 56); /* PAT7: UC */
>
> + if ( is_hardware_domain(v->domain) )
> + {
> + /* Copy values from the host. */
> + struct domain *d = v->domain;
> + unsigned int i;
> +
> + if ( mtrr_state.have_fixed )
> + for ( i = 0; i < NUM_FIXED_MSR; i++ )
> + mtrr_fix_range_msr_set(d, m, i,
> + ((uint64_t
> *)mtrr_state.fixed_ranges)[i]);
> +
> + for ( i = 0; i < num_var_ranges; i++ )
> + {
> + mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSBASE(i),
> + mtrr_state.var_ranges[i].base);
> + mtrr_var_range_msr_set(d, m, MSR_IA32_MTRR_PHYSMASK(i),
> + mtrr_state.var_ranges[i].mask);
> + }
> +
> + mtrr_def_type_msr_set(d, m,
> + mtrr_state.def_type |
> + (mtrr_state.enabled << MTRRdefType_FE_SHIFT));
This is all very clumsy (not your fault of course) - in particular, the
"enabled"
field is a two-bit value. I'd rather not see this to continue to be that way,
and hence I've created a patch to clean this up (see below; I'm intentionally
retaining my own TODO notes in there for your reference, and it's also
unlikely to apply as is because it sits on top of other changes). In that light
I
think I'd prefer if you either (later) re-based this onto my patch or reverted
back to the use of the literal 10 here.
Jan
TODO: rename _enabled to enabled for submission (but keep it this way here to
make sure further possibly necessary adjustments are noticed)
TODO: replace 10 and 11 by manifest constants (Roger supposedly will add those)
x86/mtrr: split "enabled" field into two boolean flags
The code hopefully is more readable this way.
Also switch have_fixed to bool, seeing that it already is used as a
boolean.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
--- unstable.orig/xen/arch/x86/cpu/mtrr/generic.c
+++ unstable/xen/arch/x86/cpu/mtrr/generic.c
@@ -79,7 +79,8 @@ void __init get_mtrr_state(void)
rdmsrl(MSR_MTRRdefType, msr_content);
mtrr_state.def_type = (msr_content & 0xff);
- mtrr_state.enabled = (msr_content & 0xc00) >> 10;
+ mtrr_state._enabled = (msr_content >> 11) & 1;
+ mtrr_state.fixed_enabled = (msr_content >> 10) & 1;
/* Store mtrr_cap for HVM MTRR virtualisation. */
rdmsrl(MSR_MTRRcap, mtrr_state.mtrr_cap);
@@ -158,7 +159,7 @@ static void __init print_mtrr_state(cons
unsigned int base = 0, step = 0x10000;
printk("%sMTRR fixed ranges %sabled:\n", level,
- mtrr_state.enabled & 1 ? "en" : "dis");
+ mtrr_state.fixed_enabled ? "en" : "dis");
for (; block->ranges; ++block, step >>= 2) {
for (i = 0; i < block->ranges; ++i, fr += 8) {
print_fixed(base, step, fr, level);
@@ -168,7 +169,7 @@ static void __init print_mtrr_state(cons
print_fixed_last(level);
}
printk("%sMTRR variable ranges %sabled:\n", level,
- mtrr_state.enabled & 2 ? "en" : "dis");
+ mtrr_state._enabled ? "en" : "dis");
width = (paddr_bits - PAGE_SHIFT + 3) / 4;
for (i = 0; i < num_var_ranges; ++i) {
@@ -382,8 +383,11 @@ static unsigned long set_mtrr_state(void
/* Set_mtrr_restore restores the old value of MTRRdefType,
so to set it we fiddle with the saved value */
if ((deftype & 0xff) != mtrr_state.def_type
- || ((deftype & 0xc00) >> 10) != mtrr_state.enabled) {
- deftype = (deftype & ~0xcff) | mtrr_state.def_type |
(mtrr_state.enabled << 10);
+ || ((deftype >> 11) & 1) != mtrr_state._enabled
+ || ((deftype >> 10) & 1) != mtrr_state.fixed_enabled) {
+ deftype = (deftype & ~0xcff) | mtrr_state.def_type |
+ (mtrr_state._enabled << 11) |
+ (mtrr_state.fixed_enabled << 10);
change_mask |= MTRR_CHANGE_MASK_DEFTYPE;
}
--- unstable.orig/xen/arch/x86/hvm/hvm.c
+++ unstable/xen/arch/x86/hvm/hvm.c
@@ -3497,8 +3497,9 @@ int hvm_msr_read_intercept(unsigned int
case MSR_MTRRdefType:
if ( !d->arch.cpuid->basic.mtrr )
goto gp_fault;
- *msr_content = v->arch.hvm_vcpu.mtrr.def_type
- | (v->arch.hvm_vcpu.mtrr.enabled << 10);
+ *msr_content = v->arch.hvm_vcpu.mtrr.def_type |
+ (v->arch.hvm_vcpu.mtrr._enabled << 11) |
+ (v->arch.hvm_vcpu.mtrr.fixed_enabled << 10);
break;
case MSR_MTRRfix64K_00000:
if ( !d->arch.cpuid->basic.mtrr )
--- unstable.orig/xen/arch/x86/hvm/mtrr.c
+++ unstable/xen/arch/x86/hvm/mtrr.c
@@ -195,11 +195,11 @@ static int get_mtrr_type(const struct mt
uint64_t mask = -(uint64_t)PAGE_SIZE << order;
unsigned int seg, num_var_ranges = m->mtrr_cap & 0xff;
- if ( unlikely(!(m->enabled & 0x2)) )
+ if ( unlikely(!m->_enabled) )
return MTRR_TYPE_UNCACHABLE;
pa &= mask;
- if ( (pa < 0x100000) && (m->enabled & 1) )
+ if ( (pa < 0x100000) && m->fixed_enabled )
{
/* Fixed range MTRR takes effect. */
uint32_t addr = (uint32_t)pa, index;
@@ -391,7 +391,8 @@ bool_t mtrr_def_type_msr_set(struct doma
uint64_t msr_content)
{
uint8_t def_type = msr_content & 0xff;
- uint8_t enabled = (msr_content >> 10) & 0x3;
+ uint8_t fixed_enabled = (msr_content >> 10) & 1;
+ uint8_t enabled = (msr_content >> 11) & 1;
if ( unlikely(!valid_mtrr_type(def_type)) )
{
@@ -406,10 +407,12 @@ bool_t mtrr_def_type_msr_set(struct doma
return 0;
}
- if ( m->enabled != enabled || m->def_type != def_type )
+ if ( m->_enabled != enabled || m->def_type != def_type ||
+ m->fixed_enabled != fixed_enabled )
{
- m->enabled = enabled;
+ m->_enabled = enabled;
m->def_type = def_type;
+ m->fixed_enabled = fixed_enabled;
memory_type_changed(d);
}
@@ -432,7 +435,7 @@ bool_t mtrr_fix_range_msr_set(struct dom
fixed_range_base[row] = msr_content;
- if ( m->enabled == 3 )
+ if ( m->_enabled && m->fixed_enabled )
memory_type_changed(d);
}
@@ -470,7 +473,7 @@ bool_t mtrr_var_range_msr_set(
m->overlapped = is_var_mtrr_overlapped(m);
- if ( m->enabled & 2 )
+ if ( m->_enabled )
memory_type_changed(d);
return 1;
@@ -481,10 +484,10 @@ bool mtrr_pat_not_equal(const struct vcp
const struct mtrr_state *md = &vd->arch.hvm_vcpu.mtrr;
const struct mtrr_state *ms = &vs->arch.hvm_vcpu.mtrr;
- if ( (md->enabled ^ ms->enabled) & 2 )
+ if ( md->_enabled != ms->_enabled )
return true;
- if ( md->enabled & 2 )
+ if ( md->_enabled )
{
unsigned int num_var_ranges = (uint8_t)md->mtrr_cap;
@@ -493,10 +496,10 @@ bool mtrr_pat_not_equal(const struct vcp
return true;
/* Test fixed ranges. */
- if ( (md->enabled ^ ms->enabled) & 1 )
+ if ( md->fixed_enabled != ms->fixed_enabled )
return true;
- if ( (md->enabled & 1) &&
+ if ( md->fixed_enabled &&
memcmp(md->fixed_ranges, ms->fixed_ranges,
sizeof(md->fixed_ranges)) )
return true;
@@ -684,7 +687,8 @@ static int hvm_save_mtrr_msr(struct doma
const struct mtrr_state *mtrr_state = &v->arch.hvm_vcpu.mtrr;
struct hvm_hw_mtrr hw_mtrr = {
.msr_mtrr_def_type = mtrr_state->def_type |
- (mtrr_state->enabled << 10),
+ (mtrr_state->fixed_enabled << 10) |
+ (mtrr_state->_enabled << 11),
.msr_mtrr_cap = mtrr_state->mtrr_cap,
};
unsigned int i;
--- unstable.orig/xen/include/asm-x86/mtrr.h
+++ unstable/xen/include/asm-x86/mtrr.h
@@ -50,8 +50,9 @@ struct mtrr_var_range {
struct mtrr_state {
struct mtrr_var_range *var_ranges;
mtrr_type fixed_ranges[NUM_FIXED_RANGES];
- unsigned char enabled;
- unsigned char have_fixed;
+ bool _enabled;
+ bool fixed_enabled;
+ bool have_fixed;
mtrr_type def_type;
u64 mtrr_cap;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |