|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 5/6] x86/hvm: Handle x2apic MSRs the new guest_{rd, wr}msr() infrastructure
Dispatch from the guest_{rd,wr}msr() functions. The read side should be safe
outside of current context, but the write side is definitely not. As the
toolstack has plausible reason to access the APIC registers via this interface
(not least because whether they are accessible at all depends on guest
settings), unilaterally reject access attempts outside of current context.
Rename to guest_{rd,wr}msr_x2apic() for consistency, and alter the functions
to use X86EMUL_EXCEPTION rather than X86EMUL_UNHANDLEABLE. The previous
callers turned UNHANDLEABLE into EXCEPTION, but using UNHANDLEABLE will now
interfere with the fallback to legacy MSR handling.
While altering guest_rdmsr_x2apic() make a couple of minor improvements.
Reformat the initialiser for readable[] so it indents in a more natural way,
and alter high to be a 64bit integer to avoid shifting 0 by 32 in the common
path.
Observant people might notice that we now don't let PV guests read the x2apic
MSRs. They should never have been able to in the first place.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
---
xen/arch/x86/hvm/hvm.c | 10 -------
xen/arch/x86/hvm/vlapic.c | 61 ++++++++++++++++++++++++-------------------
xen/arch/x86/msr.c | 15 +++++++++++
xen/include/asm-x86/hvm/hvm.h | 4 +--
4 files changed, 51 insertions(+), 39 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 91bc3e8..b10e05f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3432,11 +3432,6 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t
*msr_content)
*msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
break;
- case MSR_IA32_APICBASE_MSR ... MSR_IA32_APICBASE_MSR + 0x3ff:
- if ( hvm_x2apic_msr_read(v, msr, msr_content) )
- goto gp_fault;
- break;
-
case MSR_IA32_TSC_DEADLINE:
*msr_content = vlapic_tdt_msr_get(vcpu_vlapic(v));
break;
@@ -3591,11 +3586,6 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t
msr_content,
vlapic_tdt_msr_set(vcpu_vlapic(v), msr_content);
break;
- case MSR_IA32_APICBASE_MSR ... MSR_IA32_APICBASE_MSR + 0x3ff:
- if ( hvm_x2apic_msr_write(v, msr, msr_content) )
- goto gp_fault;
- break;
-
case MSR_IA32_CR_PAT:
if ( !hvm_set_guest_pat(v, msr_content) )
goto gp_fault;
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index e715729..74bbfd5 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -666,33 +666,32 @@ static int vlapic_read(
return X86EMUL_OKAY;
}
-int hvm_x2apic_msr_read(struct vcpu *v, unsigned int msr, uint64_t
*msr_content)
+int guest_rdmsr_x2apic(const struct vcpu *v, uint32_t msr, uint64_t *val)
{
- static const unsigned long readable[] =
- {
+ static const unsigned long readable[] = {
#define REG(x) (1UL << (APIC_ ## x >> 4))
- REG(ID) | REG(LVR) | REG(TASKPRI) | REG(PROCPRI) |
- REG(LDR) | REG(SPIV) | REG(ESR) | REG(ICR) |
- REG(CMCI) | REG(LVTT) | REG(LVTTHMR) | REG(LVTPC) |
- REG(LVT0) | REG(LVT1) | REG(LVTERR) | REG(TMICT) |
- REG(TMCCT) | REG(TDCR) |
+ REG(ID) | REG(LVR) | REG(TASKPRI) | REG(PROCPRI) |
+ REG(LDR) | REG(SPIV) | REG(ESR) | REG(ICR) |
+ REG(CMCI) | REG(LVTT) | REG(LVTTHMR) | REG(LVTPC) |
+ REG(LVT0) | REG(LVT1) | REG(LVTERR) | REG(TMICT) |
+ REG(TMCCT) | REG(TDCR) |
#undef REG
#define REGBLOCK(x) (((1UL << (NR_VECTORS / 32)) - 1) << (APIC_ ## x >> 4))
- REGBLOCK(ISR) | REGBLOCK(TMR) | REGBLOCK(IRR)
+ REGBLOCK(ISR) | REGBLOCK(TMR) | REGBLOCK(IRR)
#undef REGBLOCK
- };
+ };
const struct vlapic *vlapic = vcpu_vlapic(v);
- uint32_t high = 0, reg = msr - MSR_IA32_APICBASE_MSR, offset = reg << 4;
+ uint64_t high = 0;
+ uint32_t reg = msr - MSR_IA32_APICBASE_MSR, offset = reg << 4;
if ( !vlapic_x2apic_mode(vlapic) ||
(reg >= sizeof(readable) * 8) || !test_bit(reg, readable) )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
if ( offset == APIC_ICR )
- high = vlapic_read_aligned(vlapic, APIC_ICR2);
+ high = (uint64_t)vlapic_read_aligned(vlapic, APIC_ICR2) << 32;
- *msr_content = ((uint64_t)high << 32) |
- vlapic_read_aligned(vlapic, offset);
+ *val = high | vlapic_read_aligned(vlapic, offset);
return X86EMUL_OKAY;
}
@@ -983,49 +982,52 @@ int vlapic_apicv_write(struct vcpu *v, unsigned int
offset)
return X86EMUL_OKAY;
}
-int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t
msr_content)
+int guest_wrmsr_x2apic(struct vcpu *v, uint32_t msr, uint64_t msr_content)
{
struct vlapic *vlapic = vcpu_vlapic(v);
uint32_t offset = (msr - MSR_IA32_APICBASE_MSR) << 4;
+ /* The timer handling at least is unsafe outside of current context. */
+ ASSERT(v == current);
+
if ( !vlapic_x2apic_mode(vlapic) )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
switch ( offset )
{
case APIC_TASKPRI:
if ( msr_content & ~APIC_TPRI_MASK )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_SPIV:
if ( msr_content & ~(APIC_VECTOR_MASK | APIC_SPIV_APIC_ENABLED |
(VLAPIC_VERSION & APIC_LVR_DIRECTED_EOI
? APIC_SPIV_DIRECTED_EOI : 0)) )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_LVTT:
if ( msr_content & ~(LVT_MASK | APIC_TIMER_MODE_MASK) )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_LVTTHMR:
case APIC_LVTPC:
case APIC_CMCI:
if ( msr_content & ~(LVT_MASK | APIC_MODE_MASK) )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_LVT0:
case APIC_LVT1:
if ( msr_content & ~LINT_MASK )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_LVTERR:
if ( msr_content & ~LVT_MASK )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_TMICT:
@@ -1033,20 +1035,20 @@ int hvm_x2apic_msr_write(struct vcpu *v, unsigned int
msr, uint64_t msr_content)
case APIC_TDCR:
if ( msr_content & ~APIC_TDR_DIV_1 )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
break;
case APIC_ICR:
if ( (uint32_t)msr_content & ~(APIC_VECTOR_MASK | APIC_MODE_MASK |
APIC_DEST_MASK | APIC_INT_ASSERT |
APIC_INT_LEVELTRIG | APIC_SHORT_MASK) )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
break;
case APIC_SELF_IPI:
if ( msr_content & ~APIC_VECTOR_MASK )
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
offset = APIC_ICR;
msr_content = APIC_DEST_SELF | (msr_content & APIC_VECTOR_MASK);
break;
@@ -1054,13 +1056,18 @@ int hvm_x2apic_msr_write(struct vcpu *v, unsigned int
msr, uint64_t msr_content)
case APIC_EOI:
case APIC_ESR:
if ( msr_content )
+ {
default:
- return X86EMUL_UNHANDLEABLE;
+ goto gp_fault;
+ }
}
vlapic_reg_write(v, offset, msr_content);
return X86EMUL_OKAY;
+
+ gp_fault:
+ return X86EMUL_EXCEPTION;
}
static int vlapic_range(struct vcpu *v, unsigned long addr)
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 9f20fd8..3cb4158 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -139,6 +139,7 @@ int init_vcpu_msr_policy(struct vcpu *v)
int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
{
+ const struct vcpu *curr = current;
const struct domain *d = v->domain;
const struct cpuid_policy *cp = d->arch.cpuid;
const struct msr_domain_policy *dp = d->arch.msr;
@@ -175,6 +176,13 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr,
uint64_t *val)
_MSR_MISC_FEATURES_CPUID_FAULTING;
break;
+ case MSR_IA32_APICBASE_MSR ... MSR_IA32_APICBASE_MSR + 0x3ff:
+ if ( !is_hvm_domain(d) || v != curr )
+ goto gp_fault;
+
+ ret = guest_rdmsr_x2apic(v, msr, val);
+ goto out;
+
case 0x40000000 ... 0x400001ff:
if ( is_viridian_domain(d) )
{
@@ -270,6 +278,13 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
break;
}
+ case MSR_IA32_APICBASE_MSR ... MSR_IA32_APICBASE_MSR + 0x3ff:
+ if ( !is_hvm_domain(d) || v != curr )
+ goto gp_fault;
+
+ ret = guest_wrmsr_x2apic(v, msr, val);
+ goto out;
+
case 0x40000000 ... 0x400001ff:
if ( is_viridian_domain(d) )
{
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 031af12..a19b6ee 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -517,8 +517,8 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long
gla,
? (u32)__d->arch.incarnation : (u32)(v)->arch.hvm_vcpu.msr_tsc_aux; \
})
-int hvm_x2apic_msr_read(struct vcpu *v, unsigned int msr, uint64_t
*msr_content);
-int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t
msr_content);
+int guest_rdmsr_x2apic(const struct vcpu *v, uint32_t msr, uint64_t *val);
+int guest_wrmsr_x2apic(struct vcpu *v, uint32_t msr, uint64_t val);
/*
* Nested HVM
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |