Re: [PATCH v2 REPOST 04/12] tools/libxenforeignmemory: add support for resource mapping

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 24 August 2017 16:53
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Wei Liu <wei.liu2@citrix.com>; Ian
> Jackson <Ian.Jackson@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v2 REPOST 04/12]
> tools/libxenforeignmemory: add support for resource mapping
> 
> On Tue, Aug 22, 2017 at 03:50:58PM +0100, Paul Durrant wrote:
> > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h
> b/tools/libs/foreignmemory/include/xenforeignmemory.h
> > index f4814c390f..e56eb3c4d4 100644
> > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h
> > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h
> > @@ -138,6 +138,45 @@ int
> xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
> >  int xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> >                                domid_t domid);
> >
> > +typedef struct xenforeignmemory_resource_handle
> xenforeignmemory_resource_handle;
> > +
> > +/**
> > + * This function maps a guest resource.
> > + *
> > + * @parm fmem handle to the open foreignmemory interface
> > + * @parm domid the domain id
> > + * @parm type the resource type
> > + * @parm id the type-specific resource identifier
> > + * @parm frame base frame index within the resource
> > + * @parm nr_frames number of frames to map
> > + * @parm paddr pointer to an address passed through to mmap(2)
> > + * @parm prot passed through to mmap(2)
> > + * @parm flags passed through to mmap(2)
> 
> Passing mmap flags directly can be troublesome, POSIX only defines
> MAP_SHARED and MAP_PRIVATE, the rest is OS-specific AFAIK. So we
> either limit this to POSIX only flags (and enforce it), or we replace
> it with some xenforeignmemory specific flags that each OS can map to
> it's equivalents.

Given that foreign memory mapping already passes through flags I guess that, for consistency, it would be best to limit use to POSIX-only flags in all cases.

> 
> > --- a/tools/libs/foreignmemory/private.h
> > +++ b/tools/libs/foreignmemory/private.h
> > @@ -42,6 +42,36 @@ void
> *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
> >                                xen_pfn_t *arr, int num);
> >  #endif
> >
> > +struct xenforeignmemory_resource_handle {
> > +    domid_t domid;
> > +    unsigned int type;
> > +    unsigned int id;
> > +    unsigned long frame;
> > +    unsigned long nr_frames;
> > +    void *addr;
> > +    int prot;
> > +    int flags;
> > +};
> > +
> > +#ifndef __linux__
> 
> The common practice in libxenforeign seems to be to add those dummy
> handlers to each OS-specific file (see osdep_xenforeignmemory_restrict
> for example) instead of doing it in the header file.

Yes, I know, I introduced that code :-) I think this way is actually neater. If no-one objects I can add a patch in to clean up xenforeignmemory_restrict() too.

Cheers,

  Paul

> 
> Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel