diff mbox

set correct CS seg limit and flags on sipi

Message ID 20090913081951.GO22885@redhat.com
State Superseded
Headers show

Commit Message

Gleb Natapov Sept. 13, 2009, 8:19 a.m. UTC
TCG works with incorrect values somehow.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
--
			Gleb.

Comments

Jan Kiszka Sept. 13, 2009, 11:34 a.m. UTC | #1
Gleb Natapov wrote:
> TCG works with incorrect values somehow.

TCG doesn't care about limits and has only few segment type checks.

Some link to the corresponding spec section would be nice-to-have in
this commit message. I was looking for a reference what registers SIPIs
actually modify and how, but there seems to be none, at least in the
System Programming Guide.

> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/hw/apic.c b/hw/apic.c
> index 2c414c1..c89008e 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -516,7 +516,7 @@ void apic_sipi(CPUState *env)
>  
>      env->eip = 0;
>      cpu_x86_load_seg_cache(env, R_CS, s->sipi_vector << 8, s->sipi_vector << 12,
> -                           0xffff, 0);
> +                           env->segs[R_CS].limit, env->segs[R_CS].flags);
>      env->halted = 0;
>      s->wait_for_sipi = 0;
>  }
> --
> 			Gleb.

Jan
Gleb Natapov Sept. 13, 2009, 11:50 a.m. UTC | #2
On Sun, Sep 13, 2009 at 01:34:01PM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > TCG works with incorrect values somehow.
> 
> TCG doesn't care about limits and has only few segment type checks.
I already noticed that TCG implement some non existent CPU somewhat
similar to x86.

> 
> Some link to the corresponding spec section would be nice-to-have in
> this commit message. I was looking for a reference what registers SIPIs
> actually modify and how, but there seems to be none, at least in the
> System Programming Guide.
> 
This commit just fix obvious bug that caused SIPI to put garbage into
segment's flags. I don't have link to spec only common sense.

> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 2c414c1..c89008e 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -516,7 +516,7 @@ void apic_sipi(CPUState *env)
> >  
> >      env->eip = 0;
> >      cpu_x86_load_seg_cache(env, R_CS, s->sipi_vector << 8, s->sipi_vector << 12,
> > -                           0xffff, 0);
> > +                           env->segs[R_CS].limit, env->segs[R_CS].flags);
> >      env->halted = 0;
> >      s->wait_for_sipi = 0;
> >  }
> > --
> > 			Gleb.
> 
> Jan
> 
> 



--
			Gleb.
Jan Kiszka Sept. 13, 2009, 12:02 p.m. UTC | #3
Gleb Natapov wrote:
> On Sun, Sep 13, 2009 at 01:34:01PM +0200, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> TCG works with incorrect values somehow.
>> TCG doesn't care about limits and has only few segment type checks.
> I already noticed that TCG implement some non existent CPU somewhat
> similar to x86.

I once posted a patch that added full limit/flags check on memory
access. But it required more work and slowed down TCG quite noticeably.
Meanwhile we are only using KVM and don't require this precision
anymore, so this effort stalled.

> 
>> Some link to the corresponding spec section would be nice-to-have in
>> this commit message. I was looking for a reference what registers SIPIs
>> actually modify and how, but there seems to be none, at least in the
>> System Programming Guide.
>>
> This commit just fix obvious bug that caused SIPI to put garbage into
> segment's flags. I don't have link to spec only common sense.

I don't disagree. I was just wondering if it loads the specified reset
values into the flags or just keeps them untouched. BTW, looks like
BOCHS takes your path too (ie. don't touch them).

Jan
diff mbox

Patch

diff --git a/hw/apic.c b/hw/apic.c
index 2c414c1..c89008e 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -516,7 +516,7 @@  void apic_sipi(CPUState *env)
 
     env->eip = 0;
     cpu_x86_load_seg_cache(env, R_CS, s->sipi_vector << 8, s->sipi_vector << 12,
-                           0xffff, 0);
+                           env->segs[R_CS].limit, env->segs[R_CS].flags);
     env->halted = 0;
     s->wait_for_sipi = 0;
 }