Re: [Xen-devel] [PATCH v7 05/10] arch/x86: check if mapping exists before memory_mapping removes it

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.05.14 at 17:54, <avanzini.arianna@xxxxxxxxx> wrote:
> Currently, when a memory mapping is removed with the memory_mapping
> DOMCTL, no check is performed on the existence of such a mapping.
> This commit attempts to add such a consistency check to the code
> performing the unmap, emitting a warning message if the check fails.
> 
> Signed-off-by: Arianna Avanzini <avanzini.arianna@xxxxxxxxx>

With the title changed to describe the actual behavior
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> 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: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
> Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>
> 
> ---
> 
>     v7:
>         - Do not fail with an error when attempting to remove a non-existent
>           mapping: just emit a warning.
> 
> ---
>  xen/arch/x86/domctl.c     |  4 ++--
>  xen/arch/x86/mm/p2m.c     | 12 ++++++++----
>  xen/include/asm-x86/p2m.h |  2 +-
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index ae29a56..c466063 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -679,7 +679,7 @@ long arch_do_domctl(
>                             "memory_map:fail: dom%d gfn=%lx mfn=%lx 
> ret:%ld\n",
>                             d->domain_id, gfn + i, mfn + i, ret);
>                      while ( i-- )
> -                        clear_mmio_p2m_entry(d, gfn + i);
> +                        clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
>                      if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
>                           is_hardware_domain(current->domain) )
>                          printk(XENLOG_ERR
> @@ -699,7 +699,7 @@ long arch_do_domctl(
>              if ( paging_mode_translate(d) )
>                  for ( i = 0; i < nr_mfns; i++ )
>                  {
> -                    ret = clear_mmio_p2m_entry(d, gfn + i);
> +                    ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
>                      if ( ret )
>                          tmp_rc = ret;
>                  }
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 1d0528b..a99e222 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -801,10 +801,10 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long 
> gfn, mfn_t mfn)
>  }
>  
>  /* Returns: 0 for success, -errno for failure */
> -int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn)
> +int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
>  {
>      int rc = -EINVAL;
> -    mfn_t mfn;
> +    mfn_t actual_mfn;
>      p2m_access_t a;
>      p2m_type_t t;
>      struct p2m_domain *p2m = p2m_get_hostp2m(d);
> @@ -813,15 +813,19 @@ int clear_mmio_p2m_entry(struct domain *d, unsigned 
> long gfn)
>          return -EIO;
>  
>      gfn_lock(p2m, gfn, 0);
> -    mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
> +    actual_mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
>  
>      /* Do not use mfn_valid() here as it will usually fail for MMIO pages. 
> */
> -    if ( (INVALID_MFN == mfn_x(mfn)) || (t != p2m_mmio_direct) )
> +    if ( (INVALID_MFN == mfn_x(actual_mfn)) || (t != p2m_mmio_direct) )
>      {
>          gdprintk(XENLOG_ERR,
>                   "gfn_to_mfn failed! gfn=%08lx type:%d\n", gfn, t);
>          goto out;
>      }
> +    if ( mfn_x(mfn) != mfn_x(actual_mfn) )
> +        gdprintk(XENLOG_WARNING,
> +                 "no mapping between mfn %08lx and gfn %08lx\n",
> +                 mfn_x(mfn), gfn);
>      rc = p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_4K, 
> p2m_invalid,
>                         p2m->default_access);
>  
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 86847e9..e901085 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -513,7 +513,7 @@ void p2m_memory_type_changed(struct domain *d);
>  
>  /* Set mmio addresses in the p2m table (for pass-through) */
>  int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
> -int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn);
> +int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
>  
>  
>  /* 
> -- 
> 1.9.2



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel