[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hmp: Improve 'info mtree' with optional parm fo
|
From: |
Thorsten Kohfeldt |
|
Subject: |
Re: [Qemu-devel] [PATCH] hmp: Improve 'info mtree' with optional parm for mapinfo |
|
Date: |
Tue, 20 Sep 2016 20:23:31 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
Am 20.09.2016 um 02:51 schrieb Laszlo Ersek:
On 09/20/16 02:16, Thorsten Kohfeldt wrote:
Am 15.09.2016 um 11:52 schrieb Paolo Bonzini:
On 07/09/2016 02:48, Thorsten Kohfeldt wrote:
From: Thorsten Kohfeldt <address@hidden>
Date: Wed, 31 Aug 2016 22:43:14 +0200
Subject: [PATCH] hmp: Improve 'info mtree' with optional parm for
mapinfo
Motivation
When 'tuning' 'quirks' for VFIO imported devices, it is not easy to
directly grasp the implications of the priorisation algorithms in place
for the 'layered mapping' of memory regions.
Even though there are rules (documented in docs/memory.txt), once in a
while one might question the correctness of the actual implementation of
the rules.
Particularly, I believe I have uncovered a divergence of (sub-)region
priorisation/order/visibility in a corner case of importing a device
(which requires a 'quirk') with mmap enabled vs. mmap disabled.
This modification provides a means of visualising the ACTUAL
mapping/visibility/occlusion of subregions within regions, whereas the
current info mtree command only lists the tree of regions (all, visible
and invisible ones).
It is primarily intended to provide support for easy presentation of my
cause, but I strongly believe this modification also has general purpose
advantages.
It is a useful addition, but I think a simpler presentation is also
fine. What about "info mtree -f" which would visit the FlatView instead
of the tree? The patch would probably be much shorter.
For quite some time I had the patch in use as a direct modification of
'info mtree', but I felt that a general purpose use must provide an ad
hoc user selectable presentation width parameter for the map info.
I personally use a width of 65 or even 129 characters PREFIXING the
tree elements which the command currently responds.
My guess is though that most users would want a width of only 9 or 17.
So I believe that a numerical parameter is a must.
Visit the flat view -
I'm not sure I understand you. Do you suggest to traverse a completely
different data structure ?
The purpose of the suggested visualisation is to point out where
each of the tree's memory regions are "pinched" by other regions, so
their "native" contents is NOT visible any more throughout the full
region length, but (fractionally) rather another regions's content.
Thus, I personally require to traverse exactly that tree structure.
No offence, but I would rather not want to modify the patch towards
what I feel would be a completely different purpose.
I would appreciate if someone would review the patch in its current
functional form to get it queued for qemu 2.8.
My intention is to be able to rely on communication partners being
able to reproduce findings using the new command once I start to
attack the VFIO mmap flaw I talk about in the commit comment.
# PATCH (as published in mailing list) *CONVERSION* from
qemu-2.6/qemu-2.7 to qemu-master:
sed s/'[.]mhandler[.]cmd = '/'.cmd = '/ <qemu-2.6and7.patch
qemu-master.patch
# patch commit adapted that way for qemu-master and frequently rebased in
branch:
https://github.com/Thorsten-Kohfeldt/qemu/commits/upstream-pullrequest-mapinfo-V1-rebased
# sample output info mtree 9
https://gist.github.com/Thorsten-Kohfeldt/254b5a21fef497054959f58af53a44c9
# sample output info mtree 65
https://gist.github.com/Thorsten-Kohfeldt/39cf5c8521c1999518b3438315e439f4
I think your current output explains, for each part (that is, each "sample"
> of user-selected size) of each MemoryRegion, whether that sample is actually
> visible in the finally rendered, flat view of the address space(s).
> (And, if not, why not.)
If not why not: Right. That _is_ my goal.
Whereas I think Paolo's idea is to map the flat view to begin with,
> and visually associate each interval of the guest visible physical address
> space with the one region that is ultimately accessed in that interval.
> This too would explain what the guest sees where, and why. It wouldn't give
> much information about ranges that the guest can *not* access (due to
> occlusion by other regions or otherwise),
> but maybe the "why not" is not so important after all?
To the contrary IMHO. See my whole line of statements above.
The "why not" explanation is all what drove me to attempt to make this patch
an official part of qemu - as a tool for 'bugfix' discussions.
Otherwise I'd have to state "apply this patch then you see what I mean".
For me that "why not" is an important part of information from point of view
of a "memory region subtree/stack maintainer".
Please note that while hunting down that VFIO/mmap problem around last Xmas
I _was_ experimenting with flat view dumps.
But only the final mtree mapinfo was the real eye opener for me.
Regarding the FlatView thing -- QEMU already maintains a flat rendering of
> the memory region tree, so that guest accesses to the various regions can
> be quickly dispatched to the accessors that can serve these accesses.
> Memory regions are massaged (created, deleted, enabled/disabled etc) in
> "transactions", and when the outermost transaction is committed,
> the flat-view is re-rendered (which is expensive).
I knew that already, but thank you for summarising it quite to the point.
I think it would make sense to work from the pre-rendered FlatView,
> if the information you can get out of it covers your needs.
I assume you know by now that I do _not_ feel that it covers my needs.
Here's an example, from one of your sample outputs: you currently have
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
00000000000c0000-00000000000c3fff (prio 1, RW): alias pam-ram @pc.ram
00000000000c0000-00000000000c3fff [disabled]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
00000000000c0000-00000000000c3fff (prio 1, RW): alias pam-pci @pc.ram
00000000000c0000-00000000000c3fff [disabled]
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
00000000000c0000-00000000000c3fff (prio 1, R-): alias pam-rom @pc.ram
00000000000c0000-00000000000c3fff
ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
00000000000c0000-00000000000c3fff (prio 1, RW): alias pam-pci @pci
00000000000c0000-00000000000c3fff [disabled]
with
@: alias region mapped at sample
~: alias region mappable but disabled at sample
o: region occluded by some other region at sample
If you have an address in the c0000-c3fff range, you have to consult
> all four lines to see which region will match.
Actually, I feel this is too simple an example.
The real stuff starts when you mix in different priorities and mmap.
Please note that I do not remember negative priorities being employed,
but I would want to make a pass at suggesting to use them in order to
achieve a certain reduction in complexity for VFIO/mmap region stacks.
That suggestion is way easier explained when I can refer to the
info mtree mapinfo output.
Working off of the FlatView, first I think you would find the right
> resolution for the output "for free" (you wouldn't need a user sample
> size -- the interval c0000-c3fff would neither need further subdivision
> nor be blurred by over-coarse resolution).
I knew all along that this is a weak point of my tree sampler.
But I still prefer its combination of information presented.
> You could represent the c0000-c3fff interval (and every other interval
> too) with a single letter, such as P, where P would stand for
> "alias pam-rom @pc.ram 00000000000c0000-00000000000c3fff".
Second, given an address in c0000-c3fff, there would be only one range
to consult (same as QEMU itself does with FlatView), and you'd find
> the MemoryRegion visible in that range at once.
Thanks
Laszlo
Please see also my answer to Paolo's follow up mail.