Re: [Xen-devel] [PATCH v4 04/34] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Mar 24, 2016 at 03:15:25AM -0600, Jan Beulich wrote:
> >>> On 24.03.16 at 03:37, <konrad.wilk@xxxxxxxxxx> wrote:
> >> > --- a/xen/xsm/flask/hooks.c
> >> > +++ b/xen/xsm/flask/hooks.c
> >> > @@ -1658,6 +1658,40 @@ static int flask_xen_version (uint32_t op)
> >> >      }
> >> >  }
> >> >  
> >> > +static int flask_version_op (uint32_t op)
> >> > +{
> >> > +    u32 dsid = domain_sid(current->domain);
> >> > +
> >> > +    switch ( op )
> >> > +    {
> >> > +    case XEN_VERSION_version:
> >> > +    case XEN_VERSION_platform_parameters:
> >> > +    case XEN_VERSION_get_features:
> >> > +        /* These MUST always be accessible to any guest by default. */
> >> > +        return 0;
> >> 
> >> Perhaps these would better be taken care of in xsm_version_op()?
> > 
> > It would be the oddball one.
> > All of the xsm_**() in the header file (include/xsm/xsm.h) call the function
> > pointers.
> 
> True, but if there appeared any second implementation besides
> FLASK, it would need to repeat code to meet this backend
> independent policy. Anyway - I'll leave it to Daniel to judge.

/me nods.
> 
> > @@ -381,6 +389,123 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
> > arg)
> >      return -ENOSYS;
> >  }
> >  
> > +/* Computed be kernel_cache_init. */
> 
> ... by ...
> 
> And I also think kernel_cache_init is a bad name - you initialize the
> capabilities cache, not some kernel cache.

/me nods. 
> 
> > @@ -418,6 +543,20 @@ DO(ni_hypercall)(void)
> >      return -ENOSYS;
> >  }
> >  
> > +static int __init kernel_cache_init(void)
> > +{
> > +    /*
> > +     * Pre-allocate the cache so we do not have to worry about
> > +     * simultaneous invocations on safe_strcat by guests and the cache
> > +     * data becoming garbage.
> > +     */
> > +    arch_get_xen_caps(&cached_cap);
> > +    cached_cap_len = strlen(cached_cap) + 1;
> > +
> > +    return 0;
> > +}
> 
> With this I'm now missing the conversion of arch_get_xen_caps()
> to __init. Or was this meant to become a follow-up patch (since it
> might get a little larger if at once also taking care of moving the
> string literals into .init.*)?

A follow up.
> 
> Jan
> 

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