Richard Henderson <richard.hender...@linaro.org> writes: > On 6/15/21 4:32 AM, Bruno Piazera Larsen wrote: >> On 14/06/2021 19:37, Richard Henderson wrote: >>> On 6/14/21 12:16 PM, Bruno Larsen (billionai) wrote: >>>> This patch changes ppc_cpu_get_phys_page_debug so that it is now >>>> able to translate both, priviledged and real mode addresses >>>> independently of whether the CPU executing it has those permissions >>>> >>>> This was mentioned by Fabiano as something that would be very useful to >>>> help with debugging, but could possibly constitute a security issue if >>>> that debug function can be called in some way by prodution code. the >>>> solution was implemented such that it would be trivial to wrap it around >>>> ifdefs for building only with --enable-debug, for instance, but we are >>>> not sure this is the best approach, hence why it is an RFC. >>>> >>>> Suggested-by: Fabiano Rosas<faro...@linux.ibm.com> >>>> Signed-off-by: Bruno Larsen (billionai)<bruno.lar...@eldorado.org.br> >>>> --- >>>> target/ppc/mmu_helper.c | 23 +++++++++++++++++++++++ >>>> 1 file changed, 23 insertions(+) >>> >>> I think the first part is unnecessary. Either the cpu is in supervisor >>> mode or it >>> isn't, and gdb should use the correct address space. If you really want to >>> force >>> supervisor lookup from a guest that is paused in usermode, I suppose you >>> could force >>> MSR.PR=1 while you're performing the access and set it back afterward. >> I don't see why GDB should not be able to see supervisor level addresses >> just because the >> CPU can't. > > Because then when you are debugging, you then don't know whether the address > is actually > accessible in the current cpu context. >
@Bruno, so this is what I referred to somewhere else on the thread, people expect GDB to have the same access level of the currently executing code. So implementing my suggestion would break their workflow. >>> I think the second part is actively wrong -- real-mode address lookup will >>> (for the most >>> part) always succeed. Moreover, the gdb user will have no idea that you've >>> silently >>> changed addressing methods. >> >> I disagree. Real-mode address will mostly fail, since during the boot >> process Linux >> kernels set the MMU to use only virtual addresses, so real mode addresses >> only work when >> debugging the firmware or the early setup of the kernel. After that, GDB can >> basically >> only see virtual addresses. > > Exactly. But you changed that so that any unmapped address will re-try with > real-mode, > which (outside of hv) simply maps real->physical and returns the input. > > One should have to perform some special action to see addresses in a > different cpu > context. I don't think that gdb supports such a special action at the > moment. If you > want that feature though, that's where you should start. I think we can just drop this patch. The scenarios where debugging across MMU contexts happen are quite limited. My use case was a while back when implementing single-step for KVM guests; there were some situations where GDB would have issues setting breakpoints around kernel code that altered MSR_IR/DR. But that is mostly anecdotal at this point. If I ever run into that again, now I know where to look. > > > r~