On Tue, Jun 15, 2021 at 08:32:32AM -0300, 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<farosas@linux.ibm.com>
> > > Signed-off-by: Bruno Larsen (billionai)<bruno.larsen@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. when debugging, we wanna see exactly what QEMU
> sees, not what the guest sees, right?

That kind of depends whether you mean gdb attached to the gdb socket
provided by qemu - in which case I think you want it to see what the
guest sees - or gdb debugging qemu itself, in which case it does want
to see what qemu sees, but doesn't use this code path AFAIK.

> Now, if this is changing more than
> just privilege level, I agree there is a problem, but I wouldn't think it is
> the case...

> > 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.
> 
> Maybe there is a better way to handle this by having GDB warn the user that
> the CPU can not decode the address in it's current state, but I do think it
> is a good tool to have, as it would've made debugging the first RFC on this
> topic a bit easier, and farosas was actively complaining that isn't a
> feature yet.
> 
> > 
> > r~

-- 
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