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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:amc96@xxxxxxxxxxxxxxxx] On Behalf Of
> Andrew Cooper
> Sent: 21 April 2017 09:29
> 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 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.

That's a shame since nothing in the buf array changes... only what it points at.

  Paul

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