Re: [PATCH for-4.15 v2] xen/dmop: Strip __XEN_TOOLS__ header guard from public API

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/03/2021 17:12, Jan Beulich wrote:
> On 10.03.2021 16:07, Andrew Cooper wrote:
>> --- a/docs/designs/dmop.pandoc
>> +++ b/docs/designs/dmop.pandoc
>> @@ -4,9 +4,13 @@ DMOP
>>  Introduction
>>  ------------
>>  
>> -The aim of DMOP is to prevent a compromised device model from compromising
>> -domains other than the one it is providing emulation for (which is therefore
>> -likely already compromised).
>> +The DMOP hypercall has a new ABI design to solve problems in the Xen
>> +ecosystem.  First, the ABI is fully stable, to reduce the coupling between
>> +device models and the version of Xen.
>> +
>> +Secondly, for device models in userspace, the ABI is designed specifically 
>> to
>> +allow a kernel to audit the memory ranges used, without having to know the
>> +internal structure of sub-ops.
> I appreciate the editing, but the cited points still don't justify ...
>
>> --- a/xen/include/public/hvm/dm_op.h
>> +++ b/xen/include/public/hvm/dm_op.h
>> @@ -25,9 +25,6 @@
>>  #define __XEN_PUBLIC_HVM_DM_OP_H__
>>  
>>  #include "../xen.h"
>> -
>> -#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> -
>>  #include "../event_channel.h"
>>  
>>  #ifndef uint64_aligned_t
>> @@ -491,8 +488,6 @@ struct xen_dm_op {
>>      } u;
>>  };
>>  
>> -#endif /* __XEN__ || __XEN_TOOLS__ */
>> -
>>  struct xen_dm_op_buf {
>>      XEN_GUEST_HANDLE(void) h;
>>      xen_ulong_t size;
> ... this removal: What the kernel needs for its auditing (your 2nd
> point) is already outside of this guarded region, as you can see
> from the context above. You said there was a design goal of allowing
> use of this interface by (and not only through) the kernel, e.g. by
> a kernel module (which I don't think you mean to be covered by
> "device models"). If that was indeed a goal (Paul - can you confirm
> this?), this would now want listing as a 3rd item. Without such a
> statement I'd call it a bug to not have the guards there, and hence
> might either feel tempted myself to add them back at some point, or
> would ack a patch doing so.

Xen has absolutely no business dictating stuff like this.  It is an
internal and opaque property of the domain if the hypercalls happen to
originate from logic in user mode or kernel mode.  Stubdomains would
fall into the same "kernel" category as xengt in the dom0 i915 driver.

However, the actual bug I'm trying to fix with this is the need for
userspace, which doesn't link against libxc, to do

#define __XEN_TOOLS__
#include <xendevicemodel.h>

to be able to use the libxendevicemodel stable library.

The __XEN_TOOLS__ guard is buggy even ignoring the kernel device model
aspect.

~Andrew