* [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor
@ 2009-10-02 20:28 Kevin Wolf
2009-10-04 10:05 ` Aurelien Jarno
0 siblings, 1 reply; 4+ messages in thread
From: Kevin Wolf @ 2009-10-02 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf
This patch corrects the following aspects of exception generation in
fxsave/fxrstor:
* Generate #GP if the operand is not aligned to a 16 byte boundary
* Generate #UD if the LOCK prefix is used
* For CR0.EM = 1 #NM is generated, not #UD
Signed-off-by: Kevin Wolf <mail@kevin-wolf.de>
---
target-i386/op_helper.c | 10 ++++++++++
target-i386/translate.c | 8 ++++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 33d44b0..3f05532 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -4338,6 +4338,11 @@ void helper_fxsave(target_ulong ptr, int data64)
CPU86_LDouble tmp;
target_ulong addr;
+ /* The operand must be 16 byte aligned */
+ if (ptr & 0xf) {
+ raise_exception(EXCP0D_GPF);
+ }
+
fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
fptag = 0;
for(i = 0; i < 8; i++) {
@@ -4394,6 +4399,11 @@ void helper_fxrstor(target_ulong ptr, int data64)
CPU86_LDouble tmp;
target_ulong addr;
+ /* The operand must be 16 byte aligned */
+ if (ptr & 0xf) {
+ raise_exception(EXCP0D_GPF);
+ }
+
env->fpuc = lduw(ptr);
fpus = lduw(ptr + 2);
fptag = lduw(ptr + 4);
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 5b11d7f..9af2eed 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7436,9 +7436,9 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
switch(op) {
case 0: /* fxsave */
if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
- (s->flags & HF_EM_MASK))
+ (s->prefix & PREFIX_LOCK))
goto illegal_op;
- if (s->flags & HF_TS_MASK) {
+ if ((s->flags & HF_EM_MASK) || (s->flags & HF_TS_MASK)) {
gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
break;
}
@@ -7450,9 +7450,9 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
break;
case 1: /* fxrstor */
if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
- (s->flags & HF_EM_MASK))
+ (s->prefix & PREFIX_LOCK))
goto illegal_op;
- if (s->flags & HF_TS_MASK) {
+ if ((s->flags & HF_EM_MASK) || (s->flags & HF_TS_MASK)) {
gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
break;
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor
2009-10-02 20:28 [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor Kevin Wolf
@ 2009-10-04 10:05 ` Aurelien Jarno
2009-10-04 20:43 ` Kevin Wolf
0 siblings, 1 reply; 4+ messages in thread
From: Aurelien Jarno @ 2009-10-04 10:05 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
On Fri, Oct 02, 2009 at 10:28:57PM +0200, Kevin Wolf wrote:
> This patch corrects the following aspects of exception generation in
> fxsave/fxrstor:
>
> * Generate #GP if the operand is not aligned to a 16 byte boundary
Agreed.
> * Generate #UD if the LOCK prefix is used
Agreed.
> * For CR0.EM = 1 #NM is generated, not #UD
This does not match the Intel manual:
| #NM If CR0.TS[bit 3] = 1.
| #UD If CR0.EM[bit 2] = 1.
| If CPUID.01H:EDX.FXSR[bit 24] = 0.
| If the LOCK prefix is used.
> Signed-off-by: Kevin Wolf <mail@kevin-wolf.de>
> ---
> target-i386/op_helper.c | 10 ++++++++++
> target-i386/translate.c | 8 ++++----
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
> index 33d44b0..3f05532 100644
> --- a/target-i386/op_helper.c
> +++ b/target-i386/op_helper.c
> @@ -4338,6 +4338,11 @@ void helper_fxsave(target_ulong ptr, int data64)
> CPU86_LDouble tmp;
> target_ulong addr;
>
> + /* The operand must be 16 byte aligned */
> + if (ptr & 0xf) {
> + raise_exception(EXCP0D_GPF);
> + }
> +
> fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
> fptag = 0;
> for(i = 0; i < 8; i++) {
> @@ -4394,6 +4399,11 @@ void helper_fxrstor(target_ulong ptr, int data64)
> CPU86_LDouble tmp;
> target_ulong addr;
>
> + /* The operand must be 16 byte aligned */
> + if (ptr & 0xf) {
> + raise_exception(EXCP0D_GPF);
> + }
> +
> env->fpuc = lduw(ptr);
> fpus = lduw(ptr + 2);
> fptag = lduw(ptr + 4);
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 5b11d7f..9af2eed 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -7436,9 +7436,9 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
> switch(op) {
> case 0: /* fxsave */
> if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
> - (s->flags & HF_EM_MASK))
> + (s->prefix & PREFIX_LOCK))
> goto illegal_op;
> - if (s->flags & HF_TS_MASK) {
> + if ((s->flags & HF_EM_MASK) || (s->flags & HF_TS_MASK)) {
> gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
> break;
> }
> @@ -7450,9 +7450,9 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
> break;
> case 1: /* fxrstor */
> if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
> - (s->flags & HF_EM_MASK))
> + (s->prefix & PREFIX_LOCK))
> goto illegal_op;
> - if (s->flags & HF_TS_MASK) {
> + if ((s->flags & HF_EM_MASK) || (s->flags & HF_TS_MASK)) {
> gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
> break;
> }
> --
> 1.6.0.2
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor
2009-10-04 10:05 ` Aurelien Jarno
@ 2009-10-04 20:43 ` Kevin Wolf
2009-10-04 21:10 ` Aurelien Jarno
0 siblings, 1 reply; 4+ messages in thread
From: Kevin Wolf @ 2009-10-04 20:43 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
Am Sonntag, 4. Oktober 2009 12:05 schrieb Aurelien Jarno:
> On Fri, Oct 02, 2009 at 10:28:57PM +0200, Kevin Wolf wrote:
> > This patch corrects the following aspects of exception generation in
> > fxsave/fxrstor:
> >
> > * Generate #GP if the operand is not aligned to a 16 byte boundary
>
> Agreed.
>
> > * Generate #UD if the LOCK prefix is used
>
> Agreed.
>
> > * For CR0.EM = 1 #NM is generated, not #UD
>
> This does not match the Intel manual:
> | #NM If CR0.TS[bit 3] = 1.
> |
> | #UD If CR0.EM[bit 2] = 1.
> | If CPUID.01H:EDX.FXSR[bit 24] = 0.
> | If the LOCK prefix is used.
> |
Hm, you seem to have a different Intel manual. In my copy the CR0.EM part
still belongs to #NM. Also, I ran my test code in KVM for comparision and it
did generate an #NM (on two different machines, one Intel, one AMD), so I'm
quite sure this is right (well, at least not completely wrong).
On the other hand, I just had a look at the AMD documentation and it seems to
support your version... So while my hardware suggests that #NM is right, I'm
not going to insist on it. Maybe there is some hardware that actually does
generate #UD.
If you don't like to commit this part of the fix despite my explanation, just
let me know and I'll resend the patch without it.
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor
2009-10-04 20:43 ` Kevin Wolf
@ 2009-10-04 21:10 ` Aurelien Jarno
0 siblings, 0 replies; 4+ messages in thread
From: Aurelien Jarno @ 2009-10-04 21:10 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
On Sun, Oct 04, 2009 at 10:43:54PM +0200, Kevin Wolf wrote:
> Am Sonntag, 4. Oktober 2009 12:05 schrieb Aurelien Jarno:
> > On Fri, Oct 02, 2009 at 10:28:57PM +0200, Kevin Wolf wrote:
> > > This patch corrects the following aspects of exception generation in
> > > fxsave/fxrstor:
> > >
> > > * Generate #GP if the operand is not aligned to a 16 byte boundary
> >
> > Agreed.
> >
> > > * Generate #UD if the LOCK prefix is used
> >
> > Agreed.
> >
> > > * For CR0.EM = 1 #NM is generated, not #UD
> >
> > This does not match the Intel manual:
> > | #NM If CR0.TS[bit 3] = 1.
> > |
> > | #UD If CR0.EM[bit 2] = 1.
> > | If CPUID.01H:EDX.FXSR[bit 24] = 0.
> > | If the LOCK prefix is used.
> > |
>
> Hm, you seem to have a different Intel manual. In my copy the CR0.EM part
> still belongs to #NM. Also, I ran my test code in KVM for comparision and it
> did generate an #NM (on two different machines, one Intel, one AMD), so I'm
> quite sure this is right (well, at least not completely wrong).
My copy of the Intel Manual was quite outdated (May 2007). The new
version from September 2009 matches your patch, so I have applied it.
Sorry.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-10-04 21:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-02 20:28 [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor Kevin Wolf
2009-10-04 10:05 ` Aurelien Jarno
2009-10-04 20:43 ` Kevin Wolf
2009-10-04 21:10 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).