Patchwork set correct CS seg limit and flags on sipi

login
register
mail settings
Submitter Gleb Natapov
Date Sept. 13, 2009, 8:19 a.m.
Message ID <20090913081951.GO22885@redhat.com>
Download mbox | patch
Permalink /patch/33545/
State Superseded
Headers show

Comments

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

Signed-off-by: Gleb Natapov <gleb@redhat.com>
--
			Gleb.
Jan Kiszka - Sept. 13, 2009, 11:34 a.m.
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.
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.
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

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;
 }