Re: [PATCH] guestcopy: evaluate {,__}copy{,_field}_to_guest*() arguments just once

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 02/04/2020 07:20, Jan Beulich wrote:
> On 01.04.2020 23:28, Julien Grall wrote:
> > On 01/04/2020 15:29, Jan Beulich wrote:
> > > There's nothing wrong with having e.g.
> > >
> > >       copy_to_guest(uarg, ptr++, 1);
> > >
> > > yet until now this would increment "ptr" twice.
> > Is there such use in Xen today? I guess not as we would have noticed any issue.
> I'm not aware of any.
I have looked at the existing callers in staging and can't find such 
pattern as well.
> > > --- a/xen/include/asm-arm/guest_access.h
> > > +++ b/xen/include/asm-arm/guest_access.h
> > > @@ -79,7 +79,7 @@ int access_guest_memory_by_ipa(struct do
> > >        const typeof(*(ptr)) *_s = (ptr);                   \
> > >        char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> > >        void *__maybe_unused _t = (hnd).p;                  \
> > > -    ((void)((hnd).p == (ptr)));                         \
> > > +    (void)((hnd).p == _s);                              \
> > May I ask why this is a problem with 'ptr' but not 'hnd'?
> > Wouldn't it theorically possible to have an array of handle?
> Theoretically yes, but I view issues with the handle as far less
> likely than issues with any of the other parameters (in particular
> I don't see what an array of handles could be used for). So yes,
> if we want to be eager, we could deal with that one as well.
That's a fair point. I am happy either way.

I have also resent my patch (see [1]). This patch still applies on top 
of it and I have compile tested for arm32, arm64 and x86.
Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,


[1] https://lore.kernel.org/xen-devel/20200404130613.26428-1-julien@xxxxxxx/

> Jan
--
Julien Grall