[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH 1/5] x86: Fix calculation of %dr6/7 reserved bits
From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> The reserved bit calculations for %dr6 and %dr7 depend on whether the VM has the Restricted Transnational Memory feature available. Introduce adjust_dr{6,7}_rsvd() and replace the opencoded logic and constants (except for DR_STATUS_RESERVED_ONE which is (mis)used elsewhere and will be removed after future bugfixes). The use of these helpers in set_debugreg() covers toolstack values for PV guests, but HVM guests need similar treatment. The use of the guests cpuid policy is less than optimal in the create/restore paths. However in such cases, the policy will be the guest maximum policy, which will be more permissive with respect to the RTM feature. Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx> --- CC: Jan Beulich <JBeulich@xxxxxxxx> CC: Wei Liu <wl@xxxxxxx> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> --- xen/arch/x86/domain.c | 7 +++-- xen/arch/x86/hvm/hvm.c | 6 ++-- xen/arch/x86/include/asm/debugreg.h | 44 +++++++++++++++++++++++++---- xen/arch/x86/pv/misc-hypercalls.c | 16 +++-------- 4 files changed, 50 insertions(+), 23 deletions(-) diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index fe86a7f853..a39710b5af 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -1053,6 +1053,7 @@ int arch_set_info_guest( struct vcpu *v, vcpu_guest_context_u c) { struct domain *d = v->domain; + const struct cpu_policy *cp = d->arch.cpuid; unsigned int i; unsigned long flags; bool compat; @@ -1165,10 +1166,10 @@ int arch_set_info_guest( if ( is_hvm_domain(d) ) { - for ( i = 0; i < ARRAY_SIZE(v->arch.dr); ++i ) + for ( i = 0; i < ARRAY_SIZE(v->arch.dr) - 2; ++i ) v->arch.dr[i] = c(debugreg[i]); - v->arch.dr6 = c(debugreg[6]); - v->arch.dr7 = c(debugreg[7]); + v->arch.dr6 = adjust_dr6_rsvd(c(debugreg[6]), cp->feat.rtm); + v->arch.dr7 = adjust_dr7_rsvd(c(debugreg[7]), cp->feat.rtm); if ( v->vcpu_id == 0 ) d->vm_assist = c.nat->vm_assist; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 3a99c0ff20..66ead0b878 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -33,6 +33,7 @@ #include <asm/shadow.h> #include <asm/hap.h> #include <asm/current.h> +#include <asm/debugreg.h> #include <asm/e820.h> #include <asm/io.h> #include <asm/regs.h> @@ -985,6 +986,7 @@ unsigned long hvm_cr4_guest_valid_bits(const struct domain *d) static int cf_check hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) { + const struct cpu_policy *cp = d->arch.cpuid; unsigned int vcpuid = hvm_load_instance(h); struct vcpu *v; struct hvm_hw_cpu ctxt; @@ -1166,8 +1168,8 @@ static int cf_check hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h) v->arch.dr[1] = ctxt.dr1; v->arch.dr[2] = ctxt.dr2; v->arch.dr[3] = ctxt.dr3; - v->arch.dr6 = ctxt.dr6; - v->arch.dr7 = ctxt.dr7; + v->arch.dr6 = adjust_dr6_rsvd(ctxt.dr6, cp->feat.rtm); + v->arch.dr7 = adjust_dr7_rsvd(ctxt.dr7, cp->feat.rtm); hvmemul_cancel(v); diff --git a/xen/arch/x86/include/asm/debugreg.h b/xen/arch/x86/include/asm/debugreg.h index 86aa6d7143..8be60910b4 100644 --- a/xen/arch/x86/include/asm/debugreg.h +++ b/xen/arch/x86/include/asm/debugreg.h @@ -10,9 +10,18 @@ #define DR_STATUS 6 #define DR_CONTROL 7 -/* Define a few things for the status register. We can use this to determine - which debugging register was responsible for the trap. The other bits - are either reserved or not of interest to us. */ +/* + * DR6 status bits. + * N.B. For backwards compatibility, X86_DR6_RTM has inverted polarity. + */ +#define X86_DR6_B0 (1u << 0) /* Breakpoint 0 triggered */ +#define X86_DR6_B1 (1u << 1) /* Breakpoint 1 triggered */ +#define X86_DR6_B2 (1u << 2) /* Breakpoint 2 triggered */ +#define X86_DR6_B3 (1u << 3) /* Breakpoint 3 triggered */ +#define X86_DR6_BD (1u << 13) /* Debug register accessed */ +#define X86_DR6_BS (1u << 14) /* Single step */ +#define X86_DR6_BT (1u << 15) /* Task switch */ +#define X86_DR6_RTM (1u << 16) /* #DB/#BP in RTM region */ #define DR_TRAP0 (0x1) /* db0 */ #define DR_TRAP1 (0x2) /* db1 */ @@ -21,7 +30,6 @@ #define DR_STEP (0x4000) /* single-step */ #define DR_SWITCH (0x8000) /* task switch */ #define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */ -#define DR_STATUS_RESERVED_ZERO (~0xffffefffUL) /* Reserved, read as zero */ #define DR_STATUS_RESERVED_ONE 0xffff0ff0UL /* Reserved, read as one */ /* Now define a bunch of things for manipulating the control register. @@ -61,8 +69,6 @@ We can slow the instruction pipeline for instructions coming via the gdt or the ldt if we want to. I am not sure why this is an advantage */ -#define DR_CONTROL_RESERVED_ZERO (~0xffff27ffUL) /* Reserved, read as zero */ -#define DR_CONTROL_RESERVED_ONE (0x00000400UL) /* Reserved, read as one */ #define DR_LOCAL_EXACT_ENABLE (0x00000100UL) /* Local exact enable */ #define DR_GLOBAL_EXACT_ENABLE (0x00000200UL) /* Global exact enable */ #define DR_RTM_ENABLE (0x00000800UL) /* RTM debugging enable */ @@ -80,4 +86,30 @@ long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value); void activate_debugregs(const struct vcpu *); +static inline unsigned long adjust_dr6_rsvd(unsigned long dr6, bool rtm) +{ + /* + * DR6: Bits 4-11,17-31 reserved (set to 1). + * Bit 16 reserved (set to 1) if RTM unavailable. + * Bit 12 reserved (set to 0). + */ + dr6 |= 0xfffe0ff0 | (rtm ? 0 : X86_DR6_RTM); + dr6 &= 0xffffefff; + + return dr6; +} + +static inline unsigned long adjust_dr7_rsvd(unsigned long dr7, bool rtm) +{ + /* + * DR7: Bit 10 reserved (set to 1). + * Bit 11 reserved (set to 0) if RTM unavailable. + * Bits 12,14-15 reserved (set to 0). + */ + dr7 |= 0x00000400; + dr7 &= 0xffff23ff & (rtm ? 0 : ~DR_RTM_ENABLE); + + return dr7; +} + #endif /* _X86_DEBUGREG_H */ diff --git a/xen/arch/x86/pv/misc-hypercalls.c b/xen/arch/x86/pv/misc-hypercalls.c index b11bd718b7..e44f2556c8 100644 --- a/xen/arch/x86/pv/misc-hypercalls.c +++ b/xen/arch/x86/pv/misc-hypercalls.c @@ -56,6 +56,7 @@ long do_fpu_taskswitch(int set) long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value) { struct vcpu *curr = current; + const struct cpu_policy *cp = curr->domain->arch.cpuid; switch ( reg ) { @@ -86,12 +87,7 @@ long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value) if ( value != (uint32_t)value ) return -EINVAL; - /* - * DR6: Bits 4-11,16-31 reserved (set to 1). - * Bit 12 reserved (set to 0). - */ - value &= ~DR_STATUS_RESERVED_ZERO; /* reserved bits => 0 */ - value |= DR_STATUS_RESERVED_ONE; /* reserved bits => 1 */ + value = adjust_dr6_rsvd(value, cp->feat.rtm); v->arch.dr6 = value; if ( v == curr ) @@ -108,12 +104,8 @@ long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value) if ( value != (uint32_t)value ) return -EINVAL; - /* - * DR7: Bit 10 reserved (set to 1). - * Bits 11-12,14-15 reserved (set to 0). - */ - value &= ~DR_CONTROL_RESERVED_ZERO; /* reserved bits => 0 */ - value |= DR_CONTROL_RESERVED_ONE; /* reserved bits => 1 */ + value = adjust_dr7_rsvd(value, cp->feat.rtm); + /* * Privileged bits: * GD (bit 13): must be 0. -- 2.41.0
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |