Re: [Xen-devel] [PATCH v8 6/8] xen/arm: introduce GNTTABOP_cache_flush

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.10.14 at 11:35, <Ian.Campbell@xxxxxxxxxx> wrote:
> On Tue, 2014-10-21 at 10:14 +0100, Jan Beulich wrote:
>> >>> On 21.10.14 at 11:04, <Ian.Campbell@xxxxxxxxxx> wrote:
>> > On Mon, 2014-10-20 at 17:31 +0100, Stefano Stabellini wrote:
>> >> On Mon, 20 Oct 2014, Jan Beulich wrote:
>> >> > >>> On 20.10.14 at 17:24, <Ian.Campbell@xxxxxxxxxx> wrote:
>> >> > > On Mon, 2014-10-20 at 10:48 +0100, Stefano Stabellini wrote:
>> >> > >>  
>> >> > >> @@ -574,6 +576,25 @@ struct gnttab_swap_grant_ref {
>> >> > >>  typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
>> >> > >>  DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
>> >> > >>  
>> >> > >> +/*
>> >> > >> + * Issue one or more cache maintenance operations on a portion of a
>> >> > >> + * page granted to the calling domain by a foreign domain.
>> >> > >> + */
>> >> > >> +struct gnttab_cache_flush {
>> >> > >> +    union {
>> >> > >> +        uint64_t dev_bus_addr;
>> >> > >> +        grant_ref_t ref;
>> >> > >> +    } a;
>> >> > >> +    uint16_t offset; /* offset from start of grant */
>> >> > >> +    uint16_t length; /* size within the grant */
>> >> > > 
>> >> > > So are these not valid when used with dev_bus_addr? I can see that 
>> >> > > being
>> >> > > the case for offset, but length too?
>> >> > 
>> >> > Both ought to be valid, and dev_bus_addr should be page aligned.
>> >> 
>> >> That's right.
>> > 
>> > I would have expected you to correct the comment in the repost then
>> > (which I notice also still says arm in the subject)
>> 
>> I have to admit I'm having trouble seeing which of the comments
>> you think is wrong/misleading.
> 
> start of grant/size within the grant, in the context of using
> dev_bus_addr, seemed strange/inconsistent.
> 
> But you made me think again and of course dev_bus_addr is required to
> refer to a granted page isn't it. So I retract that comment.
> 
> Is it not worth being explicit about dev_bus_addr being page aligned and
> equal to something previously returned by a map operation?

Maybe, but that would then apply to pre-existing structures/fields
too.

Jan


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