Re: [PATCH v3 2/2] xen: move arm/include/asm/vm_event.h to asm-generic

2023-08-31 Thread Jan Beulich 
On 31.08.2023 16:02, Oleksii wrote:
> On Thu, 2023-08-31 at 11:46 +0200, Jan Beulich wrote:
>> On 30.08.2023 18:57, Oleksii Kurochko wrote:
>>> ---
>>>  xen/arch/arm/include/asm/vm_event.h    | 66 --
>>> 
>>>  xen/include/asm-generic/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/asm-generic/asm/vm_event.h
>>
>> While it's a comment on the first patch, it's really better making
>> here:
>> Did you look at Linux? They don't put an intermediate asm/ here.
>> Instead
>> see their scripts/Makefile.asm-generic. That way an arch still has
>> control which generic headers it gets access to, without duplicating
>> any
>> of them.
> 
> IIUC scripts/Makefile.asm-generic script is needed to generate a
> wrapper for a header which will contain:
> #include 
> if it is mentioned generic-y += some.h in
> arch/${ARCH}/include/asm/Kbuild.
> 
> But do we need this generated wrapper header?
> Do we need all the support of generic-y in Kbuild, etc.?

For a single header we could likely get away without. But vm_event.h is
only the first example.

> In the previous patch of this patch series, it was added inclusion of
> $(srctree)/include/asm-generic after the inclusion of
> $(srctree)/arch/$(SRCARCH)/include so it will first look if the arch-
> specific header exists, and if not then use generic one.
> 
> Probably I misunderstood you and your suggestion was to have
> scripts/Makefile.asm-generic which will generate folder asm/ with
> necessary headers in arch specific folder?

Yes.

> So basically it was just a
> question if asm/ folder should exist in $(srctree)/include/asm-generic
> or not?

Not really, no. For "#include " to work, you can't simply omit
asm/ under asm-generic/. That's where the generated wrapper headers
come into play.

> One more thing I would like to clarify is the duplicating of the
> headers you mentioned above.
> But if the architecture doesn't want to use a generic header, then it
> still needs to add the header to arch/${ARCH}/include/asm and remove
> mention of the header from arch/${ARCH}/include/asm/Kbuild.

But then it's using its own custom header, not a duplication of whatever
is the (in the case here) stub one.

Jan

Re: [PATCH v3 2/2] xen: move arm/include/asm/vm_event.h to asm-generic

2023-08-31 Thread Oleksii 
On Thu, 2023-08-31 at 11:46 +0200, Jan Beulich wrote:
> On 30.08.2023 18:57, Oleksii Kurochko wrote:
> > asm/vm_event.h is common for ARM and RISC-V so it will be moved to
> > asm-generic dir.
> > 
> > Original asm/vm_event.h from ARM was updated:
> >  * use SPDX-License-Identifier.
> >  * update comment messages of stubs.
> >  * update #ifdef.
> >  * change public/domctl.h to public/vm_event.h.
> > 
> > Signed-off-by: Oleksii Kurochko 
> > Acked-by: Stefano Stabellini 
> > ---
> > Changes in V3:
> >  - add Acked-by: Stefano Stabellini  for
> > "xen: move arm/include/asm/vm_event.h to asm-generic"
> >  - update SPDX tag.
> >  - move asm/vm_event.h to asm-generic.
> > ---
> > Changes in V2:
> >  - change public/domctl.h to public/vm_event.h.
> >  - update commit message of [PATCH v2 2/2] xen: move
> > arm/include/asm/vm_event.h to stubs
> > ---
> >  xen/arch/arm/include/asm/vm_event.h    | 66 --
> > 
> >  xen/include/asm-generic/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/asm-generic/asm/vm_event.h
> 
> While it's a comment on the first patch, it's really better making
> here:
> Did you look at Linux? They don't put an intermediate asm/ here.
> Instead
> see their scripts/Makefile.asm-generic. That way an arch still has
> control which generic headers it gets access to, without duplicating
> any
> of them.

IIUC scripts/Makefile.asm-generic script is needed to generate a
wrapper for a header which will contain:
#include 
if it is mentioned generic-y += some.h in
arch/${ARCH}/include/asm/Kbuild.

But do we need this generated wrapper header?
Do we need all the support of generic-y in Kbuild, etc.?

In the previous patch of this patch series, it was added inclusion of
$(srctree)/include/asm-generic after the inclusion of
$(srctree)/arch/$(SRCARCH)/include so it will first look if the arch-
specific header exists, and if not then use generic one.

Probably I misunderstood you and your suggestion was to have
scripts/Makefile.asm-generic which will generate folder asm/ with
necessary headers in arch specific folder? So basically it was just a
question if asm/ folder should exist in $(srctree)/include/asm-generic
or not?

One more thing I would like to clarify is the duplicating of the
headers you mentioned above.
But if the architecture doesn't want to use a generic header, then it
still needs to add the header to arch/${ARCH}/include/asm and remove
mention of the header from arch/${ARCH}/include/asm/Kbuild.

~ Oleksii

Re: [PATCH v3 2/2] xen: move arm/include/asm/vm_event.h to asm-generic

2023-08-31 Thread Oleksii 
On Thu, 2023-08-31 at 11:46 +0200, Jan Beulich wrote:
> On 30.08.2023 18:57, Oleksii Kurochko wrote:
> > asm/vm_event.h is common for ARM and RISC-V so it will be moved to
> > asm-generic dir.
> > 
> > Original asm/vm_event.h from ARM was updated:
> >  * use SPDX-License-Identifier.
> >  * update comment messages of stubs.
> >  * update #ifdef.
> >  * change public/domctl.h to public/vm_event.h.
> > 
> > Signed-off-by: Oleksii Kurochko 
> > Acked-by: Stefano Stabellini 
> > ---
> > Changes in V3:
> >  - add Acked-by: Stefano Stabellini  for
> > "xen: move arm/include/asm/vm_event.h to asm-generic"
> >  - update SPDX tag.
> >  - move asm/vm_event.h to asm-generic.
> > ---
> > Changes in V2:
> >  - change public/domctl.h to public/vm_event.h.
> >  - update commit message of [PATCH v2 2/2] xen: move
> > arm/include/asm/vm_event.h to stubs
> > ---
> >  xen/arch/arm/include/asm/vm_event.h    | 66 --
> > 
> >  xen/include/asm-generic/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/asm-generic/asm/vm_event.h
> 
> While it's a comment on the first patch, it's really better making
> here:
> Did you look at Linux? They don't put an intermediate asm/ here.
> Instead
> see their scripts/Makefile.asm-generic. That way an arch still has
> control which generic headers it gets access to, without duplicating
> any
> of them.
Thanks. I'll look at the script.

~ Oleksii

Re: [PATCH v3 2/2] xen: move arm/include/asm/vm_event.h to asm-generic

2023-08-31 Thread Jan Beulich 
On 30.08.2023 18:57, Oleksii Kurochko wrote:
> asm/vm_event.h is common for ARM and RISC-V so it will be moved to
> asm-generic dir.
> 
> Original asm/vm_event.h from ARM was updated:
>  * use SPDX-License-Identifier.
>  * update comment messages of stubs.
>  * update #ifdef.
>  * change public/domctl.h to public/vm_event.h.
> 
> Signed-off-by: Oleksii Kurochko 
> Acked-by: Stefano Stabellini 
> ---
> Changes in V3:
>  - add Acked-by: Stefano Stabellini  for "xen: move 
> arm/include/asm/vm_event.h to asm-generic"
>  - update SPDX tag.
>  - move asm/vm_event.h to asm-generic.
> ---
> Changes in V2:
>  - change public/domctl.h to public/vm_event.h.
>  - update commit message of [PATCH v2 2/2] xen: move 
> arm/include/asm/vm_event.h to stubs
> ---
>  xen/arch/arm/include/asm/vm_event.h| 66 --
>  xen/include/asm-generic/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/asm-generic/asm/vm_event.h

While it's a comment on the first patch, it's really better making here:
Did you look at Linux? They don't put an intermediate asm/ here. Instead
see their scripts/Makefile.asm-generic. That way an arch still has
control which generic headers it gets access to, without duplicating any
of them.

Jan

[PATCH v3 2/2] xen: move arm/include/asm/vm_event.h to asm-generic

2023-08-30 Thread Oleksii Kurochko 
asm/vm_event.h is common for ARM and RISC-V so it will be moved to
asm-generic dir.

Original asm/vm_event.h from ARM was updated:
 * use SPDX-License-Identifier.
 * update comment messages of stubs.
 * update #ifdef.
 * change public/domctl.h to public/vm_event.h.

Signed-off-by: Oleksii Kurochko 
Acked-by: Stefano Stabellini 
---
Changes in V3:
 - add Acked-by: Stefano Stabellini  for "xen: move 
arm/include/asm/vm_event.h to asm-generic"
 - update SPDX tag.
 - move asm/vm_event.h to asm-generic.
---
Changes in V2:
 - change public/domctl.h to public/vm_event.h.
 - update commit message of [PATCH v2 2/2] xen: move arm/include/asm/vm_event.h 
to stubs
---
 xen/arch/arm/include/asm/vm_event.h| 66 --
 xen/include/asm-generic/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/asm-generic/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..00
--- 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 (ta...@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 .
- */
-
-#ifndef __ASM_ARM_VM_EVENT_H__
-#define __ASM_ARM_VM_EVENT_H__
-
-#include 
-#include 
-
-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(>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/asm-generic/asm/vm_event.h 
b/xen/include/asm-generic/asm/vm_event.h
new file mode 100644
index 00..29ab1b01b4
--- /dev/null
+++ b/xen/include/asm-generic/asm/vm_event.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier:  GPL-2.0-only */
+/*
+ * vm_event.h: stubs for architecture specific vm_event handling routines
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
+ */
+
+#ifndef __ASM_STUB_VM_EVENT_H__
+#define __ASM_STUB_VM_EVENT_H__
+
+#include 
+#include 
+
+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(>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