| Message ID | 798b7eec-e31e-1798-773d-c2865fba4be2@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | allow xc_domain_maximum_gpfn() to observe full GFN value | expand |
On 18/06/2021 11:23, Jan Beulich wrote: > Some hypercalls, memory-op in particular, can return values requiring > more than 31 bits to represent. Hence the underlying layers need to make > sure they won't truncate such values. (Note that for Solaris the > function also gets renamed, to match the other OSes.) Spot the environment which obviously hasn't been compiled since the 4.5(?) timeframe... Also, I'm fairly sure the comment in the NetBSD version is false when it says it is copying the Linux way of doing things - I'm pretty sure it means copying the FreeBSD way. > Signed-off-by: Jan Beulich <jbeulich@suse.com> I think the commit message needs to state that this doesn't fix truncation in the Linux or Solaris, nor the truncation in the xencall{0..5}() public APIs. It only fixes truncation issues for FreeBSD, MiniOS and NetBSD. With a suitable adjustment, and ideally a fix to the NetBSD comment, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 18.06.2021 15:26, Andrew Cooper wrote: > On 18/06/2021 11:23, Jan Beulich wrote: >> Some hypercalls, memory-op in particular, can return values requiring >> more than 31 bits to represent. Hence the underlying layers need to make >> sure they won't truncate such values. (Note that for Solaris the >> function also gets renamed, to match the other OSes.) > > Spot the environment which obviously hasn't been compiled since the > 4.5(?) timeframe... > > Also, I'm fairly sure the comment in the NetBSD version is false when it > says it is copying the Linux way of doing things - I'm pretty sure it > means copying the FreeBSD way. It doesn't say "copy", but "mimic", and I think what it means is the effect for its callers. Therefore I think the comment makes sense in its current shape. And I don't think I should change it right here anyway. >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > I think the commit message needs to state that this doesn't fix > truncation in the Linux or Solaris, nor the truncation in the > xencall{0..5}() public APIs. It only fixes truncation issues for > FreeBSD, MiniOS and NetBSD. I've added "Due to them merely propagating ioctl()'s return value, this change is benign on Linux and Solaris. IOW there's an actual effect here only for the BSDs and MiniOS, but even then further adjustments are needed at the xencall<N>() level." > With a suitable adjustment, and ideally a fix to the NetBSD comment, > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks. Jan
--- a/tools/libs/call/freebsd.c +++ b/tools/libs/call/freebsd.c @@ -62,7 +62,7 @@ int osdep_xencall_close(xencall_handle * return close(fd); } -int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) +long osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) { int fd = xcall->fd; int ret; --- a/tools/libs/call/linux.c +++ b/tools/libs/call/linux.c @@ -80,7 +80,7 @@ int osdep_xencall_close(xencall_handle * return 0; } -int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) +long osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) { return ioctl(xcall->fd, IOCTL_PRIVCMD_HYPERCALL, hypercall); } --- a/tools/libs/call/minios.c +++ b/tools/libs/call/minios.c @@ -38,7 +38,7 @@ int osdep_xencall_close(xencall_handle * return 0; } -int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) +long osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) { multicall_entry_t call; int i, ret; --- a/tools/libs/call/netbsd.c +++ b/tools/libs/call/netbsd.c @@ -96,7 +96,7 @@ void osdep_free_pages(xencall_handle *xc free(ptr); } -int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) +long osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) { int fd = xcall->fd; int error = ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall); --- a/tools/libs/call/private.h +++ b/tools/libs/call/private.h @@ -55,7 +55,7 @@ struct xencall_handle { int osdep_xencall_open(xencall_handle *xcall); int osdep_xencall_close(xencall_handle *xcall); -int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall); +long osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall); void *osdep_alloc_pages(xencall_handle *xcall, size_t nr_pages); void osdep_free_pages(xencall_handle *xcall, void *p, size_t nr_pages); --- a/tools/libs/call/solaris.c +++ b/tools/libs/call/solaris.c @@ -80,7 +80,7 @@ void osdep_free_hypercall_buffer(xencall free(ptr); } -int do_xen_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) +long osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall) { int fd = xcall->fd; return ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
Some hypercalls, memory-op in particular, can return values requiring more than 31 bits to represent. Hence the underlying layers need to make sure they won't truncate such values. (Note that for Solaris the function also gets renamed, to match the other OSes.) Signed-off-by: Jan Beulich <jbeulich@suse.com>