Re: [Xen-devel] [PATCH v4 3/7] libxl: introduce a new structure to represent static shared memory regions

[Julien Grall <julien.grall@xxxxxxx>]

On 02/06/2018 04:06 PM, Zhongze Liu wrote:
> Hi,
Hi,


> 2018-02-06 23:46 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:
> > Hi,
> >
> > On 02/06/2018 03:41 PM, Zhongze Liu wrote:
> > > Thanks for reviewing.
> > >
> > > 2018-02-06 19:27 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:
> > > > Hi,
> > > >
> > > > On 01/30/2018 05:50 PM, Zhongze Liu wrote:
> > > > > Add a new structure to the IDL familiy to represent static shared memory
> > > > > regions
> > >
> > > [...]
> > >
> > > > > +libxl_static_shm = Struct("static_shm", [
> > > > > +    ("id", string),
> > > > > +    ("offset", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
> > > > > +    ("begin", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
> > > > > +    ("end", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
> > > >
> > > >
> > > > We might want to store the size rather than the end. This would allow us
> > > > to
> > > > cover region up to the address 2^64-1.
> > > >
> > > > Also, this would make clearer whether end is included in the region or
> > > > not.
> > > I think making the range inclusive and documenting it would have the
> > > same effect.
> > > But I'm not sure which syntax is more friendly to the users. What do you
> > > think?
> >
> > You would still run into some problem. Indeed LIBX_SSHM_RANGE_UNKNOWN is
> > defined as UINT64_MAX. So how would you differentiate them?
> But saying inclusive, I was actually trying to say "including the page
> that begins at @end", so
Which is quite confusing. Usually when I see a range, I assume that the 
end will be the actual end. Not PAGE_ALIGN(end).
> the only possibility when  LIBXL_SSHM_RANGE_UNKNOWN would be a valid value for
> @end is when the page granularity is 1byte, which, I think, is not
> very likely to happen.
>
> But soon I find this might lead to more confusion. Now I agree with
> you that we should use
> the begin/size syntax instead of the current one.
Yes, begin/size is straightforward. There are no way to lead to map one 
less (or extra) page by confusion.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel