From: Arianna Avanzini <avanzini.arianna@gmail.com>
To: xen-devel@lists.xen.org
Cc: Ian.Campbell@eu.citrix.com, paolo.valente@unimore.it,
keir@xen.org, stefano.stabellini@eu.citrix.com,
Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com,
tim@xen.org, julien.grall@citrix.com, etrudeau@broadcom.com,
JBeulich@suse.com, avanzini.arianna@gmail.com,
viktor.kleinik@globallogic.com
Subject: [PATCH v5 0/8] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
Date: Mon, 7 Apr 2014 01:31:52 +0200 [thread overview]
Message-ID: <1396827120-30617-1-git-send-email-avanzini.arianna@gmail.com> (raw)
Hello,
here is my fifth attempt at proposing an implementation of the memory_mapping
DOMCTL for the ARM architecture. The following patchset adds both the proposed
implementation of the hypercall and some related changes made to existing code
after having the benefit of many comments to the previous proposal attempts
([1]). As of those previous discussions, I'm not summarizing them in the below
description to prevent this cover letter from becoming much longer than
appropriate, but more details should be available, if needed, in the cover
letter of the third and fourth patchsets ([2]).
I am aware of the fact that there is still on-going discussion about the
previous version of this patchset ([3]), but, as also suggested by Stefano
Stabellini, I thought I'd send this new version to try to get rid of the most
immediate issues that have been highlighted.
The first commit in this patchset implements the bookkeeping operations deemed
necessary to make a domain's iomem_caps coherent with the I/O-memory mapping
operations performed during domain build. As of now, the change is dom0-related,
even if the function affected by the change if generic.
The second commit adds to the apply_p2m_changes() function some consistency
checks useful for the REMOVE operation. More in detail, the REMOVE switch of
the function now checks:
. the validity of the mapping: if the mapping is found not to be valid, the
page is skipped;
. the fact that the gfn whose mapping is to be removed is actually mapped to
the machine address given as parameter.
With respect to v4, this commit uses the "maddr" parameter of the function to
keep the machine address used for the above-described comparison, incrementing
it where necessary (also when first and second level mappings are not valid), as
suggested by Julien Grall and Ian Campbell. It also gets the actual mfn mapped
to the gfn given as parameter to the function in the REMOVE case of the switch
construct, as in other cases it might be invalid and its value might be
uncorrectly used, as pointed out by Julien Grall. It also removes a harmful
ASSERT on the validity of the mapping, which was introduced by the v4 patchset
in the REMOVE case; the v5 commit turns it to an if(), as suggested by Julien
Grall.
The third commit changes the interface of the existing map_mmio_regions()
function for ARM. As of now, the function takes parameters of paddr_t type; this
interface needs caller functions to correctly page-align addresses. This commit
lets this function take page frame numbers as parameters instead, trying also to
modify callers to use the new interface. With respect to v4, the v5 commit adds
a macro for the paddr_to_pfn(PAGE_ALIGN(...)) pattern which was repeatedly used
throughout the patch, as pointed out by Julien Grall.
This commit still attempts to produce the minimum necessary changes; as a
consequence, it still does not modify also the interface of apply_p2m_changes(),
which instead was suggested by Stefano Stabellini.
The fourth commit adds to clear_mmio_p2m_entry() a check on the actual existence
of a mapping between the guest frame number and a machine frame number given as
parameter; the check is performed before removing such a mapping. This change
has been implemented after having been discussed with Jan Beulich, and should be
the equivalent of that added by commit 0002 of this patchset to the ARM-specific
code removing a p2m mapping.
The fifth commit moves the existing memory_mapping DOMCTL for x86 to
xen/common/domctl.c and attempts to adapt the code to be used for both the x86
and ARM architectures. More in detail, the arch-specific operations necessary to
map and unmap memory ranges are abstracted out in a pair of map_mmio_regions()
and unmap_mmio_regions() functions.
With respect to the v4 patchset, this commit attempts to address the following
issues.
. The unmap_mmio_regions() function for the x86 architecture now returns a
proper error code, instead of relying on a wrongly-used true/false return
value, as suggested by Jan Beulich.
. It restores the correct handling of return values from the iomem_deny_access()
and unmap_mmio_regions() in the remove path of the hypercall, while the v4
version uncorrectly let an error code returned by unmap_mmio_regions() be
ignored, as pointed out by Jan Beulich and discussed with Stefano Stabellini.
. It adds to the new DOMCTL a comment which attempts to explain how error codes
from the above functions are handled.
. It lets the DOMCTL compute gfn_end - 1 and mfn_end - 1 only once, as suggested
by Jan Beulich.
. It uses a new local variable to keep the return value instead of re-using the
"add" local variable, which was confusing as pointed out by Ian Campbell.
. It renames the newly-added header, common to the x86 and ARM architectures,
from p2m.h to p2m-common.h, as suggested by Jan Beulich.
The sixth commit adds to libxl the handling code for a new optional parameter to
the "iomem" configuration option. The new parameter, called "gfn", allows to
specify the start guest address used for the mapping of the given machine
address range; in case no start guest address is specified, the patch lets libxl
default its value to the start machine address.
With respect to the v4 corresponding one, this commit tries to address the
following issues, pointed out by Julien Grall and Ian Campbell.
. It uses -1 as a default value for the optional "gfn" parameter.
. It lets the default value of "gfn" be initialized with a proper call to the
libxl_iomem_range_init() function.
. It defers to libxl the assignment of the "gfn" parameter to the start machine
address, if "gfn" was initialized to its default value.
. It uses a local variable to keep the return value of the sscanf() function
as to makes the code more readable.
The seventh commit adds two new architecture helpers to libxl: these helpers
should allow the caller to establish if a domain uses an auto-translated
physmap. This is instrumental to commit 0008, which needs to call the newly-
added DOMCTL only for auto-translated guests.
The eighth commit adds to libxl code to handle the iomem configuration option by
invoking the memory_mapping DOMCTL. The hypercall is invoked only if the domain
is auto-translated, by using the arch-specific helpers introduced with the
previous commit.
The code has been tested on a cubieboard2, with Linux v3.13 as a dom0 and ERIKA
Enterprise ([4]) as a domU.
As usual, any feedback about this new version is more than welcome.
Arianna
[1] http://markmail.org/message/d5vhhiin5tr2yee7
http://markmail.org/message/dtzsvauzob332gnq
http://markmail.org/message/fy7ioiv37gbp22an
http://markmail.org/message/qca2nxrpe2dfwdks
[2] http://lists.xen.org/archives/html/xen-devel/2014-03/msg01724.html
http://lists.xen.org/archives/html/xen-devel/2014-03/msg03138.html
[3] http://lists.xen.org/archives/html/xen-devel/2014-04/msg00118.html
http://lists.xen.org/archives/html/xen-devel/2014-04/msg00123.html
[4] http://erika.tuxfamily.org/drupal/
Arianna Avanzini (8):
arch, arm: domain build: let dom0 access I/O memory of mapped devices
arch, arm: add consistency check to REMOVE p2m changes
arch, arm: let map_mmio_regions() take pfn as parameters
arch, x86: check if mapping exists before memory_mapping removes it
xen, common: add the XEN_DOMCTL_memory_mapping hypercall
tools, libxl: parse optional start gfn from the iomem config option
tools, libxl: add helpers to establish if guest is auto-translated
tools, libxl: handle the iomem parameter with the memory_mapping hcall
docs/man/xl.cfg.pod.5 | 13 +++++--
tools/libxl/libxl.h | 10 +++++
tools/libxl/libxl_arch.h | 2 +
tools/libxl/libxl_arm.c | 5 +++
tools/libxl/libxl_create.c | 19 ++++++++++
tools/libxl/libxl_types.idl | 7 +++-
tools/libxl/libxl_x86.c | 6 +++
tools/libxl/xl_cmdimpl.c | 19 +++++-----
xen/arch/arm/domain_build.c | 18 +++++++--
xen/arch/arm/gic.c | 21 ++++++-----
xen/arch/arm/p2m.c | 52 ++++++++++++++++++++++----
xen/arch/arm/platforms/exynos5.c | 12 +++---
xen/arch/arm/platforms/omap5.c | 21 ++++++-----
xen/arch/arm/platforms/xgene-storm.c | 4 +-
xen/arch/x86/domctl.c | 70 -----------------------------------
xen/arch/x86/mm/p2m.c | 65 ++++++++++++++++++++++++++++++---
xen/common/domctl.c | 71 ++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/mm.h | 2 +
xen/include/asm-arm/p2m.h | 8 ++--
xen/include/asm-x86/p2m.h | 3 +-
xen/include/xen/p2m-common.h | 16 ++++++++
21 files changed, 313 insertions(+), 131 deletions(-)
create mode 100644 xen/include/xen/p2m-common.h
--
1.9.1
next reply other threads:[~2014-04-06 23:31 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-06 23:31 Arianna Avanzini [this message]
2014-04-06 23:31 ` [PATCH v5 1/8] arch, arm: domain build: let dom0 access I/O memory of mapped devices Arianna Avanzini
2014-04-06 23:31 ` [PATCH v5 2/8] arch, arm: add consistency check to REMOVE p2m changes Arianna Avanzini
2014-04-07 11:05 ` Julien Grall
2014-04-09 13:39 ` Ian Campbell
2014-04-22 19:27 ` Julien Grall
2014-04-06 23:31 ` [PATCH v5 3/8] arch, arm: let map_mmio_regions() take pfn as parameters Arianna Avanzini
2014-04-07 14:56 ` Julien Grall
2014-04-09 13:54 ` Ian Campbell
2014-04-12 9:20 ` Arianna Avanzini
2014-04-06 23:31 ` [PATCH v5 4/8] arch, x86: check if mapping exists before memory_mapping removes it Arianna Avanzini
2014-04-06 23:31 ` [PATCH v5 5/8] xen, common: add the XEN_DOMCTL_memory_mapping hypercall Arianna Avanzini
2014-04-07 7:55 ` Jan Beulich
2014-04-09 14:03 ` Ian Campbell
2014-04-06 23:31 ` [PATCH v5 6/8] tools, libxl: parse optional start gfn from the iomem config option Arianna Avanzini
2014-04-09 14:10 ` Ian Campbell
2014-04-06 23:31 ` [PATCH v5 7/8] tools, libxl: add helpers to establish if guest is auto-translated Arianna Avanzini
2014-04-06 23:32 ` [PATCH v5 8/8] tools, libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
2014-04-09 14:25 ` Ian Campbell
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=1396827120-30617-1-git-send-email-avanzini.arianna@gmail.com \
--to=avanzini.arianna@gmail.com \
--cc=Ian.Campbell@eu.citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=JBeulich@suse.com \
--cc=dario.faggioli@citrix.com \
--cc=etrudeau@broadcom.com \
--cc=julien.grall@citrix.com \
--cc=keir@xen.org \
--cc=paolo.valente@unimore.it \
--cc=stefano.stabellini@eu.citrix.com \
--cc=tim@xen.org \
--cc=viktor.kleinik@globallogic.com \
--cc=xen-devel@lists.xen.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).