[PATCH] Synchronize privcmd header constants

[PATCH] Synchronize privcmd header constants

[Andres Lagar-Cavilla]

 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
- Update libxc appropriately
- Remove the unnecessary constant in domctl.h

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

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 {

Re: [PATCH] Synchronize privcmd header constants

[Ian Campbell]

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@lagarcavilla.org>
> 
> 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 {

Re: [PATCH] Synchronize privcmd header constants

[Andres Lagar-Cavilla]

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

On Oct 18, 2012, at 4:08 AM, Ian Campbell <ian.campbell@citrix.com> 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@lagarcavilla.org>
>> 
>> 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 {
> 
> 

Re: [PATCH] Synchronize privcmd header constants

[Roger Pau Monné]

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@citrix.com> 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@lagarcavilla.org>
>>>
>>> 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 {
>>
>>
> 

Re: [PATCH] Synchronize privcmd header constants

[Andres Lagar-Cavilla]

On Oct 19, 2012, at 4:37 AM, Roger Pau Monné <roger.pau@citrix.com> 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@citrix.com> 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@lagarcavilla.org>
>>>> 
>>>> 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 {
>>> 
>>> 
>> 
> 

Re: [PATCH] Synchronize privcmd header constants

[Roger Pau Monné]

On 19/10/12 17:46, Andres Lagar-Cavilla wrote:
> 
> On Oct 19, 2012, at 4:37 AM, Roger Pau Monné <roger.pau@citrix.com> 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?

Solaris (Illumos) is currently pretty broken, and has gone the KVM side
as far as I know, so I wouldn't expect any response.

Re: [PATCH] Synchronize privcmd header constants

[Pasi Kärkkäinen]

On Fri, Oct 19, 2012 at 05:49:57PM +0200, Roger Pau Monné wrote:
> On 19/10/12 17:46, Andres Lagar-Cavilla wrote:
> > 
> > On Oct 19, 2012, at 4:37 AM, Roger Pau Monné <roger.pau@citrix.com> 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?
> 
> Solaris (Illumos) is currently pretty broken, and has gone the KVM side
> as far as I know, so I wouldn't expect any response.
> 

Around one month ago there was an Illumos developer posting to xen-devel,
he was working on porting the OpenSolaris XVM (Xen hypervisor) patches to xen-unstable.

Also he said the current Illumos kernel still works as dom0, for him at least.

-- Pasi

Re: [PATCH] Synchronize privcmd header constants

[Ian Campbell]

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@citrix.com> and applied, thanks &
sorry for the delay.

> Thanks
> Andres
> 
> On Oct 18, 2012, at 4:08 AM, Ian Campbell <ian.campbell@citrix.com> 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@lagarcavilla.org>
> >> 
> >> 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 {
> > 
> > 
>