Re: [Xen-devel] [PATCH] Synchronize privcmd header constants

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-10-19 at 03:20 +0100, Andres Lagar-Cavilla wrote:
> I've had a look. The xen.org tree knows about three other OSes: minios, 
> solaris and netbsd. None knows about paged out frames. None uses the 
> XEN_DOMCTL_PFINFO_PAGEDTAB constant in domctl.h
> 
> 1. The domctl.h constant can still go away without hurting other OSes.
> 2. It is trivial to add the PRIVCMD_MMAPBATCH_* constants to the privcmd.h of 
> other OSes. It can't hurt. I can do it here. Each OS Xen maintainer would 
> have to take care of syncing that up in the respective upstream. However ...
> 3. Not that trivial to teach all these OSes about paged out frames. Does 
> anyone care?
> 
> Please advise.

I'm convinced, I'd forgotten that only Linux dom0 did paging.

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> and applied, thanks &
sorry for the delay.

> Thanks
> Andres
> 
> On Oct 18, 2012, at 4:08 AM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> 
> > On Fri, 2012-10-12 at 16:30 +0100, Andres Lagar-Cavilla wrote:
> >> tools/include/xen-sys/Linux/privcmd.h |   3 +++
> >> tools/libxc/xc_linux_osdep.c          |  10 +++++-----
> >> xen/include/public/domctl.h           |   1 -
> >> 3 files changed, 8 insertions(+), 6 deletions(-)
> >> 
> >> 
> >> Since Linux's git commit ceb90fa0a8008059ecbbf9114cb89dc71a730bb6, the
> >> privcmd.h interface between Linux and libxc specifies two new constants,
> >> PRIVCMD_MMAPBATCH_MFN_ERROR and PRIVCMD_MMAPBATCH_PAGED_ERROR. These 
> >> constants
> >> represent the error codes encoded in the top nibble of an mfn slot passed 
> >> to
> >> the legacy MMAPBATCH ioctl.
> >> 
> >> In particular, libxenctrl checks for the equivalent of the latter constant 
> >> when
> >> dealing with paged out frames that might be the target of a foreign map.
> >> 
> >> Previously, the relevant constant was defined in the domctl hypervisor
> >> interface header (XEN_DOMCTL_PFINFO_PAGEDTAB). Because this top-nibble 
> >> encoding
> >> is a contract between the dom0 kernel and libxc, a domctl.h definition is
> >> misplaced.
> >> 
> >> - Sync the privcmd.h header to that now available in upstream Linux
> > 
> > Although the ioctl is Linux specific is the top-nibble behaviour (and
> > therefore the #define) common to other dom0s like *BSD? Can a BSD person
> > confirm that this change won't breaking things for them please.
> > 
> >> - Update libxc appropriately
> >> - Remove the unnecessary constant in domctl.h
> >> 
> >> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> >> 
> >> diff -r 4eed5e64544f -r 5171750d133e tools/include/xen-sys/Linux/privcmd.h
> >> --- a/tools/include/xen-sys/Linux/privcmd.h
> >> +++ b/tools/include/xen-sys/Linux/privcmd.h
> >> @@ -64,6 +64,9 @@ typedef struct privcmd_mmapbatch {
> >>    xen_pfn_t __user *arr; /* array of mfns - top nibble set on err */
> >> } privcmd_mmapbatch_t; 
> >> 
> >> +#define PRIVCMD_MMAPBATCH_MFN_ERROR     0xf0000000U
> >> +#define PRIVCMD_MMAPBATCH_PAGED_ERROR   0x80000000U
> >> +
> >> typedef struct privcmd_mmapbatch_v2 {
> >>    unsigned int num; /* number of pages to populate */
> >>    domid_t dom;      /* target domain */
> >> diff -r 4eed5e64544f -r 5171750d133e tools/libxc/xc_linux_osdep.c
> >> --- a/tools/libxc/xc_linux_osdep.c
> >> +++ b/tools/libxc/xc_linux_osdep.c
> >> @@ -129,7 +129,7 @@ static int xc_map_foreign_batch_single(i
> >> 
> >>     do
> >>     {
> >> -        *mfn ^= XEN_DOMCTL_PFINFO_PAGEDTAB;
> >> +        *mfn ^= PRIVCMD_MMAPBATCH_PAGED_ERROR;
> >>         usleep(100);
> >>         rc = ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx);
> >>     }
> >> @@ -166,8 +166,8 @@ static void *linux_privcmd_map_foreign_b
> >> 
> >>         for ( i = 0; i < num; i++ )
> >>         {
> >> -            if ( (arr[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) ==
> >> -                 XEN_DOMCTL_PFINFO_PAGEDTAB )
> >> +            if ( (arr[i] & PRIVCMD_MMAPBATCH_MFN_ERROR) ==
> >> +                           PRIVCMD_MMAPBATCH_PAGED_ERROR )
> >>             {
> >>                 unsigned long paged_addr = (unsigned long)addr + (i << 
> >> XC_PAGE_SHIFT);
> >>                 rc = xc_map_foreign_batch_single(fd, dom, &arr[i],
> >> @@ -323,12 +323,12 @@ static void *linux_privcmd_map_foreign_b
> >>             default:
> >>                 err[i] = -EINVAL;
> >>                 continue;
> >> -            case XEN_DOMCTL_PFINFO_PAGEDTAB:
> >> +            case PRIVCMD_MMAPBATCH_PAGED_ERROR:
> >>                 if ( rc != -ENOENT )
> >>                 {
> >>                     err[i] = rc ?: -EINVAL;
> >>                     continue;
> >> -                 }
> >> +                }
> >>                 rc = xc_map_foreign_batch_single(fd, dom, pfn + i,
> >>                         (unsigned long)addr + ((unsigned 
> >> long)i<<XC_PAGE_SHIFT));
> >>                 if ( rc < 0 )
> >> diff -r 4eed5e64544f -r 5171750d133e xen/include/public/domctl.h
> >> --- a/xen/include/public/domctl.h
> >> +++ b/xen/include/public/domctl.h
> >> @@ -135,7 +135,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_getme
> >> #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
> >> #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
> >> #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page */
> >> -#define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
> >> #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
> >> 
> >> struct xen_domctl_getpageframeinfo {
> > 
> > 
> 



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