|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen staging] x86/hvm: Handle x2apic MSRs via the new guest_{rd, wr}msr() infrastructure
commit 7824baee56248474346da138b906a3a5c5420458
Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Mon Feb 26 12:45:58 2018 +0000
Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Wed Dec 5 21:06:47 2018 +0000
x86/hvm: Handle x2apic MSRs via 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 no legitimate 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>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
xen/arch/x86/hvm/hvm.c | 10 -------
xen/arch/x86/hvm/vlapic.c | 65 +++++++++++++++++++++++++------------------
xen/arch/x86/msr.c | 15 ++++++++++
xen/include/asm-x86/hvm/hvm.h | 4 +--
4 files changed, 55 insertions(+), 39 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 46cb92e9bc..0039e8cf38 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3414,11 +3414,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_X2APIC_FIRST ... MSR_X2APIC_LAST:
- 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;
@@ -3578,11 +3573,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_X2APIC_FIRST ... MSR_X2APIC_LAST:
- 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 05afb7aed6..1f58b36499 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -649,33 +649,39 @@ static int vlapic_mmio_read(struct vcpu *v, unsigned long
address,
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_X2APIC_FIRST, offset = reg << 4;
+ uint64_t high = 0;
+ uint32_t reg = msr - MSR_X2APIC_FIRST, offset = reg << 4;
+
+ /*
+ * The read side looks as if it might be safe to use outside of current
+ * context, but the write side is most certainly not. As we don't need
+ * any non-current access, enforce symmetry with the write side.
+ */
+ ASSERT(v == current);
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;
}
@@ -957,49 +963,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_X2APIC_FIRST) << 4;
+ /* The timer handling at least is unsafe outside of current context. */
+ ASSERT(v == current);
+
if ( !vlapic_x2apic_mode(vlapic) )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
switch ( offset )
{
case APIC_TASKPRI:
if ( msr_content & ~APIC_TPRI_MASK )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
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;
+ return X86EMUL_EXCEPTION;
break;
case APIC_LVTT:
if ( msr_content & ~(LVT_MASK | APIC_TIMER_MODE_MASK) )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
break;
case APIC_LVTTHMR:
case APIC_LVTPC:
case APIC_CMCI:
if ( msr_content & ~(LVT_MASK | APIC_MODE_MASK) )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
break;
case APIC_LVT0:
case APIC_LVT1:
if ( msr_content & ~LINT_MASK )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
break;
case APIC_LVTERR:
if ( msr_content & ~LVT_MASK )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
break;
case APIC_TMICT:
@@ -1007,20 +1016,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;
+ return X86EMUL_EXCEPTION;
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;
+ return X86EMUL_EXCEPTION;
vlapic_set_reg(vlapic, APIC_ICR2, msr_content >> 32);
break;
case APIC_SELF_IPI:
if ( msr_content & ~APIC_VECTOR_MASK )
- return X86EMUL_UNHANDLEABLE;
+ return X86EMUL_EXCEPTION;
offset = APIC_ICR;
msr_content = APIC_DEST_SELF | (msr_content & APIC_VECTOR_MASK);
break;
@@ -1028,8 +1037,10 @@ 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;
+ return X86EMUL_EXCEPTION;
+ }
}
vlapic_reg_write(v, offset, msr_content);
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 76cb6efa52..85a58c0b58 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -117,6 +117,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_policy *mp = d->arch.msr;
@@ -150,6 +151,13 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr,
uint64_t *val)
*val = msrs->misc_features_enables.raw;
break;
+ case MSR_X2APIC_FIRST ... MSR_X2APIC_LAST:
+ if ( !is_hvm_domain(d) || v != curr )
+ goto gp_fault;
+
+ ret = guest_rdmsr_x2apic(v, msr, val);
+ break;
+
case 0x40000000 ... 0x400001ff:
if ( is_viridian_domain(d) )
{
@@ -297,6 +305,13 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
break;
}
+ case MSR_X2APIC_FIRST ... MSR_X2APIC_LAST:
+ if ( !is_hvm_domain(d) || v != curr )
+ goto gp_fault;
+
+ ret = guest_wrmsr_x2apic(v, msr, val);
+ break;
+
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 3d3250dff0..d68604127f 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -333,8 +333,8 @@ void hvm_toggle_singlestep(struct vcpu *v);
int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
struct npfec npfec);
-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);
/* Check CR4/EFER values */
const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
--
generated by git-patchbot for /home/xen/git/xen.git#staging
_______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |