Re: [PATCH V5 09/12] xen/vm_event: Decouple vm_event and mem_access.

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 13/02/15 16:33, Tamas K Lengyel wrote:
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index f988291..f89361e 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -357,6 +357,67 @@ int vm_event_get_response(struct domain *d, struct vm_event_domain *ved, vm_even
>      return 1;
>  }
>  
> +/*
> + * Pull all responses from the given ring and unpause the corresponding vCPU
> + * if required. Based on the response type, here we can also call custom
> + * handlers.
> + *
> + * Note: responses are handled the same way regardless of which ring they
> + * arrive on.
> + */
> +void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
> +{
> +    vm_event_response_t rsp;
> +
> +    /* Pull all responses off the ring. */
> +    while ( vm_event_get_response(d, ved, &rsp) )
> +    {
> +        struct vcpu *v;
> +
> +        if ( rsp.version != VM_EVENT_INTERFACE_VERSION )
> +        {
> +            gdprintk(XENLOG_WARNING, "vm_event interface version mismatch!");
> +            continue;
> +        }
> +
> +#ifndef NDEBUG
> +        if ( rsp.flags & VM_EVENT_FLAG_DUMMY )
> +            continue;
> +#endif
> +
> +        /* Validate the vcpu_id in the response. */
> +        if ( (rsp.vcpu_id >= d->max_vcpus) || !d->vcpu[rsp.vcpu_id] )
> +            continue;
> +
> +        v = d->vcpu[rsp.vcpu_id];
> +
> +        /*
> +         * In some cases the response type needs extra handling, so here
> +         * we call the appropriate handlers.
> +         */
> +        switch ( rsp.reason )
> +        {
> +
> +#ifdef HAS_MEM_ACCESS
> +        case VM_EVENT_REASON_MEM_ACCESS:
> +            mem_access_resume(v, &rsp);
> +            break;
> +#endif
> +
> +#ifdef HAS_MEM_PAGING
> +        case VM_EVENT_REASON_MEM_PAGING:
> +            p2m_mem_paging_resume(d, &rsp);
> +            break;
> +#endif
> +

You need a default clause which captures unknown/invalid responses, logs
a message for debugging purposes, and ceases any further processing.  It
is not sensible for an unknown response to unpause the vcpu.

This will require you to have a whitelist of known reasons which simply
break.

~Andrew

> +        };
> +
> +        /* Unpause domain. */
> +        if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
> +            vm_event_vcpu_unpause(v);
> +    }
> +}
> +
>  void vm_event_cancel_slot(struct domain *d, struct vm_event_domain *ved)
>  {
>      vm_event_ring_lock(ved);
>