All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Fabiano Rosas <farosas@linux.ibm.com>
To: Richard Henderson <richard.henderson@linaro.org>,
	Bruno Piazera Larsen <bruno.larsen@eldorado.org.br>,
	qemu-devel@nongnu.org
Cc: luis.pires@eldorado.org.br, Greg Kurz <groug@kaod.org>,
	lucas.araujo@eldorado.org.br, fernando.valle@eldorado.org.br,
	qemu-ppc@nongnu.org, matheus.ferst@eldorado.org.br,
	david@gibson.dropbear.id.au
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	[thread overview]
Message-ID: <877diuq06o.fsf@linux.ibm.com> (raw)
In-Reply-To: <7ce3cd57-0abf-f0d9-11ec-6fdc42b89b62@linaro.org>

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~


  reply	other threads:[~2021-06-15 21:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 19:16 [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus Bruno Larsen (billionai)
2021-06-14 19:16 ` [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledged addresses Bruno Larsen (billionai)
2021-06-14 19:37   ` Philippe Mathieu-Daudé
2021-06-15  1:41     ` David Gibson
2021-06-15 12:12     ` Bruno Piazera Larsen
2021-06-14 21:25   ` Fabiano Rosas
2021-06-15 11:59     ` Bruno Piazera Larsen
2021-06-14 22:37   ` Richard Henderson
2021-06-15 11:32     ` Bruno Piazera Larsen
2021-06-15 20:00       ` Richard Henderson
2021-06-15 21:37         ` Fabiano Rosas [this message]
2021-06-16 12:07           ` Bruno Piazera Larsen
2021-06-16  6:18       ` David Gibson
2021-06-14 19:29 ` [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus Greg Kurz
2021-06-14 21:04 ` Fabiano Rosas
2021-06-15  1:18   ` David Gibson
2021-06-15  1:41 ` David Gibson
2021-06-15  3:20   ` Richard Henderson
2021-06-15 12:25     ` Bruno Piazera Larsen
2021-06-16  6:16       ` David Gibson
2021-06-15 13:57 ` Cédric Le Goater
2021-06-15 14:14   ` Philippe Mathieu-Daudé
2021-06-15 14:57   ` Bruno Piazera Larsen
2021-06-15 15:57     ` Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877diuq06o.fsf@linux.ibm.com \
    --to=farosas@linux.ibm.com \
    --cc=bruno.larsen@eldorado.org.br \
    --cc=david@gibson.dropbear.id.au \
    --cc=fernando.valle@eldorado.org.br \
    --cc=groug@kaod.org \
    --cc=lucas.araujo@eldorado.org.br \
    --cc=luis.pires@eldorado.org.br \
    --cc=matheus.ferst@eldorado.org.br \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.