Re: [Xen-devel] [PATCH 2/3] xen/privcmd: report paged-out frames in PRIVCMD_MMAPBATCH ioctl

[Andres Lagar-Cavilla <andreslc@xxxxxxxxxxxxxx>]

On Aug 23, 2012, at 1:13 PM, David Vrabel wrote:

> From: David Vrabel <david.vrabel@xxxxxxxxxx>
> 
> libxc handles paged-out frames in xc_map_foreign_bulk() and friends by
> retrying the map operation.  libxc expects the PRIVCMD_MMAPBATCH ioctl
> to report paged out frames by setting bit 31 in the mfn.
> 
> Do this for the PRIVCMD_MMAPBATCH ioctl if
> xen_remap_domain_mfn_range() returned -ENOENT.
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> ---
> drivers/xen/privcmd.c |   11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index ccee0f1..f8c1b6d 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -255,10 +255,15 @@ static int mmap_batch_fn(void *data, void *state)
> {
>       xen_pfn_t *mfnp = data;
>       struct mmap_batch_state *st = state;
> +     int ret;
> 
> -     if (xen_remap_domain_mfn_range(st->vma, st->va & PAGE_MASK, *mfnp, 1,
> -                                    st->vma->vm_page_prot, st->domain) < 0) {
> -             *mfnp |= 0xf0000000U;
> +     ret = xen_remap_domain_mfn_range(st->vma, st->va & PAGE_MASK, *mfnp, 1,
> +                                      st->vma->vm_page_prot, st->domain);
> +     if (ret < 0) {
> +             if (ret == -ENOENT)
> +                     *mfnp |= 0x80000000U;

As Konrad pointed out separately, constants here would be great. 

These two constants are defined in <xen>/include/public/domctl.h. In 
particular, the 0x80..0 constant is defined there but *not* used by the 
hypervisor. I don't see a problem in (re)defining it in the linux interface 
headers shared between libxc and the kernel -- this is where it really belongs. 
It could be even phased out of domctl.h in 4.2 (unlikely) or 4.3.

As for the 0xf0..0 constant I have no strong opinion -- pre-existing problem.

> +             else
> +                     *mfnp |= 0xf0000000U;
>               st->err++;
>       }
>       st->va += PAGE_SIZE;

Libxc expects errno to be ENOENT if at least one map hypercall returned ENOENT. 
So you need to keep an extra latch in the state, e.g. st->enoents. Or recycle 
st->err to be a tristate (ok, error, enoent), since I don't see any actual use 
of err count. Whichever way, privcmd_ioctl_mmap_batch needs to find out if it 
needs to return ENOENT. Note this requirement also extends to 
PRIVCMD_MMAPBATCH_V2.

Andres
 
> -- 
> 1.7.2.5
> 


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