Hello Jan,
> -----Original Message-----
> From: Jan Beulich [mailto:[email protected]]
> Sent: Tuesday, July 3, 2018 3:34 PM
> To: Xin Li <[email protected]>; Daniel de Graaf <[email protected]>
> Cc: Andrew Cooper <[email protected]>; Ming Lu
> <[email protected]>; Sergey Dyasli <[email protected]>; Wei Liu
> <[email protected]>; Xin Li (Talons) <[email protected]>; George Dunlap
> <[email protected]>; Stefano Stabellini <[email protected]>; xen-
> [email protected]; Konrad Rzeszutek Wilk <[email protected]>; Tim
> (Xen.org) <[email protected]>
> Subject: Re: [PATCH 2/2] xen/xsm: Add new SILO mode for XSM
>
> >>> On 03.07.18 at 03:26, <[email protected]> wrote:
> > v2
> > To further discuss:
> > 1) is the new Kconfig option XSM_SILO necessary?
> > we can handle SILO similar as DUMMY, using exsting CONFIG_XSM.
> >
> > 2) explain "unmediated communication channel"
>
> I'm confused: As said in the reply to patch 1, this section is generally
> expected
> to contain information on what has changed from the prior version. I take it
> the
> above item instead related to the "To further discuss" sub-heading.
>
> > 3) is it OK to use the indirect call dummy_xsm_ops.evtchn_unbound?
>
> I'm not convinced it was worthwhile to send v2 with all of these still open.
OK.
>
> > +/*
> > + * Check if inter-domain communication is allowed.
> > + * Return true when pass check.
> > + */
> > +static bool silo_mode_dom_check(struct domain *ldom, struct domain
> > +*rdom) {
> > + struct domain *cur_dom = current->domain;
>
> const (three times altogether)
OK.
static bool silo_mode_dom_check(const struct domain *ldom,
const struct domain
*rdom)
{
const struct domain *cur_dom = current->domain;
return (is_control_domain(cur_dom) || is_control_domain(ldom) ||
is_control_domain(rdom) || ldom == rdom);
}
>
> > + return (is_control_domain(cur_dom) || is_control_domain(ldom) ||
> > + is_control_domain(rdom) || ldom == rdom); }
> > +
> > +static int silo_evtchn_unbound(struct domain *d1, struct evtchn *chn,
> > + domid_t id2) {
> > + int rc = -EPERM;
> > + struct domain *d2 = rcu_lock_domain_by_id(id2);
> > + if ( d2 != NULL && silo_mode_dom_check(d1, d2) )
>
> Blank line please between declaration(s) and statement(s). And const on the
> local variable declaration again.
>
> Also, is DOMID_SELF really not allowed here for id2? I don't think so,
> looking at
> e.g. evtchn_alloc_unbound().
static int silo_evtchn_unbound(struct domain *d1, struct evtchn *chn,
domid_t id2)
{
int rc = -EPERM;
struct domain *d2;
if ( id2 == DOMID_SELF )
id2 = current->domain->domain_id;
d2 = rcu_lock_domain_by_id(id2);
if ( d2 != NULL && silo_mode_dom_check(d1, d2) )
rc = dummy_xsm_ops.evtchn_unbound(d1, chn, id2);
rcu_unlock_domain(d2);
return rc;
}
>
> > +static int silo_grant_copy(struct domain *d1, struct domain *d2) {
> > + if ( silo_mode_dom_check(d1, d2) )
> > + return dummy_xsm_ops.grant_copy(d1, d2);
> > + return -EPERM;
> > +}
>
> I know transitive grants are a bad child, but they shouldn't be left out
> altogether in deciding what SILO mode is going to mean. In fact it looks to me
> as if there was no XSM check at all for the second half of a transitive grant
> copy's domain handling (the recursive
> acquire_grant_for_copy() call), which would mean that the fencing SILO mode
> looks to mean to establish wouldn't be complete. Daniel?
>
> Speaking of completeness: What about TMEM? Isn't one of the two pool types
> also meant to allow page sharing between domains?
>
> Jan
>
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
