Re: [PATCH] common: don't require use of DOMID_SELF

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2021 10:59, Julien Grall wrote:
> On 14/01/2021 15:30, Jan Beulich wrote:
>> On 14.01.2021 15:43, Julien Grall wrote:
>>> On 14/01/2021 14:02, Jan Beulich wrote:
>>>> It's not overly difficult for a domain to figure out its ID, so
>>>> requiring the use of DOMID_SELF in a very limited set of places isn't
>>>> really helpful towards keeping the ID opaque to the guest.
>>>
>>> So I agree that a domid can be figured out really easily today and in
>>> principle it would be fine to relax it.
>>>
>>> However, most of the guest OSes will care about running on older Xen
>>> versions. Therefore they are not going to be able to use this relaxation.
>>>
>>> So I am not entirely convinced the relaxation is actually worth it for
>>> existing hypercalls.
>>
>> I'm aware of the concern (Andrew has voiced it both here and in
>> earlier contexts), but personally I think undue restrictions should
>> not be retained just because they used to be enforced.
> 
> I don't disagree about the undue restrictions. My main concern is it 
> makes more difficult for a developper to write portable code. The more 
> when there is no easy way to find out the differences between Xen 
> versions (see more below).
> 
>> We've gone
>> this same route in a few other occasions not overly long ago, and
>> iirc there are two patches going in a similar direction (different
>> area of course) that I have still pending and which neither got
>> ack-ed nor firmly rejected. >> Anyway, if we decide to relax it, then I 
>> think we should update the
>>> public headers because an OS using this relaxation will not work on
>>> older Xen. A developper will not be able to know that without looking at
>>> the implementation.
>>
>> Well, DOMID_SELF exists because that's the preferred form to use.
>> I can certainly add commentary, but it would feel a little odd to
>> do so.
> 
> Lets imagine your are the developer of a new OS but don't know Xen 
> internal. How would you find out the difference between Xen interfaces?
> 
> With no documentation you have two choices:
>     1) Dig into Xen code to understand the parameters
>     2) Rely on the testing to find interface
> 
> Neither of the two solutions are great for the developper.

But why would such a developer research how to figure out the domain
ID when they can use DOMID_SELF? As said, it's not difficult, but
one still needs to know a few details and write some extra code.

>> To be honest I'm also not sure how helpful this is going to
>> be, considering that consumers often have their own clones of our
>> headers.
> Right, but IMHO, anyone writing code that interface with the hypervisor 
> should read the latest documentation/interface.
> 
> At the moment, most of our documentations are in the public headers. So 
> it makes a suitable place.
> 
> We may need to duplicate the comment, but it is small enough to be fine.

Okay, I've added this hunk for the grant table side:

--- unstable.orig/xen/include/public/grant_table.h
+++ unstable/xen/include/public/grant_table.h
@@ -447,6 +447,12 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_
  * source_offset specifies an offset in the source frame, dest_offset
  * the offset in the target frame and  len specifies the number of
  * bytes to be copied.
+ *
+ * Note that operations not specifying GNTCOPY_*_gref will be restricted
+ * to the local domain for the respective operands (source and/or
+ * destination.  Note further that prior to Xen 4.15 only DOMID_SELF
+ * would be accepted to specify this, i.e. the actual ID of the local
+ * domain can only be used successfully on 4.15 and newer.
  */
 
 #define _GNTCOPY_source_gref      (0)

The duplication will be more noticable for the get_pg_owner() aspect,
as that is used by multiple hypercall handlers, which I first need to
all hunt down, locate a suitable place for putting the comment in the
headers, and then perhaps - since I have to do this hunting down now
anyway - also see whether Andrew's concern needs further code changes
to address. So far for a simple code change, where I don't really buy
in to the benefit of doing this extra exercise ...

Jan