[PATCH v8 0/7] Introduce generic headers

[PATCH v8 0/7] Introduce generic headers

[Oleksii Kurochko]

Some headers are common between several architectures, so the current patch series
provide them.

Another one reason to have them as generic is a simplification of adding support
necessary to make a complete Xen build as it was/is being done in the patch series [1]
and [2].

Also, instead of providing generic/stub headers, it was used
"#ifdef CONFIG_* #include <asm/*.h> #endif" instead of providing empty headers.

This patch series is a pre-requisite for "Enable build of full Xen for RISC-V" [3].

[1] https://lore.kernel.org/xen-devel/cover.1694543103.git.sanastasio@raptorengineering.com/
[2] https://lore.kernel.org/xen-devel/cover.1692181079.git.oleksii.kurochko@gmail.com/
[3] https://lore.kernel.org/xen-devel/cover.1700761381.git.oleksii.kurochko@gmail.com/
---
Changes in V8:
 - Add Acked-by Acked-by: Tamas K Lengyel <tamas@tklengyel.com> for patches:
    [PATCH v7 4/7] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
    [PATCH v7 3/7] xen/asm-generic: introduce stub header monitor.h
 - Update the commit messages for patches:
    [PATCH v7 5/7] xen/asm-generic: introduce generic device.h
    [PATCH v7 6/7] xen/arm: switch Arm to use asm-generic/device.h
 - Drop #ifdef HAS_PCI in asm-generic/device.h to be consistent with other pci-related
   things in the header.
 - Code styles fixes.    
---
Changes in V7:
 - The following patches were dropped because of rebasing as they were merged
   to staging:
   - [PATCH v6 3/9] xen/asm-generic: introduce generic div64.h header
   - [PATCH v6 6/9] xen/asm-generic: introduce stub header softirq.h
   - [PATCH v6 7/9] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
   - [PATCH v6 8/9] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
- [PATCH v6 5/9] xen/asm-generic: introduce stub header numa.h was dropped becaus of
  the patch: "[PATCH v2] NUMA: no need for asm/numa.h when !NUMA"
- Drop definition of arch_monitor_domctl_event for PPC.
- define arch_monitor_domctl_event in asm-generic/monitor.h.
- Add "define HAS_ARCH_MONITOR_DOMCTL_EVENT" in arm/.../monitor.h as it has arch
  specific implementation.
- keeping DEVICE_PCI_HOSTBRIDGE available for every build based on the reply:
    https://lore.kernel.org/xen-devel/926a5c12-7f02-42ec-92a8-1c82d060c710@xen.org/
- add comment above enum device_type.h with explanation about DEV_TYPE_MAX.        
- drop #ifdef HAS_PCI around "(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE))" in ARM code.
- separate patch  "[PATCH v6 9/9] xen/asm-generic: introduce generic device.h" into 3 patches.
---
Changes in V6:
 - Fix the build script to work properly with EXTRA_FIXED_RANDCONFIG.
 - Introduce separate randconfig yaml with fixed configs for RISC-V.
 - Disable CONFIG_GRANT_TABLE and CONFIG_MEM_ACCESS for PPC and RISC-V.
 - Remove change in Kconfig/common for CONFIG_GRANT_TABLE and CONFIG_MEM_ACCESS.
 - Rebase on top of the latest staging. 
---
Changes in V5:
 - Update the patch series message as patch related to delay.h was merged.
 - Rebase on top of staging because half of the patches of the patch series were
   merged to staging branch.
 - Add A-by for some of the patches.
 - Add "depends on X86 || Arm" for CONFIG_GRANT_TABLE and CONFIG_MEM_ACCESS to be
   sure it won't be turned on by randconfig in CI.
 - Partly switch Arm and PPC to asm-generic/monitor.h.
 - Some other minor changes
---
Changes in V4:
 - Update the cover letter message
 - Add Reviewed-by/Acked-by for patches:
    [PATCH v3 01/14] xen/asm-generic: introduce stub header paging.h
    [PATCH v3 03/14] xen/asm-generic: introduce generic hypercall.h
    [PATCH v3 04/14] xen/asm-generic: introduce generic header iocap.h
    [PATCH v3 05/14] xen/asm-generic: introduce stub header <asm/random.h>
    [PATCH v3 06/14] xen/asm-generic: introduce generic header percpu.h
    [PATCH v3 07/14] xen/asm-generic: introduce generalized hardirq.h
    [PATCH v3 08/14] xen/asm-generic: introduce generic div64.h header
    [PATCH v3 09/14] xen/asm-generic: introduce generic header altp2m.h
    [PATCH v3 10/14] xen/asm-generic: introduce stub header monitor.h
    [PATCH v3 11/14] xen/asm-generic: introduce stub header numa.h
    [PATCH v3 12/14] xen/asm-generic: introduce stub header softirq.h
 - Fix some code style and minor issues.
 - Use asm-generic version of device.h for Arm and PPC.
---
Changes in V3:
 - Update the commit message of the cover letter.
 - Drop the following patch as it can be arch-specific enough:
   * [PATCH v2 09/15] xen/asm-generic: introduce generic header smp.h
 - Drop correspondent arch specific headers and use asm-generic version of
   a header.
 - Back to the patch series patches:
   * xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
   * xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
---
Changes in V2:
 - Update the commit message of the cover letter.
 - Drop the following patches because they are arch-specific or was sent as a separate patch:
   - xen/asm-generic: introduce stub header event.h
	 - xen/asm-generic: introduce stub header spinlock.h
	 - [PATCH v1 03/29] xen/asm-generic: introduce stub header cpufeature.h
	 - [PATCH v1 07/29] xen/asm-generic: introduce stub header guest_atomics.h
	 - [PATCH v1 10/29] xen/asm-generic: introduce stub header iommu.h
	 - [PATCH v1 12/29] xen/asm-generic: introduce stub header pci.h because separate patch was sent [5]
	 - [PATCH v1 14/29] xen/asm-generic: introduce stub header setup.h
	 - [PATCH v1 15/29] xen/asm-generic: introduce stub header xenoprof.h because of [3].
	 - [PATCH v1 16/29] xen/asm-generic: introduce stub header flushtlb.h
	 - [PATCH v1 22/29] xen/asm-generic: introduce stub header delay.h because of [3]
	 - [PATCH v1 23/29] xen/asm-generic: introduce stub header domain.h
	 - [PATCH v1 24/29] xen/asm-generic: introduce stub header guest_access.h
	 - [PATCH v1 25/29] xen/asm-generic: introduce stub header irq.h ( probably not so generic as I expected, I'll back to it if it will be necessary in the future )
	 - [PATCH v1 28/29] xen/asm-generic: introduce stub header p2m.h ( probably not so generic as I expected, I'll back to it if it will be necessary in the future )
 - For the rest of the patches please look at changes for each patch separately.
---

Oleksii Kurochko (7):
  automation: ensure values in EXTRA_FIXED_RANDCONFIG are separated by
    new line
  automation: introduce fixed randconfig for RISC-V
  xen/asm-generic: introduce stub header monitor.h
  xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
  xen/asm-generic: introduce generic device.h
  xen/arm: switch Arm to use asm-generic/device.h
  xen/ppc: switch PPC to use asm-generic/device.h

 automation/gitlab-ci/build.yaml               |   8 +-
 .../gitlab-ci/riscv-fixed-randconfig.yaml     |   7 ++
 automation/scripts/build                      |   6 +-
 xen/arch/arm/device.c                         |   5 +
 xen/arch/arm/domain_build.c                   |   2 +-
 xen/arch/arm/gic-v2.c                         |   4 +-
 xen/arch/arm/gic-v3.c                         |   6 +-
 xen/arch/arm/gic.c                            |   4 +-
 xen/arch/arm/include/asm/Makefile             |   1 +
 xen/arch/arm/include/asm/monitor.h            |  25 +---
 xen/arch/arm/p2m.c                            |   1 +
 xen/arch/arm/traps.c                          |   1 +
 xen/arch/ppc/configs/ppc64_defconfig          |   1 +
 xen/arch/ppc/include/asm/Makefile             |   1 +
 xen/arch/ppc/include/asm/device.h             |  53 --------
 xen/arch/ppc/include/asm/mem_access.h         |   5 -
 xen/arch/ppc/include/asm/monitor.h            |  28 +----
 xen/arch/ppc/stubs.c                          |   8 --
 xen/arch/riscv/configs/tiny64_defconfig       |   1 +
 .../asm => include/asm-generic}/device.h      | 117 +++++++++++-------
 xen/include/asm-generic/monitor.h             |  64 ++++++++++
 xen/include/xen/mem_access.h                  |   2 +
 22 files changed, 174 insertions(+), 176 deletions(-)
 create mode 100644 automation/gitlab-ci/riscv-fixed-randconfig.yaml
 delete mode 100644 xen/arch/ppc/include/asm/device.h
 delete mode 100644 xen/arch/ppc/include/asm/mem_access.h
 rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (66%)
 create mode 100644 xen/include/asm-generic/monitor.h

-- 
2.43.0

[PATCH v8 1/7] automation: ensure values in EXTRA_FIXED_RANDCONFIG are separated by new line

[Oleksii Kurochko]

Kconfig tool expects each configuration to be on a new line.

The current version of the build script puts all of ${EXTRA_FIXED_RANDCONFIG}
in a single line and configs are seperated by spaces.

As a result, only the first configuration in ${EXTRA_FIXED_RANDCONFIG} will
be used.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V8:
 - Nothing changed. Only rebase
---
Changes in V7:
 - Nothing changed. Only rebase
---
Changes in V6:
 - The patch was introduced in this version of patch series.
---
 automation/scripts/build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/automation/scripts/build b/automation/scripts/build
index b3c71fb6fb..13b043923d 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -14,7 +14,7 @@ if [[ "${RANDCONFIG}" == "y" ]]; then
 
     # Append job-specific fixed configuration
     if [[ -n "${EXTRA_FIXED_RANDCONFIG}" ]]; then
-        echo "${EXTRA_FIXED_RANDCONFIG}" >> xen/tools/kconfig/allrandom.config
+        sed "s/ /\n/g" <<< "${EXTRA_FIXED_RANDCONFIG}" > xen/tools/kconfig/allrandom.config
     fi
 
     make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
@@ -28,9 +28,11 @@ else
     echo "CONFIG_DEBUG=${debug}" >> xen/.config
 
     if [[ -n "${EXTRA_XEN_CONFIG}" ]]; then
-        echo "${EXTRA_XEN_CONFIG}" >> xen/.config
+        sed "s/ /\n/g" <<< "${EXTRA_XEN_CONFIG}" >> xen/.config
     fi
 
+    cat xen/.config
+
     make -j$(nproc) -C xen olddefconfig
 fi
 
-- 
2.43.0

[PATCH v8 2/7] automation: introduce fixed randconfig for RISC-V

[Oleksii Kurochko]

This patch introduces the file riscv-fixed-randconfig.yaml,
which includes all configurations that should be disabled for
randconfig builds.

Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
The patch were introduced after discussion in a topic:
 https://lore.kernel.org/xen-devel/cover.1701966261.git.oleksii.kurochko@gmail.com/
 ---
Changes in V8:
 - Nothing changed. Only rebase
---
Changes in V7:
 - Nothing changed. Only rebase
---
Changes in V6:
 - The patch was introduced in this version of patch series.
---
 automation/gitlab-ci/build.yaml                  | 8 ++++----
 automation/gitlab-ci/riscv-fixed-randconfig.yaml | 7 +++++++
 2 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 automation/gitlab-ci/riscv-fixed-randconfig.yaml

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 6d2cb18b88..376eb17f9c 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -512,6 +512,8 @@ alpine-3.18-gcc-debug-arm64-boot-cpupools:
       CONFIG_BOOT_TIME_CPUPOOLS=y
 
 # RISC-V 64 cross-build
+include: 'automation/gitlab-ci/riscv-fixed-randconfig.yaml'
+
 archlinux-current-gcc-riscv64:
   extends: .gcc-riscv64-cross-build
   variables:
@@ -532,8 +534,7 @@ archlinux-current-gcc-riscv64-randconfig:
     CONTAINER: archlinux:current-riscv64
     KBUILD_DEFCONFIG: tiny64_defconfig
     RANDCONFIG: y
-    EXTRA_FIXED_RANDCONFIG:
-      CONFIG_COVERAGE=n
+    EXTRA_FIXED_RANDCONFIG: !reference [.riscv-fixed-randconfig, variables, EXTRA_FIXED_RANDCONFIG]
 
 archlinux-current-gcc-riscv64-debug-randconfig:
   extends: .gcc-riscv64-cross-build-debug
@@ -541,8 +542,7 @@ archlinux-current-gcc-riscv64-debug-randconfig:
     CONTAINER: archlinux:current-riscv64
     KBUILD_DEFCONFIG: tiny64_defconfig
     RANDCONFIG: y
-    EXTRA_FIXED_RANDCONFIG:
-      CONFIG_COVERAGE=n
+    EXTRA_FIXED_RANDCONFIG: !reference [.riscv-fixed-randconfig, variables, EXTRA_FIXED_RANDCONFIG]
 
 # Power cross-build
 debian-bullseye-gcc-ppc64le:
diff --git a/automation/gitlab-ci/riscv-fixed-randconfig.yaml b/automation/gitlab-ci/riscv-fixed-randconfig.yaml
new file mode 100644
index 0000000000..f1282b40c9
--- /dev/null
+++ b/automation/gitlab-ci/riscv-fixed-randconfig.yaml
@@ -0,0 +1,7 @@
+.riscv-fixed-randconfig:
+  variables:
+    EXTRA_FIXED_RANDCONFIG:
+      CONFIG_COVERAGE=n
+      CONFIG_EXPERT=y
+      CONFIG_GRANT_TABLE=n
+      CONFIG_MEM_ACCESS=n
-- 
2.43.0

[PATCH v8 3/7] xen/asm-generic: introduce stub header monitor.h

[Oleksii Kurochko]

The header is shared between several archs so it is
moved to asm-generic.

Switch partly Arm and PPC to asm-generic/monitor.h and only
arch_monitor_get_capabilities() left in arch-specific/monitor.h.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Changes in V8:
 - Add Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Changes in V7:
 - Drop definition of arch_monitor_domctl_event for PPC.
 - define arch_monitor_domctl_event in asm-generic/monitor.h.
 - add "define HAS_ARCH_MONITOR_DOMCTL_EVENT" in arm/.../monitor.h as it has arch specific implementation.
---
Changes in V6:
 - Rebase only.
---
Changes in V5:
  - Switched partly Arm and PPC to asm-generic monitor.h only
    arch_monitor_get_capabilities() left in arch-specific/monitor.h.
  - Updated the commit message.
---
Changes in V4:
 - Removed the double blank line.
 - Added Acked-by: Jan Beulich <jbeulich@suse.com>.
 - Update the commit message
---
Changes in V3:
 - Use forward-declaration of struct domain instead of " #include <xen/sched.h> ".
 - Add ' include <xen/errno.h> '
 - Drop PPC's monitor.h.
---
Changes in V2:
	- remove inclusion of "+#include <public/domctl.h>"
	- add "struct xen_domctl_monitor_op;"
	- remove one of SPDX tags.
---
 xen/arch/arm/include/asm/monitor.h | 25 +-----------
 xen/arch/ppc/include/asm/monitor.h | 28 +------------
 xen/arch/ppc/stubs.c               |  8 ----
 xen/include/asm-generic/monitor.h  | 64 ++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 58 deletions(-)
 create mode 100644 xen/include/asm-generic/monitor.h

diff --git a/xen/arch/arm/include/asm/monitor.h b/xen/arch/arm/include/asm/monitor.h
index 7567be66bd..77a3c1a36c 100644
--- a/xen/arch/arm/include/asm/monitor.h
+++ b/xen/arch/arm/include/asm/monitor.h
@@ -25,34 +25,13 @@
 #include <xen/sched.h>
 #include <public/domctl.h>
 
-static inline
-void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
-{
-}
+#define HAS_ARCH_MONITOR_DOMCTL_EVENT
 
-static inline
-int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
-{
-    /* No arch-specific monitor ops on ARM. */
-    return -EOPNOTSUPP;
-}
+#include <asm-generic/monitor.h>
 
 int arch_monitor_domctl_event(struct domain *d,
                               struct xen_domctl_monitor_op *mop);
 
-static inline
-int arch_monitor_init_domain(struct domain *d)
-{
-    /* No arch-specific domain initialization on ARM. */
-    return 0;
-}
-
-static inline
-void arch_monitor_cleanup_domain(struct domain *d)
-{
-    /* No arch-specific domain cleanup on ARM. */
-}
-
 static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 {
     uint32_t capabilities = 0;
diff --git a/xen/arch/ppc/include/asm/monitor.h b/xen/arch/ppc/include/asm/monitor.h
index e5b0282bf1..89000dacc6 100644
--- a/xen/arch/ppc/include/asm/monitor.h
+++ b/xen/arch/ppc/include/asm/monitor.h
@@ -6,33 +6,7 @@
 #include <public/domctl.h>
 #include <xen/errno.h>
 
-static inline
-void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
-{
-}
-
-static inline
-int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
-{
-    /* No arch-specific monitor ops on PPC. */
-    return -EOPNOTSUPP;
-}
-
-int arch_monitor_domctl_event(struct domain *d,
-                              struct xen_domctl_monitor_op *mop);
-
-static inline
-int arch_monitor_init_domain(struct domain *d)
-{
-    /* No arch-specific domain initialization on PPC. */
-    return 0;
-}
-
-static inline
-void arch_monitor_cleanup_domain(struct domain *d)
-{
-    /* No arch-specific domain cleanup on PPC. */
-}
+#include <asm-generic/monitor.h>
 
 static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 {
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index a96e45626d..da193839bd 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -95,14 +95,6 @@ void arch_get_domain_info(const struct domain *d,
     BUG_ON("unimplemented");
 }
 
-/* monitor.c */
-
-int arch_monitor_domctl_event(struct domain *d,
-                              struct xen_domctl_monitor_op *mop)
-{
-    BUG_ON("unimplemented");
-}
-
 /* smp.c */
 
 void arch_flush_tlb_mask(const cpumask_t *mask)
diff --git a/xen/include/asm-generic/monitor.h b/xen/include/asm-generic/monitor.h
new file mode 100644
index 0000000000..1ade289099
--- /dev/null
+++ b/xen/include/asm-generic/monitor.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * include/asm-generic/monitor.h
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ */
+
+#ifndef __ASM_GENERIC_MONITOR_H__
+#define __ASM_GENERIC_MONITOR_H__
+
+#include <xen/errno.h>
+#include <xen/lib.h>
+
+struct domain;
+struct xen_domctl_monitor_op;
+
+static inline
+void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
+{
+}
+
+static inline
+int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
+{
+    /* No arch-specific monitor ops on GENERIC. */
+    return -EOPNOTSUPP;
+}
+
+#ifndef HAS_ARCH_MONITOR_DOMCTL_EVENT
+static inline
+int arch_monitor_domctl_event(struct domain *d,
+                              struct xen_domctl_monitor_op *mop)
+{
+    BUG_ON("unimplemented");
+}
+#endif
+
+static inline
+int arch_monitor_init_domain(struct domain *d)
+{
+    /* No arch-specific domain initialization on GENERIC. */
+    return 0;
+}
+
+static inline
+void arch_monitor_cleanup_domain(struct domain *d)
+{
+    /* No arch-specific domain cleanup on GENERIC. */
+}
+
+#endif /* __ASM_GENERIC_MONITOR_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

[PATCH v8 4/7] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>

[Oleksii Kurochko]

ifdefing inclusion of <asm/mem_access.h> in <xen/mem_access.h>
allows to avoid generation of empty <asm/mem_access.h> header
for the case when !CONFIG_MEM_ACCESS.

For Arm it was explicitly added inclusion of <asm/mem_access.h> for p2m.c
and traps.c because they require some functions from <asm/mem_access.h> which
aren't available in case of !CONFIG_MEM_ACCESS.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Changes in V8:
 - Add Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Changes in V7:
 - Nothing changed. Only rebase
---
Changes in V6:
 - Remove the way how CONFIG_MEM_ACCESS is disabled for PPC and RISC-V.
 - Disable the config in ppc64_defconfig and tiny64_defconfig (RISC-V).
---
Changes in V5:
 - Added dependencies for "Config MEM_ACCESS" to be sure that randconfig will not
   turn on the config.
---
Changes in V4:
 - Nothing changed. Only rebase.
---
Changes in V3:
 - Remove unnecessary comment.
---
 xen/arch/arm/p2m.c                      | 1 +
 xen/arch/arm/traps.c                    | 1 +
 xen/arch/ppc/configs/ppc64_defconfig    | 1 +
 xen/arch/ppc/include/asm/mem_access.h   | 5 -----
 xen/arch/riscv/configs/tiny64_defconfig | 1 +
 xen/include/xen/mem_access.h            | 2 ++
 6 files changed, 6 insertions(+), 5 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/mem_access.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b991b76ce4..2465c266e9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -7,6 +7,7 @@
 #include <asm/event.h>
 #include <asm/flushtlb.h>
 #include <asm/guest_walk.h>
+#include <asm/mem_access.h>
 #include <asm/page.h>
 #include <asm/traps.h>
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9c10e8f78c..8ddca643d4 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -35,6 +35,7 @@
 #include <asm/cpufeature.h>
 #include <asm/event.h>
 #include <asm/hsr.h>
+#include <asm/mem_access.h>
 #include <asm/mmio.h>
 #include <asm/regs.h>
 #include <asm/smccc.h>
diff --git a/xen/arch/ppc/configs/ppc64_defconfig b/xen/arch/ppc/configs/ppc64_defconfig
index f7cc075e45..48a053237a 100644
--- a/xen/arch/ppc/configs/ppc64_defconfig
+++ b/xen/arch/ppc/configs/ppc64_defconfig
@@ -6,6 +6,7 @@
 # CONFIG_HYPFS is not set
 # CONFIG_GRANT_TABLE is not set
 # CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
+# CONFIG_MEM_ACCESS is not set
 
 CONFIG_PPC64=y
 CONFIG_DEBUG=y
diff --git a/xen/arch/ppc/include/asm/mem_access.h b/xen/arch/ppc/include/asm/mem_access.h
deleted file mode 100644
index e7986dfdbd..0000000000
--- a/xen/arch/ppc/include/asm/mem_access.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_MEM_ACCESS_H__
-#define __ASM_PPC_MEM_ACCESS_H__
-
-#endif /* __ASM_PPC_MEM_ACCESS_H__ */
diff --git a/xen/arch/riscv/configs/tiny64_defconfig b/xen/arch/riscv/configs/tiny64_defconfig
index 3c9a2ff941..09defe236b 100644
--- a/xen/arch/riscv/configs/tiny64_defconfig
+++ b/xen/arch/riscv/configs/tiny64_defconfig
@@ -6,6 +6,7 @@
 # CONFIG_HYPFS is not set
 # CONFIG_GRANT_TABLE is not set
 # CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
+# CONFIG_MEM_ACCESS is not set
 
 CONFIG_RISCV_64=y
 CONFIG_DEBUG=y
diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
index 4e4811680d..87d93b31f6 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -33,7 +33,9 @@
  */
 struct vm_event_st;
 
+#ifdef CONFIG_MEM_ACCESS
 #include <asm/mem_access.h>
+#endif
 
 /*
  * Additional access types, which are used to further restrict
-- 
2.43.0

[PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Oleksii Kurochko]

Arm, PPC and RISC-V introduce the same things in asm/device.h, so
generic device.h was introduced.
Arm's device.h was taken as a base with the following changes:
 - #ifdef ACPI related things.
 - Rename #ifdef guards.
 - Add SPDX tag.
 - #ifdef CONFIG_HAS_DEVICE_TREE related things.
 - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V8:
 - drop #ifdef HAS_PCI around DEV_PCI in enum device_type to be consistent with DEVICE_PCI_HOSTBRIDGE
 - drop the comment above enum device_type, after DEV_PCI is alaways present in enum device_type, so
   there is no need anymore in comment and DEV_TYPE_MAX.
 - drop #ifdef HAS_PCI around dev_is_pci() as DEV_PCI is alwayds define now.
 - rename args _name, _namestr, _class to name, namestr, class.
 - drop unnessary backslash after .class = class in defines ACPI_DEVICE_START() and DT_DEVICE_START()
 - update the commit message 
---
Changes in V7:
 - keeping DEVICE_PCI_HOSTBRIDGE available for every build based on the reply:
    https://lore.kernel.org/xen-devel/926a5c12-7f02-42ec-92a8-1c82d060c710@xen.org/
 - add comment above enum device_type.h with explanation about DEV_TYPE_MAX.        
 - separate patch  "[PATCH v6 9/9] xen/asm-generic: introduce generic device.h" into 3 patches.
---
Changes in V6:
 - Rebase only.
---
Changes in V5:
  - Removed generated file: xen/include/headers++.chk.new
  - Removed pointless #ifdef CONFIG_HAS_DEVICE_TREE ... #endif for PPC as
    CONFIG_HAS_DEVICE_TREE will be always used for PPC.
---
Changes in V4:
 - Updated the commit message
 - Switched Arm and PPC to asm-generic version of device.h
 - Replaced HAS_PCI with CONFIG_HAS_PCI
 - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
 - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
 - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
 - Updated the commit message ( remove a note with question about
   if device.h should be in asm-generic or not )
 - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
 - Rationalized usage of CONFIG_HAS_* in device.h
 - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
---
Changes in V3:
 - ifdef device tree related things.
 - update the commit message
---
Changes in V2:
	- take ( as common ) device.h from Arm as PPC and RISC-V use it as a base.
	- #ifdef PCI related things.
	- #ifdef ACPI related things.
	- rename DEVICE_GIC to DEVIC_IC.
	- rename #ifdef guards.
	- switch Arm and PPC to generic device.h
	- add SPDX tag
	- update the commit message
---
 xen/include/asm-generic/device.h | 149 +++++++++++++++++++++++++++++++
 1 file changed, 149 insertions(+)
 create mode 100644 xen/include/asm-generic/device.h

diff --git a/xen/include/asm-generic/device.h b/xen/include/asm-generic/device.h
new file mode 100644
index 0000000000..6e56658271
--- /dev/null
+++ b/xen/include/asm-generic/device.h
@@ -0,0 +1,149 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_DEVICE_H__
+#define __ASM_GENERIC_DEVICE_H__
+
+#include <xen/stdbool.h>
+
+enum device_type
+{
+#ifdef CONFIG_HAS_DEVICE_TREE
+    DEV_DT,
+#endif
+    DEV_PCI
+};
+
+enum device_class
+{
+    DEVICE_SERIAL,
+    DEVICE_IOMMU,
+    DEVICE_INTERRUPT_CONTROLLER,
+    DEVICE_PCI_HOSTBRIDGE,
+    /* Use for error */
+    DEVICE_UNKNOWN,
+};
+
+struct dev_archdata {
+#ifdef CONFIG_HAS_PASSTHROUGH
+    void *iommu;    /* IOMMU private data */
+#endif
+};
+
+/* struct device - The basic device structure */
+struct device
+{
+    enum device_type type;
+#ifdef CONFIG_HAS_DEVICE_TREE
+    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
+#endif
+    struct dev_archdata archdata;
+#ifdef CONFIG_HAS_PASSTHROUGH
+    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
+#endif
+};
+
+typedef struct device device_t;
+
+#ifdef CONFIG_HAS_DEVICE_TREE
+
+#include <xen/device_tree.h>
+
+#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
+
+/**
+ *  device_init - Initialize a device
+ *  @dev: device to initialize
+ *  @class: class of the device (serial, network...)
+ *  @data: specific data for initializing the device
+ *
+ *  Return 0 on success.
+ */
+int device_init(struct dt_device_node *dev, enum device_class class,
+                const void *data);
+
+/**
+ * device_get_type - Get the type of the device
+ * @dev: device to match
+ *
+ * Return the device type on success or DEVICE_ANY on failure
+ */
+enum device_class device_get_class(const struct dt_device_node *dev);
+
+#define DT_DEVICE_START(name_, namestr_, class_)            \
+static const struct device_desc __dev_desc_##name_ __used   \
+__section(".dev.info") = {                                  \
+    .name = namestr_,                                       \
+    .class = class_,
+
+#define DT_DEVICE_END                                       \
+};
+
+#else /* !CONFIG_HAS_DEVICE_TREE */
+#define dev_is_dt(dev) ((void)(dev), false)
+#endif /* CONFIG_HAS_DEVICE_TREE */
+
+#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
+
+struct device_desc {
+    /* Device name */
+    const char *name;
+    /* Device class */
+    enum device_class class;
+
+#ifdef CONFIG_HAS_DEVICE_TREE
+
+    /* List of devices supported by this driver */
+    const struct dt_device_match *dt_match;
+    /*
+     * Device initialization.
+     *
+     * -EAGAIN is used to indicate that device probing is deferred.
+     */
+    int (*init)(struct dt_device_node *dev, const void *data);
+
+#endif
+};
+
+#ifdef CONFIG_ACPI
+
+struct acpi_device_desc {
+    /* Device name */
+    const char *name;
+    /* Device class */
+    enum device_class class;
+    /* type of device supported by the driver */
+    const int class_type;
+    /* Device initialization */
+    int (*init)(const void *data);
+};
+
+/**
+ *  acpi_device_init - Initialize a device
+ *  @class: class of the device (serial, network...)
+ *  @data: specific data for initializing the device
+ *
+ *  Return 0 on success.
+ */
+int acpi_device_init(enum device_class class,
+                     const void *data, int class_type);
+
+#define ACPI_DEVICE_START(name_, namestr_, class_)              \
+static const struct acpi_device_desc __dev_desc_##name_ __used  \
+__section(".adev.info") = {                                     \
+    .name = namestr_,                                           \
+    .class = class_,
+
+#define ACPI_DEVICE_END                                         \
+};
+
+#endif /* CONFIG_ACPI */
+
+#endif /* __ASM_GENERIC_DEVICE_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.43.0

[PATCH v8 6/7] xen/arm: switch Arm to use asm-generic/device.h

[Oleksii Kurochko]

The following changes were done as a result of switching to
asm-generic/device.h:
 * DEVICE_GIC was renamed to DEVICE_INTERRUPT_CONTROLLER according
   to definition of enum device_class in asm-generic/device.h.
 * acpi-related things in Arm code were guarded by #ifdef CONFIG_ACPI
   as struct acpi_device_desc was guarded in asm-generic, also functions
   acpi_device_init() was guarded too as they are using structure
   acpi_device_desc inside.
 * drop arm/include/asm/device.h and update arm/include/asm/Makefile
   to use asm-generic/device.h instead.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V8:
 - update the commit message
---
Changes in V7:
 - newly introduced patch which is based on the previous version of the patch:
     [PATCH v6 9/9] xen/asm-generic: introduce generic device.h
---
 xen/arch/arm/device.c             |   5 ++
 xen/arch/arm/domain_build.c       |   2 +-
 xen/arch/arm/gic-v2.c             |   4 +-
 xen/arch/arm/gic-v3.c             |   6 +-
 xen/arch/arm/gic.c                |   4 +-
 xen/arch/arm/include/asm/Makefile |   1 +
 xen/arch/arm/include/asm/device.h | 124 ------------------------------
 7 files changed, 14 insertions(+), 132 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/device.h

diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 1f631d3274..3e02cff008 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -16,7 +16,10 @@
 #include <xen/lib.h>
 
 extern const struct device_desc _sdevice[], _edevice[];
+
+#ifdef CONFIG_ACPI
 extern const struct acpi_device_desc _asdevice[], _aedevice[];
+#endif
 
 int __init device_init(struct dt_device_node *dev, enum device_class class,
                        const void *data)
@@ -45,6 +48,7 @@ int __init device_init(struct dt_device_node *dev, enum device_class class,
     return -EBADF;
 }
 
+#ifdef CONFIG_ACPI
 int __init acpi_device_init(enum device_class class, const void *data, int class_type)
 {
     const struct acpi_device_desc *desc;
@@ -61,6 +65,7 @@ int __init acpi_device_init(enum device_class class, const void *data, int class
 
     return -EBADF;
 }
+#endif
 
 enum device_class device_get_class(const struct dt_device_node *dev)
 {
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 46161848dc..085d88671e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1651,7 +1651,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
      * Replace these nodes with our own. Note that the original may be
      * used_by DOMID_XEN so this check comes first.
      */
-    if ( device_get_class(node) == DEVICE_GIC )
+    if ( device_get_class(node) == DEVICE_INTERRUPT_CONTROLLER )
         return make_gic_node(d, kinfo->fdt, node);
     if ( dt_match_node(timer_matches, node) )
         return make_timer_node(kinfo);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cf392bfd1c..5d6885e389 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1366,7 +1366,7 @@ static const struct dt_device_match gicv2_dt_match[] __initconst =
     { /* sentinel */ },
 };
 
-DT_DEVICE_START(gicv2, "GICv2", DEVICE_GIC)
+DT_DEVICE_START(gicv2, "GICv2", DEVICE_INTERRUPT_CONTROLLER)
         .dt_match = gicv2_dt_match,
         .init = gicv2_dt_preinit,
 DT_DEVICE_END
@@ -1381,7 +1381,7 @@ static int __init gicv2_acpi_preinit(const void *data)
     return 0;
 }
 
-ACPI_DEVICE_START(agicv2, "GICv2", DEVICE_GIC)
+ACPI_DEVICE_START(agicv2, "GICv2", DEVICE_INTERRUPT_CONTROLLER)
         .class_type = ACPI_MADT_GIC_VERSION_V2,
         .init = gicv2_acpi_preinit,
 ACPI_DEVICE_END
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index bf0e5c1b75..1cb1360606 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1909,7 +1909,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
     { /* sentinel */ },
 };
 
-DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
+DT_DEVICE_START(gicv3, "GICv3", DEVICE_INTERRUPT_CONTROLLER)
         .dt_match = gicv3_dt_match,
         .init = gicv3_dt_preinit,
 DT_DEVICE_END
@@ -1924,12 +1924,12 @@ static int __init gicv3_acpi_preinit(const void *data)
     return 0;
 }
 
-ACPI_DEVICE_START(agicv3, "GICv3", DEVICE_GIC)
+ACPI_DEVICE_START(agicv3, "GICv3", DEVICE_INTERRUPT_CONTROLLER)
         .class_type = ACPI_MADT_GIC_VERSION_V3,
         .init = gicv3_acpi_preinit,
 ACPI_DEVICE_END
 
-ACPI_DEVICE_START(agicv4, "GICv4", DEVICE_GIC)
+ACPI_DEVICE_START(agicv4, "GICv4", DEVICE_INTERRUPT_CONTROLLER)
         .class_type = ACPI_MADT_GIC_VERSION_V4,
         .init = gicv3_acpi_preinit,
 ACPI_DEVICE_END
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index d922ea67aa..b5a9c8266c 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -234,7 +234,7 @@ static void __init gic_dt_preinit(void)
         if ( !dt_get_parent(node) )
             continue;
 
-        rc = device_init(node, DEVICE_GIC, NULL);
+        rc = device_init(node, DEVICE_INTERRUPT_CONTROLLER, NULL);
         if ( !rc )
         {
             /* NOTE: Only one GIC is supported */
@@ -262,7 +262,7 @@ static void __init gic_acpi_preinit(void)
 
     dist = container_of(header, struct acpi_madt_generic_distributor, header);
 
-    if ( acpi_device_init(DEVICE_GIC, NULL, dist->version) )
+    if ( acpi_device_init(DEVICE_INTERRUPT_CONTROLLER, NULL, dist->version) )
         panic("Unable to find compatible GIC in the ACPI table\n");
 }
 #else
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 505cb49090..4a4036c951 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += altp2m.h
+generic-y += device.h
 generic-y += hardirq.h
 generic-y += iocap.h
 generic-y += paging.h
diff --git a/xen/arch/arm/include/asm/device.h b/xen/arch/arm/include/asm/device.h
deleted file mode 100644
index b5d451e087..0000000000
--- a/xen/arch/arm/include/asm/device.h
+++ /dev/null
@@ -1,124 +0,0 @@
-#ifndef __ASM_ARM_DEVICE_H
-#define __ASM_ARM_DEVICE_H
-
-enum device_type
-{
-    DEV_DT,
-    DEV_PCI,
-};
-
-struct dev_archdata {
-    void *iommu;    /* IOMMU private data */
-};
-
-/* struct device - The basic device structure */
-struct device
-{
-    enum device_type type;
-#ifdef CONFIG_HAS_DEVICE_TREE
-    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
-#endif
-    struct dev_archdata archdata;
-    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
-};
-
-typedef struct device device_t;
-
-#include <xen/device_tree.h>
-
-#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
-#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
-
-enum device_class
-{
-    DEVICE_SERIAL,
-    DEVICE_IOMMU,
-    DEVICE_GIC,
-    DEVICE_PCI_HOSTBRIDGE,
-    /* Use for error */
-    DEVICE_UNKNOWN,
-};
-
-struct device_desc {
-    /* Device name */
-    const char *name;
-    /* Device class */
-    enum device_class class;
-    /* List of devices supported by this driver */
-    const struct dt_device_match *dt_match;
-    /*
-     * Device initialization.
-     *
-     * -EAGAIN is used to indicate that device probing is deferred.
-     */
-    int (*init)(struct dt_device_node *dev, const void *data);
-};
-
-struct acpi_device_desc {
-    /* Device name */
-    const char *name;
-    /* Device class */
-    enum device_class class;
-    /* type of device supported by the driver */
-    const int class_type;
-    /* Device initialization */
-    int (*init)(const void *data);
-};
-
-/**
- *  acpi_device_init - Initialize a device
- *  @class: class of the device (serial, network...)
- *  @data: specific data for initializing the device
- *
- *  Return 0 on success.
- */
-int acpi_device_init(enum device_class class,
-                     const void *data, int class_type);
-
-/**
- *  device_init - Initialize a device
- *  @dev: device to initialize
- *  @class: class of the device (serial, network...)
- *  @data: specific data for initializing the device
- *
- *  Return 0 on success.
- */
-int device_init(struct dt_device_node *dev, enum device_class class,
-                const void *data);
-
-/**
- * device_get_type - Get the type of the device
- * @dev: device to match
- *
- * Return the device type on success or DEVICE_ANY on failure
- */
-enum device_class device_get_class(const struct dt_device_node *dev);
-
-#define DT_DEVICE_START(_name, _namestr, _class)                    \
-static const struct device_desc __dev_desc_##_name __used           \
-__section(".dev.info") = {                                          \
-    .name = _namestr,                                               \
-    .class = _class,                                                \
-
-#define DT_DEVICE_END                                               \
-};
-
-#define ACPI_DEVICE_START(_name, _namestr, _class)                    \
-static const struct acpi_device_desc __dev_desc_##_name __used           \
-__section(".adev.info") = {                       \
-    .name = _namestr,                                               \
-    .class = _class,                                                \
-
-#define ACPI_DEVICE_END                                               \
-};
-
-#endif /* __ASM_ARM_DEVICE_H */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
-- 
2.43.0

[PATCH v8 7/7] xen/ppc: switch PPC to use asm-generic/device.h

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V7:
 - newly introduced patch which is based on the previous version of the patch:
     [PATCH v6 9/9] xen/asm-generic: introduce generic device.h
---

 xen/arch/ppc/include/asm/Makefile |  1 +
 xen/arch/ppc/include/asm/device.h | 53 -------------------------------
 2 files changed, 1 insertion(+), 53 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/device.h

diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 3fd893f3e0..ced02e26ed 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += altp2m.h
+generic-y += device.h
 generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
diff --git a/xen/arch/ppc/include/asm/device.h b/xen/arch/ppc/include/asm/device.h
deleted file mode 100644
index 8253e61d51..0000000000
--- a/xen/arch/ppc/include/asm/device.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_DEVICE_H__
-#define __ASM_PPC_DEVICE_H__
-
-enum device_type
-{
-    DEV_DT,
-    DEV_PCI,
-};
-
-struct device {
-    enum device_type type;
-#ifdef CONFIG_HAS_DEVICE_TREE
-    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
-#endif
-};
-
-enum device_class
-{
-    DEVICE_SERIAL,
-    DEVICE_IOMMU,
-    DEVICE_PCI_HOSTBRIDGE,
-    /* Use for error */
-    DEVICE_UNKNOWN,
-};
-
-struct device_desc {
-    /* Device name */
-    const char *name;
-    /* Device class */
-    enum device_class class;
-    /* List of devices supported by this driver */
-    const struct dt_device_match *dt_match;
-    /*
-     * Device initialization.
-     *
-     * -EAGAIN is used to indicate that device probing is deferred.
-     */
-    int (*init)(struct dt_device_node *dev, const void *data);
-};
-
-typedef struct device device_t;
-
-#define DT_DEVICE_START(name_, namestr_, class_)                    \
-static const struct device_desc __dev_desc_##name_ __used           \
-__section(".dev.info") = {                                          \
-    .name = namestr_,                                               \
-    .class = class_,                                                \
-
-#define DT_DEVICE_END                                               \
-};
-
-#endif /* __ASM_PPC_DEVICE_H__ */
-- 
2.43.0

Re: [PATCH v8 1/7] automation: ensure values in EXTRA_FIXED_RANDCONFIG are separated by new line

[Michal Orzel]

Hi Oleksii,

On 09/02/2024 19:00, Oleksii Kurochko wrote:
> 
> 
> Kconfig tool expects each configuration to be on a new line.
> 
> The current version of the build script puts all of ${EXTRA_FIXED_RANDCONFIG}
> in a single line and configs are seperated by spaces.
> 
> As a result, only the first configuration in ${EXTRA_FIXED_RANDCONFIG} will
> be used.
There is no need for this patch. If you want the variables to be separated by new lines,
just use a '|' symbol after EXTRA_FIXED_RANDCONFIG (see all definitions of EXTRA_XEN_CONFIG).

~Michal

Re: [PATCH v8 2/7] automation: introduce fixed randconfig for RISC-V

[Michal Orzel]

Hi Oleksii,

On 09/02/2024 19:00, Oleksii Kurochko wrote:
> 
> 
> This patch introduces the file riscv-fixed-randconfig.yaml,
> which includes all configurations that should be disabled for
> randconfig builds.
> 
> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> The patch were introduced after discussion in a topic:
>  https://lore.kernel.org/xen-devel/cover.1701966261.git.oleksii.kurochko@gmail.com/
>  ---
> Changes in V8:
>  - Nothing changed. Only rebase
> ---
> Changes in V7:
>  - Nothing changed. Only rebase
> ---
> Changes in V6:
>  - The patch was introduced in this version of patch series.
> ---
>  automation/gitlab-ci/build.yaml                  | 8 ++++----
>  automation/gitlab-ci/riscv-fixed-randconfig.yaml | 7 +++++++
>  2 files changed, 11 insertions(+), 4 deletions(-)
>  create mode 100644 automation/gitlab-ci/riscv-fixed-randconfig.yaml
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 6d2cb18b88..376eb17f9c 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -512,6 +512,8 @@ alpine-3.18-gcc-debug-arm64-boot-cpupools:
>        CONFIG_BOOT_TIME_CPUPOOLS=y
> 
>  # RISC-V 64 cross-build
> +include: 'automation/gitlab-ci/riscv-fixed-randconfig.yaml'
I don't think there is a need to create a new file for that.
You could define an anchor in build.yaml:

# RISC-V 64 cross-build
.riscv-fixed-randconfig:
  variables: &riscv-fixed-randconfig
    EXTRA_FIXED_RANDCONFIG: |
      CONFIG_COVERAGE=n
      CONFIG_EXPERT=y
      CONFIG_GRANT_TABLE=n
      CONFIG_MEM_ACCESS=n

and reference it in the job:

archlinux-current-gcc-riscv64-randconfig:
  extends: .gcc-riscv64-cross-build
  variables:
    CONTAINER: archlinux:current-riscv64
    KBUILD_DEFCONFIG: tiny64_defconfig
    RANDCONFIG: y
    <<: *riscv-fixed-randconfig

~Michal

Re: [PATCH v8 1/7] automation: ensure values in EXTRA_FIXED_RANDCONFIG are separated by new line

[Oleksii]

On Mon, 2024-02-12 at 09:12 +0100, Michal Orzel wrote:
> Hi Oleksii,
Hi Michal,

> 
> On 09/02/2024 19:00, Oleksii Kurochko wrote:
> > 
> > 
> > Kconfig tool expects each configuration to be on a new line.
> > 
> > The current version of the build script puts all of
> > ${EXTRA_FIXED_RANDCONFIG}
> > in a single line and configs are seperated by spaces.
> > 
> > As a result, only the first configuration in
> > ${EXTRA_FIXED_RANDCONFIG} will
> > be used.
> There is no need for this patch. If you want the variables to be
> separated by new lines,
> just use a '|' symbol after EXTRA_FIXED_RANDCONFIG (see all
> definitions of EXTRA_XEN_CONFIG).

Thanks a lot. I'll drop this path then.

~ Oleksii

Re: [PATCH v8 2/7] automation: introduce fixed randconfig for RISC-V

[Oleksii]

On Mon, 2024-02-12 at 09:39 +0100, Michal Orzel wrote:
> Hi Oleksii,
Hi Michal,

> 
> On 09/02/2024 19:00, Oleksii Kurochko wrote:
> > 
> > 
> > This patch introduces the file riscv-fixed-randconfig.yaml,
> > which includes all configurations that should be disabled for
> > randconfig builds.
> > 
> > Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > The patch were introduced after discussion in a topic:
> >  
> > https://lore.kernel.org/xen-devel/cover.1701966261.git.oleksii.kurochko@gmail.com
> > /
> >  ---
> > Changes in V8:
> >  - Nothing changed. Only rebase
> > ---
> > Changes in V7:
> >  - Nothing changed. Only rebase
> > ---
> > Changes in V6:
> >  - The patch was introduced in this version of patch series.
> > ---
> >  automation/gitlab-ci/build.yaml                  | 8 ++++----
> >  automation/gitlab-ci/riscv-fixed-randconfig.yaml | 7 +++++++
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> >  create mode 100644 automation/gitlab-ci/riscv-fixed-
> > randconfig.yaml
> > 
> > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-
> > ci/build.yaml
> > index 6d2cb18b88..376eb17f9c 100644
> > --- a/automation/gitlab-ci/build.yaml
> > +++ b/automation/gitlab-ci/build.yaml
> > @@ -512,6 +512,8 @@ alpine-3.18-gcc-debug-arm64-boot-cpupools:
> >        CONFIG_BOOT_TIME_CPUPOOLS=y
> > 
> >  # RISC-V 64 cross-build
> > +include: 'automation/gitlab-ci/riscv-fixed-randconfig.yaml'
> I don't think there is a need to create a new file for that.
> You could define an anchor in build.yaml:
> 
> # RISC-V 64 cross-build
> .riscv-fixed-randconfig:
>   variables: &riscv-fixed-randconfig
>     EXTRA_FIXED_RANDCONFIG: |
>       CONFIG_COVERAGE=n
>       CONFIG_EXPERT=y
>       CONFIG_GRANT_TABLE=n
>       CONFIG_MEM_ACCESS=n
> 
> and reference it in the job:
> 
> archlinux-current-gcc-riscv64-randconfig:
>   extends: .gcc-riscv64-cross-build
>   variables:
>     CONTAINER: archlinux:current-riscv64
>     KBUILD_DEFCONFIG: tiny64_defconfig
>     RANDCONFIG: y
>     <<: *riscv-fixed-randconfig
I've created a new file just for convenience, build.yaml. It is pretty
large, and it's not always easy to navigate, especially when you don't
remember a specific name.
This is not directly related to this patch, but it seems to me that it
would be better to create arch-specific files and include them in
build.yaml.

If it would be better not to create a new file, I am okay to drop it
and add everything to build.yaml.

Thanks.

~ Oleksii

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Jan Beulich]

On 09.02.2024 19:00, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/include/asm-generic/device.h
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_DEVICE_H__
> +#define __ASM_GENERIC_DEVICE_H__
> +
> +#include <xen/stdbool.h>
> +
> +enum device_type
> +{
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +    DEV_DT,
> +#endif
> +    DEV_PCI
> +};
> +
> +enum device_class
> +{
> +    DEVICE_SERIAL,
> +    DEVICE_IOMMU,
> +    DEVICE_INTERRUPT_CONTROLLER,
> +    DEVICE_PCI_HOSTBRIDGE,
> +    /* Use for error */
> +    DEVICE_UNKNOWN,
> +};
> +
> +struct dev_archdata {
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +    void *iommu;    /* IOMMU private data */
> +#endif
> +};
> +
> +/* struct device - The basic device structure */
> +struct device
> +{
> +    enum device_type type;
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> +#endif
> +    struct dev_archdata archdata;
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
> +#endif
> +};
> +
> +typedef struct device device_t;
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +
> +#include <xen/device_tree.h>
> +
> +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
> +
> +/**
> + *  device_init - Initialize a device
> + *  @dev: device to initialize
> + *  @class: class of the device (serial, network...)
> + *  @data: specific data for initializing the device
> + *
> + *  Return 0 on success.
> + */
> +int device_init(struct dt_device_node *dev, enum device_class class,
> +                const void *data);
> +
> +/**
> + * device_get_type - Get the type of the device
> + * @dev: device to match
> + *
> + * Return the device type on success or DEVICE_ANY on failure
> + */
> +enum device_class device_get_class(const struct dt_device_node *dev);
> +
> +#define DT_DEVICE_START(name_, namestr_, class_)            \

I don't think the trailing underscores are needed or helpful here
(or in the ACPI counterpart), ...

> +static const struct device_desc __dev_desc_##name_ __used   \
> +__section(".dev.info") = {                                  \
> +    .name = namestr_,                                       \
> +    .class = class_,

... seeing this all it would have taken was to avoid the two words
"name" and "class" (by e.g. using "ident" and "cls").

Nevertheless:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH v8 3/7] xen/asm-generic: introduce stub header monitor.h

[Julien Grall]

Hi Oleksii,

On 09/02/2024 18:00, Oleksii Kurochko wrote:
> The header is shared between several archs so it is
> moved to asm-generic.
> 
> Switch partly Arm and PPC to asm-generic/monitor.h and only
> arch_monitor_get_capabilities() left in arch-specific/monitor.h.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Changes in V8:
>   - Add Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Changes in V7:
>   - Drop definition of arch_monitor_domctl_event for PPC.
>   - define arch_monitor_domctl_event in asm-generic/monitor.h.
>   - add "define HAS_ARCH_MONITOR_DOMCTL_EVENT" in arm/.../monitor.h as it has arch specific implementation.
> ---
> Changes in V6:
>   - Rebase only.
> ---
> Changes in V5:
>    - Switched partly Arm and PPC to asm-generic monitor.h only
>      arch_monitor_get_capabilities() left in arch-specific/monitor.h.
>    - Updated the commit message.
> ---
> Changes in V4:
>   - Removed the double blank line.
>   - Added Acked-by: Jan Beulich <jbeulich@suse.com>.
>   - Update the commit message
> ---
> Changes in V3:
>   - Use forward-declaration of struct domain instead of " #include <xen/sched.h> ".
>   - Add ' include <xen/errno.h> '
>   - Drop PPC's monitor.h.
> ---
> Changes in V2:
> 	- remove inclusion of "+#include <public/domctl.h>"
> 	- add "struct xen_domctl_monitor_op;"
> 	- remove one of SPDX tags.
> ---
>   xen/arch/arm/include/asm/monitor.h | 25 +-----------
>   xen/arch/ppc/include/asm/monitor.h | 28 +------------

Looking at MAINTAINERS, monitor.h was covered by "VM EVENT, MEM ACCESS 
and MONITOR". As you move to ...

>   xen/arch/ppc/stubs.c               |  8 ----
>   xen/include/asm-generic/monitor.h  | 64 ++++++++++++++++++++++++++++++

... asm-generic/, I believe it will now fall under "THE REST". So I 
think MAINTAINERS needs to be updated to also cover asm-generic/monitor.h.

Looking at what was already committed, I think asm-generic/vm_event.h 
should also be added in MAINTAINERS. Can you send a separate patch for that?

Cheers,

-- 
Julien Grall

Re: [PATCH v8 3/7] xen/asm-generic: introduce stub header monitor.h

[Julien Grall]

Hi,

On 13/02/2024 17:40, Julien Grall wrote:
> Hi Oleksii,
> 
> On 09/02/2024 18:00, Oleksii Kurochko wrote:
>> The header is shared between several archs so it is
>> moved to asm-generic.
>>
>> Switch partly Arm and PPC to asm-generic/monitor.h and only
>> arch_monitor_get_capabilities() left in arch-specific/monitor.h.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>> ---
>> Changes in V8:
>>   - Add Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>> ---
>> Changes in V7:
>>   - Drop definition of arch_monitor_domctl_event for PPC.
>>   - define arch_monitor_domctl_event in asm-generic/monitor.h.
>>   - add "define HAS_ARCH_MONITOR_DOMCTL_EVENT" in arm/.../monitor.h as 
>> it has arch specific implementation.
>> ---
>> Changes in V6:
>>   - Rebase only.
>> ---
>> Changes in V5:
>>    - Switched partly Arm and PPC to asm-generic monitor.h only
>>      arch_monitor_get_capabilities() left in arch-specific/monitor.h.
>>    - Updated the commit message.
>> ---
>> Changes in V4:
>>   - Removed the double blank line.
>>   - Added Acked-by: Jan Beulich <jbeulich@suse.com>.
>>   - Update the commit message
>> ---
>> Changes in V3:
>>   - Use forward-declaration of struct domain instead of " #include 
>> <xen/sched.h> ".
>>   - Add ' include <xen/errno.h> '
>>   - Drop PPC's monitor.h.
>> ---
>> Changes in V2:
>>     - remove inclusion of "+#include <public/domctl.h>"
>>     - add "struct xen_domctl_monitor_op;"
>>     - remove one of SPDX tags.
>> ---
>>   xen/arch/arm/include/asm/monitor.h | 25 +-----------
>>   xen/arch/ppc/include/asm/monitor.h | 28 +------------
> 
> Looking at MAINTAINERS, monitor.h was covered by "VM EVENT, MEM ACCESS 
> and MONITOR". As you move to ...
> 
>>   xen/arch/ppc/stubs.c               |  8 ----
>>   xen/include/asm-generic/monitor.h  | 64 ++++++++++++++++++++++++++++++
> 
> ... asm-generic/, I believe it will now fall under "THE REST". So I 
> think MAINTAINERS needs to be updated to also cover asm-generic/monitor.h.
> 
> Looking at what was already committed, I think asm-generic/vm_event.h 
> should also be added in MAINTAINERS. Can you send a separate patch for 
> that?

Oh, I just noticed that we have the following entry:

xen/include/*/monitor.h

So the header is already covered. Sorry for the noise.

As the code is falling under the "VM EVENT..." subsystem, then Tamas's 
acked is technically sufficient for this patch (even for the PPC change).

But just in case you need one for Arm:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH v8 4/7] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>

[Julien Grall]

Hi Oleksii,

On 09/02/2024 18:00, Oleksii Kurochko wrote:
> ifdefing inclusion of <asm/mem_access.h> in <xen/mem_access.h>
> allows to avoid generation of empty <asm/mem_access.h> header
> for the case when !CONFIG_MEM_ACCESS.
> 
> For Arm it was explicitly added inclusion of <asm/mem_access.h> for p2m.c
> and traps.c because they require some functions from <asm/mem_access.h> which
> aren't available in case of !CONFIG_MEM_ACCESS.

Ideally we should protect the code relying on <asm/mem_access.h>. But 
that's a separate improvement for the Arm folks to deal with. So:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Julien Grall]

Hi Oleksii,

On 09/02/2024 18:00, Oleksii Kurochko wrote:
> diff --git a/xen/include/asm-generic/device.h b/xen/include/asm-generic/device.h
> new file mode 100644
> index 0000000000..6e56658271
> --- /dev/null
> +++ b/xen/include/asm-generic/device.h
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_DEVICE_H__
> +#define __ASM_GENERIC_DEVICE_H__
> +
> +#include <xen/stdbool.h>
> +
> +enum device_type
> +{
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +    DEV_DT,
> +#endif
> +    DEV_PCI
> +};
> +
> +enum device_class
> +{
> +    DEVICE_SERIAL,
> +    DEVICE_IOMMU,
> +    DEVICE_INTERRUPT_CONTROLLER,
> +    DEVICE_PCI_HOSTBRIDGE,
> +    /* Use for error */
> +    DEVICE_UNKNOWN,
> +};
> +
> +struct dev_archdata {
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +    void *iommu;    /* IOMMU private data */
> +#endif > +};

It is a bit too late to change, but I thought I would point it if 
someone wants to send a follow-up. It is a bit odd to have a structure 
dev_archdata that, if I am not mistaken, is only used ...

> +
> +/* struct device - The basic device structure */
> +struct device
> +{
> +    enum device_type type;
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +    struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> +#endif
> +    struct dev_archdata archdata;

... in struct device. Looking at the use, I believe this was only 
introduced to try to keep that SMMU code close to Linux. I would 
consider to fold the other structure and update dev_archdata() in 
drivers/passthrough/arm/smmu.c.

> +#ifdef CONFIG_HAS_PASSTHROUGH
> +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
> +#endif
> +};
> +
> +typedef struct device device_t;
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +
> +#include <xen/device_tree.h>
> +
> +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
> +
> +/**
> + *  device_init - Initialize a device
> + *  @dev: device to initialize
> + *  @class: class of the device (serial, network...)
> + *  @data: specific data for initializing the device
> + *
> + *  Return 0 on success.
> + */
> +int device_init(struct dt_device_node *dev, enum device_class class,
> +                const void *data);
> +
> +/**
> + * device_get_type - Get the type of the device
> + * @dev: device to match
> + *
> + * Return the device type on success or DEVICE_ANY on failure
> + */
> +enum device_class device_get_class(const struct dt_device_node *dev);
> +
> +#define DT_DEVICE_START(name_, namestr_, class_)            \
> +static const struct device_desc __dev_desc_##name_ __used   \
> +__section(".dev.info") = {                                  \
> +    .name = namestr_,                                       \
> +    .class = class_,
> +
> +#define DT_DEVICE_END                                       \
> +};
> +
> +#else /* !CONFIG_HAS_DEVICE_TREE */
> +#define dev_is_dt(dev) ((void)(dev), false)
> +#endif /* CONFIG_HAS_DEVICE_TREE */
> +
> +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
> +
> +struct device_desc {
> +    /* Device name */
> +    const char *name;
> +    /* Device class */
> +    enum device_class class;
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +
> +    /* List of devices supported by this driver */
> +    const struct dt_device_match *dt_match;
> +    /*
> +     * Device initialization.
> +     *
> +     * -EAGAIN is used to indicate that device probing is deferred.
> +     */
> +    int (*init)(struct dt_device_node *dev, const void *data);
> +
> +#endif
> +};
I am not sure I fully understand why "device_desc" is not protected by 
CONFIG_HAS_DEVICE_TREE. The structure doesn't mean much when the config 
is disabled. Can you clarify?

Cheers,

-- 
Julien Grall

Re: [PATCH v8 6/7] xen/arm: switch Arm to use asm-generic/device.h

[Julien Grall]

Hi Oleksii,

On 09/02/2024 18:00, Oleksii Kurochko wrote:
> The following changes were done as a result of switching to
> asm-generic/device.h:
>   * DEVICE_GIC was renamed to DEVICE_INTERRUPT_CONTROLLER according
>     to definition of enum device_class in asm-generic/device.h.
>   * acpi-related things in Arm code were guarded by #ifdef CONFIG_ACPI
>     as struct acpi_device_desc was guarded in asm-generic, also functions
>     acpi_device_init() was guarded too as they are using structure
>     acpi_device_desc inside.
>   * drop arm/include/asm/device.h and update arm/include/asm/Makefile
>     to use asm-generic/device.h instead.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V8:
>   - update the commit message
> ---
> Changes in V7:
>   - newly introduced patch which is based on the previous version of the patch:
>       [PATCH v6 9/9] xen/asm-generic: introduce generic device.h
> ---
>   xen/arch/arm/device.c             |   5 ++
>   xen/arch/arm/domain_build.c       |   2 +-
>   xen/arch/arm/gic-v2.c             |   4 +-
>   xen/arch/arm/gic-v3.c             |   6 +-
>   xen/arch/arm/gic.c                |   4 +-
>   xen/arch/arm/include/asm/Makefile |   1 +
>   xen/arch/arm/include/asm/device.h | 124 ------------------------------
>   7 files changed, 14 insertions(+), 132 deletions(-)
>   delete mode 100644 xen/arch/arm/include/asm/device.h
> 
> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
> index 1f631d3274..3e02cff008 100644
> --- a/xen/arch/arm/device.c
> +++ b/xen/arch/arm/device.c
> @@ -16,7 +16,10 @@
>   #include <xen/lib.h>
>   
>   extern const struct device_desc _sdevice[], _edevice[];
> +
> +#ifdef CONFIG_ACPI
>   extern const struct acpi_device_desc _asdevice[], _aedevice[];
> +#endif

Can you also update the linker script to protect the following code? I.e

#ifdef CONFIG_ACPI
   . = ALIGN(8);
   .adev.info : {
       _asdevice = .;
       *(.adev.info)
       _aedevice = .;
   } :text
#endif

With this change:

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH v8 3/7] xen/asm-generic: introduce stub header monitor.h

[Oleksii]

Hi Julien,

> On 13/02/2024 17:40, Julien Grall wrote:
> > Hi Oleksii,
> > 
> > On 09/02/2024 18:00, Oleksii Kurochko wrote:
> > > The header is shared between several archs so it is
> > > moved to asm-generic.
> > > 
> > > Switch partly Arm and PPC to asm-generic/monitor.h and only
> > > arch_monitor_get_capabilities() left in arch-specific/monitor.h.
> > > 
> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > > Acked-by: Jan Beulich <jbeulich@suse.com>
> > > Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> > > ---
> > > Changes in V8:
> > >   - Add Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> > > ---
> > > Changes in V7:
> > >   - Drop definition of arch_monitor_domctl_event for PPC.
> > >   - define arch_monitor_domctl_event in asm-generic/monitor.h.
> > >   - add "define HAS_ARCH_MONITOR_DOMCTL_EVENT" in
> > > arm/.../monitor.h as 
> > > it has arch specific implementation.
> > > ---
> > > Changes in V6:
> > >   - Rebase only.
> > > ---
> > > Changes in V5:
> > >    - Switched partly Arm and PPC to asm-generic monitor.h only
> > >      arch_monitor_get_capabilities() left in arch-
> > > specific/monitor.h.
> > >    - Updated the commit message.
> > > ---
> > > Changes in V4:
> > >   - Removed the double blank line.
> > >   - Added Acked-by: Jan Beulich <jbeulich@suse.com>.
> > >   - Update the commit message
> > > ---
> > > Changes in V3:
> > >   - Use forward-declaration of struct domain instead of "
> > > #include 
> > > <xen/sched.h> ".
> > >   - Add ' include <xen/errno.h> '
> > >   - Drop PPC's monitor.h.
> > > ---
> > > Changes in V2:
> > >     - remove inclusion of "+#include <public/domctl.h>"
> > >     - add "struct xen_domctl_monitor_op;"
> > >     - remove one of SPDX tags.
> > > ---
> > >   xen/arch/arm/include/asm/monitor.h | 25 +-----------
> > >   xen/arch/ppc/include/asm/monitor.h | 28 +------------
> > 
> > Looking at MAINTAINERS, monitor.h was covered by "VM EVENT, MEM
> > ACCESS 
> > and MONITOR". As you move to ...
> > 
> > >   xen/arch/ppc/stubs.c               |  8 ----
> > >   xen/include/asm-generic/monitor.h  | 64
> > > ++++++++++++++++++++++++++++++
> > 
> > ... asm-generic/, I believe it will now fall under "THE REST". So I
> > think MAINTAINERS needs to be updated to also cover asm-
> > generic/monitor.h.
> > 
> > Looking at what was already committed, I think asm-
> > generic/vm_event.h 
> > should also be added in MAINTAINERS. Can you send a separate patch
> > for 
> > that?
> 
> Oh, I just noticed that we have the following entry:
> 
> xen/include/*/monitor.h
> 
> So the header is already covered. Sorry for the noise.
> 
> As the code is falling under the "VM EVENT..." subsystem, then
> Tamas's 
> acked is technically sufficient for this patch (even for the PPC
> change).
> 
> But just in case you need one for Arm:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>
Thanks.

~ Oleksii

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Oleksii]

On Mon, 2024-02-12 at 15:19 +0100, Jan Beulich wrote:
> On 09.02.2024 19:00, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/include/asm-generic/device.h
> > @@ -0,0 +1,149 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ASM_GENERIC_DEVICE_H__
> > +#define __ASM_GENERIC_DEVICE_H__
> > +
> > +#include <xen/stdbool.h>
> > +
> > +enum device_type
> > +{
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +    DEV_DT,
> > +#endif
> > +    DEV_PCI
> > +};
> > +
> > +enum device_class
> > +{
> > +    DEVICE_SERIAL,
> > +    DEVICE_IOMMU,
> > +    DEVICE_INTERRUPT_CONTROLLER,
> > +    DEVICE_PCI_HOSTBRIDGE,
> > +    /* Use for error */
> > +    DEVICE_UNKNOWN,
> > +};
> > +
> > +struct dev_archdata {
> > +#ifdef CONFIG_HAS_PASSTHROUGH
> > +    void *iommu;    /* IOMMU private data */
> > +#endif
> > +};
> > +
> > +/* struct device - The basic device structure */
> > +struct device
> > +{
> > +    enum device_type type;
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +    struct dt_device_node *of_node; /* Used by drivers imported
> > from Linux */
> > +#endif
> > +    struct dev_archdata archdata;
> > +#ifdef CONFIG_HAS_PASSTHROUGH
> > +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU
> > instance data */
> > +#endif
> > +};
> > +
> > +typedef struct device device_t;
> > +
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +
> > +#include <xen/device_tree.h>
> > +
> > +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
> > +
> > +/**
> > + *  device_init - Initialize a device
> > + *  @dev: device to initialize
> > + *  @class: class of the device (serial, network...)
> > + *  @data: specific data for initializing the device
> > + *
> > + *  Return 0 on success.
> > + */
> > +int device_init(struct dt_device_node *dev, enum device_class
> > class,
> > +                const void *data);
> > +
> > +/**
> > + * device_get_type - Get the type of the device
> > + * @dev: device to match
> > + *
> > + * Return the device type on success or DEVICE_ANY on failure
> > + */
> > +enum device_class device_get_class(const struct dt_device_node
> > *dev);
> > +
> > +#define DT_DEVICE_START(name_, namestr_, class_)            \
> 
> I don't think the trailing underscores are needed or helpful here
> (or in the ACPI counterpart), ...
> 
> > +static const struct device_desc __dev_desc_##name_ __used   \
> > +__section(".dev.info") = {                                  \
> > +    .name = namestr_,                                       \
> > +    .class = class_,
> 
> ... seeing this all it would have taken was to avoid the two words
> "name" and "class" (by e.g. using "ident" and "cls").
> 
> Nevertheless:
> Acked-by: Jan Beulich <jbeulich@suse.com>
Thanks.

I'll apply your comments in the next patch version.

~ Oleksii

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Oleksii]

On Tue, 2024-02-13 at 18:09 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 09/02/2024 18:00, Oleksii Kurochko wrote:
> > diff --git a/xen/include/asm-generic/device.h b/xen/include/asm-
> > generic/device.h
> > new file mode 100644
> > index 0000000000..6e56658271
> > --- /dev/null
> > +++ b/xen/include/asm-generic/device.h
> > @@ -0,0 +1,149 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ASM_GENERIC_DEVICE_H__
> > +#define __ASM_GENERIC_DEVICE_H__
> > +
> > +#include <xen/stdbool.h>
> > +
> > +enum device_type
> > +{
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +    DEV_DT,
> > +#endif
> > +    DEV_PCI
> > +};
> > +
> > +enum device_class
> > +{
> > +    DEVICE_SERIAL,
> > +    DEVICE_IOMMU,
> > +    DEVICE_INTERRUPT_CONTROLLER,
> > +    DEVICE_PCI_HOSTBRIDGE,
> > +    /* Use for error */
> > +    DEVICE_UNKNOWN,
> > +};
> > +
> > +struct dev_archdata {
> > +#ifdef CONFIG_HAS_PASSTHROUGH
> > +    void *iommu;    /* IOMMU private data */
> > +#endif > +};
> 
> It is a bit too late to change, but I thought I would point it if 
> someone wants to send a follow-up. It is a bit odd to have a
> structure 
> dev_archdata that, if I am not mistaken, is only used ...
> 
> > +
> > +/* struct device - The basic device structure */
> > +struct device
> > +{
> > +    enum device_type type;
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +    struct dt_device_node *of_node; /* Used by drivers imported
> > from Linux */
> > +#endif
> > +    struct dev_archdata archdata;
> 
> ... in struct device. Looking at the use, I believe this was only 
> introduced to try to keep that SMMU code close to Linux. I would 
> consider to fold the other structure and update dev_archdata() in 
> drivers/passthrough/arm/smmu.c.
I can do that in separate patch. struct dev_archdata was left because
of drivers/passthrough/arm/smmu.c.

> 
> > +#ifdef CONFIG_HAS_PASSTHROUGH
> > +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU
> > instance data */
> > +#endif
> > +};
> > +
> > +typedef struct device device_t;
> > +
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +
> > +#include <xen/device_tree.h>
> > +
> > +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
> > +
> > +/**
> > + *  device_init - Initialize a device
> > + *  @dev: device to initialize
> > + *  @class: class of the device (serial, network...)
> > + *  @data: specific data for initializing the device
> > + *
> > + *  Return 0 on success.
> > + */
> > +int device_init(struct dt_device_node *dev, enum device_class
> > class,
> > +                const void *data);
> > +
> > +/**
> > + * device_get_type - Get the type of the device
> > + * @dev: device to match
> > + *
> > + * Return the device type on success or DEVICE_ANY on failure
> > + */
> > +enum device_class device_get_class(const struct dt_device_node
> > *dev);
> > +
> > +#define DT_DEVICE_START(name_, namestr_, class_)            \
> > +static const struct device_desc __dev_desc_##name_ __used   \
> > +__section(".dev.info") = {                                  \
> > +    .name = namestr_,                                       \
> > +    .class = class_,
> > +
> > +#define DT_DEVICE_END                                       \
> > +};
> > +
> > +#else /* !CONFIG_HAS_DEVICE_TREE */
> > +#define dev_is_dt(dev) ((void)(dev), false)
> > +#endif /* CONFIG_HAS_DEVICE_TREE */
> > +
> > +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
> > +
> > +struct device_desc {
> > +    /* Device name */
> > +    const char *name;
> > +    /* Device class */
> > +    enum device_class class;
> > +
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > +
> > +    /* List of devices supported by this driver */
> > +    const struct dt_device_match *dt_match;
> > +    /*
> > +     * Device initialization.
> > +     *
> > +     * -EAGAIN is used to indicate that device probing is
> > deferred.
> > +     */
> > +    int (*init)(struct dt_device_node *dev, const void *data);
> > +
> > +#endif
> > +};
> I am not sure I fully understand why "device_desc" is not protected
> by 
> CONFIG_HAS_DEVICE_TREE. The structure doesn't mean much when the
> config 
> is disabled. Can you clarify?
I thought that one day struct device_desc and acpi_device_desc will be
"merged", and so decided just to #ifdef only DEVICE_TREE specific
fields.
Another one reason it is if to protect fully struct device_desc then it
would be needed more #ifdef in arm/device.c ( for example,
device_init() should be all protected then ) what will require to ifdef
all calls of device_init(). As an option device_init can can be defined
in case when !CONFIG_HAS_DEVICE_TREE as:
   int __init device_init(struct dt_device_node *dev, enum device_class
   class,
                          const void *data)
   {
    return -EBADF;
   }
   
The similar thing will be needed for device_get_class() in Arm's device.c.

Would it be better to ifdef full struct device_desc ?

~ Oleksii

Re: [PATCH v8 6/7] xen/arm: switch Arm to use asm-generic/device.h

[Oleksii]

Hi Julien,

On Tue, 2024-02-13 at 18:12 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 09/02/2024 18:00, Oleksii Kurochko wrote:
> > The following changes were done as a result of switching to
> > asm-generic/device.h:
> >   * DEVICE_GIC was renamed to DEVICE_INTERRUPT_CONTROLLER according
> >     to definition of enum device_class in asm-generic/device.h.
> >   * acpi-related things in Arm code were guarded by #ifdef
> > CONFIG_ACPI
> >     as struct acpi_device_desc was guarded in asm-generic, also
> > functions
> >     acpi_device_init() was guarded too as they are using structure
> >     acpi_device_desc inside.
> >   * drop arm/include/asm/device.h and update
> > arm/include/asm/Makefile
> >     to use asm-generic/device.h instead.
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V8:
> >   - update the commit message
> > ---
> > Changes in V7:
> >   - newly introduced patch which is based on the previous version
> > of the patch:
> >       [PATCH v6 9/9] xen/asm-generic: introduce generic device.h
> > ---
> >   xen/arch/arm/device.c             |   5 ++
> >   xen/arch/arm/domain_build.c       |   2 +-
> >   xen/arch/arm/gic-v2.c             |   4 +-
> >   xen/arch/arm/gic-v3.c             |   6 +-
> >   xen/arch/arm/gic.c                |   4 +-
> >   xen/arch/arm/include/asm/Makefile |   1 +
> >   xen/arch/arm/include/asm/device.h | 124 -------------------------
> > -----
> >   7 files changed, 14 insertions(+), 132 deletions(-)
> >   delete mode 100644 xen/arch/arm/include/asm/device.h
> > 
> > diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
> > index 1f631d3274..3e02cff008 100644
> > --- a/xen/arch/arm/device.c
> > +++ b/xen/arch/arm/device.c
> > @@ -16,7 +16,10 @@
> >   #include <xen/lib.h>
> >   
> >   extern const struct device_desc _sdevice[], _edevice[];
> > +
> > +#ifdef CONFIG_ACPI
> >   extern const struct acpi_device_desc _asdevice[], _aedevice[];
> > +#endif
> 
> Can you also update the linker script to protect the following code?
Sure, I'll update that. Also please look at my comment to the previous
patch. Perhaps, it will be needed to update this patch to ifdef
device_init() and device_get_class(), and provide stubs for them in
case of !CONFIG_HAS_DEVICE_TREE.


> I.e
> 
> #ifdef CONFIG_ACPI
>    . = ALIGN(8);
>    .adev.info : {
>        _asdevice = .;
>        *(.adev.info)
>        _aedevice = .;
>    } :text
> #endif
> 
> With this change:
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>
Thanks.

~ Oleksii

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Julien Grall]

Hi Oleksii,

On 14/02/2024 09:32, Oleksii wrote:
> On Tue, 2024-02-13 at 18:09 +0000, Julien Grall wrote:
>>> +#ifdef CONFIG_HAS_PASSTHROUGH
>>> +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU
>>> instance data */
>>> +#endif
>>> +};
>>> +
>>> +typedef struct device device_t;
>>> +
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>> +
>>> +#include <xen/device_tree.h>
>>> +
>>> +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
>>> +
>>> +/**
>>> + *  device_init - Initialize a device
>>> + *  @dev: device to initialize
>>> + *  @class: class of the device (serial, network...)
>>> + *  @data: specific data for initializing the device
>>> + *
>>> + *  Return 0 on success.
>>> + */
>>> +int device_init(struct dt_device_node *dev, enum device_class
>>> class,
>>> +                const void *data);
>>> +
>>> +/**
>>> + * device_get_type - Get the type of the device
>>> + * @dev: device to match
>>> + *
>>> + * Return the device type on success or DEVICE_ANY on failure
>>> + */
>>> +enum device_class device_get_class(const struct dt_device_node
>>> *dev);
>>> +
>>> +#define DT_DEVICE_START(name_, namestr_, class_)            \
>>> +static const struct device_desc __dev_desc_##name_ __used   \
>>> +__section(".dev.info") = {                                  \
>>> +    .name = namestr_,                                       \
>>> +    .class = class_,
>>> +
>>> +#define DT_DEVICE_END                                       \
>>> +};
>>> +
>>> +#else /* !CONFIG_HAS_DEVICE_TREE */
>>> +#define dev_is_dt(dev) ((void)(dev), false)
>>> +#endif /* CONFIG_HAS_DEVICE_TREE */
>>> +
>>> +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
>>> +
>>> +struct device_desc {
>>> +    /* Device name */
>>> +    const char *name;
>>> +    /* Device class */
>>> +    enum device_class class;
>>> +
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>> +
>>> +    /* List of devices supported by this driver */
>>> +    const struct dt_device_match *dt_match;
>>> +    /*
>>> +     * Device initialization.
>>> +     *
>>> +     * -EAGAIN is used to indicate that device probing is
>>> deferred.
>>> +     */
>>> +    int (*init)(struct dt_device_node *dev, const void *data);
>>> +
>>> +#endif
>>> +};
>> I am not sure I fully understand why "device_desc" is not protected
>> by
>> CONFIG_HAS_DEVICE_TREE. The structure doesn't mean much when the
>> config
>> is disabled. Can you clarify?
> I thought that one day struct device_desc and acpi_device_desc will be
> "merged", and so decided just to #ifdef only DEVICE_TREE specific
> fields.

It might be possible to merge the two if we were using an union for the 
ACPI/DT specific part. However the majority of the parsing code needs to 
differ. So I am not convinced there would be any value to merge the two 
structures.

> Another one reason it is if to protect fully struct device_desc then it
> would be needed more #ifdef in arm/device.c ( for example,
> device_init() should be all protected then ) what will require to ifdef
> all calls of device_init(). As an option device_init can can be defined
> in case when !CONFIG_HAS_DEVICE_TREE as:
>     int __init device_init(struct dt_device_node *dev, enum device_class
>     class,
>                            const void *data)
>     {
>      return -EBADF;
>     }
>     
> The similar thing will be needed for device_get_class() in Arm's device.c.

I agree that in theory device_init() & co should be protected with 
CONFIG_HAS_DEVICE_TREE. However, it is not possible to compile Xen on 
Arm without the Device-Tree part today. So I don't view adding the 
#ifdef or any extra stub as necessary today.

This may be useful in the future though. Note this is not a request to 
modify the patch more than...

> 
> Would it be better to ifdef full struct device_desc ?
.. moving structure within the #ifdef.

Cheers,

-- 
Julien Grall

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Oleksii]

> 
> On 14/02/2024 09:32, Oleksii wrote:
> > On Tue, 2024-02-13 at 18:09 +0000, Julien Grall wrote:
> > > > +#ifdef CONFIG_HAS_PASSTHROUGH
> > > > +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU
> > > > instance data */
> > > > +#endif
> > > > +};
> > > > +
> > > > +typedef struct device device_t;
> > > > +
> > > > +#ifdef CONFIG_HAS_DEVICE_TREE
> > > > +
> > > > +#include <xen/device_tree.h>
> > > > +
> > > > +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
> > > > +
> > > > +/**
> > > > + *  device_init - Initialize a device
> > > > + *  @dev: device to initialize
> > > > + *  @class: class of the device (serial, network...)
> > > > + *  @data: specific data for initializing the device
> > > > + *
> > > > + *  Return 0 on success.
> > > > + */
> > > > +int device_init(struct dt_device_node *dev, enum device_class
> > > > class,
> > > > +                const void *data);
> > > > +
> > > > +/**
> > > > + * device_get_type - Get the type of the device
> > > > + * @dev: device to match
> > > > + *
> > > > + * Return the device type on success or DEVICE_ANY on failure
> > > > + */
> > > > +enum device_class device_get_class(const struct dt_device_node
> > > > *dev);
> > > > +
> > > > +#define DT_DEVICE_START(name_, namestr_, class_)            \
> > > > +static const struct device_desc __dev_desc_##name_ __used   \
> > > > +__section(".dev.info") = {                                  \
> > > > +    .name = namestr_,                                       \
> > > > +    .class = class_,
> > > > +
> > > > +#define DT_DEVICE_END                                       \
> > > > +};
> > > > +
> > > > +#else /* !CONFIG_HAS_DEVICE_TREE */
> > > > +#define dev_is_dt(dev) ((void)(dev), false)
> > > > +#endif /* CONFIG_HAS_DEVICE_TREE */
> > > > +
> > > > +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
> > > > +
> > > > +struct device_desc {
> > > > +    /* Device name */
> > > > +    const char *name;
> > > > +    /* Device class */
> > > > +    enum device_class class;
> > > > +
> > > > +#ifdef CONFIG_HAS_DEVICE_TREE
> > > > +
> > > > +    /* List of devices supported by this driver */
> > > > +    const struct dt_device_match *dt_match;
> > > > +    /*
> > > > +     * Device initialization.
> > > > +     *
> > > > +     * -EAGAIN is used to indicate that device probing is
> > > > deferred.
> > > > +     */
> > > > +    int (*init)(struct dt_device_node *dev, const void *data);
> > > > +
> > > > +#endif
> > > > +};
> > > I am not sure I fully understand why "device_desc" is not
> > > protected
> > > by
> > > CONFIG_HAS_DEVICE_TREE. The structure doesn't mean much when the
> > > config
> > > is disabled. Can you clarify?
> > I thought that one day struct device_desc and acpi_device_desc will
> > be
> > "merged", and so decided just to #ifdef only DEVICE_TREE specific
> > fields.
> 
> It might be possible to merge the two if we were using an union for
> the 
> ACPI/DT specific part. However the majority of the parsing code needs
> to 
> differ. So I am not convinced there would be any value to merge the
> two 
> structures.
In this case, let's have two separate structures.

This is not the current situation, and I don't have a specific example.
It appears that all architectures will use Device Tree or ACPI.
However, does it make sense to keep 'struct device_desc' more generic
to accommodate non-DT or non-ACPI cases?

I am okay with making the following change, but I am just curious if
what I mentioned above makes sense at all:

#ifdef CONFIG_HAS_DEVICE_TREE
struct device_desc {
    /* Device name */
    const char *name;
    /* Device class */
    enum device_class class;

    /* List of devices supported by this driver */
    const struct dt_device_match *dt_match;
    /*
     * Device initialization.
     *
     * -EAGAIN is used to indicate that device probing is deferred.
     */
    int (*init)(struct dt_device_node *dev, const void *data);
};
#endif /* CONFIG_HAS_DEVICE_TREE */ 

> 
> > Another one reason it is if to protect fully struct device_desc
> > then it
> > would be needed more #ifdef in arm/device.c ( for example,
> > device_init() should be all protected then ) what will require to
> > ifdef
> > all calls of device_init(). As an option device_init can can be
> > defined
> > in case when !CONFIG_HAS_DEVICE_TREE as:
> >     int __init device_init(struct dt_device_node *dev, enum
> > device_class
> >     class,
> >                            const void *data)
> >     {
> >      return -EBADF;
> >     }
> >     
> > The similar thing will be needed for device_get_class() in Arm's
> > device.c.
> 
> I agree that in theory device_init() & co should be protected with 
> CONFIG_HAS_DEVICE_TREE. However, it is not possible to compile Xen on
> Arm without the Device-Tree part today. So I don't view adding the 
> #ifdef or any extra stub as necessary today.
> 
> This may be useful in the future though. Note this is not a request
> to 
> modify the patch more than...
> 
> > 
> > Would it be better to ifdef full struct device_desc ?
> .. moving structure within the #ifdef.
Well, I'll update the commit message of the next patch that it is not
possible to compile Xen without CONFIG_HAS_DEVICE_TREE, so
device_init() and Co won't be protected by CONFIG_HAS_DEVICE_TREE.

~ Oleksii

Re: [PATCH v8 5/7] xen/asm-generic: introduce generic device.h

[Julien Grall]

Hi Oleksii,

On 15/02/2024 16:54, Oleksii wrote:
>>
>> On 14/02/2024 09:32, Oleksii wrote:
>>> On Tue, 2024-02-13 at 18:09 +0000, Julien Grall wrote:
>>>>> +#ifdef CONFIG_HAS_PASSTHROUGH
>>>>> +    struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU
>>>>> instance data */
>>>>> +#endif
>>>>> +};
>>>>> +
>>>>> +typedef struct device device_t;
>>>>> +
>>>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>>>> +
>>>>> +#include <xen/device_tree.h>
>>>>> +
>>>>> +#define dev_is_dt(dev)  ((dev)->type == DEV_DT)
>>>>> +
>>>>> +/**
>>>>> + *  device_init - Initialize a device
>>>>> + *  @dev: device to initialize
>>>>> + *  @class: class of the device (serial, network...)
>>>>> + *  @data: specific data for initializing the device
>>>>> + *
>>>>> + *  Return 0 on success.
>>>>> + */
>>>>> +int device_init(struct dt_device_node *dev, enum device_class
>>>>> class,
>>>>> +                const void *data);
>>>>> +
>>>>> +/**
>>>>> + * device_get_type - Get the type of the device
>>>>> + * @dev: device to match
>>>>> + *
>>>>> + * Return the device type on success or DEVICE_ANY on failure
>>>>> + */
>>>>> +enum device_class device_get_class(const struct dt_device_node
>>>>> *dev);
>>>>> +
>>>>> +#define DT_DEVICE_START(name_, namestr_, class_)            \
>>>>> +static const struct device_desc __dev_desc_##name_ __used   \
>>>>> +__section(".dev.info") = {                                  \
>>>>> +    .name = namestr_,                                       \
>>>>> +    .class = class_,
>>>>> +
>>>>> +#define DT_DEVICE_END                                       \
>>>>> +};
>>>>> +
>>>>> +#else /* !CONFIG_HAS_DEVICE_TREE */
>>>>> +#define dev_is_dt(dev) ((void)(dev), false)
>>>>> +#endif /* CONFIG_HAS_DEVICE_TREE */
>>>>> +
>>>>> +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
>>>>> +
>>>>> +struct device_desc {
>>>>> +    /* Device name */
>>>>> +    const char *name;
>>>>> +    /* Device class */
>>>>> +    enum device_class class;
>>>>> +
>>>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>>>> +
>>>>> +    /* List of devices supported by this driver */
>>>>> +    const struct dt_device_match *dt_match;
>>>>> +    /*
>>>>> +     * Device initialization.
>>>>> +     *
>>>>> +     * -EAGAIN is used to indicate that device probing is
>>>>> deferred.
>>>>> +     */
>>>>> +    int (*init)(struct dt_device_node *dev, const void *data);
>>>>> +
>>>>> +#endif
>>>>> +};
>>>> I am not sure I fully understand why "device_desc" is not
>>>> protected
>>>> by
>>>> CONFIG_HAS_DEVICE_TREE. The structure doesn't mean much when the
>>>> config
>>>> is disabled. Can you clarify?
>>> I thought that one day struct device_desc and acpi_device_desc will
>>> be
>>> "merged", and so decided just to #ifdef only DEVICE_TREE specific
>>> fields.
>>
>> It might be possible to merge the two if we were using an union for
>> the
>> ACPI/DT specific part. However the majority of the parsing code needs
>> to
>> differ. So I am not convinced there would be any value to merge the
>> two
>> structures.
> In this case, let's have two separate structures.
> 
> This is not the current situation, and I don't have a specific example.
> It appears that all architectures will use Device Tree or ACPI.
> However, does it make sense to keep 'struct device_desc' more generic
> to accommodate non-DT or non-ACPI cases?

I am not entirely sure what else to say. As I wrote before yes it could 
be made generic. But right now I don't see any values.

If you have any idea how to share the structure. Then feel free to make 
a proposal and I will review it.

Cheers,

-- 
Julien Grall