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

[Arianna Avanzini <avanzini.arianna@gmail.com>]

On 07/01/2014 12:45 PM, Julien Grall wrote:
> Hi Arianna,
> 

Hello,

> Do you have any plan to send a new version soon? I'm working on device
> pass-through and this series is a dependency/blocker for my series.
> 
> IIRC, this version was already in good shape. There is only few comments.
> 

I'm currently working on the v9 series and will be sending it no later than
tomorrow.

Sorry for the delay,
Arianna


> Regards,
> 
> On 05/25/2014 11:51 AM, Arianna Avanzini wrote:
>> Hello,
>>
>> this is a new version - the eighth - of my implementation proposal for the
>> memory_mapping DOMCTL for the ARM architecture. As usual, this cover letter just
>> lists the most relevant changes performed on the patchset, while a more detailed
>> description can be found in the description and changelog of each patch; also,
>> some more information about the patch series' origin can be found in the last
>> full cover letter ([1]).
>>
>> First of all, the commit descriptions have been reworded to get rid of tentative
>> phrases that were pointless in that context, as suggested by Dario Faggioli.
>>
>> Patch 0002 has been modified to revert some uncorrect changes due to my
>> misunderstanding of previous comments, as pointed out by Julien Grall. Also,
>> according to his suggestions, an error message has been added to let a failure
>> encountered during an unmap be less obscure. The error path has been improved
>> to include unmapping previously-created page-table mappings, unlocking the
>> p2m_lock and flushing the TLBs in case some mappings have been successfully
>> removed.
>>
>> Patch 0004 has been changed to address issues pointed out by Julien Grall and
>> Ian Campbell. Now the correct DIV_ROUND_UP() macro is used instead of the
>> ternary operator, and the map_one_mmio() function, defined for the xgene-storm
>> platform, accepts pfns as arguments instead of paddr_t. Also, the function
>> apply_p2m_changes() is now called from map_mmio_regions() with the correct
>> count.
>>
>> A new patch 0005 has been added to let the map_mmio_regions() function for ARM
>> unmap partially-mapped I/O-memory regions if mapping a page fails, as suggested
>> by Julien Grall. The approach which has been used includes having the function
>> apply_p2m_changes() return, in the case of an error, the number of pages that
>> have been successfully mapped, therefore allowing the caller to undo what has
>> been done, if necessary. The definition of unmap_mmio_regions() for ARM has been
>> moved to this patch and now uses the correct count as indicated by Julien Grall.
>>
>> The title and description of patch 0006 have been changed to (hopefully) better
>> fit the patch content, as requested by Jan Beulich.
>>
>> The patch factoring out map and unmap operations from the x86 version of the
>> memory_mapping DOMCTL has been split into two, as suggested by Jan Beulich, to
>> make the review easier: patch 0007 now just cleans up existing code, by letting
>> mfn + nr_mfns be computed only once and by changing the name of the tmp_rc local
>> variable to rc; patch 0009 instead actually factors out the operations.
>>
>> A new patch 0008 now adds an empty stub for the ARM architecture of the MTRR-
>> related function memory_type_changed(), currently available only for x86, and
>> makes its definition common to both x86 and ARM. This was requested by Jan
>> Beulich and Ian Campbell.
>>
>> The call to PERROR() in patch 0012 has been fixed, as it showed an error message
>> whose meaning was the opposite of the correct one, as pointed out by Jan
>> Beulich.
>>
>> The patch which implemented a separation of the functions of the memory_mapping
>> and iomem_permission DOMCTLs has now been split into two, as suggested by Jan
>> Beulich: patch 0013 now contains only libxl-related preparatory changes, while
>> patch 0014 actually implements the separation. As for patch 0014, changes
>> performed on the iomem_permission DOMCTL handling code have been dropped, while
>> the permission checks performed by memory_mapping have been changed to be more
>> conservative and check both the calling domain and the grantee domain's
>> permissions.
>> As of now, patch 0014 performs the separation only for I/O memory and not for
>> I/O ports (as was instead suggested by Jan Beulich) because, as far as I know,
>> Julien Grall is working on the I/O ports patch series, and I'm afraid I might be
>> conflicting with his work. I am however surely willing to add a patch for that
>> if he is OK with it.
>>
>> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
>> and ERIKA Enterprise ([2]) as a domU. The x86 bits have been tested on an x86_64
>> machine, as I forgot to say in the last versions of the patchset and Jan Beulich
>> pointed out; in particular, this last version, has been tested 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://erika.tuxfamily.org/drupal/
>>
>> Arianna Avanzini (14):
>>   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/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: move memory_type_changed() function to common code
>>   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: 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                | 18 +++++--
>>  tools/libxc/xc_domain.c              | 12 +++++
>>  tools/libxl/libxl.h                  | 10 ++++
>>  tools/libxl/libxl_create.c           | 34 +++++++++++++
>>  tools/libxl/libxl_internal.h         |  1 +
>>  tools/libxl/libxl_pci.c              | 26 ++++------
>>  tools/libxl/libxl_types.idl          |  4 ++
>>  tools/libxl/xl_cmdimpl.c             | 17 ++++---
>>  xen/arch/arm/Makefile                |  1 +
>>  xen/arch/arm/domain_build.c          | 18 +++++--
>>  xen/arch/arm/gic.c                   | 15 +++---
>>  xen/arch/arm/mtrr.c                  |  6 +++
>>  xen/arch/arm/p2m.c                   | 99 ++++++++++++++++++++++++++++--------
>>  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                | 53 +++++++++++++++++--
>>  xen/common/domctl.c                  | 55 +++++++++++++++++++-
>>  xen/include/asm-arm/mm.h             |  2 +
>>  xen/include/asm-arm/mtrr.h           |  6 +++
>>  xen/include/asm-arm/p2m.h            | 11 ++--
>>  xen/include/asm-x86/mtrr.h           |  2 +-
>>  xen/include/asm-x86/p2m.h            |  3 +-
>>  xen/include/xen/mtrr.h               |  8 +++
>>  xen/include/xen/p2m-common.h         | 16 ++++++
>>  26 files changed, 365 insertions(+), 170 deletions(-)
>>  create mode 100644 xen/arch/arm/mtrr.c
>>  create mode 100644 xen/include/asm-arm/mtrr.h
>>  create mode 100644 xen/include/xen/mtrr.h
>>  create mode 100644 xen/include/xen/p2m-common.h
>>
> 
> 


-- 
/*
 * Arianna Avanzini
 * avanzini.arianna@gmail.com
 * 73628@studenti.unimore.it
 */