| Message ID | 20181018204303.GO573482@lisa.in-ulm.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Fix definition and use of DR6_RESERVED | expand |
On Thu, Oct 18, 2018 at 1:43 PM, Christian Ehrhardt <lk@c--e.de> wrote: > > Bit 16 in %dr6 (RTM) is no longer reserved. > > Adjust the definition of DR6_RESERVED and provide new constants > DR6_INIT (moved from kvm_host.h) and DR_RTM. Fix existing users > of DR6_RESERVED that want DR6_INIT instead. Since DR6_RESERVED is visible in /usr/include, I don't see how you can claim to fix existing users of DR6_RESERVED that want DR6_INIT instead. The best you can do is to fix the kernel. What about all of the user code in the world?
Cc Thomas, On Fri, 19 Oct 2018 at 05:11, Christian Ehrhardt <lk@c--e.de> wrote: > > > Bit 16 in %dr6 (RTM) is no longer reserved. > > Adjust the definition of DR6_RESERVED and provide new constants > DR6_INIT (moved from kvm_host.h) and DR_RTM. Fix existing users > of DR6_RESERVED that want DR6_INIT instead. > > Signed-off-by: Christian Ehrhardt <lk@c--e.de> > --- > arch/x86/include/asm/kvm_host.h | 1 - > arch/x86/include/uapi/asm/debugreg.h | 5 ++++- > arch/x86/kernel/process_32.c | 2 +- > arch/x86/kernel/process_64.c | 2 +- > 4 files changed, 6 insertions(+), 4 deletions(-) You should Cc x86 maintainers for these changes. In addition, I think you don't need to update TSX stuff outside of KVM since TSX is unsupported by linux kernel until now. Regards, Wanpeng Li > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 09b2e3e2cf1b..4e686713c45f 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -179,7 +179,6 @@ enum { > #define DR6_BS (1 << 14) > #define DR6_RTM (1 << 16) > #define DR6_FIXED_1 0xfffe0ff0 > -#define DR6_INIT 0xffff0ff0 > #define DR6_VOLATILE 0x0001e00f > > #define DR7_BP_EN_MASK 0x000000ff > diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/uapi/asm/debugreg.h > index d95d080b30e3..bba5b98fbd46 100644 > --- a/arch/x86/include/uapi/asm/debugreg.h > +++ b/arch/x86/include/uapi/asm/debugreg.h > @@ -16,7 +16,9 @@ > are either reserved or not of interest to us. */ > > /* Define reserved bits in DR6 which are always set to 1 */ > -#define DR6_RESERVED (0xFFFF0FF0) > +#define DR6_RESERVED (0xFFFE0FF0) > +/* Initial (reset) value of DR6 */ > +#define DR6_INIT (0xFFFF0FF0) > > #define DR_TRAP0 (0x1) /* db0 */ > #define DR_TRAP1 (0x2) /* db1 */ > @@ -26,6 +28,7 @@ > > #define DR_STEP (0x4000) /* single-step */ > #define DR_SWITCH (0x8000) /* task switch */ > +#define DR_RTM (0x10000) /* #DB/#BP in RTM region */ > > /* Now define a bunch of things for manipulating the control register. > The top two bytes of the control register consist of 4 fields of 4 > diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c > index 5046a3c9dec2..8b39b6155ff8 100644 > --- a/arch/x86/kernel/process_32.c > +++ b/arch/x86/kernel/process_32.c > @@ -104,7 +104,7 @@ void __show_regs(struct pt_regs *regs, enum show_regs_mode mode) > > /* Only print out debug registers if they are in their non-default state. */ > if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) && > - (d6 == DR6_RESERVED) && (d7 == 0x400)) > + (d6 == DR6_INIT) && (d7 == 0x400)) > return; > > printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n", > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c > index ea5ea850348d..25de610826a8 100644 > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -129,7 +129,7 @@ void __show_regs(struct pt_regs *regs, enum show_regs_mode mode) > > /* Only print out debug registers if they are in their non-default state. */ > if (!((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) && > - (d6 == DR6_RESERVED) && (d7 == 0x400))) { > + (d6 == DR6_INIT) && (d7 == 0x400))) { > printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", > d0, d1, d2); > printk(KERN_DEFAULT "DR3: %016lx DR6: %016lx DR7: %016lx\n", > -- > 2.17.1 >
Hi, On Fri, Oct 19, 2018 at 08:56:35AM +0800, Wanpeng Li wrote: > Cc Thomas, > On Fri, 19 Oct 2018 at 05:11, Christian Ehrhardt <lk@c--e.de> wrote: > > > > > > Bit 16 in %dr6 (RTM) is no longer reserved. > > > > Adjust the definition of DR6_RESERVED and provide new constants > > DR6_INIT (moved from kvm_host.h) and DR_RTM. Fix existing users > > of DR6_RESERVED that want DR6_INIT instead. > > > > Signed-off-by: Christian Ehrhardt <lk@c--e.de> > > --- > > arch/x86/include/asm/kvm_host.h | 1 - > > arch/x86/include/uapi/asm/debugreg.h | 5 ++++- > > arch/x86/kernel/process_32.c | 2 +- > > arch/x86/kernel/process_64.c | 2 +- > > 4 files changed, 6 insertions(+), 4 deletions(-) > > You should Cc x86 maintainers for these changes. In addition, I think > you don't need to update TSX stuff outside of KVM since TSX is > unsupported by linux kernel until now. Thanks for the reply. The change is mostly nit-picking and changes UAPI and the %dr6 change does not depend on this one. Thus I'll let this change drop as I have no way to assess and deal with the userland impact. regards Christian
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 09b2e3e2cf1b..4e686713c45f 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -179,7 +179,6 @@ enum { #define DR6_BS (1 << 14) #define DR6_RTM (1 << 16) #define DR6_FIXED_1 0xfffe0ff0 -#define DR6_INIT 0xffff0ff0 #define DR6_VOLATILE 0x0001e00f #define DR7_BP_EN_MASK 0x000000ff diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/uapi/asm/debugreg.h index d95d080b30e3..bba5b98fbd46 100644 --- a/arch/x86/include/uapi/asm/debugreg.h +++ b/arch/x86/include/uapi/asm/debugreg.h @@ -16,7 +16,9 @@ are either reserved or not of interest to us. */ /* Define reserved bits in DR6 which are always set to 1 */ -#define DR6_RESERVED (0xFFFF0FF0) +#define DR6_RESERVED (0xFFFE0FF0) +/* Initial (reset) value of DR6 */ +#define DR6_INIT (0xFFFF0FF0) #define DR_TRAP0 (0x1) /* db0 */ #define DR_TRAP1 (0x2) /* db1 */ @@ -26,6 +28,7 @@ #define DR_STEP (0x4000) /* single-step */ #define DR_SWITCH (0x8000) /* task switch */ +#define DR_RTM (0x10000) /* #DB/#BP in RTM region */ /* Now define a bunch of things for manipulating the control register. The top two bytes of the control register consist of 4 fields of 4 diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 5046a3c9dec2..8b39b6155ff8 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -104,7 +104,7 @@ void __show_regs(struct pt_regs *regs, enum show_regs_mode mode) /* Only print out debug registers if they are in their non-default state. */ if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) && - (d6 == DR6_RESERVED) && (d7 == 0x400)) + (d6 == DR6_INIT) && (d7 == 0x400)) return; printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n", diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index ea5ea850348d..25de610826a8 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -129,7 +129,7 @@ void __show_regs(struct pt_regs *regs, enum show_regs_mode mode) /* Only print out debug registers if they are in their non-default state. */ if (!((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) && - (d6 == DR6_RESERVED) && (d7 == 0x400))) { + (d6 == DR6_INIT) && (d7 == 0x400))) { printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2); printk(KERN_DEFAULT "DR3: %016lx DR6: %016lx DR7: %016lx\n",
Bit 16 in %dr6 (RTM) is no longer reserved. Adjust the definition of DR6_RESERVED and provide new constants DR6_INIT (moved from kvm_host.h) and DR_RTM. Fix existing users of DR6_RESERVED that want DR6_INIT instead. Signed-off-by: Christian Ehrhardt <lk@c--e.de> --- arch/x86/include/asm/kvm_host.h | 1 - arch/x86/include/uapi/asm/debugreg.h | 5 ++++- arch/x86/kernel/process_32.c | 2 +- arch/x86/kernel/process_64.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-)