| Message ID | 1371567181-4917-2-git-send-email-chouteau@adacore.com |
|---|---|
| State | New |
| Headers | show |
On 18.06.2013, at 16:53, Fabien Chouteau wrote: > On PPC 6xx, data and code have separated TLBs. Until now QEMU was only > looking at data TLBs, which is not good when GDB wants to read code. > > This patch adds a second call to get_physical_address() with an > ACCESS_CODE type of access when the first call with ACCESS_INT fails. > > Signed-off-by: Fabien Chouteau <chouteau@adacore.com> > --- > target-ppc/mmu_helper.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c > index 910e022..19f0b8c 100644 > --- a/target-ppc/mmu_helper.c > +++ b/target-ppc/mmu_helper.c > @@ -1378,7 +1378,15 @@ hwaddr cpu_get_phys_page_debug(CPUPPCState *env, target_ulong addr) > } > > if (unlikely(get_physical_address(env, &ctx, addr, 0, ACCESS_INT) != 0)) { > - return -1; > + > + /* Some MMUs have separate TLBs for code and data. If we only try an > + * ACCESS_INT, we may not be able to read instructions mapped by code > + * TLBs, so we also try a ACCESS_CODE. This is pretty ugly, but I don't see any other way to conveniently give gdb the information it needs. Let's ask Jan whether he has an idea. Maybe a monitor command to switch memory access modes? Alex > + */ > + if (unlikely(get_physical_address(env, &ctx, addr, 0, > + ACCESS_CODE) != 0)) { > + return -1; > + } > } > > return ctx.raddr & TARGET_PAGE_MASK; > -- > 1.7.9.5 >
On 2013-06-18 17:34, Alexander Graf wrote: > > On 18.06.2013, at 16:53, Fabien Chouteau wrote: > >> On PPC 6xx, data and code have separated TLBs. Until now QEMU was only >> looking at data TLBs, which is not good when GDB wants to read code. >> >> This patch adds a second call to get_physical_address() with an >> ACCESS_CODE type of access when the first call with ACCESS_INT fails. >> >> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >> --- >> target-ppc/mmu_helper.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c >> index 910e022..19f0b8c 100644 >> --- a/target-ppc/mmu_helper.c >> +++ b/target-ppc/mmu_helper.c >> @@ -1378,7 +1378,15 @@ hwaddr cpu_get_phys_page_debug(CPUPPCState *env, target_ulong addr) >> } >> >> if (unlikely(get_physical_address(env, &ctx, addr, 0, ACCESS_INT) != 0)) { >> - return -1; >> + >> + /* Some MMUs have separate TLBs for code and data. If we only try an >> + * ACCESS_INT, we may not be able to read instructions mapped by code >> + * TLBs, so we also try a ACCESS_CODE. > > This is pretty ugly, but I don't see any other way to conveniently give gdb the information it needs. Let's ask Jan whether he has an idea. Maybe a monitor command to switch memory access modes? I suppose the gdb frontend is not willing to tell us what kind of memory it accesses (code disassembling or data access etc.), right? So we can only guess here, ie. try both. I don't see how a monitor-based side channel could help. If we touched gdb, we could also touch the remote protocol (for memory accesses). Jan
On 21.06.2013, at 20:10, Jan Kiszka wrote: > On 2013-06-18 17:34, Alexander Graf wrote: >> >> On 18.06.2013, at 16:53, Fabien Chouteau wrote: >> >>> On PPC 6xx, data and code have separated TLBs. Until now QEMU was only >>> looking at data TLBs, which is not good when GDB wants to read code. >>> >>> This patch adds a second call to get_physical_address() with an >>> ACCESS_CODE type of access when the first call with ACCESS_INT fails. >>> >>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >>> --- >>> target-ppc/mmu_helper.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c >>> index 910e022..19f0b8c 100644 >>> --- a/target-ppc/mmu_helper.c >>> +++ b/target-ppc/mmu_helper.c >>> @@ -1378,7 +1378,15 @@ hwaddr cpu_get_phys_page_debug(CPUPPCState *env, target_ulong addr) >>> } >>> >>> if (unlikely(get_physical_address(env, &ctx, addr, 0, ACCESS_INT) != 0)) { >>> - return -1; >>> + >>> + /* Some MMUs have separate TLBs for code and data. If we only try an >>> + * ACCESS_INT, we may not be able to read instructions mapped by code >>> + * TLBs, so we also try a ACCESS_CODE. >> >> This is pretty ugly, but I don't see any other way to conveniently give gdb the information it needs. Let's ask Jan whether he has an idea. Maybe a monitor command to switch memory access modes? > > I suppose the gdb frontend is not willing to tell us what kind of memory > it accesses (code disassembling or data access etc.), right? So we can > only guess here, ie. try both. I don't see how a monitor-based side > channel could help. If we touched gdb, we could also touch the remote > protocol (for memory accesses). Thanks, applied to ppc-next. Teaching gdb a new protocol is way out of scope for these rare cases :). Alex
On 06/22/2013 02:26 AM, Alexander Graf wrote: > Thanks, applied to ppc-next. Teaching gdb a new protocol is way out of scope for these rare cases :). > Thanks guys,
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c index 910e022..19f0b8c 100644 --- a/target-ppc/mmu_helper.c +++ b/target-ppc/mmu_helper.c @@ -1378,7 +1378,15 @@ hwaddr cpu_get_phys_page_debug(CPUPPCState *env, target_ulong addr) } if (unlikely(get_physical_address(env, &ctx, addr, 0, ACCESS_INT) != 0)) { - return -1; + + /* Some MMUs have separate TLBs for code and data. If we only try an + * ACCESS_INT, we may not be able to read instructions mapped by code + * TLBs, so we also try a ACCESS_CODE. + */ + if (unlikely(get_physical_address(env, &ctx, addr, 0, + ACCESS_CODE) != 0)) { + return -1; + } } return ctx.raddr & TARGET_PAGE_MASK;
On PPC 6xx, data and code have separated TLBs. Until now QEMU was only looking at data TLBs, which is not good when GDB wants to read code. This patch adds a second call to get_physical_address() with an ACCESS_CODE type of access when the first call with ACCESS_INT fails. Signed-off-by: Fabien Chouteau <chouteau@adacore.com> --- target-ppc/mmu_helper.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)