[Xen-devel] Ping: [PATCH] x86: restrict HVMOP_pagetable_dying to current

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.10.18 at 14:03, <JBeulich@xxxxxxxx> wrote:
>>>> On 05.10.18 at 13:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 05/10/18 12:29, Jan Beulich wrote:
>>> This is not used (and probably was never meant to be) by the tool stack.
>>> Limiting it to the current domain in particular allows to eliminate a
>>> bogus use of vCPU 0 in pagetable_dying().
>>>
>>> Remove the now unnecessary domain/vCPU parameters from the wrapper/hook
>>> functions at the same time.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -4895,10 +4895,12 @@ long do_hvm_op(unsigned long op, XEN_GUE
>>>              return -ESRCH;
>>>  
>>>          rc = -EINVAL;
>>> -        if ( is_hvm_domain(d) && paging_mode_shadow(d) )
>>> +        if ( unlikely(d != current->domain) )
>>> +            rc = -EOPNOTSUPP;
>>> +        else if ( is_hvm_domain(d) && paging_mode_shadow(d) )
>>>              rc = xsm_hvm_param(XSM_TARGET, d, op);
>> 
>> As we're switching to current-only, shouldn't this turn to XSM_HOOK ?
> 
> Not sure - I simply didn't want to fiddle with any of the semantics,
> and keeping it as it is may be sub-optimal, but is certainly not going
> to be wrong.

Are you fine with the above, or do you demand the change in
order to give your ack?

>> Everything else LGTM, with one small suggestion....
>> 
>>>          if ( !rc )
>>> -            pagetable_dying(d, a.gpa);
>>> +            pagetable_dying(a.gpa);
>>>  
>>>          rcu_unlock_domain(d);
>>>          break;
>>> --- a/xen/include/asm-x86/paging.h
>>> +++ b/xen/include/asm-x86/paging.h
>>> @@ -345,7 +345,7 @@ void paging_write_p2m_entry(struct p2m_d
>>>  
>>>  /* Called from the guest to indicate that the a process is being
>>>   * torn down and its pagetables will soon be discarded */
>>> -void pagetable_dying(struct domain *d, paddr_t gpa);
>>> +void pagetable_dying(paddr_t gpa);
>> 
>> Fix the comment style while in this area?
> 
> Well, I can certainly do so - I didn't because I didn't touch the
> comment itself.

I didn't think it was necessary to re-submit with just this adjustment,
the more that it was a suggestion only anyway. Is there anything
else that needs taking care of before I can get you ack for the
non-mm parts?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel