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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.04.14 at 15:44, <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.

I think this goes too far: Did you check that all existing tool stacks
actually pass a valid MFN for the unmap? It would seem quite natural
to me if some didn't, since tool stacks can be expected to know what
they're doing.

> @@ -819,15 +819,23 @@ 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) )

And if we went the route you propose, the INVALID_MFN check here
could be dropped (as being redundant with the check added below, so
long as the passed in MFN isn't INVALID_MFN).

>      {
>          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_ERR,
> +                 "mapping between mfn %08lx and gfn %08lx does not exist "
> +                 "(mapped to %08lx)\n",
> +                 mfn_x(mfn), gfn, mfn_x(actual_mfn));
> +        goto out;
> +    }

But more likely you will want to make this a warning only, i.e. a
message logged without causing the function to fail. And we would
then see whether anyone complains about this having become too
verbose (in which case we might need to drop the message again).

Jan


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