Re: [PATCH v9 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM

[Julien Grall <julien.grall@linaro.org>]

Hi Arianna,

Lots of patches has been pushed upstream since last week (GICv2 and P2M,
reworking).

This series doesn't apply cleanly anymore and need some rework. Could
you rebase it to the latest master?

Thanks,

On 07/02/2014 07:42 PM, Arianna Avanzini wrote:
> Hello,
> here is finally the nineth version of my implementation proposal for the
> hypercall XEN_DOMCTL_memory_mapping for the ARM architecture. First of all,
> I am deeply sorry for the delay and for blocking patchsets of which this one
> is a dependency. As usual, I'm trying to keep this cover letter as brief as
> I can by listing here only the most relevant changes, while a more detailed
> description can be found in the commit message and in the changelog of each
> patch. Also, more information about the patch series can be found in the
> last full cover letter ([1]).
> Please note that the one that was the series' first patch has been merged
> two weeks ago by Ian Campbell ([2]), and this is why it is missing from this
> patchset.
> 
> Patch 0001 has been fixed according to suggestions provided by Julien Grall
> and Ian Campbell; it is also worthy to say that now the added checks don't stop
> unmapping when failing to unmap an I/O-memory mapping, but merely emit a
> warning. This choice has two benefits: it prevents I/O-memory regions from
> remaining partially accessible to a domain, and it lets the handling of such a
> failure match the one introduced for x86 by patch 0005 in the lighter warn-
> only form requested by Jan Beulich.
> 
> Patch 0004 has been modified according to directives given by Julien Grall and
> Ian Campbell: it now lets the function apply_p2m_changes() unwind its own
> progress when failing to INSERT a new mapping.
> 
> Patch 0007 now adds the MTRR-related memory_type_changed() function for ARM
> as just a stub instead of creating and ad-hoc file and header, as suggested by
> Julien Grall and seconded by Jan Beulich and Ian Campbell.
> 
> Patch 0008 has been fixed according to requests from Jan Beulich.
> 
> Patch 0009 no more exposes mfn_t to the common code, as requested by Julien
> Grall and approved by Ian Campbell and Tim Deegan.
> 
> Patch 0011 has been fixed as per Julien Grall's indications.
> 
> A new patch 0012 has been added to the patch series. The patch attempts to
> address an issue pointed out by Jan Beulich, concerning the passthru of
> different GPUs to different domain, each one designed as primary VGA device
> of its own domain with gfx_passthru = 1 in the domain config. Patch 0013
> would grant access permission to the VGA framebuffer to each domain with
> gfx_passthru in its config, while access permission must be granted only to
> the domain having the primary GPU passthru to it.
> Unfortunately, I haven't found a way to distinguish the primary GPU from the
> other ones, and I decided to rely on user input. Patch 0012, in fact, adds a
> new option to libxl configuration, which allows the user to specify if one of
> the passthru VGA devices is primary. Only a domain with both gfx_passthru = 1
> and gfx_passthru_primary = 1 (and its stubdomain, see the comment on patch
> 0013 below) will have access permission to VGA-specific I/O-memory regions.
> Please note that the default for the new option is true, to avoid breaking
> interaction with QEMU when possible; AFAIK QEMU will however try to map the
> I/O-memory region to all guests requesting GFX passthrough.
> 
> Patch 0013 now grants access permission to VGA-specific I/O-memory regions
> both to the domain with gfx_passthru = 1 and gfx_passthru_primary = 1 and its
> stubdomain, as, after patch 0014, the memory_mapping DOMCTL will check for
> both, as pointed out by Julien Grall.
> 
> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
> and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on an x86_64
> machine with Linux 3.15.0-rc5 as both dom0 and domU.
> Any feedback about this new version of the patchset is more than welcome,
> Arianna
> 
> [1] http://lists.xen.org/archives/html/xen-devel/2014-03/msg01724.html
> [2] http://markmail.org/message/svp3byd62eol375y
> [3] http://erika.tuxfamily.org/drupal/
> 
> Arianna Avanzini (14):
>   arch/arm: add consistency check to REMOVE p2m changes
>   arch/arm: let map_mmio_regions() take pfn as parameters
>   arch/arm: let map_mmio_regions() use start and count
>   arch/arm: unmap partially-mapped I/O-memory regions
>   arch/x86: warn if to-be-removed mapping does not exist
>   arch/x86: cleanup memory_mapping DOMCTL
>   xen/common: add ARM stub for the function memory_type_changed()
>   xen/x86: factor out map and unmap from the memory_mapping DOMCTL
>   xen/common: move the memory_mapping DOMCTL hypercall to common code
>   tools/libxl: parse optional start gfn from the iomem config option
>   tools/libxl: handle the iomem parameter with the memory_mapping hcall
>   tools/libxl: read from config if passing through primary GPU
>   tools/libxl: explicitly grant access to needed I/O-memory ranges
>   xen/common: do not implicitly permit access to mapped I/O memory
> 
>  docs/man/xl.cfg.pod.5                | 25 +++++++++---
>  tools/libxc/xc_domain.c              | 10 +++++
>  tools/libxl/libxl.h                  | 17 ++++++++
>  tools/libxl/libxl_create.c           | 51 ++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h         |  1 +
>  tools/libxl/libxl_pci.c              | 26 +++++-------
>  tools/libxl/libxl_types.idl          |  7 +++-
>  tools/libxl/xl_cmdimpl.c             | 19 +++++----
>  xen/arch/arm/domain_build.c          |  7 ++--
>  xen/arch/arm/gic.c                   | 15 +++----
>  xen/arch/arm/p2m.c                   | 77 ++++++++++++++++++++++++++++++------
>  xen/arch/arm/platforms/exynos5.c     |  9 ++---
>  xen/arch/arm/platforms/omap5.c       | 17 ++++----
>  xen/arch/arm/platforms/xgene-storm.c | 16 +++++---
>  xen/arch/x86/domctl.c                | 76 -----------------------------------
>  xen/arch/x86/mm/p2m.c                | 55 ++++++++++++++++++++++++--
>  xen/common/domctl.c                  | 54 ++++++++++++++++++++++++-
>  xen/include/asm-arm/mm.h             |  2 +
>  xen/include/asm-arm/p2m.h            | 11 +++---
>  xen/include/asm-x86/p2m.h            |  3 +-
>  xen/include/xen/p2m-common.h         | 16 ++++++++
>  21 files changed, 354 insertions(+), 160 deletions(-)
>  create mode 100644 xen/include/xen/p2m-common.h
> 


-- 
Julien Grall