From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH V5 09/12] xen/vm_event: Decouple vm_event
 and mem_access.
Date: Fri, 13 Feb 2015 21:05:20 +0000
Message-ID: <54DE6710.4040504@citrix.com>
References: <1423845203-18941-1-git-send-email-tamas.lengyel@zentific.com>
	<1423845203-18941-10-git-send-email-tamas.lengyel@zentific.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1423845203-18941-10-git-send-email-tamas.lengyel@zentific.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Tamas K Lengyel <tamas.lengyel@zentific.com>, xen-devel@lists.xen.org
Cc: kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, tim@xen.org, steve@zentific.com, jbeulich@suse.com, eddie.dong@intel.com, andres@lagarcavilla.org, jun.nakajima@intel.com, rshriram@cs.ubc.ca, keir@xen.org, dgdegra@tycho.nsa.gov, yanghy@cn.fujitsu.com, ian.jackson@eu.citrix.com
List-Id: xen-devel@lists.xenproject.org

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);
>