Message ID | 1471089120.12231.48.camel@kernel.crashing.org |
---|---|
State | New |
Headers | show |
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
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.
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.
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
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.
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
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 --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;
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(+)