| Message ID | 201503091502.11484.wpaul@windriver.com |
|---|---|
| State | New |
| Headers | show |
Hi Bill, sending text only e-mails might help. Usually git send-email is better than using KMail or other mail software. Use tools like git gui to get a correct signature. Any optional personal comments should come directly after the line with ---. Could you please re-send your patch? Check it before sending with scripts/checkpatch.pl. Maybe this looks like much regulations to fix a bug, but some rules are necessary to keep a project like QEMU alive. Cc'ing Paolo and Richard because this might be an important bug fix for the next release. (scripts/get_maintainer.pl tells which maintainers should be cc'ed). Regards Stefan Am 09.03.2015 um 23:02 schrieb Bill Paul: > > Nobody has commented on this yet. According to my reading of the Intel > documentation, the SYSRET instruction is supposed to force the RPL > bits of the %ss register to 3 when returning to user mode. The actual > sequence is: > > SS.Selector <-- (IA32_STAR[63:48]+8) OR 3; (* RPL forced to 3 *) > > However, the code in helper_sysret() leaves them at 0 (in other words, > the "OR 3" part of the above sequence is missing). It does set the > privilege level bits of %cs correctly though. > > This has caused me trouble with some of my VxWorks development: code > that runs okay on real hardware will crash on QEMU, unless I apply the > patch below. > > Can someone confirm that this is in fact a real bug? The Intel > architecture manual seems quite clear about the SYSRET behavior. The > bug seems to have been around as far back as QEMU 0.10.5. > > I am using QEMU 2.2.0 on FreeBSD/amd64 9.1-RELEASE. > > -Bill > > Signed-off-by: Bill Paul <wpaul@windriver.com> > > --- > > target-i386/seg_helper.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c > > index fa374d0..2bc757a 100644 > > --- a/target-i386/seg_helper.c > > +++ b/target-i386/seg_helper.c > > @@ -1043,7 +1043,7 @@ void helper_sysret(CPUX86State *env, int dflag) > > DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK); > > env->eip = (uint32_t)env->regs[R_ECX]; > > } > > - cpu_x86_load_seg_cache(env, R_SS, selector + 8, > > + cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3, > > 0, 0xffffffff, > > DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | > > DESC_S_MASK | (3 << DESC_DPL_SHIFT) | > > @@ -1056,7 +1056,7 @@ void helper_sysret(CPUX86State *env, int dflag) > > DESC_S_MASK | (3 << DESC_DPL_SHIFT) | > > DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK); > > env->eip = (uint32_t)env->regs[R_ECX]; > > - cpu_x86_load_seg_cache(env, R_SS, selector + 8, > > + cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3, > > 0, 0xffffffff, > > DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | > > DESC_S_MASK | (3 << DESC_DPL_SHIFT) | > > -- > > 1.8.0 > > -- > > ============================================================================= > > -Bill Paul (510) 749-2329 | Senior Member of Technical Staff, > > wpaul@windriver.com | Master of Unix-Fu - Wind River Systems > > ============================================================================= > > "I put a dollar in a change machine. Nothing changed." - George Carlin > > ============================================================================= >
Of all the gin joints in all the towns in all the world, Stefan Weil had to walk into mine at 15:23:36 on Monday 09 March 2015 and say: > Hi Bill, > > sending text only e-mails might help. Usually git send-email is better > than using KMail or other mail software. You know, I thought I was sending in plain text. Somewhere along the line KMail lied to me. > Use tools like git gui to get a correct signature. Any optional personal > comments should come directly after > the line with ---. After? Or before? > Could you please re-send your patch? Check it before > sending with scripts/checkpatch.pl. > > Maybe this looks like much regulations to fix a bug, but some rules are > necessary to keep a project > like QEMU alive. I'm going to send it once more because it was entirely my fault that my stupid mailer kept sending in HTML mode, but that's it. -Bill -- ============================================================================= -Bill Paul (510) 749-2329 | Senior Member of Technical Staff, wpaul@windriver.com | Master of Unix-Fu - Wind River Systems ============================================================================= "I put a dollar in a change machine. Nothing changed." - George Carlin =============================================================================
Please see my comment below. Am 09.03.2015 um 23:46 schrieb Bill Paul: > Of all the gin joints in all the towns in all the world, Stefan Weil had to > walk into mine at 15:23:36 on Monday 09 March 2015 and say: > >> Hi Bill, >> >> sending text only e-mails might help. Usually git send-email is better >> than using KMail or other mail software. > You know, I thought I was sending in plain text. Somewhere along the line > KMail lied to me. Even when using plain text, mailing programs will often wrap lines and or do other nasty things. Create your patch with "git format-patch", add optional personal comments after the ---, use scripts/get_maintainer.pl and scripts/checkpatch.pl on your patch, and then send it with "git send-email". > >> Use tools like git gui to get a correct signature. Any optional personal >> comments should come directly after >> the line with ---. > After? Or before? See http://patchwork.ozlabs.org/patch/447644/for an example. >> Could you please re-send your patch? Check it before >> sending with scripts/checkpatch.pl. >> >> Maybe this looks like much regulations to fix a bug, but some rules are >> necessary to keep a project >> like QEMU alive. > I'm going to send it once more because it was entirely my fault that my stupid > mailer kept sending in HTML mode, but that's it. > > -Bill Cheers Stefan
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c index fa374d0..2bc757a 100644 --- a/target-i386/seg_helper.c +++ b/target-i386/seg_helper.c @@ -1043,7 +1043,7 @@ void helper_sysret(CPUX86State *env, int dflag) DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK); env->eip = (uint32_t)env->regs[R_ECX]; } - cpu_x86_load_seg_cache(env, R_SS, selector + 8, + cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3, 0, 0xffffffff, DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | DESC_S_MASK | (3 << DESC_DPL_SHIFT) | @@ -1056,7 +1056,7 @@ void helper_sysret(CPUX86State *env, int dflag) DESC_S_MASK | (3 << DESC_DPL_SHIFT) | DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK); env->eip = (uint32_t)env->regs[R_ECX]; - cpu_x86_load_seg_cache(env, R_SS, selector + 8, + cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3, 0, 0xffffffff, DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
Nobody has commented on this yet. According to my reading of the Intel documentation, the SYSRET instruction is supposed to force the RPL bits of the %ss register to 3 when returning to user mode. The actual sequence is: SS.Selector <-- (IA32_STAR[63:48]+8) OR 3; (* RPL forced to 3 *) However, the code in helper_sysret() leaves them at 0 (in other words, the "OR 3" part of the above sequence is missing). It does set the privilege level bits of %cs correctly though. This has caused me trouble with some of my VxWorks development: code that runs okay on real hardware will crash on QEMU, unless I apply the patch below. Can someone confirm that this is in fact a real bug? The Intel architecture manual seems quite clear about the SYSRET behavior. The bug seems to have been around as far back as QEMU 0.10.5. I am using QEMU 2.2.0 on FreeBSD/amd64 9.1-RELEASE. -Bill Signed-off-by: Bill Paul <wpaul@windriver.com> --- target-i386/seg_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)