[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4] altp2m: Allow specifying external-only use-case
>>> On 20.03.17 at 20:27, <tamas.lengyel@xxxxxxxxxxxx> wrote: > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx> > Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx> > Acked-by: Jan Beulich <jbeulich@xxxxxxxx> I'll need to make this conditional upon a few more adjustments: > @@ -4370,18 +4370,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] ) I think it would be better for readers if you compared against XEN_ALTP2M_disabled here. > + { > + rc = -EINVAL; > + goto out; > + } > + > + if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, > + d->arch.hvm_domain.params[HVM_PARAM_ALTP2M])) ) Note the implicit truncation here. Not a problem at present, but at the very least I'd like to ask for the function parameter to become unsigned int. Furthermore, wasn't HVMOP_altp2m_vcpu_enable_notify supposed to always be available to the guest (as long as altp2m is enabled)? You don't allow this here anymore. > --- a/xen/include/public/hvm/params.h > +++ b/xen/include/public/hvm/params.h > @@ -228,8 +228,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 This needs to be more precise: VMFUNC is an access mechanism too, and aiui this isn't meant to be disabled by external_only. > + */ > #define HVM_PARAM_ALTP2M 35 > +#define XEN_ALTP2M_disabled 0 > +#define XEN_ALTP2M_mixed 1 > +#define XEN_ALTP2M_external_only 2 I'd drop the _only suffix. > --- a/xen/include/xsm/dummy.h > +++ b/xen/include/xsm/dummy.h > @@ -555,10 +555,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 XEN_ALTP2M_mixed: > + return xsm_default_action(XSM_TARGET, current->domain, d); > + case XEN_ALTP2M_external_only: > + return xsm_default_action(XSM_DM_PRIV, current->domain, d); > + default: > + return -EPERM; I think -EPERM is correct at most for XEN_ALTP2M_disabled, all others should get -EINVAL or -EOPNOTSUPP or some such. Perhaps that also doesn't really belong here, but rather into the caller (which right now produces -EINVAL for XEN_ALTP2M_disabled only). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |