| Message ID | 20210126224800.1246-6-bouyer@netbsd.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] libs/foreignmemory: Implement on NetBSD | expand |
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
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.
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
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
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 !
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) {
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(-)