Re: [Qemu-devel] [PATCH] hmp: Improve 'info mtree' with optional parm for mapinfo

[Thorsten Kohfeldt]

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.
>
> Thanks,
>
> Paolo

Paolo,

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.


@ALL
I have provided 2 patch branches in github,
one for qemu-2.7.0 and
one for qemu-current-master (this needed a tiny sed-conversion, see below).
I also placed some example info mtree mapinfo output on gist.github:


# patch commit for qemu-2.7 (same patch also works for qemu-2.6):
https://github.com/Thorsten-Kohfeldt/qemu/commit/5633a3cdbf6fd7cccd098fb83f591fbb15e8d383
# in branch:
https://github.com/Thorsten-Kohfeldt/qemu/commits/monitor_--hmp_info_mtree_mapinfo-width

# 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:
https://github.com/Thorsten-Kohfeldt/qemu/commit/60b8c1be1a0119df1f1859b0d484e06e709c2ea2
# in branch:
https://github.com/Thorsten-Kohfeldt/qemu/commits/upstream-pullrequest-mapinfo-V1


# sample output info mtree 9
https://gist.github.com/Thorsten-Kohfeldt/254b5a21fef497054959f58af53a44c9

# sample output info mtree 65
https://gist.github.com/Thorsten-Kohfeldt/39cf5c8521c1999518b3438315e439f4


Regards,

Thorsten