| Message ID | 1305862499-12328-1-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
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
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
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 >
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
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
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
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
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
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
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.
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
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
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
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(-)