[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledge
From: |
Fabiano Rosas |
Subject: |
Re: [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledged addresses |
Date: |
Tue, 15 Jun 2021 18:37:19 -0300 |
Richard Henderson <richard.henderson@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<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.
>
> 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~
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus, Greg Kurz, 2021/06/14
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus, Fabiano Rosas, 2021/06/14
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus, David Gibson, 2021/06/14
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus, Cédric Le Goater, 2021/06/15