|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 4/7] xen, common: add the XEN_DOMCTL_memory_mapping hypercall
Hi Arianna,
Thank you for the patch.
On 03/25/2014 02:02 AM, Arianna Avanzini wrote:
> This commit introduces a first attempt of implementation of the
> XEN_DOMCTL_memory_mapping hypercall for ARM. As the implementation
> would have been almost identical to the one for x86, this code has
> been made common to both the architectures. The only difference
> between the two procedures lies in the arch-specific implementation
> of the map_mmio_regions() and unmap_mmio_regions() functions.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@xxxxxxxxx>
> Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Cc: Paolo Valente <paolo.valente@xxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
> Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>
>
> ---
>
> v4:
> - Use a define for paddr_bits instead of a new variable.
> - Define prototypes for common functions map_mmio_regions() and
> unmap_mmio_regions() only once in a common header.
> - Fix type and signedness of local variables used as indexes in
> map_mmio_regions() and unmap_mmio_regions() for x86.
> - Clear p2m entries in map_mmio_regions() for x86 only if
> set_mmio_p2m_entry() returned with and error.
> - Make ranges inclusive of the end address in map_mmio_regions()
> and unmap_mmio_regions() for x86.
> - Turn hard tabs into spaces.
>
> v3:
> - Move code to xen/common/domctl.c; abstract out differences between
> the x86 and ARM code:
> . add map_mmio_regions() and unmap_mmio_regions() functions for x86;
> . add a paddr_bits variable for ARM.
> - Use pfn as parameters to the unmap_mmio_regions() function.
> - Compute gfn + nr_mfns and mfn + nr_mfns only once.
>
> v2:
> - Move code to xen/arm/domctl.c.
> - Use the already-defined PADDR_BITS constant in the new DOMCTL.
> - Use paddr_t as arguments to the map_mmio_regions() function.
> - Page-align addresses given as arguments to map_mmio_regions() and
> unmap_mmio_regions().
>
> ---
> xen/arch/arm/p2m.c | 12 ++++++++
> xen/arch/x86/domctl.c | 70
> -----------------------------------------------
> xen/arch/x86/mm/p2m.c | 40 +++++++++++++++++++++++++++
> xen/common/domctl.c | 65 +++++++++++++++++++++++++++++++++++++++++++
> xen/include/asm-arm/p2m.h | 9 ++----
> xen/include/asm-x86/p2m.h | 1 +
> xen/include/xen/p2m.h | 16 +++++++++++
> 7 files changed, 137 insertions(+), 76 deletions(-)
> create mode 100644 xen/include/xen/p2m.h
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index c8e77b9..961213d 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -480,6 +480,18 @@ int map_mmio_regions(struct domain *d,
> MATTR_DEV, p2m_mmio_direct);
> }
>
> +int unmap_mmio_regions(struct domain *d,
> + unsigned long start_gfn,
> + unsigned long end_gfn,
> + unsigned long mfn)
> +{
> + return apply_p2m_changes(d, REMOVE,
> + pfn_to_paddr(start_gfn),
> + pfn_to_paddr(end_gfn),
> + pfn_to_paddr(mfn),
> + MATTR_DEV, p2m_mmio_direct);
> +}
> +
> int guest_physmap_add_entry(struct domain *d,
> unsigned long gpfn,
> unsigned long mfn,
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 26635ff..69a7fbf 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -639,76 +639,6 @@ long arch_do_domctl(
> }
> break;
>
> - case XEN_DOMCTL_memory_mapping:
> - {
> - unsigned long gfn = domctl->u.memory_mapping.first_gfn;
> - unsigned long mfn = domctl->u.memory_mapping.first_mfn;
> - unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
> - int add = domctl->u.memory_mapping.add_mapping;
> - unsigned long i;
> -
> - ret = -EINVAL;
> - if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
> - ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
> - (gfn + nr_mfns - 1) < gfn ) /* wrap? */
> - break;
> -
> - ret = -EPERM;
> - if ( !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns -
> 1) )
> - break;
> -
> - ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add);
> - if ( ret )
> - break;
> -
> - if ( add )
> - {
> - printk(XENLOG_G_INFO
> - "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
> - d->domain_id, gfn, mfn, nr_mfns);
> -
> - ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
> - if ( !ret && paging_mode_translate(d) )
> - {
> - for ( i = 0; !ret && i < nr_mfns; i++ )
> - if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
> - ret = -EIO;
> - if ( ret )
> - {
> - printk(XENLOG_G_WARNING
> - "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
> - d->domain_id, gfn + i, mfn + i);
> - while ( i-- )
> - clear_mmio_p2m_entry(d, gfn + i);
> - if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
> - is_hardware_domain(current->domain) )
> - printk(XENLOG_ERR
> - "memory_map: failed to deny dom%d access to
> [%lx,%lx]\n",
> - d->domain_id, mfn, mfn + nr_mfns - 1);
> - }
> - }
> - }
> - else
> - {
> - printk(XENLOG_G_INFO
> - "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
> - d->domain_id, gfn, mfn, nr_mfns);
> -
> - if ( paging_mode_translate(d) )
> - for ( i = 0; i < nr_mfns; i++ )
> - add |= !clear_mmio_p2m_entry(d, gfn + i);
> - ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
> - if ( !ret && add )
> - ret = -EIO;
> - if ( ret && is_hardware_domain(current->domain) )
> - printk(XENLOG_ERR
> - "memory_map: error %ld %s dom%d access to
> [%lx,%lx]\n",
> - ret, add ? "removing" : "denying", d->domain_id,
> - mfn, mfn + nr_mfns - 1);
> - }
> - }
> - break;
> -
> case XEN_DOMCTL_ioport_mapping:
> {
> #define MAX_IOPORTS 0x10000
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 8f380ed..a44d7a3 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1624,6 +1624,46 @@ unsigned long paging_gva_to_gfn(struct vcpu *v,
> return hostmode->gva_to_gfn(v, hostp2m, va, pfec);
> }
>
> +int map_mmio_regions(struct domain *d,
> + unsigned long start_gfn,
> + unsigned long end_gfn,
> + unsigned long mfn)
> +{
> + int ret = 0;
> + unsigned long i;
> + unsigned long nr_mfns = end_gfn - start_gfn + 1;
> +
> + if ( !paging_mode_translate(d) )
> + return 0;
> +
> + for ( i = 0; !ret && i < nr_mfns; i++ )
> + if ( !set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i)) )
> + ret = -EIO;
> + if ( ret )
> + while ( i-- )
> + clear_mmio_p2m_entry(d, start_gfn + i);
> +
> + return ret;
> +}
> +
> +int unmap_mmio_regions(struct domain *d,
> + unsigned long start_gfn,
> + unsigned long end_gfn,
> + unsigned long mfn)
> +{
> + int ret = 0;
> + unsigned long nr_mfns = end_gfn - start_gfn + 1;
> + unsigned long i;
> +
> + if ( !paging_mode_translate(d) )
> + return 0;
> +
> + for ( i = 0; i < nr_mfns; i++ )
> + ret |= !clear_mmio_p2m_entry(d, start_gfn + i);
> +
> + return ret;
> +}
> +
> /*** Audit ***/
>
> #if P2M_AUDIT
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 7cf610a..ae120d9 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -818,6 +818,71 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> u_domctl)
> }
> break;
>
> + case XEN_DOMCTL_memory_mapping:
> + {
> + unsigned long gfn = op->u.memory_mapping.first_gfn;
> + unsigned long mfn = op->u.memory_mapping.first_mfn;
> + unsigned long nr_mfns = op->u.memory_mapping.nr_mfns;
> + unsigned long mfn_end = mfn + nr_mfns;
> + unsigned long gfn_end = gfn + nr_mfns;
> + int add = op->u.memory_mapping.add_mapping;
> +
> + ret = -EINVAL;
> + if ( (mfn_end - 1) < mfn || /* wrap? */
> + ((mfn | (mfn_end - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
> + (gfn_end - 1) < gfn ) /* wrap? */
> + return ret;
Would not it be better to only rely on the GFN when the toolstack is
removing the mapping?
I know this is not the previous behavior, but most of the hypercall
which deal with removing mapping only take GFN.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |