Re: [PATCH v1 2/2] xen: move arm/include/asm/vm_event.h to stubs

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-08-28 at 18:05 +0200, Jan Beulich wrote:
> On 28.08.2023 17:57, Oleksii Kurochko wrote:
> > asm/vm_event.h is common for ARM and RISC-V so it will be moved to
> > stubs dir.
> > 
> > Original asm/vm_event.h from ARM was updated:
> >  * use SPDX-License-Identifier.
> >  * update comment messages of stubs.
> >  * update #ifdef
> 
> When generalizing such a header, more tidying wants doing imo:
> 
> > --- /dev/null
> > +++ b/xen/include/stubs/asm/vm_event.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier:  GPL-2.0 */
> > +/*
> > + * vm_event.h: stubs for architecture specific vm_event handling
> > routines
> > + *
> > + * Copyright (c) 2015 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> > + */
> > +
> > +#ifndef __ASM_STUB_VM_EVENT_H__
> > +#define __ASM_STUB_VM_EVENT_H__
> > +
> > +#include <xen/sched.h>
> > +#include <public/domctl.h>
> 
> I can't spot why this is being included here. All that's needed ought
> to
> be public/vm_event.h, and even that only if we were to continue to
You are right. I'll change public/domctl.h to public/vm_event.h.

> use
> vm_event_response_t in the function definitions (which isn't really
> necessary).
> 
> > +static inline int vm_event_init_domain(struct domain *d)
> > +{
> > +    /* Nothing to do. */
> > +    return 0;
> > +}
> > +
> > +static inline void vm_event_cleanup_domain(struct domain *d)
> > +{
> > +    memset(&d->monitor, 0, sizeof(d->monitor));
> 
> This looks to be the sole reason that xen/sched.h is needed. I
> question
> the existence of that field in the first place when this stub is
> being
> used. But I guess cleaning that up as well might be going too far.
What do you mean by the existence of the field? Looking at declaration
of struct domain, monitor field always exists.

Could we leave initialisation of d->monitor for now?

~ Oleksii