Re: [Xen-devel] [PATCH V4 4/5] xen, libxc: Request page fault injection via libxc

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.09.14 at 12:01, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> +    /*
> +     * Inject per-domain pending hw/sw trap (this will most likely
> +     * be a page fault injected by memory introspection code).
> +     */
> +    else if ( d->arch.hvm_domain.inject_trap.vector != -1 &&
> +              hvm_can_inject_domain_pf(v) )

Now these two don't fit together: The first check for _any_
vector, while the latter considers just #PF.

> +    {
> +        hvm_inject_trap(&d->arch.hvm_domain.inject_trap);
> +        d->arch.hvm_domain.inject_trap.vector = -1;
> +    }

And this is clearly lacking serialization (or a comment saying why
serialization isn't needed here).

> @@ -6086,19 +6123,51 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>              goto param_fail8;
>  
>          rc = -ENOENT;
> -        if ( tr.vcpuid >= d->max_vcpus || (v = d->vcpu[tr.vcpuid]) == NULL )
> -            goto param_fail8;
> -        
> -        if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
> -            rc = -EBUSY;
> -        else 
> +
> +        if ( tr.vcpuid == (uint32_t)~0 ) /* Any VCPU. */
>          {
> -            v->arch.hvm_vcpu.inject_trap.vector = tr.vector;
> -            v->arch.hvm_vcpu.inject_trap.type = tr.type;
> -            v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
> -            v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
> -            v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
> -            rc = 0;
> +            unsigned int i;
> +
> +            for ( i = 0; i < d->max_vcpus; i++ )
> +                if ( (v = d->vcpu[i]) != NULL &&
> +                     v->arch.hvm_vcpu.inject_trap.vector != -1 )
> +                {
> +                    rc = -EBUSY;
> +                    break;
> +                }
> +
> +            if ( d->arch.hvm_domain.inject_trap.vector != -1 )
> +                rc = -EBUSY;
> +
> +            if ( rc != -EBUSY )
> +            {
> +                d->arch.hvm_domain.inject_trap.vector = tr.vector;
> +                d->arch.hvm_domain.inject_trap.type = tr.type;
> +                d->arch.hvm_domain.inject_trap.error_code = tr.error_code;
> +                d->arch.hvm_domain.inject_trap.insn_len = tr.insn_len;
> +                d->arch.hvm_domain.inject_trap.cr2 = tr.cr2;
> +                d->arch.hvm_domain.inject_trap.cr3 = tr.cr3;
> +                rc = 0;
> +            }
> +        }
> +        else /* Specific VCPU. */
> +        {
> +            if ( tr.vcpuid >= d->max_vcpus || (v = d->vcpu[tr.vcpuid]) == 
> NULL )
> +                goto param_fail8;
> +
> +            if ( v->arch.hvm_vcpu.inject_trap.vector != -1 ||
> +                 d->arch.hvm_domain.inject_trap.vector != -1 )
> +                rc = -EBUSY;
> +            else
> +            {
> +                v->arch.hvm_vcpu.inject_trap.vector = tr.vector;
> +                v->arch.hvm_vcpu.inject_trap.type = tr.type;
> +                v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
> +                v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
> +                v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
> +                v->arch.hvm_vcpu.inject_trap.cr3 = tr.cr3;
> +                rc = 0;
> +            }
>          }

As does - afaict - this code.

> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -197,6 +197,13 @@ struct xen_hvm_inject_trap {
>      uint32_t insn_len;
>      /* CR2 for page faults */
>      uint64_aligned_t cr2;
> +    /*
> +     * Only used if vcpuid == ~0 (wildcard for any VCPU).
> +     * In that case, injection data is set per-domain, and any VCPU
> +     * running a process with matching CR3 in user mode will inject
> +     * the trap.
> +     */
> +    uint64_aligned_t cr3;

The comment should say "Currently only used ...", and the code
should then check this (returning the usual -EOPNOTSUPP). Or
alternatively implement it right away (which may be the better
route taking into consideration the first of the comments above).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel