[Qemu-devel] [PATCH v3] target-i386: Fix segment cache dump

[Qemu-devel] [PATCH v3] target-i386: Fix segment cache dump

[Tobias Markus]

When in Long Mode, cpu_x86_seg_cache() logs "DS16" because the Default
operation size bit (D/B bit) is not set for Long Mode Data Segments since
there are only Data Segments in Long Mode and no explicit 16/32/64-bit
Descriptors.
This patch fixes this by checking the Long Mode Active bit of the hidden
flags variable and logging "DS" if it is set. (I.e. in Long Mode all Data
Segments are logged as "DS")

Signed-off-by: Tobias Markus <tobias@markus-regensburg.de>
---
v3: * Manually Break Lines
    * Again change line wrapping as suggested by Eric Blake
v2: * Fix line wrapping as suggested in IRC
    * Break the line
Note that alternatively, Data Segments in Long Mode could be logged as "DS64" to avoid confusion about the missing 64 postfix. (But that would be, strictly speaking, wrong because there are only DSs and no DS16/32/64 in Long Mode.)
PS: This is my first contribution to an Open Source Project and I would be very happy about constructive feedback, especially about possible wrong line wrapping.
  target-i386/helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index bf3e2ac..0edb93b 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -147,7 +147,9 @@ cpu_x86_dump_seg_cache(CPUX86State *env, FILE *f, fprintf_function cpu_fprintf,
             cpu_fprintf(f, " [%c%c", (sc->flags & DESC_C_MASK) ? 'C' : '-',
                         (sc->flags & DESC_R_MASK) ? 'R' : '-');
         } else {
-            cpu_fprintf(f, (sc->flags & DESC_B_MASK) ? "DS  " : "DS16");
+            cpu_fprintf(f,
+                        (sc->flags & DESC_B_MASK || env->hflags & HF_LMA_MASK)
+                        ? "DS  " : "DS16");
             cpu_fprintf(f, " [%c%c", (sc->flags & DESC_E_MASK) ? 'E' : '-',
                         (sc->flags & DESC_W_MASK) ? 'W' : '-');
         }
-- 
1.8.4

Re: [Qemu-devel] [PATCH v3] target-i386: Fix segment cache dump

[Richard Henderson]

On 08/25/2013 03:20 AM, Tobias Markus wrote:
> When in Long Mode, cpu_x86_seg_cache() logs "DS16" because the Default
> operation size bit (D/B bit) is not set for Long Mode Data Segments since
> there are only Data Segments in Long Mode and no explicit 16/32/64-bit
> Descriptors.
> This patch fixes this by checking the Long Mode Active bit of the hidden
> flags variable and logging "DS" if it is set. (I.e. in Long Mode all Data
> Segments are logged as "DS")
> 
> Signed-off-by: Tobias Markus <tobias@markus-regensburg.de>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] PING Re: [PATCH v3] target-i386: Fix segment cache dump

[Tobias Markus]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/25/2013 12:20 PM, Tobias Markus wrote:
> When in Long Mode, cpu_x86_seg_cache() logs "DS16" because the
> Default operation size bit (D/B bit) is not set for Long Mode Data
> Segments since there are only Data Segments in Long Mode and no
> explicit 16/32/64-bit Descriptors. This patch fixes this by
> checking the Long Mode Active bit of the hidden flags variable and
> logging "DS" if it is set. (I.e. in Long Mode all Data Segments are
> logged as "DS")
> 
> Signed-off-by: Tobias Markus <tobias@markus-regensburg.de> --- v3:
> * Manually Break Lines * Again change line wrapping as suggested by
> Eric Blake v2: * Fix line wrapping as suggested in IRC * Break the
> line Note that alternatively, Data Segments in Long Mode could be
> logged as "DS64" to avoid confusion about the missing 64 postfix.
> (But that would be, strictly speaking, wrong because there are only
> DSs and no DS16/32/64 in Long Mode.) PS: This is my first
> contribution to an Open Source Project and I would be very happy
> about constructive feedback, especially about possible wrong line
> wrapping. target-i386/helper.c | 4 +++- 1 file changed, 3
> insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/helper.c b/target-i386/helper.c index
> bf3e2ac..0edb93b 100644 --- a/target-i386/helper.c +++
> b/target-i386/helper.c @@ -147,7 +147,9 @@
> cpu_x86_dump_seg_cache(CPUX86State *env, FILE *f, fprintf_function
> cpu_fprintf, cpu_fprintf(f, " [%c%c", (sc->flags & DESC_C_MASK) ?
> 'C' : '-', (sc->flags & DESC_R_MASK) ? 'R' : '-'); } else { -
> cpu_fprintf(f, (sc->flags & DESC_B_MASK) ? "DS  " : "DS16"); +
> cpu_fprintf(f, +                        (sc->flags & DESC_B_MASK ||
> env->hflags & HF_LMA_MASK) +                        ? "DS  " :
> "DS16"); cpu_fprintf(f, " [%c%c", (sc->flags & DESC_E_MASK) ? 'E' :
> '-', (sc->flags & DESC_W_MASK) ? 'W' : '-'); }
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.21 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlIzG6kACgkQAO6N0EYmC9ZTGQCfXUfs8zrnlwOAtLqqbKW1F8Z/
PjEAoJGDaLQGzQ/qYEOEo0KpO4ZhE7kE
=Lfdr
-----END PGP SIGNATURE-----

[Qemu-devel] PING Re: [PATCH v3] target-i386: Fix segment cache dump

[Tobias Markus]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

PING
(Mail resent - sorry, Thunderbird wrapped line in the first mail)

PS: Sorry for the two wrapped lines below ---, but probably not worth
resending the patch just for that.

On 08/25/2013 12:20 PM, Tobias Markus wrote:
> When in Long Mode, cpu_x86_seg_cache() logs "DS16" because the Default
> operation size bit (D/B bit) is not set for Long Mode Data Segments since
> there are only Data Segments in Long Mode and no explicit 16/32/64-bit
> Descriptors.
> This patch fixes this by checking the Long Mode Active bit of the hidden
> flags variable and logging "DS" if it is set. (I.e. in Long Mode all Data
> Segments are logged as "DS")
> 
> Signed-off-by: Tobias Markus <tobias@markus-regensburg.de>
> ---
> v3: * Manually Break Lines
>     * Again change line wrapping as suggested by Eric Blake
> v2: * Fix line wrapping as suggested in IRC
>     * Break the line
> Note that alternatively, Data Segments in Long Mode could be logged as "DS64" to avoid confusion about the missing 64 postfix. (But that would be, strictly speaking, wrong because there are only DSs and no DS16/32/64 in Long Mode.)
> PS: This is my first contribution to an Open Source Project and I would be very happy about constructive feedback, especially about possible wrong line wrapping.
>   target-i386/helper.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf3e2ac..0edb93b 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -147,7 +147,9 @@ cpu_x86_dump_seg_cache(CPUX86State *env, FILE *f, fprintf_function cpu_fprintf,
>              cpu_fprintf(f, " [%c%c", (sc->flags & DESC_C_MASK) ? 'C' : '-',
>                          (sc->flags & DESC_R_MASK) ? 'R' : '-');
>          } else {
> -            cpu_fprintf(f, (sc->flags & DESC_B_MASK) ? "DS  " : "DS16");
> +            cpu_fprintf(f,
> +                        (sc->flags & DESC_B_MASK || env->hflags & HF_LMA_MASK)
> +                        ? "DS  " : "DS16");
>              cpu_fprintf(f, " [%c%c", (sc->flags & DESC_E_MASK) ? 'E' : '-',
>                          (sc->flags & DESC_W_MASK) ? 'W' : '-');
>          }
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.21 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlIzH3gACgkQAO6N0EYmC9ZZaACeLJZ4fJ9QARYuBHXwKCyV6uMN
X/4AnjP9gqnop9fjXQWXhFDxqEgofkw7
=BEj3
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [Qemu-trivial] PING Re: [PATCH v3] target-i386: Fix segment cache dump

[Michael Tokarev]

13.09.2013 18:05, Tobias Markus wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 08/25/2013 12:20 PM, Tobias Markus wrote:
>> When in Long Mode, cpu_x86_seg_cache() logs "DS16" because the
>> Default operation size bit (D/B bit) is not set for Long Mode Data
>> Segments since there are only Data Segments in Long Mode and no
>> explicit 16/32/64-bit Descriptors. This patch fixes this by
>> checking the Long Mode Active bit of the hidden flags variable and
>> logging "DS" if it is set. (I.e. in Long Mode all Data Segments are
>> logged as "DS")

Thanks, applied to the trivial-patches queue.

Andreas: if you think it should go to i386 queue instead please
just reply to this email saying so, before I'll send a pull request
to Anthony.  Before a pull request it is really trivial to remove
one patch from a queue.

/mjt