Re: [RFC PATCH V3 11/12] xen/vm_event: Decouple vm_event and mem_access.

["Jan Beulich" <JBeulich@suse.com>]

>>> On 29.01.15 at 22:46, <tamas.lengyel@zentific.com> wrote:
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -52,9 +52,10 @@ obj-y += tmem_xen.o
>  obj-y += radix-tree.o
>  obj-y += rbtree.o
>  obj-y += lzo.o
> +obj-y += vm_event.o
> +obj-y += monitor.o

Please make the (not) sorting situation worse than it already is - the
original intention was for entries to be alphabetically sorted here.

> --- /dev/null
> +++ b/xen/common/monitor.c
> @@ -0,0 +1,64 @@
> +/******************************************************************************
> + * monitor.c
> + *
> + * VM event monitor support.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include <xen/sched.h>
> +#include <xen/vm_event.h>
> +#include <xen/mem_access.h>
> +
> +void monitor_resume(struct domain *d)
> +{
> +    vm_event_response_t rsp;
> +
> +    /* Pull all responses off the ring. */
> +    while ( vm_event_get_response(d, &d->vm_event->monitor, &rsp) )
> +    {
> +        struct vcpu *v;
> +
> +        if ( rsp.version != VM_EVENT_INTERFACE_VERSION )
> +            continue;
> +
> +#ifndef NDEBUG
> +        if ( rsp.flags & VM_EVENT_FLAG_DUMMY )
> +            continue;
> +#endif

This code was unconditional in the original function. Such a change
needs mentioning in the description.

> @@ -49,7 +53,7 @@ int mem_access_send_req(struct domain *d, vm_event_request_t *req)
>      return -ENOSYS;
>  }
>  
> -static inline void mem_access_resume(struct domain *d) {}
> +static inline void mem_access_resume(struct vcpu *vcpu, vm_event_response_t *rsp) {}

Long line.

> --- /dev/null
> +++ b/xen/include/xen/monitor.h
> @@ -0,0 +1,38 @@
> +/******************************************************************************
> + * monitor.h
> + *
> + * Monitor event support.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#ifndef _XEN_ASM_MONITOR_H
> +#define _XEN_ASM_MONITOR_H
> +
> +#include <public/memory.h>

This can't be enough (nor can I see why it's needed here), or else ...

> +/* Resumes the running of the VCPU, restarting the last instruction */
> +void monitor_resume(struct domain *d);

... struct domain may end up being a forward reference (with scope
restricted to monitor_resume()).

Jan