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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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...

> 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