All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
@ 2011-05-20  3:34 David Gibson
  2011-05-20  7:40 ` Alexander Graf
  2011-05-21  9:46 ` Andreas Färber
  0 siblings, 2 replies; 14+ messages in thread
From: David Gibson @ 2011-05-20  3:34 UTC (permalink / raw)
  To: agraf, qemu-devel; +Cc: andreas.faerber, paulus, kennethsalerno

Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
registers, by translating writes to them into writes to the SLB.  This is
not used by any current Linux kernel, but it is used by the openbios used
in the qemu mac99 model.

Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
handling introduced a bug in this code, breaking the openbios currently in
qemu.  Specifically, there was an off by one error bitshuffling the
register format used by mtsr into the format needed for the SLB load,
causing the flag bits to end up in the wrong place.  This caused the
storage keys to be wrong under openbios, meaning that the translation code
incorrectly thought a legitimate access was a permission violation.

This patch fixes the bug, at the same time it fixes some build bug in the
MMU debugging code (only exposed when DEBUG_MMU is enabled).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/helper.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 4238be6..4700632 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -606,7 +606,7 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h,
             r = pte64_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
-                    pteg_base + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
+                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
                     (int)((pte0 >> 1) & 1), ctx->ptem);
         } else
 #endif
@@ -621,7 +621,7 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h,
             r = pte32_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
-                    pteg_base + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
+                    pteg_off + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
                     (int)((pte0 >> 6) & 1), ctx->ptem);
         }
         switch (r) {
@@ -918,8 +918,7 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
                     if (eaddr != 0xEFFFFFFF)
                         LOG_MMU("1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
                                 " vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx
-                                " hash=" TARGET_FMT_plx " pg_addr="
-                                TARGET_FMT_plx "\n", env->htab_base,
+                                " hash=" TARGET_FMT_plx "\n", env->htab_base,
                                 env->htab_mask, vsid, ctx->ptem, ctx->hash[1]);
                     ret2 = find_pte(env, ctx, 1, rw, type,
                                     target_page_bits);
@@ -2140,7 +2139,7 @@ void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
         /* VSID = VSID */
         rs |= (value & 0xfffffff) << 12;
         /* flags = flags */
-        rs |= ((value >> 27) & 0xf) << 9;
+        rs |= ((value >> 27) & 0xf) << 8;
 
         ppc_store_slb(env, rb, rs);
     } else
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-20  3:34 [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64 David Gibson
@ 2011-05-20  7:40 ` Alexander Graf
  2011-05-20 22:37   ` Andreas Färber
  2011-05-21  9:40   ` Andreas Färber
  2011-05-21  9:46 ` Andreas Färber
  1 sibling, 2 replies; 14+ messages in thread
From: Alexander Graf @ 2011-05-20  7:40 UTC (permalink / raw)
  To: David Gibson; +Cc: andreas.faerber, paulus, qemu-devel, kennethsalerno


On 20.05.2011, at 05:34, David Gibson wrote:

> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
> registers, by translating writes to them into writes to the SLB.  This is
> not used by any current Linux kernel, but it is used by the openbios used
> in the qemu mac99 model.
> 
> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
> handling introduced a bug in this code, breaking the openbios currently in
> qemu.  Specifically, there was an off by one error bitshuffling the
> register format used by mtsr into the format needed for the SLB load,
> causing the flag bits to end up in the wrong place.  This caused the
> storage keys to be wrong under openbios, meaning that the translation code
> incorrectly thought a legitimate access was a permission violation.
> 
> This patch fixes the bug, at the same time it fixes some build bug in the
> MMU debugging code (only exposed when DEBUG_MMU is enabled).

Thanks, applied to ppc-next :)


Alex

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-20  7:40 ` Alexander Graf
@ 2011-05-20 22:37   ` Andreas Färber
  2011-05-21  1:58     ` Alexander Graf
  2011-05-21  9:40   ` Andreas Färber
  1 sibling, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2011-05-20 22:37 UTC (permalink / raw)
  To: David Gibson, Alexander Graf
  Cc: Kenneth Salerno, paulus, QEMU-devel Developers

Am 20.05.2011 um 09:40 schrieb Alexander Graf:

> On 20.05.2011, at 05:34, David Gibson wrote:
>
>> Early ppc64 CPUs include a hack to partially simulate the ppc32  
>> segment
>> registers, by translating writes to them into writes to the SLB.   
>> This is
>> not used by any current Linux kernel, but it is used by the  
>> openbios used
>> in the qemu mac99 model.
>>
>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>> handling introduced a bug in this code, breaking the openbios  
>> currently in
>> qemu.  Specifically, there was an off by one error bitshuffling the
>> register format used by mtsr into the format needed for the SLB load,
>> causing the flag bits to end up in the wrong place.  This caused the
>> storage keys to be wrong under openbios, meaning that the  
>> translation code
>> incorrectly thought a legitimate access was a permission violation.
>>
>> This patch fixes the bug, at the same time it fixes some build bug  
>> in the
>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>
> Thanks, applied to ppc-next :)

Nack, this does not fix ppc64 for me! How did you test it, Alex???

Andreas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-20 22:37   ` Andreas Färber
@ 2011-05-21  1:58     ` Alexander Graf
  2011-05-21  9:39       ` Andreas Färber
  2011-05-21  9:39       ` Alexander Graf
  0 siblings, 2 replies; 14+ messages in thread
From: Alexander Graf @ 2011-05-21  1:58 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kenneth Salerno, paulus, QEMU-devel Developers, David Gibson


Am 21.05.2011 um 00:37 schrieb Andreas Färber <andreas.faerber@web.de>:

> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
> 
>> On 20.05.2011, at 05:34, David Gibson wrote:
>> 
>>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
>>> registers, by translating writes to them into writes to the SLB.  This is
>>> not used by any current Linux kernel, but it is used by the openbios used
>>> in the qemu mac99 model.
>>> 
>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>>> handling introduced a bug in this code, breaking the openbios currently in
>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>> register format used by mtsr into the format needed for the SLB load,
>>> causing the flag bits to end up in the wrong place.  This caused the
>>> storage keys to be wrong under openbios, meaning that the translation code
>>> incorrectly thought a legitimate access was a permission violation.
>>> 
>>> This patch fixes the bug, at the same time it fixes some build bug in the
>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>> 
>> Thanks, applied to ppc-next :)
> 
> Nack, this does not fix ppc64 for me! How did you test it, Alex???

I booted a ppc64 kernel on a ppc64 Linux system with tcg and it booted fine for me. Maybe you're hitting yet another issue?

Alex

> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21  1:58     ` Alexander Graf
@ 2011-05-21  9:39       ` Andreas Färber
  2011-05-21  9:44         ` Alexander Graf
  2011-05-21 12:16         ` Alexander Graf
  2011-05-21  9:39       ` Alexander Graf
  1 sibling, 2 replies; 14+ messages in thread
From: Andreas Färber @ 2011-05-21  9:39 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Kenneth Salerno, paulus, QEMU-devel Developers, David Gibson

Am 21.05.2011 um 03:58 schrieb Alexander Graf:

> Am 21.05.2011 um 00:37 schrieb Andreas Färber  
> <andreas.faerber@web.de>:
>
>> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
>>
>>> On 20.05.2011, at 05:34, David Gibson wrote:
>>>
>>>> Early ppc64 CPUs include a hack to partially simulate the ppc32  
>>>> segment
>>>> registers, by translating writes to them into writes to the SLB.   
>>>> This is
>>>> not used by any current Linux kernel, but it is used by the  
>>>> openbios used
>>>> in the qemu mac99 model.
>>>>
>>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the  
>>>> SLB
>>>> handling introduced a bug in this code, breaking the openbios  
>>>> currently in
>>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>>> register format used by mtsr into the format needed for the SLB  
>>>> load,
>>>> causing the flag bits to end up in the wrong place.  This caused  
>>>> the
>>>> storage keys to be wrong under openbios, meaning that the  
>>>> translation code
>>>> incorrectly thought a legitimate access was a permission violation.
>>>>
>>>> This patch fixes the bug, at the same time it fixes some build  
>>>> bug in the
>>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>>>
>>> Thanks, applied to ppc-next :)
>>
>> Nack, this does not fix ppc64 for me! How did you test it, Alex???
>
> I booted a ppc64 kernel on a ppc64 Linux system with tcg and it  
> booted fine for me. Maybe you're hitting yet another issue?

Which OpenBIOS did you use? The 32-bit version using mtsrin works now,  
but not the 64-bit HEAD version that I attached, which uses slb*  
instructions instead.

Andreas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21  1:58     ` Alexander Graf
  2011-05-21  9:39       ` Andreas Färber
@ 2011-05-21  9:39       ` Alexander Graf
  1 sibling, 0 replies; 14+ messages in thread
From: Alexander Graf @ 2011-05-21  9:39 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kenneth Salerno, Paul Mackerras, QEMU-devel Developers, David Gibson


On 21.05.2011, at 03:58, Alexander Graf wrote:

> 
> Am 21.05.2011 um 00:37 schrieb Andreas Färber <andreas.faerber@web.de>:
> 
>> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
>> 
>>> On 20.05.2011, at 05:34, David Gibson wrote:
>>> 
>>>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
>>>> registers, by translating writes to them into writes to the SLB.  This is
>>>> not used by any current Linux kernel, but it is used by the openbios used
>>>> in the qemu mac99 model.
>>>> 
>>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>>>> handling introduced a bug in this code, breaking the openbios currently in
>>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>>> register format used by mtsr into the format needed for the SLB load,
>>>> causing the flag bits to end up in the wrong place.  This caused the
>>>> storage keys to be wrong under openbios, meaning that the translation code
>>>> incorrectly thought a legitimate access was a permission violation.
>>>> 
>>>> This patch fixes the bug, at the same time it fixes some build bug in the
>>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>>> 
>>> Thanks, applied to ppc-next :)
>> 
>> Nack, this does not fix ppc64 for me! How did you test it, Alex???
> 
> I booted a ppc64 kernel on a ppc64 Linux system with tcg and it booted fine for me. Maybe you're hitting yet another issue?

Sorry for not being overly precise; host userland was running in 32-bit.


Alex

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-20  7:40 ` Alexander Graf
  2011-05-20 22:37   ` Andreas Färber
@ 2011-05-21  9:40   ` Andreas Färber
  2011-05-21  9:46     ` Alexander Graf
  1 sibling, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2011-05-21  9:40 UTC (permalink / raw)
  To: Alexander Graf; +Cc: kennethsalerno, paulus, qemu-devel, David Gibson

Am 20.05.2011 um 09:40 schrieb Alexander Graf:

> On 20.05.2011, at 05:34, David Gibson wrote:
>
>> Early ppc64 CPUs include a hack to partially simulate the ppc32  
>> segment
>> registers, by translating writes to them into writes to the SLB.   
>> This is
>> not used by any current Linux kernel, but it is used by the  
>> openbios used
>> in the qemu mac99 model.
>>
>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>> handling introduced a bug in this code, breaking the openbios  
>> currently in
>> qemu.  Specifically, there was an off by one error bitshuffling the
>> register format used by mtsr into the format needed for the SLB load,
>> causing the flag bits to end up in the wrong place.  This caused the
>> storage keys to be wrong under openbios, meaning that the  
>> translation code
>> incorrectly thought a legitimate access was a permission violation.
>>
>> This patch fixes the bug, at the same time it fixes some build bug  
>> in the
>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>
> Thanks, applied to ppc-next :)

Hm. Don't you think we should split off the unrelated debug code fix  
for bisecting?

Andreas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21  9:39       ` Andreas Färber
@ 2011-05-21  9:44         ` Alexander Graf
  2011-05-21 12:16         ` Alexander Graf
  1 sibling, 0 replies; 14+ messages in thread
From: Alexander Graf @ 2011-05-21  9:44 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kenneth Salerno, paulus, QEMU-devel Developers, David Gibson


On 21.05.2011, at 11:39, Andreas Färber wrote:

> Am 21.05.2011 um 03:58 schrieb Alexander Graf:
> 
>> Am 21.05.2011 um 00:37 schrieb Andreas Färber <andreas.faerber@web.de>:
>> 
>>> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
>>> 
>>>> On 20.05.2011, at 05:34, David Gibson wrote:
>>>> 
>>>>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
>>>>> registers, by translating writes to them into writes to the SLB.  This is
>>>>> not used by any current Linux kernel, but it is used by the openbios used
>>>>> in the qemu mac99 model.
>>>>> 
>>>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>>>>> handling introduced a bug in this code, breaking the openbios currently in
>>>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>>>> register format used by mtsr into the format needed for the SLB load,
>>>>> causing the flag bits to end up in the wrong place.  This caused the
>>>>> storage keys to be wrong under openbios, meaning that the translation code
>>>>> incorrectly thought a legitimate access was a permission violation.
>>>>> 
>>>>> This patch fixes the bug, at the same time it fixes some build bug in the
>>>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>>>> 
>>>> Thanks, applied to ppc-next :)
>>> 
>>> Nack, this does not fix ppc64 for me! How did you test it, Alex???
>> 
>> I booted a ppc64 kernel on a ppc64 Linux system with tcg and it booted fine for me. Maybe you're hitting yet another issue?
> 
> Which OpenBIOS did you use? The 32-bit version using mtsrin works now, but not the 64-bit HEAD version that I attached, which uses slb* instructions instead.

Ah, I see. I tested the binary that's in pc-bios, so the 32-bit version obviously. So your 64-bit build regresses?


Alex

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21  9:40   ` Andreas Färber
@ 2011-05-21  9:46     ` Alexander Graf
  2011-05-21 11:13       ` David Gibson
  0 siblings, 1 reply; 14+ messages in thread
From: Alexander Graf @ 2011-05-21  9:46 UTC (permalink / raw)
  To: Andreas Färber; +Cc: kennethsalerno, paulus, qemu-devel, David Gibson


On 21.05.2011, at 11:40, Andreas Färber wrote:

> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
> 
>> On 20.05.2011, at 05:34, David Gibson wrote:
>> 
>>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
>>> registers, by translating writes to them into writes to the SLB.  This is
>>> not used by any current Linux kernel, but it is used by the openbios used
>>> in the qemu mac99 model.
>>> 
>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>>> handling introduced a bug in this code, breaking the openbios currently in
>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>> register format used by mtsr into the format needed for the SLB load,
>>> causing the flag bits to end up in the wrong place.  This caused the
>>> storage keys to be wrong under openbios, meaning that the translation code
>>> incorrectly thought a legitimate access was a permission violation.
>>> 
>>> This patch fixes the bug, at the same time it fixes some build bug in the
>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>> 
>> Thanks, applied to ppc-next :)
> 
> Hm. Don't you think we should split off the unrelated debug code fix for bisecting?

Not sure if it's worth the effort. If you were bisecting before that, you probably had DEBUG_MMU disabled anyways, because you'd otherwise get build breakages before that specific commit anyways, so the commit behaves as if it's only the SLB fix.


Alex

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-20  3:34 [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64 David Gibson
  2011-05-20  7:40 ` Alexander Graf
@ 2011-05-21  9:46 ` Andreas Färber
  1 sibling, 0 replies; 14+ messages in thread
From: Andreas Färber @ 2011-05-21  9:46 UTC (permalink / raw)
  To: David Gibson; +Cc: kennethsalerno, paulus, agraf, qemu-devel

Am 20.05.2011 um 05:34 schrieb David Gibson:

> Early ppc64 CPUs include a hack to partially simulate the ppc32  
> segment
> registers, by translating writes to them into writes to the SLB.   
> This is
> not used by any current Linux kernel, but it is used by the openbios  
> used
> in the qemu mac99 model.
>
> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
> handling introduced a bug in this code, breaking the openbios  
> currently in
> qemu.  Specifically, there was an off by one error bitshuffling the
> register format used by mtsr into the format needed for the SLB load,
> causing the flag bits to end up in the wrong place.  This caused the
> storage keys to be wrong under openbios, meaning that the  
> translation code
> incorrectly thought a legitimate access was a permission violation.
>
> This patch fixes the bug, at the same time it fixes some build bug  
> in the
> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target-ppc/helper.c |    9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index 4238be6..4700632 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c

> @@ -2140,7 +2139,7 @@ void ppc_store_sr (CPUPPCState *env, int  
> srnum, target_ulong value)
>         /* VSID = VSID */
>         rs |= (value & 0xfffffff) << 12;
>         /* flags = flags */
> -        rs |= ((value >> 27) & 0xf) << 9;
> +        rs |= ((value >> 27) & 0xf) << 8;
>
>         ppc_store_slb(env, rb, rs);
>     } else

This part fixes OpenBIOS legacy ppc64 support.

Acked-by: Andreas Färber <andreas.faerber@web.de>

Andreas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21  9:46     ` Alexander Graf
@ 2011-05-21 11:13       ` David Gibson
  2011-05-21 11:22         ` Alexander Graf
  0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2011-05-21 11:13 UTC (permalink / raw)
  To: Alexander Graf; +Cc: Andreas Färber, paulus, qemu-devel, kennethsalerno

On Sat, May 21, 2011 at 11:46:12AM +0200, Alexander Graf wrote:
> 
> On 21.05.2011, at 11:40, Andreas Färber wrote:
> 
> > Am 20.05.2011 um 09:40 schrieb Alexander Graf:
> > 
> >> On 20.05.2011, at 05:34, David Gibson wrote:
> >> 
> >>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
> >>> registers, by translating writes to them into writes to the SLB.  This is
> >>> not used by any current Linux kernel, but it is used by the openbios used
> >>> in the qemu mac99 model.
> >>> 
> >>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
> >>> handling introduced a bug in this code, breaking the openbios currently in
> >>> qemu.  Specifically, there was an off by one error bitshuffling the
> >>> register format used by mtsr into the format needed for the SLB load,
> >>> causing the flag bits to end up in the wrong place.  This caused the
> >>> storage keys to be wrong under openbios, meaning that the translation code
> >>> incorrectly thought a legitimate access was a permission violation.
> >>> 
> >>> This patch fixes the bug, at the same time it fixes some build bug in the
> >>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
> >> 
> >> Thanks, applied to ppc-next :)
> > 
> > Hm. Don't you think we should split off the unrelated debug code fix for bisecting?
> 
> Not sure if it's worth the effort. If you were bisecting before
> that, you probably had DEBUG_MMU disabled anyways, because you'd
> otherwise get build breakages before that specific commit anyways,
> so the commit behaves as if it's only the SLB fix.

Right, I don't see how the debug fixes break bisect in any way.  Or
least not in any way they werem't already broken.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21 11:13       ` David Gibson
@ 2011-05-21 11:22         ` Alexander Graf
  0 siblings, 0 replies; 14+ messages in thread
From: Alexander Graf @ 2011-05-21 11:22 UTC (permalink / raw)
  To: David Gibson; +Cc: Andreas Färber, paulus, qemu-devel, kennethsalerno


On 21.05.2011, at 13:13, David Gibson wrote:

> On Sat, May 21, 2011 at 11:46:12AM +0200, Alexander Graf wrote:
>> 
>> On 21.05.2011, at 11:40, Andreas Färber wrote:
>> 
>>> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
>>> 
>>>> On 20.05.2011, at 05:34, David Gibson wrote:
>>>> 
>>>>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
>>>>> registers, by translating writes to them into writes to the SLB.  This is
>>>>> not used by any current Linux kernel, but it is used by the openbios used
>>>>> in the qemu mac99 model.
>>>>> 
>>>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>>>>> handling introduced a bug in this code, breaking the openbios currently in
>>>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>>>> register format used by mtsr into the format needed for the SLB load,
>>>>> causing the flag bits to end up in the wrong place.  This caused the
>>>>> storage keys to be wrong under openbios, meaning that the translation code
>>>>> incorrectly thought a legitimate access was a permission violation.
>>>>> 
>>>>> This patch fixes the bug, at the same time it fixes some build bug in the
>>>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>>>> 
>>>> Thanks, applied to ppc-next :)
>>> 
>>> Hm. Don't you think we should split off the unrelated debug code fix for bisecting?
>> 
>> Not sure if it's worth the effort. If you were bisecting before
>> that, you probably had DEBUG_MMU disabled anyways, because you'd
>> otherwise get build breakages before that specific commit anyways,
>> so the commit behaves as if it's only the SLB fix.
> 
> Right, I don't see how the debug fixes break bisect in any way.  Or
> least not in any way they werem't already broken.

Yup, let's just make sure to have you submit separate patches next time, so everyone is happy :)


Alex

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
  2011-05-21  9:39       ` Andreas Färber
  2011-05-21  9:44         ` Alexander Graf
@ 2011-05-21 12:16         ` Alexander Graf
  1 sibling, 0 replies; 14+ messages in thread
From: Alexander Graf @ 2011-05-21 12:16 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kenneth Salerno, paulus, QEMU-devel Developers, David Gibson


On 21.05.2011, at 11:39, Andreas Färber wrote:

> Am 21.05.2011 um 03:58 schrieb Alexander Graf:
> 
>> Am 21.05.2011 um 00:37 schrieb Andreas Färber <andreas.faerber@web.de>:
>> 
>>> Am 20.05.2011 um 09:40 schrieb Alexander Graf:
>>> 
>>>> On 20.05.2011, at 05:34, David Gibson wrote:
>>>> 
>>>>> Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
>>>>> registers, by translating writes to them into writes to the SLB.  This is
>>>>> not used by any current Linux kernel, but it is used by the openbios used
>>>>> in the qemu mac99 model.
>>>>> 
>>>>> Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
>>>>> handling introduced a bug in this code, breaking the openbios currently in
>>>>> qemu.  Specifically, there was an off by one error bitshuffling the
>>>>> register format used by mtsr into the format needed for the SLB load,
>>>>> causing the flag bits to end up in the wrong place.  This caused the
>>>>> storage keys to be wrong under openbios, meaning that the translation code
>>>>> incorrectly thought a legitimate access was a permission violation.
>>>>> 
>>>>> This patch fixes the bug, at the same time it fixes some build bug in the
>>>>> MMU debugging code (only exposed when DEBUG_MMU is enabled).
>>>> 
>>>> Thanks, applied to ppc-next :)
>>> 
>>> Nack, this does not fix ppc64 for me! How did you test it, Alex???
>> 
>> I booted a ppc64 kernel on a ppc64 Linux system with tcg and it booted fine for me. Maybe you're hitting yet another issue?
> 
> Which OpenBIOS did you use? The 32-bit version using mtsrin works now, but not the 64-bit HEAD version that I attached, which uses slb* instructions instead.

After some debugging, we tracked the 64-bit issue down to a bug in OpenBIOS, so things are fine in Qemu land now :)


Alex

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64
@ 2011-05-20  3:33 David Gibson
  0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2011-05-20  3:33 UTC (permalink / raw)
  To: agraf, qemu-devel; +Cc: andreas.farber, kennethsalerno, paulus

Early ppc64 CPUs include a hack to partially simulate the ppc32 segment
registers, by translating writes to them into writes to the SLB.  This is
not used by any current Linux kernel, but it is used by the openbios used
in the qemu mac99 model.

Commit 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d, cleaning up the SLB
handling introduced a bug in this code, breaking the openbios currently in
qemu.  Specifically, there was an off by one error bitshuffling the
register format used by mtsr into the format needed for the SLB load,
causing the flag bits to end up in the wrong place.  This caused the
storage keys to be wrong under openbios, meaning that the translation code
incorrectly thought a legitimate access was a permission violation.

This patch fixes the bug, at the same time it fixes some build bug in the
MMU debugging code (only exposed when DEBUG_MMU is enabled).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/helper.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 4238be6..4700632 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -606,7 +606,7 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h,
             r = pte64_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
-                    pteg_base + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
+                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
                     (int)((pte0 >> 1) & 1), ctx->ptem);
         } else
 #endif
@@ -621,7 +621,7 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h,
             r = pte32_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
-                    pteg_base + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
+                    pteg_off + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
                     (int)((pte0 >> 6) & 1), ctx->ptem);
         }
         switch (r) {
@@ -918,8 +918,7 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
                     if (eaddr != 0xEFFFFFFF)
                         LOG_MMU("1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
                                 " vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx
-                                " hash=" TARGET_FMT_plx " pg_addr="
-                                TARGET_FMT_plx "\n", env->htab_base,
+                                " hash=" TARGET_FMT_plx "\n", env->htab_base,
                                 env->htab_mask, vsid, ctx->ptem, ctx->hash[1]);
                     ret2 = find_pte(env, ctx, 1, rw, type,
                                     target_page_bits);
@@ -2140,7 +2139,7 @@ void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
         /* VSID = VSID */
         rs |= (value & 0xfffffff) << 12;
         /* flags = flags */
-        rs |= ((value >> 27) & 0xf) << 9;
+        rs |= ((value >> 27) & 0xf) << 8;
 
         ppc_store_slb(env, rb, rs);
     } else
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-05-21 12:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-20  3:34 [Qemu-devel] [PATCH] Fix a bug in mtsr/mtsrin emulation on ppc64 David Gibson
2011-05-20  7:40 ` Alexander Graf
2011-05-20 22:37   ` Andreas Färber
2011-05-21  1:58     ` Alexander Graf
2011-05-21  9:39       ` Andreas Färber
2011-05-21  9:44         ` Alexander Graf
2011-05-21 12:16         ` Alexander Graf
2011-05-21  9:39       ` Alexander Graf
2011-05-21  9:40   ` Andreas Färber
2011-05-21  9:46     ` Alexander Graf
2011-05-21 11:13       ` David Gibson
2011-05-21 11:22         ` Alexander Graf
2011-05-21  9:46 ` Andreas Färber
  -- strict thread matches above, loose matches on Subject: below --
2011-05-20  3:33 David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.