Re: [PATCH for-4.16] freebsd/privcmd: fix MMAP_RESOURCE ioctl definition

[Andrew Cooper <amc96@xxxxxxxx>]

On 15/11/2021 12:19, Roger Pau Monné wrote:
> On Mon, Nov 15, 2021 at 12:03:26PM +0000, Andrew Cooper wrote:
>> On 15/11/2021 11:08, Roger Pau Monne wrote:
>>> Current ioctl definition was wrong in both FreeBSD and Xen sources, as
>>> the MMAP_RESOURCE ioctl needs to copy back the size of the resource
>>> when passed a zero address and size. FreeBSD encodes in the definition
>>> of the ioctl number whether parameters should be copied in (W) and/or
>>> copied out (R). The current definition for MMAP_RESOURCE is lacking
>>> the copy out part (R), and thus the call to query the size of a
>>> resource would always return 0.
>>>
>>> This change will break the current ioctl interface, the tools can
>>> however fall back to using the foreign memory interface in order to
>>> map resources from guests.
>>>
>>> This was a shortcoming from when the hypercall and ioctl gained the
>>> ability to query the size of the resources, as originally the
>>> MMAP_RESOURCE ioctl didn't need to copy out any data.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> ---
>>> Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
>>>
>>> The change only affects FreeBSD, and it's only a change in a
>>> definition of an ioctl, so it's unlikely to break existing code logic.
>>> Without this change Xen tools won't be able to use the MMAP_RESOURCE
>>> ioctl.
>> I guess you found this while trying to fix test-resource, in which case
>> a further argument for the change is "the unit tests now pass on FreeBSD" ?
> Indeed. It seems like this is the only instance of a resource size
> query that we have implemented so far.

Well - it's used on the domain create path ever since being introduced
to avoid the "shoot a hole in a superpage in order to map the grant
table" problem, but it does have a fallback which is probably why it
went unnoticed.

~Andrew