[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions |
Date: |
Mon, 28 Sep 2015 15:48:17 -0300 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, Sep 21, 2015 at 02:05:52PM +0200, Paolo Bonzini wrote:
> On 15/09/2015 20:45, Richard Henderson wrote:
> > Best guess, since I can't find any code that actually uses them.
> > Linux actively turns them off at boot...
>
> I've sent a kvm-unit-tests patch to test debug extensions. It shows
> that debug extensions work, but the following needs to be squashed in
> patch 4:
>
> diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
> index c258598..b24e446 100644
> --- a/target-i386/bpt_helper.c
> +++ b/target-i386/bpt_helper.c
> @@ -134,14 +134,14 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t
> new_dr7)
(I've expanded the diff context below, to make review easier)
> /* If nothing is changing except the global/local enable bits,
> then we can make the change more efficient. */
> if (((old_dr7 ^ new_dr7) & ~0xff) == 0) {
> /* Fold the global and local enable bits together into the
> global fields, then xor to show which registers have
> changed collective enable state. */
> int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
>
> for (i = 0; i < DR7_MAX_BP; i++) {
> - if (mod & (2 << i * 2)) {
> - /* We know that register i has changed enable state;
> - recheck what that state should be and apply. */
> - if (hw_breakpoint_enabled(new_dr7, i)) {
> - iobpt |= hw_breakpoint_insert(env, i);
> - } else {
> - hw_breakpoint_remove(env, i);
> - }
> + if ((mod & (2 << i * 2)) && !hw_breakpoint_enabled(new_dr7, i)) {
> + hw_breakpoint_remove(env, i);
> + }
> + }
> + env->dr[7] = new_dr7 | DR7_FIXED_1;
Why is DR7_FIXED_1 forced here, but not at the other branch below?
Also, the previous code didn't force DR7_FIXED_1, so shouldn't we implement
this fix in a separate patch instead of squashing it into patch 4/8?
> + for (i = 0; i < DR7_MAX_BP; i++) {
> + if (mod & (2 << i * 2) && hw_breakpoint_enabled(new_dr7, i)) {
> + iobpt |= hw_breakpoint_insert(env, i);
> } else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
> && hw_breakpoint_enabled(new_dr7, i)) {
> iobpt |= HF_IOBPT_MASK;
> }
> }
> } else {
> for (i = 0; i < DR7_MAX_BP; i++) {
> hw_breakpoint_remove(env, i);
> }
> env->dr[7] = new_dr7;
> for (i = 0; i < DR7_MAX_BP; i++) {
> iobpt |= hw_breakpoint_insert(env, i);
> }
> }
--
Eduardo
- [Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions, (continued)
- [Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions, Richard Henderson, 2015/09/15
- [Qemu-devel] [PATCH 6/8] target-i386: Optimize setting dr[0-3], Richard Henderson, 2015/09/15
- [Qemu-devel] [PATCH 8/8] target-i386: Check CR4[DE] for processing DR4/DR5, Richard Henderson, 2015/09/15
- [Qemu-devel] [PATCH 7/8] target-i386: Handle I/O breakpoints, Richard Henderson, 2015/09/15
- [Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file, Richard Henderson, 2015/09/15
- Re: [Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions, Paolo Bonzini, 2015/09/21