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

[Andres Lagar-Cavilla <andres.lagarcavilla@xxxxxxxxx>]

On Oct 19, 2012, at 4:37 AM, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:

> On 19/10/12 04:20, 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.
> 
> I've checked and NetBSD doesn't use XEN_DOMCTL_PFINFO_PAGEDTAB, so as
> Andres says I guess it's safe to remove it.
> 
>> 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?
> 
> Well, I'm sure the NetBSD community would be interested in this, but
> finding someone to actually work on it is a whole different story…

Thanks Roger. Ian should I expect a response from Solaris?
Andres
> 
>> Please advise.
>> 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