[PATCH v2] libs/foreignmemory: Implement on NetBSD

Manuel Bouyer posted 1 patch 3 years, 2 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210126224800.1246-6-bouyer@netbsd.org
tools/libs/foreignmemory/Makefile  |  2 +-
tools/libs/foreignmemory/netbsd.c  | 66 +++++++++++++++++++++++++-----
tools/libs/foreignmemory/private.h |  6 +--
3 files changed, 60 insertions(+), 14 deletions(-)
[PATCH v2] libs/foreignmemory: Implement on NetBSD
Posted by Manuel Bouyer 3 years, 2 months ago
Implement foreignmemory interface on NetBSD. The compat interface is now used
only on __sun__

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
---
 tools/libs/foreignmemory/Makefile  |  2 +-
 tools/libs/foreignmemory/netbsd.c  | 66 +++++++++++++++++++++++++-----
 tools/libs/foreignmemory/private.h |  6 +--
 3 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/tools/libs/foreignmemory/Makefile b/tools/libs/foreignmemory/Makefile
index 13850f7988..f191cdbed0 100644
--- a/tools/libs/foreignmemory/Makefile
+++ b/tools/libs/foreignmemory/Makefile
@@ -8,7 +8,7 @@ SRCS-y                 += core.c
 SRCS-$(CONFIG_Linux)   += linux.c
 SRCS-$(CONFIG_FreeBSD) += freebsd.c
 SRCS-$(CONFIG_SunOS)   += compat.c solaris.c
-SRCS-$(CONFIG_NetBSD)  += compat.c netbsd.c
+SRCS-$(CONFIG_NetBSD)  += netbsd.c
 SRCS-$(CONFIG_MiniOS)  += minios.c
 
 include $(XEN_ROOT)/tools/libs/libs.mk
diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
index 54a418ebd6..a7e1d72ffc 100644
--- a/tools/libs/foreignmemory/netbsd.c
+++ b/tools/libs/foreignmemory/netbsd.c
@@ -19,7 +19,9 @@
 
 #include <unistd.h>
 #include <fcntl.h>
+#include <errno.h>
 #include <sys/mman.h>
+#include <sys/ioctl.h>
 
 #include "private.h"
 
@@ -66,15 +68,17 @@ int osdep_xenforeignmemory_close(xenforeignmemory_handle *fmem)
     return close(fd);
 }
 
-void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
-                              void *addr, int prot, int flags,
-                              xen_pfn_t *arr, int num)
+void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
+                                 uint32_t dom, void *addr,
+                                 int prot, int flags, size_t num,
+                                 const xen_pfn_t arr[/*num*/], int err[/*num*/])
+
 {
     int fd = fmem->fd;
-    privcmd_mmapbatch_t ioctlx;
-    addr = mmap(addr, num*XC_PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
+    privcmd_mmapbatch_v2_t ioctlx;
+    addr = mmap(addr, num*PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( addr == MAP_FAILED ) {
-        PERROR("osdep_map_foreign_batch: mmap failed");
+        PERROR("osdep_xenforeignmemory_map: mmap failed");
         return NULL;
     }
 
@@ -82,11 +86,12 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
     ioctlx.dom=dom;
     ioctlx.addr=(unsigned long)addr;
     ioctlx.arr=arr;
-    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx) < 0 )
+    ioctlx.err=err;
+    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH_V2, &ioctlx) < 0 )
     {
         int saved_errno = errno;
-        PERROR("osdep_map_foreign_batch: ioctl failed");
-        (void)munmap(addr, num*XC_PAGE_SIZE);
+        PERROR("osdep_xenforeignmemory_map: ioctl failed");
+        (void)munmap(addr, num*PAGE_SIZE);
         errno = saved_errno;
         return NULL;
     }
@@ -97,7 +102,48 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num*XC_PAGE_SIZE);
+    return munmap(addr, num*PAGE_SIZE);
+}
+
+int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
+                                    domid_t domid)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+int osdep_xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
+}
+
+int osdep_xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    privcmd_mmap_resource_t mr = {
+        .dom = fres->domid,
+        .type = fres->type,
+        .id = fres->id,
+        .idx = fres->frame,
+        .num = fres->nr_frames,
+    };
+    int rc;
+
+    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
+                      fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
+    if ( fres->addr == MAP_FAILED )
+        return -1;
+
+    mr.addr = (uintptr_t)fres->addr;
+
+    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
+    if ( rc )
+    {
+        PERROR("ioctl failed");
+    }
+
+    return 0;
 }
 
 /*
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index ebd45c4785..7e734ac61e 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -36,9 +36,9 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num);
 
-#if defined(__NetBSD__) || defined(__sun__)
+#if defined(__sun__)
 /* Strictly compat for those two only only */
-void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
+void *osdep_map_foreign_batch(xenforeignmemory_handle *fmem, uint32_t dom,
                               void *addr, int prot, int flags,
                               xen_pfn_t *arr, int num);
 #endif
@@ -54,7 +54,7 @@ struct xenforeignmemory_resource_handle {
     int flags;
 };
 
-#if !defined(__linux__) && !defined(__FreeBSD__)
+#ifdef __sun__
 static inline int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
                                                   domid_t domid)
 {
-- 
2.29.2


Re: [PATCH v2] libs/foreignmemory: Implement on NetBSD
Posted by Andrew Cooper 3 years, 2 months ago
On 26/01/2021 22:47, Manuel Bouyer wrote:
> diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
> index 54a418ebd6..a7e1d72ffc 100644
> --- a/tools/libs/foreignmemory/netbsd.c
> +++ b/tools/libs/foreignmemory/netbsd.c
> @@ -97,7 +102,48 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
>  int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
>                                   void *addr, size_t num)
>  {
> -    return munmap(addr, num*XC_PAGE_SIZE);
> +    return munmap(addr, num*PAGE_SIZE);
> +}
> +
> +int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> +                                    domid_t domid)
> +{
> +    errno = EOPNOTSUPP;
> +    return -1;
> +}
> +
> +int osdep_xenforeignmemory_unmap_resource(
> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
> +{
> +    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
> +}
> +
> +int osdep_xenforeignmemory_map_resource(
> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
> +{
> +    privcmd_mmap_resource_t mr = {
> +        .dom = fres->domid,
> +        .type = fres->type,
> +        .id = fres->id,
> +        .idx = fres->frame,
> +        .num = fres->nr_frames,
> +    };
> +    int rc;
> +
> +    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
> +                      fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
> +    if ( fres->addr == MAP_FAILED )
> +        return -1;
> +
> +    mr.addr = (uintptr_t)fres->addr;
> +
> +    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
> +    if ( rc )
> +    {
> +        PERROR("ioctl failed");

return -1;

I was rebasing my resource_size fix over this patch.

It would be easiest for me if I fix up and commit this patch, if
everyone is happy with that.

~Andrew

Re: [PATCH v2] libs/foreignmemory: Implement on NetBSD
Posted by Andrew Cooper 3 years, 2 months ago
On 28/01/2021 10:52, Andrew Cooper wrote:
> On 26/01/2021 22:47, Manuel Bouyer wrote:
>> diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
>> index 54a418ebd6..a7e1d72ffc 100644
>> --- a/tools/libs/foreignmemory/netbsd.c
>> +++ b/tools/libs/foreignmemory/netbsd.c
>> @@ -97,7 +102,48 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
>>  int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
>>                                   void *addr, size_t num)
>>  {
>> -    return munmap(addr, num*XC_PAGE_SIZE);
>> +    return munmap(addr, num*PAGE_SIZE);
>> +}
>> +
>> +int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
>> +                                    domid_t domid)
>> +{
>> +    errno = EOPNOTSUPP;
>> +    return -1;
>> +}
>> +
>> +int osdep_xenforeignmemory_unmap_resource(
>> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
>> +{
>> +    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
>> +}
>> +
>> +int osdep_xenforeignmemory_map_resource(
>> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
>> +{
>> +    privcmd_mmap_resource_t mr = {
>> +        .dom = fres->domid,
>> +        .type = fres->type,
>> +        .id = fres->id,
>> +        .idx = fres->frame,
>> +        .num = fres->nr_frames,
>> +    };
>> +    int rc;
>> +
>> +    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
>> +                      fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
>> +    if ( fres->addr == MAP_FAILED )
>> +        return -1;
>> +
>> +    mr.addr = (uintptr_t)fres->addr;
>> +
>> +    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
>> +    if ( rc )
>> +    {
>> +        PERROR("ioctl failed");
> return -1;
>
> I was rebasing my resource_size fix over this patch.
>
> It would be easiest for me if I fix up and commit this patch, if
> everyone is happy with that.

FAOD I've committed a fixed up version of this patch as discussed.

~Andrew

Re: [PATCH v2] libs/foreignmemory: Implement on NetBSD
Posted by Manuel Bouyer 3 years, 1 month ago
On Thu, Jan 28, 2021 at 11:42:45AM +0000, Andrew Cooper wrote:
> FAOD I've committed a fixed up version of this patch as discussed.

thanks !

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Re: [PATCH v2] libs/foreignmemory: Implement on NetBSD
Posted by Roger Pau Monné 3 years, 2 months ago
On Tue, Jan 26, 2021 at 11:47:52PM +0100, Manuel Bouyer wrote:
> Implement foreignmemory interface on NetBSD. The compat interface is now used
> only on __sun__
> 
> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
> ---
>  tools/libs/foreignmemory/Makefile  |  2 +-
>  tools/libs/foreignmemory/netbsd.c  | 66 +++++++++++++++++++++++++-----
>  tools/libs/foreignmemory/private.h |  6 +--
>  3 files changed, 60 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/libs/foreignmemory/Makefile b/tools/libs/foreignmemory/Makefile
> index 13850f7988..f191cdbed0 100644
> --- a/tools/libs/foreignmemory/Makefile
> +++ b/tools/libs/foreignmemory/Makefile
> @@ -8,7 +8,7 @@ SRCS-y                 += core.c
>  SRCS-$(CONFIG_Linux)   += linux.c
>  SRCS-$(CONFIG_FreeBSD) += freebsd.c
>  SRCS-$(CONFIG_SunOS)   += compat.c solaris.c
> -SRCS-$(CONFIG_NetBSD)  += compat.c netbsd.c
> +SRCS-$(CONFIG_NetBSD)  += netbsd.c
>  SRCS-$(CONFIG_MiniOS)  += minios.c
>  
>  include $(XEN_ROOT)/tools/libs/libs.mk
> diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
> index 54a418ebd6..a7e1d72ffc 100644
> --- a/tools/libs/foreignmemory/netbsd.c
> +++ b/tools/libs/foreignmemory/netbsd.c
> @@ -19,7 +19,9 @@
>  
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <errno.h>
>  #include <sys/mman.h>
> +#include <sys/ioctl.h>
>  
>  #include "private.h"
>  
> @@ -66,15 +68,17 @@ int osdep_xenforeignmemory_close(xenforeignmemory_handle *fmem)
>      return close(fd);
>  }
>  
> -void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
> -                              void *addr, int prot, int flags,
> -                              xen_pfn_t *arr, int num)
> +void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
> +                                 uint32_t dom, void *addr,
> +                                 int prot, int flags, size_t num,
> +                                 const xen_pfn_t arr[/*num*/], int err[/*num*/])
> +
>  {
>      int fd = fmem->fd;
> -    privcmd_mmapbatch_t ioctlx;
> -    addr = mmap(addr, num*XC_PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
> +    privcmd_mmapbatch_v2_t ioctlx;
> +    addr = mmap(addr, num*PAGE_SIZE, prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
>      if ( addr == MAP_FAILED ) {
> -        PERROR("osdep_map_foreign_batch: mmap failed");
> +        PERROR("osdep_xenforeignmemory_map: mmap failed");
>          return NULL;
>      }
>  
> @@ -82,11 +86,12 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
>      ioctlx.dom=dom;
>      ioctlx.addr=(unsigned long)addr;
>      ioctlx.arr=arr;
> -    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx) < 0 )
> +    ioctlx.err=err;
> +    if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH_V2, &ioctlx) < 0 )
>      {
>          int saved_errno = errno;
> -        PERROR("osdep_map_foreign_batch: ioctl failed");
> -        (void)munmap(addr, num*XC_PAGE_SIZE);
> +        PERROR("osdep_xenforeignmemory_map: ioctl failed");
> +        (void)munmap(addr, num*PAGE_SIZE);
>          errno = saved_errno;
>          return NULL;
>      }
> @@ -97,7 +102,48 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
>  int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
>                                   void *addr, size_t num)
>  {
> -    return munmap(addr, num*XC_PAGE_SIZE);
> +    return munmap(addr, num*PAGE_SIZE);
> +}
> +
> +int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> +                                    domid_t domid)
> +{
> +    errno = EOPNOTSUPP;
> +    return -1;
> +}
> +
> +int osdep_xenforeignmemory_unmap_resource(
> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
> +{
> +    return fres ? munmap(fres->addr, fres->nr_frames << PAGE_SHIFT) : 0;
> +}
> +
> +int osdep_xenforeignmemory_map_resource(
> +    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
> +{
> +    privcmd_mmap_resource_t mr = {
> +        .dom = fres->domid,
> +        .type = fres->type,
> +        .id = fres->id,
> +        .idx = fres->frame,
> +        .num = fres->nr_frames,
> +    };
> +    int rc;
> +
> +    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
> +                      fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
> +    if ( fres->addr == MAP_FAILED )
> +        return -1;
> +
> +    mr.addr = (uintptr_t)fres->addr;
> +
> +    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
> +    if ( rc )
> +    {
> +        PERROR("ioctl failed");
> +    }
> +
> +    return 0;

You need to return rc here, or else the failure won't be propagated to
the caller.

If you agree I don't think I have further comments:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH v2] libs/foreignmemory: Implement on NetBSD
Posted by Andrew Cooper 3 years, 2 months ago
On 26/01/2021 22:47, Manuel Bouyer wrote:
> Implement foreignmemory interface on NetBSD. The compat interface is now used
> only on __sun__
>
> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>

Just as a note.  I've also got a bugfix for this library (as well as
every other level of the stack), which will need an incremental change
in osdep_xenforeignmemory_map_resource().

See
https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=commitdiff;h=3768453802baece85140e579814af9c27f70d99a
for the details, but you may also need to bugfix the kernel side of the
IOCTL if you borrowed the FreeBSD copy to begin with.

What is probably easiest is for this patch to be committed first, then I
will rework the resource size fix() to make the same delta to NetBSD's
copy as Linux/FreeBSD.

~Andrew