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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 21 Oct 2014, Jan Beulich wrote:
> >>> On 20.10.14 at 18:58, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > @@ -490,6 +495,42 @@ static int _set_status(unsigned gt_version,
> >          return _set_status_v2(domid, readonly, mapflag, shah, act, status);
> >  }
> >  
> > +static int grant_map_exists(const struct domain *ld,
> > +                            struct grant_table *rgt,
> > +                            unsigned long mfn,
> > +                            grant_ref_t *ref_count)
> > +{
> > +    const struct active_grant_entry *act;
> > +    grant_ref_t ref, max_iter;
> > +    
> > +    ASSERT(spin_is_locked(&rgt->lock));
> > +
> > +    max_iter = min(*ref_count + (1 << GNTTABOP_CONTINUATION_ARG_SHIFT), 
> > nr_grant_entries(rgt));
> 
> Long line.
> 
> > @@ -2617,17 +2761,34 @@ do_grant_table_op(
> >          }
> >          break;
> >      }
> > +    case GNTTABOP_cache_flush:
> > +    {
> > +        grant_ref_t ref_count = opaque_in;
> 
> I can't see the need for this variable.

We use opaque_in's address below but opaque_in is an unsigned int, not
grant_ref_t. It doesn't make sense to cast &opaque_in to (grant_ref_t*),
because if they were different sizes, it couldn't really help.
So I used a local variable.


> > --- a/xen/include/public/grant_table.h
> > +++ b/xen/include/public/grant_table.h
> > @@ -309,6 +309,8 @@ typedef uint16_t grant_status_t;
> >  #define GNTTABOP_get_status_frames    9
> >  #define GNTTABOP_get_version          10
> >  #define GNTTABOP_swap_grant_ref          11
> > +#define GNTTABOP_cache_flush             12
> > +/* max GNTTABOP is 4095 */
> 
> This is an implementation detail, i.e. such a comment doesn't belong
> in the public header.

xen/include/xen/grant_table.h?

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