Xen devel - Oct 2012 - [PATCH] xen: Add versions of rcu_lock_*_domain without IS_PRIV

While this patch is a part of my XSM IS_PRIV series, I am reposting it
alone because the XSM build of xen-unstable is currently broken. I can
also repost the remaining patches series if that would be helpful.

--------------------------------->8----------------------------------

These functions will be used to avoid duplication of IS_PRIV calls
that will be introduced in XSM hooks. This also fixes a build error
with XSM enabled introduced by 25925:d1c3375c3f11 which depends on
this patch.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/common/domain.c     | 21 +++++++++++++++++++++
 xen/include/xen/sched.h | 11 +++++++++++
 2 files changed, 32 insertions(+)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index a1aa05e..52489b3 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -420,6 +420,13 @@ struct domain *rcu_lock_domain_by_id(domid_t dom)
     return d;
 }
 
+struct domain *rcu_lock_domain_by_any_id(domid_t dom)
+{
+    if ( dom == DOMID_SELF )
+        return rcu_lock_current_domain();
+    return rcu_lock_domain_by_id(dom);
+}
+
 int rcu_lock_target_domain_by_id(domid_t dom, struct domain **d)
 {
     if ( dom == DOMID_SELF )
@@ -454,6 +461,20 @@ int rcu_lock_remote_target_domain_by_id(domid_t dom, struct
domain **d)
     return 0;
 }
 
+int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d)
+{
+    if ( (*d = rcu_lock_domain_by_id(dom)) == NULL )
+        return -ESRCH;
+
+    if ( *d == current->domain )
+    {
+        rcu_unlock_domain(*d);
+        return -EPERM;
+    }
+
+    return 0;
+}
+
 int domain_kill(struct domain *d)
 {
     int rc = 0;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 53804c8..b0def4a 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -447,6 +447,11 @@ struct domain *domain_create(
 struct domain *rcu_lock_domain_by_id(domid_t dom);
 
 /*
+ * As above function, but resolves DOMID_SELF to current domain
+ */
+struct domain *rcu_lock_domain_by_any_id(domid_t dom);
+
+/*
  * As above function, but accounts for current domain context:
  *  - Translates target DOMID_SELF into caller''s domain id; and
  *  - Checks that caller has permission to act on the target domain.
@@ -460,6 +465,12 @@ int rcu_lock_target_domain_by_id(domid_t dom, struct domain
**d);
  */
 int rcu_lock_remote_target_domain_by_id(domid_t dom, struct domain **d);
 
+/*
+ * As rcu_lock_domain_by_id(), but will fail EPERM or ESRCH rather than resolve
+ * to local domain.
+ */
+int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d);
+
 /* Finish a RCU critical region started by rcu_lock_domain_by_id(). */
 static inline void rcu_unlock_domain(struct domain *d)
 {
-- 
1.7.11.7

Must we have two new calls for translating a domid to a domain? It''s
getting
to be a confusing mess isn''t it? Also, here, a more consistent name for
rcu_lock_domain_by_any_id would be rcu_lock_any_domain_by_id, I think?

 -- Keir


On 15/10/2012 15:02, "Daniel De Graaf" <dgdegra@tycho.nsa.gov>
wrote:
> While this patch is a part of my XSM IS_PRIV series, I am reposting it
> alone because the XSM build of xen-unstable is currently broken. I can
> also repost the remaining patches series if that would be helpful.
> 
> --------------------------------->8----------------------------------
> 
> These functions will be used to avoid duplication of IS_PRIV calls
> that will be introduced in XSM hooks. This also fixes a build error
> with XSM enabled introduced by 25925:d1c3375c3f11 which depends on
> this patch.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  xen/common/domain.c     | 21 +++++++++++++++++++++
>  xen/include/xen/sched.h | 11 +++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index a1aa05e..52489b3 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -420,6 +420,13 @@ struct domain *rcu_lock_domain_by_id(domid_t dom)
>      return d;
>  }
>  
> +struct domain *rcu_lock_domain_by_any_id(domid_t dom)
> +{
> +    if ( dom == DOMID_SELF )
> +        return rcu_lock_current_domain();
> +    return rcu_lock_domain_by_id(dom);
> +}
> +
>  int rcu_lock_target_domain_by_id(domid_t dom, struct domain **d)
>  {
>      if ( dom == DOMID_SELF )
> @@ -454,6 +461,20 @@ int rcu_lock_remote_target_domain_by_id(domid_t dom,
> struct domain **d)
>      return 0;
>  }
>  
> +int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d)
> +{
> +    if ( (*d = rcu_lock_domain_by_id(dom)) == NULL )
> +        return -ESRCH;
> +
> +    if ( *d == current->domain )
> +    {
> +        rcu_unlock_domain(*d);
> +        return -EPERM;
> +    }
> +
> +    return 0;
> +}
> +
>  int domain_kill(struct domain *d)
>  {
>      int rc = 0;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 53804c8..b0def4a 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -447,6 +447,11 @@ struct domain *domain_create(
>  struct domain *rcu_lock_domain_by_id(domid_t dom);
>  
>  /*
> + * As above function, but resolves DOMID_SELF to current domain
> + */
> +struct domain *rcu_lock_domain_by_any_id(domid_t dom);
> +
> +/*
>   * As above function, but accounts for current domain context:
>   *  - Translates target DOMID_SELF into caller''s domain id; and
>   *  - Checks that caller has permission to act on the target domain.
> @@ -460,6 +465,12 @@ int rcu_lock_target_domain_by_id(domid_t dom, struct
> domain **d);
>   */
>  int rcu_lock_remote_target_domain_by_id(domid_t dom, struct domain **d);
>  
> +/*
> + * As rcu_lock_domain_by_id(), but will fail EPERM or ESRCH rather than
> resolve
> + * to local domain.
> + */
> +int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d);
> +
>  /* Finish a RCU critical region started by rcu_lock_domain_by_id(). */
>  static inline void rcu_unlock_domain(struct domain *d)
>  {

On 10/15/2012 11:26 AM, Keir Fraser wrote:
> Must we have two new calls for translating a domid to a domain?
It''s getting
> to be a confusing mess isn''t it? Also, here, a more consistent
name for
> rcu_lock_domain_by_any_id would be rcu_lock_any_domain_by_id, I think?
> 
>  -- Keir
The original version of this patch queue removed the two _target_ calls;
that removal is not in the current versions to avoid breaking code that is
not yet converted (ARM and two other callers not converted).

The name rcu_lock_any_domain_by_id is also fine, although it seems to imply
that rcu_lock_domain_by_id cannot lock any domain, when the real difference
is if they accept DOMID_SELF (hence why I chose to say any_id). Would you
like me to send a patch changing the name?
 
> 
> On 15/10/2012 15:02, "Daniel De Graaf"
<dgdegra@tycho.nsa.gov> wrote:
> 
>> While this patch is a part of my XSM IS_PRIV series, I am reposting it
>> alone because the XSM build of xen-unstable is currently broken. I can
>> also repost the remaining patches series if that would be helpful.
>>
>>
--------------------------------->8----------------------------------
>>
>> These functions will be used to avoid duplication of IS_PRIV calls
>> that will be introduced in XSM hooks. This also fixes a build error
>> with XSM enabled introduced by 25925:d1c3375c3f11 which depends on
>> this patch.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> ---
>>  xen/common/domain.c     | 21 +++++++++++++++++++++
>>  xen/include/xen/sched.h | 11 +++++++++++
>>  2 files changed, 32 insertions(+)
>>
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index a1aa05e..52489b3 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -420,6 +420,13 @@ struct domain *rcu_lock_domain_by_id(domid_t dom)
>>      return d;
>>  }
>>  
>> +struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>> +{
>> +    if ( dom == DOMID_SELF )
>> +        return rcu_lock_current_domain();
>> +    return rcu_lock_domain_by_id(dom);
>> +}
>> +
>>  int rcu_lock_target_domain_by_id(domid_t dom, struct domain **d)
>>  {
>>      if ( dom == DOMID_SELF )
>> @@ -454,6 +461,20 @@ int rcu_lock_remote_target_domain_by_id(domid_t
dom,
>> struct domain **d)
>>      return 0;
>>  }
>>  
>> +int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d)
>> +{
>> +    if ( (*d = rcu_lock_domain_by_id(dom)) == NULL )
>> +        return -ESRCH;
>> +
>> +    if ( *d == current->domain )
>> +    {
>> +        rcu_unlock_domain(*d);
>> +        return -EPERM;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  int domain_kill(struct domain *d)
>>  {
>>      int rc = 0;
>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>> index 53804c8..b0def4a 100644
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -447,6 +447,11 @@ struct domain *domain_create(
>>  struct domain *rcu_lock_domain_by_id(domid_t dom);
>>  
>>  /*
>> + * As above function, but resolves DOMID_SELF to current domain
>> + */
>> +struct domain *rcu_lock_domain_by_any_id(domid_t dom);
>> +
>> +/*
>>   * As above function, but accounts for current domain context:
>>   *  - Translates target DOMID_SELF into caller''s domain id;
and
>>   *  - Checks that caller has permission to act on the target domain.
>> @@ -460,6 +465,12 @@ int rcu_lock_target_domain_by_id(domid_t dom,
struct
>> domain **d);
>>   */
>>  int rcu_lock_remote_target_domain_by_id(domid_t dom, struct domain
**d);
>>  
>> +/*
>> + * As rcu_lock_domain_by_id(), but will fail EPERM or ESRCH rather
than
>> resolve
>> + * to local domain.
>> + */
>> +int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d);
>> +
>>  /* Finish a RCU critical region started by rcu_lock_domain_by_id(). */
>>  static inline void rcu_unlock_domain(struct domain *d)
>>  {
> 

-- 
Daniel De Graaf
National Security Agency

On 15/10/2012 16:31, "Daniel De Graaf" <dgdegra@tycho.nsa.gov>
wrote:
> On 10/15/2012 11:26 AM, Keir Fraser wrote:
>> Must we have two new calls for translating a domid to a domain?
It''s getting
>> to be a confusing mess isn''t it? Also, here, a more consistent
name for
>> rcu_lock_domain_by_any_id would be rcu_lock_any_domain_by_id, I think?
>> 
>>  -- Keir
> 
> The original version of this patch queue removed the two _target_ calls;
> that removal is not in the current versions to avoid breaking code that is
> not yet converted (ARM and two other callers not converted).
> 
> The name rcu_lock_any_domain_by_id is also fine, although it seems to imply
> that rcu_lock_domain_by_id cannot lock any domain, when the real difference
> is if they accept DOMID_SELF (hence why I chose to say any_id). Would you
> like me to send a patch changing the name?
Oh I see. No I think that''s fine then.

 -- Keir
>> 
>> On 15/10/2012 15:02, "Daniel De Graaf"
<dgdegra@tycho.nsa.gov> wrote:
>> 
>>> While this patch is a part of my XSM IS_PRIV series, I am reposting
it
>>> alone because the XSM build of xen-unstable is currently broken. I
can
>>> also repost the remaining patches series if that would be helpful.
>>> 
>>>
--------------------------------->8----------------------------------
>>> 
>>> These functions will be used to avoid duplication of IS_PRIV calls
>>> that will be introduced in XSM hooks. This also fixes a build error
>>> with XSM enabled introduced by 25925:d1c3375c3f11 which depends on
>>> this patch.
>>> 
>>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> ---
>>>  xen/common/domain.c     | 21 +++++++++++++++++++++
>>>  xen/include/xen/sched.h | 11 +++++++++++
>>>  2 files changed, 32 insertions(+)
>>> 
>>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>>> index a1aa05e..52489b3 100644
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -420,6 +420,13 @@ struct domain *rcu_lock_domain_by_id(domid_t
dom)
>>>      return d;
>>>  }
>>>  
>>> +struct domain *rcu_lock_domain_by_any_id(domid_t dom)
>>> +{
>>> +    if ( dom == DOMID_SELF )
>>> +        return rcu_lock_current_domain();
>>> +    return rcu_lock_domain_by_id(dom);
>>> +}
>>> +
>>>  int rcu_lock_target_domain_by_id(domid_t dom, struct domain **d)
>>>  {
>>>      if ( dom == DOMID_SELF )
>>> @@ -454,6 +461,20 @@ int
rcu_lock_remote_target_domain_by_id(domid_t dom,
>>> struct domain **d)
>>>      return 0;
>>>  }
>>>  
>>> +int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d)
>>> +{
>>> +    if ( (*d = rcu_lock_domain_by_id(dom)) == NULL )
>>> +        return -ESRCH;
>>> +
>>> +    if ( *d == current->domain )
>>> +    {
>>> +        rcu_unlock_domain(*d);
>>> +        return -EPERM;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  int domain_kill(struct domain *d)
>>>  {
>>>      int rc = 0;
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index 53804c8..b0def4a 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -447,6 +447,11 @@ struct domain *domain_create(
>>>  struct domain *rcu_lock_domain_by_id(domid_t dom);
>>>  
>>>  /*
>>> + * As above function, but resolves DOMID_SELF to current domain
>>> + */
>>> +struct domain *rcu_lock_domain_by_any_id(domid_t dom);
>>> +
>>> +/*
>>>   * As above function, but accounts for current domain context:
>>>   *  - Translates target DOMID_SELF into caller''s domain
id; and
>>>   *  - Checks that caller has permission to act on the target
domain.
>>> @@ -460,6 +465,12 @@ int rcu_lock_target_domain_by_id(domid_t dom,
struct
>>> domain **d);
>>>   */
>>>  int rcu_lock_remote_target_domain_by_id(domid_t dom, struct domain
**d);
>>>  
>>> +/*
>>> + * As rcu_lock_domain_by_id(), but will fail EPERM or ESRCH rather
than
>>> resolve
>>> + * to local domain.
>>> + */
>>> +int rcu_lock_remote_domain_by_id(domid_t dom, struct domain **d);
>>> +
>>>  /* Finish a RCU critical region started by
rcu_lock_domain_by_id(). */
>>>  static inline void rcu_unlock_domain(struct domain *d)
>>>  {
>> 
>