[PATCH v1 0/2] introduce stub directory to storing empty/stub headers

[PATCH v1 0/2] introduce stub directory to storing empty/stub headers

[Oleksii Kurochko]

A lot of empty/stub headers should be introduced during the early steps of adding
support of new architecture.

An example can be found here:
1. https://lore.kernel.org/xen-devel/cover.1692181079.git.oleksii.kurochko@gmail.com/
2. https://lore.kernel.org/xen-devel/a92f99e8f697da99d77bfde562a549dbef3760ce.1692816595.git.sanastasio@raptorengineering.com/

As part of the patch series, asm/vm_event.h was moved to the stubs directory because
It is the same for ARM, PPC, and RISC-V.

Oleksii Kurochko (2):
  xen: add stubs dir to include path
  xen: move arm/include/asm/vm_event.h to stubs

 xen/Makefile                        |  1 +
 xen/arch/arm/include/asm/vm_event.h | 66 -----------------------------
 xen/include/stubs/asm/vm_event.h    | 55 ++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 66 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/vm_event.h
 create mode 100644 xen/include/stubs/asm/vm_event.h

-- 
2.41.0

[PATCH v1 1/2] xen: add stubs dir to include path

[Oleksii Kurochko]

stubs dir will contain empty/stubs generic for all architectures
headers.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/Makefile b/xen/Makefile
index f57e5a596c..64c3542c84 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -438,6 +438,7 @@ ifdef building_out_of_srctree
 endif
 CFLAGS += -I$(srctree)/include
 CFLAGS += -I$(srctree)/arch/$(SRCARCH)/include
+CFLAGS += -I$(srctree)/include/stubs
 
 # Note that link order matters!
 ALL_OBJS-y                := common/built_in.o
-- 
2.41.0

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

[Oleksii Kurochko]

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

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/arm/include/asm/vm_event.h | 66 -----------------------------
 xen/include/stubs/asm/vm_event.h    | 55 ++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 66 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/vm_event.h
 create mode 100644 xen/include/stubs/asm/vm_event.h

diff --git a/xen/arch/arm/include/asm/vm_event.h b/xen/arch/arm/include/asm/vm_event.h
deleted file mode 100644
index 4d861373b3..0000000000
--- a/xen/arch/arm/include/asm/vm_event.h
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * vm_event.h: architecture specific vm_event handling routines
- *
- * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope 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, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef __ASM_ARM_VM_EVENT_H__
-#define __ASM_ARM_VM_EVENT_H__
-
-#include <xen/sched.h>
-#include <public/domctl.h>
-
-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));
-}
-
-static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
-                                              vm_event_response_t *rsp)
-{
-    /* Not supported on ARM. */
-}
-
-static inline
-void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
-{
-    /* Not supported on ARM. */
-}
-
-static inline
-void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
-{
-    /* Not supported on ARM. */
-}
-
-static inline
-void vm_event_sync_event(struct vcpu *v, bool value)
-{
-    /* Not supported on ARM. */
-}
-
-static inline
-void vm_event_reset_vmtrace(struct vcpu *v)
-{
-    /* Not supported on ARM. */
-}
-
-#endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/stubs/asm/vm_event.h b/xen/include/stubs/asm/vm_event.h
new file mode 100644
index 0000000000..6bda6ce7df
--- /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@tklengyel.com)
+ */
+
+#ifndef __ASM_STUB_VM_EVENT_H__
+#define __ASM_STUB_VM_EVENT_H__
+
+#include <xen/sched.h>
+#include <public/domctl.h>
+
+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));
+}
+
+static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
+                                              vm_event_response_t *rsp)
+{
+    /* Nothing to do. */
+}
+
+static inline
+void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
+{
+    /* Nothing to do. */
+}
+
+static inline
+void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
+{
+    /* Nothing to do. */
+}
+
+static inline
+void vm_event_sync_event(struct vcpu *v, bool value)
+{
+    /* Nothing to do. */
+}
+
+static inline
+void vm_event_reset_vmtrace(struct vcpu *v)
+{
+    /* Nothing to do. */
+}
+
+#endif /* __ASM_STUB_VM_EVENT_H__ */
-- 
2.41.0

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

[Jan Beulich]

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@tklengyel.com)
> + */
> +
> +#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 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.

Jan

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

[Oleksii]

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@tklengyel.com)
> > + */
> > +
> > +#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

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

[Jan Beulich]

On 29.08.2023 14:14, Oleksii wrote:
> On Mon, 2023-08-28 at 18:05 +0200, Jan Beulich wrote:
>> On 28.08.2023 17:57, Oleksii Kurochko wrote:
>>> +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.

Right, and that's what I consider questionable.

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

As said, asking you to also do that aspect of cleanup is probably
going too far, so yes, I guess you can leave that alone.

Jan