| Message ID | AANLkTinWu7ABrSP9EVZFeC==+ZS+qDOur8rAPNjdzo6i@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 10/06/2010 11:34 PM, Blue Swirl wrote: > Compiling with GCC 4.6.0 20100925 produced warnings: > /src/qemu/target-i386/op_helper.c: In function 'switch_tss': > /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' > set but not used [-Werror=unused-but-set-variable] > > Fix by deleting the variable. Again, this warning tells us the emulation is incorrect, so it's wrong to remove it. Paolo
On Thu, 7 Oct 2010, Paolo Bonzini wrote: > On 10/06/2010 11:34 PM, Blue Swirl wrote: > > Compiling with GCC 4.6.0 20100925 produced warnings: > > /src/qemu/target-i386/op_helper.c: In function 'switch_tss': > > /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' > > set but not used [-Werror=unused-but-set-variable] > > > > Fix by deleting the variable. > > Again, this warning tells us the emulation is incorrect, so it's wrong to > remove it. > http://support.amd.com/us/Processor_TechDocs/24593.pdf 12.2.5 T (Trap) Bit--■Bit 0 of byte 64h, static field. This bit, when set to1, causes a debug exception (#DB) to occur on a task switch. See "Beakpoint Instruction (INT3)" page 340 for additional information. current code never checks and never traps, which is indeed not correct.
On Thu, Oct 7, 2010 at 7:27 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 10/06/2010 11:34 PM, Blue Swirl wrote: >> >> Compiling with GCC 4.6.0 20100925 produced warnings: >> /src/qemu/target-i386/op_helper.c: In function 'switch_tss': >> /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' >> set but not used [-Werror=unused-but-set-variable] >> >> Fix by deleting the variable. > > Again, this warning tells us the emulation is incorrect, so it's wrong to > remove it. The warning tells us that the emulation is unfinished. It's probably unfinished because debugging TSS switches hasn't ever been needed by anyone. If anyone in the future cares enough about T bit, it's easy to implement it even after this part has been removed. But if someone proposes a better patch, I'd be happy to use that instead of this.
On Thu, Oct 7, 2010 at 8:27 AM, malc <av1474@comtv.ru> wrote: > On Thu, 7 Oct 2010, Paolo Bonzini wrote: > >> On 10/06/2010 11:34 PM, Blue Swirl wrote: >> > Compiling with GCC 4.6.0 20100925 produced warnings: >> > /src/qemu/target-i386/op_helper.c: In function 'switch_tss': >> > /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' >> > set but not used [-Werror=unused-but-set-variable] >> > >> > Fix by deleting the variable. >> >> Again, this warning tells us the emulation is incorrect, so it's wrong to >> remove it. >> > > http://support.amd.com/us/Processor_TechDocs/24593.pdf > > 12.2.5 > T (Trap) Bit--■Bit 0 of byte 64h, static field. This bit, when set to1, > causes a debug exception (#DB) to occur on a task switch. See "Beakpoint > Instruction (INT3)" page 340 for additional information. Yes, I read that before making the patch. But page 340 didn't seem to give any relevant information.
On Thu, 7 Oct 2010, Blue Swirl wrote: > On Thu, Oct 7, 2010 at 8:27 AM, malc <av1474@comtv.ru> wrote: > > On Thu, 7 Oct 2010, Paolo Bonzini wrote: > > > >> On 10/06/2010 11:34 PM, Blue Swirl wrote: > >> > Compiling with GCC 4.6.0 20100925 produced warnings: > >> > /src/qemu/target-i386/op_helper.c: In function 'switch_tss': > >> > /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' > >> > set but not used [-Werror=unused-but-set-variable] > >> > > >> > Fix by deleting the variable. > >> > >> Again, this warning tells us the emulation is incorrect, so it's wrong to > >> remove it. > >> > > > > http://support.amd.com/us/Processor_TechDocs/24593.pdf > > > > 12.2.5 > > T (Trap) Bit--■Bit 0 of byte 64h, static field. This bit, when set to1, > > causes a debug exception (#DB) to occur on a task switch. See "Beakpoint > > Instruction (INT3)" page 340 for additional information. > > Yes, I read that before making the patch. But page 340 didn't seem to > give any relevant information. 13.2.4 Breakpoint Instruction (INT3) Is what was referred to apparently.
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c index ec6b3e9..a5462a9 100644 --- a/target-i386/op_helper.c +++ b/target-i386/op_helper.c @@ -280,7 +280,7 @@ static void switch_tss(int tss_selector, int tss_limit, tss_limit_max, type, old_tss_limit_max, old_type, v1, v2, i; target_ulong tss_base; uint32_t new_regs[8], new_segs[6]; - uint32_t new_eflags, new_eip, new_cr3, new_ldt, new_trap; + uint32_t new_eflags, new_eip, new_cr3, new_ldt; uint32_t old_eflags, eflags_mask; SegmentCache *dt; int index; @@ -334,7 +334,6 @@ static void switch_tss(int tss_selector, for(i = 0; i < 6; i++) new_segs[i] = lduw_kernel(tss_base + (0x48 + i * 4)); new_ldt = lduw_kernel(tss_base + 0x60); - new_trap = ldl_kernel(tss_base + 0x64); } else { /* 16 bit */ new_cr3 = 0; @@ -347,7 +346,6 @@ static void switch_tss(int tss_selector, new_ldt = lduw_kernel(tss_base + 0x2a); new_segs[R_FS] = 0; new_segs[R_GS] = 0; - new_trap = 0; } /* NOTE: we must avoid memory exceptions during the task switch,
Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-i386/op_helper.c: In function 'switch_tss': /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' set but not used [-Werror=unused-but-set-variable] Fix by deleting the variable. Alternatively the flag could be implemented by generating a debug exception if the flag is set. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- target-i386/op_helper.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)