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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/10/18 12:12, Jan Beulich wrote:
>>>> 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?

Lets not waste time arguing.  Acked-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>

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