Re: [Xen-devel] [PATCH 2/2] x86/altp2m: allow specifying external-only use-case

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.08.16 at 01:56, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> @@ -5238,18 +5238,19 @@ static int do_altp2m_op(
>          goto out;
>      }
>  
> -    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
> +    if ( !d->arch.hvm_domain.params[HVM_PARAM_ALTP2M] )
> +    {
> +        rc = -EINVAL;
> +        goto out;
> +    }
> +
> +    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d,
> +                d->arch.hvm_domain.params[HVM_PARAM_ALTP2M])) )

As per the 3rd of the non-commit message notes in
https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg00865.html
(subject to a still outstanding response from Daniel or others)
I'd prefer if no new uses of XSM_OTHER got added.

> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -225,8 +225,16 @@
>  /* Location of the VM Generation ID in guest physical address space. */
>  #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
>  
> -/* Boolean: Enable altp2m */
> +/*
> + * Set mode for altp2m:
> + *  disabled: don't activate altp2m (default)
> + *  mixed: allow access to altp2m for both in-guest and external tools
> + *  external_only: allow access to external privileged tools only
> + */
>  #define HVM_PARAM_ALTP2M       35
> +#define HVMALTP2M_disabled      0
> +#define HVMALTP2M_mixed         1
> +#define HVMALTP2M_external_only 2

No new classes / groups of identifiers in the public interface please
which are in violation of the name spacing rules (read: should be
prefixed by XEN_).

Apart from that - would a guest-only mode make sense, i.e. would
making this individual flags rather than an enumeration like thing be
better?

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -554,10 +554,18 @@ static XSM_INLINE int 
> xsm_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)
>      return xsm_default_action(action, current->domain, d);
>  }
>  
> -static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d)
> +static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, 
> int mode)
>  {
> -    XSM_ASSERT_ACTION(XSM_TARGET);
> -    return xsm_default_action(action, current->domain, d);
> +    XSM_ASSERT_ACTION(XSM_OTHER);
> +    switch ( mode )
> +    {
> +    case HVMALTP2M_mixed:
> +        return xsm_default_action(XSM_TARGET, current->domain, d);
> +    case HVMALTP2M_external_only:
> +        return xsm_default_action(XSM_PRIV, current->domain, d);

Does this really need to be PRIV instead of just DM_PRIV?

Jan


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