diff mbox

monitor: Add an "xlate" command for translating a virtual address

Message ID 1471089120.12231.48.camel@kernel.crashing.org
State New
Headers show

Commit Message

Benjamin Herrenschmidt Aug. 13, 2016, 11:52 a.m. UTC
This is very handy when debugging a guest, especially when it's
stuck on accessing some HW and the only way to figure out what
specific piece of HW is to translate the virtual address to
a hardware address that can then be matched with the mtree

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 hmp-commands.hx | 15 +++++++++++++++
 monitor.c       | 11 +++++++++++
 2 files changed, 26 insertions(+)

Comments

Peter Maydell Aug. 14, 2016, 6:55 p.m. UTC | #1
On 13 August 2016 at 12:52, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This is very handy when debugging a guest, especially when it's
> stuck on accessing some HW and the only way to figure out what
> specific piece of HW is to translate the virtual address to
> a hardware address that can then be matched with the mtree
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Should this wrap cpu_get_phys_page_attrs_debug() instead
(and report the attributes to the user)?

thanks
-- PMM
Benjamin Herrenschmidt Aug. 14, 2016, 9:24 p.m. UTC | #2
On Sun, 2016-08-14 at 19:55 +0100, Peter Maydell wrote:
> On 13 August 2016 at 12:52, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > 
> > This is very handy when debugging a guest, especially when it's
> > stuck on accessing some HW and the only way to figure out what
> > specific piece of HW is to translate the virtual address to
> > a hardware address that can then be matched with the mtree
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Should this wrap cpu_get_phys_page_attrs_debug() instead
> (and report the attributes to the user)?

Ah good idea, I'll look into it.

On a vaguely related note, do you know if anybody is planning (or
trying to) overhaul that API to get the access type as an input as well
? We can have widely different translations for data and instructions
and that has been a problem (for example if some OS uses a split MMU
mode like that, currently -d in_asm will try to disassemble using data
translation).

Cheers,
Ben.
Benjamin Herrenschmidt Aug. 14, 2016, 10:55 p.m. UTC | #3
On Sun, 2016-08-14 at 19:55 +0100, Peter Maydell wrote:
> On 13 August 2016 at 12:52, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > 
> > This is very handy when debugging a guest, especially when it's
> > stuck on accessing some HW and the only way to figure out what
> > specific piece of HW is to translate the virtual address to
> > a hardware address that can then be matched with the mtree
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Should this wrap cpu_get_phys_page_attrs_debug() instead
> (and report the attributes to the user)?

Looking at this... the attributes are a bit of a mess aren't they ?

The requester_id is pretty much PCI specific and only useful for
load/stores coming from a device (for IOMMUs), the "secure" bit
seems to be an ARM thing and is an output from translation,
what about "user" ?

IE, it's a blend of things that are input to an access and things
that are output from translate as far as I can tell ...

For the monitor, I'm thinking of just printing "secure", what do
you think ?

Cheers,
Ben.
Peter Maydell Aug. 15, 2016, 10:02 a.m. UTC | #4
On 14 August 2016 at 23:55, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Looking at this... the attributes are a bit of a mess aren't they ?
>
> The requester_id is pretty much PCI specific and only useful for
> load/stores coming from a device (for IOMMUs), the "secure" bit
> seems to be an ARM thing and is an output from translation,
> what about "user" ?
> IE, it's a blend of things that are input to an access and things
> that are output from translate as far as I can tell ...

The intention is that they are attributes as you would see on
the bus for a memory transaction. In particular for ARM the
'secure' bit corresponds to a transaction attribute indicating
that the access is to the 'secure' physical address space.
'user' also corresponds to an ARM hardware bus signal indicating
the privilege level of the access [devices can behave differently
for user vs non-user accesses, for instance].
requester_id is the bus attribute for 'who sent this transaction'.
In real ARM AXI bus hardware this can distinguish which core
in the CPU sent the transaction, though we don't model this in
QEMU. It's not PCI specific, it just happens that our current
use for it is PCI related. (The ARM GICv3 ITS interrupt controller
hardware distinguishes which device just asked it to do something
by looking at the ID of the memory write that the device did.
It looks like x86 has a use for it too.)

'secure' is used also by x86 but for somewhat different purposes
relating to SMM.

Some of these attributes are determined before the MMU
(eg "user", "requester ID"), and some of them are (for
a particular CPU architecture) determined by the MMU
from the translation tables ("secure"). From the point of
view of the CPU as a whole (or other transaction master)
they're all outputs, though.

thanks
-- PMM
Benjamin Herrenschmidt Aug. 15, 2016, 11:34 a.m. UTC | #5
On Mon, 2016-08-15 at 11:02 +0100, Peter Maydell wrote:
> 
> Some of these attributes are determined before the MMU
> (eg "user", "requester ID"), and some of them are (for
> a particular CPU architecture) determined by the MMU
> from the translation tables ("secure"). From the point of
> view of the CPU as a whole (or other transaction master)
> they're all outputs, though.

So you suggest I just print out all the fields as the output
of translate along with the translated address ?

Cheers,
Ben.
Peter Maydell Aug. 15, 2016, 11:44 a.m. UTC | #6
On 15 August 2016 at 12:34, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Mon, 2016-08-15 at 11:02 +0100, Peter Maydell wrote:
>> Some of these attributes are determined before the MMU
>> (eg "user", "requester ID"), and some of them are (for
>> a particular CPU architecture) determined by the MMU
>> from the translation tables ("secure"). From the point of
>> view of the CPU as a whole (or other transaction master)
>> they're all outputs, though.
>
> So you suggest I just print out all the fields as the output
> of translate along with the translated address ?

Certainly I think the QMP protocol command should just
return all the info (I think current best-practice is
to implement new HMP commands as wrappers round an
equivalent QMP command). At that point we get into UI
design questions about whether we should just print
everything for the HMP command output, or have it default
to just-address with an option to print with the attributes.
I don't have a strong opinion there.

thanks
-- PMM
Benjamin Herrenschmidt Aug. 15, 2016, 11:56 a.m. UTC | #7
On Mon, 2016-08-15 at 12:44 +0100, Peter Maydell wrote:
> Certainly I think the QMP protocol command should just
> return all the info (I think current best-practice is
> to implement new HMP commands as wrappers round an
> equivalent QMP command). At that point we get into UI
> design questions about whether we should just print
> everything for the HMP command output, or have it default
> to just-address with an option to print with the attributes.
> I don't have a strong opinion there.

Ok I have no experience with QMP, I'll have a look, but from a UI
perspective, it's easier to not bother too much and just print it
all, something like:

translated_addr [secure=x,user=y,req_id=id]

This is low level debugging stuff, the user can cope with the verbose
output.

Ben.
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 848efee..2d0ce4e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -569,6 +569,21 @@  Write to I/O port.
 ETEXI
 
     {
+        .name       = "xlate",
+        .args_type  = "addr:l",
+        .params     = "addr",
+        .help       = "Translate virtual address via the MMU",
+        .mhandler.cmd = hmp_mmu_xlate,
+    },
+
+STEXI
+@item xlate @var{addr}
+@findex xlate
+Translate virtual address
+
+ETEXI
+
+    {
         .name       = "sendkey",
         .args_type  = "keys:s,hold-time:i?",
         .params     = "keys [hold_ms]",
diff --git a/monitor.c b/monitor.c
index 5d68a5d..b2b3a6e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1505,6 +1505,17 @@  static void hmp_ioport_write(Monitor *mon, const QDict *qdict)
     }
 }
 
+static void hmp_mmu_xlate(Monitor *mon, const QDict *qdict)
+{
+    CPUState *cpu = mon_get_cpu();
+    vaddr va = qdict_get_int(qdict, "addr");
+    hwaddr pa;
+
+    pa = cpu_get_phys_page_debug(cpu, va);
+    pa |= (va & ~TARGET_PAGE_MASK);
+    monitor_printf(mon, "0x%#" HWADDR_PRIx "\n", pa);
+}
+
 static void hmp_boot_set(Monitor *mon, const QDict *qdict)
 {
     Error *local_err = NULL;