Re: [Xen-devel] [PATCH 1/4] hvm/dmop: Box dmop_args rather than passing multiple parameters around

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/04/2017 09:10, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:amc96@xxxxxxxxxxxxxxxx] On Behalf Of
>> Andrew Cooper
>> Sent: 21 April 2017 09:04
>> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jennifer Herbert
>> <jennifer.herbert@xxxxxxxxxx>; Xen-devel <xen-devel@xxxxxxxxxxxxx>
>> Cc: Jan Beulich <JBeulich@xxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
>> Subject: Re: [PATCH 1/4] hvm/dmop: Box dmop_args rather than passing
>> multiple parameters around
>>
>> On 21/04/2017 08:54, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: jennifer.herbert@xxxxxxxxxx [mailto:jennifer.herbert@xxxxxxxxxx]
>>>> Sent: 20 April 2017 19:00
>>>> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
>>>> Cc: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>; Andrew Cooper
>>>> <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>;
>>>> Jan Beulich <JBeulich@xxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
>>>> Subject: [PATCH 1/4] hvm/dmop: Box dmop_args rather than passing
>>>> multiple parameters around
>>>>
>>>> From: Jennifer Herbert <Jennifer.Herbert@xxxxxxxxxx>
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Jennifer Herbert <Jennifer.Herbert@xxxxxxxxxx>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> --
>>>> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
>>>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>>> CC: Julien Grall <julien.grall@xxxxxxx>
>>>> ---
>>>>  xen/arch/x86/hvm/dm.c | 47 ++++++++++++++++++++++++++++--------
>> ----
>>>> -------
>>>>  1 file changed, 28 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
>>>> index d72b7bd..fb4bcec 100644
>>>> --- a/xen/arch/x86/hvm/dm.c
>>>> +++ b/xen/arch/x86/hvm/dm.c
>>>> @@ -25,6 +25,13 @@
>>>>
>>>>  #include <xsm/xsm.h>
>>>>
>>>> +struct dmop_args {
>>>> +    domid_t domid;
>>>> +    unsigned int nr_bufs;
>>>> +    /* Reserve enough buf elements for all current hypercalls. */
>>>> +    struct xen_dm_op_buf buf[2];
>>>> +};
>>>> +
>>>>  static bool copy_buf_from_guest(const xen_dm_op_buf_t bufs[],
>>>>                                  unsigned int nr_bufs, void *dst,
>>>>                                  unsigned int idx, size_t dst_size)
>>>> @@ -287,16 +294,14 @@ static int inject_event(struct domain *d,
>>>>      return 0;
>>>>  }
>>>>
>>>> -static int dm_op(domid_t domid,
>>>> -                 unsigned int nr_bufs,
>>>> -                 xen_dm_op_buf_t bufs[])
>>>> +static int dm_op(struct dmop_args *op_args)
>>> Shouldn't this be a const pointer?
>> No.  copy_to_guest_buf() uses a non const reference of op_args-
>>> buf[$IDX].
> Can't that be const too (as I commented in the relevant patch)?

No.  That is not legal in the C typesystem.

copy_to_guest_offset(args->buf[buf_idx].h, ...) really really uses a non
constant .h here.

The broken quirk of the of the C typesystem which loses const when
following pointers doesn't apply here, because buf[] is an embedded
array and properly inherits the constness of the args pointer.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel