[PATCH v3 0/3] make build of vm_event/mem_access/monitor optional

[PATCH v3 0/3] make build of vm_event/mem_access/monitor optional

[Sergiy Kibrik]

This series aims to provide a possibility to build Xen without mem_access and
related features. It is already largely covered by corresponding
CONFIG_MEM_ACCESS option, yet there're still some parts remaining.
Hopefully this would help to reduce dead code a bit.

As coverage of MEM_ACCESS config option begins to extend beyond actual
mem_access code it has been suggested to rename it into VM_EVENT, as a more
general option controlling mem_access, vm_event and monitor code.

One patch for automation from v2 series had been squashed into #1 patch in
v3 series, v2 reference below:
https://lore.kernel.org/xen-devel/cover.1737452864.git.Sergiy_Kibrik@epam.com/

  -Sergiy

Sergiy Kibrik (2):
  xen: kconfig: rename MEM_ACCESS -> VM_EVENT
  x86:monitor: control monitor.c build with CONFIG_VM_EVENT option

Stefano Stabellini (1):
  xen: mem_access: conditionally compile vm_event.c & monitor.c

 automation/gitlab-ci/build.yaml         |  2 +-
 xen/arch/arm/Makefile                   |  6 +++---
 xen/arch/arm/configs/tiny64_defconfig   |  2 +-
 xen/arch/arm/include/asm/mem_access.h   |  4 ++--
 xen/arch/arm/vsmc.c                     |  3 ++-
 xen/arch/ppc/configs/ppc64_defconfig    |  2 +-
 xen/arch/riscv/configs/tiny64_defconfig |  2 +-
 xen/arch/x86/Kconfig                    |  2 +-
 xen/arch/x86/Makefile                   |  2 +-
 xen/arch/x86/mm/Makefile                |  2 +-
 xen/common/Kconfig                      |  2 +-
 xen/common/Makefile                     |  6 +++---
 xen/common/domctl.c                     |  2 +-
 xen/include/xen/mem_access.h            |  6 +++---
 xen/include/xen/monitor.h               |  9 +++++++++
 xen/include/xen/vm_event.h              | 14 +++++++++++---
 xen/include/xsm/dummy.h                 |  2 +-
 xen/include/xsm/xsm.h                   |  4 ++--
 xen/xsm/dummy.c                         |  2 +-
 xen/xsm/flask/hooks.c                   |  4 ++--
 20 files changed, 48 insertions(+), 30 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] xen: kconfig: rename MEM_ACCESS -> VM_EVENT

[Sergiy Kibrik]

Use more generic CONFIG_VM_EVENT name throughout Xen code instead of
CONFIG_MEM_ACCESS. This reflects the fact that vm_event is a higher level
feature, with mem_access & monitor depending on it.

CC: Jan Beulich <jbeulich@suse.com>
Suggested-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
changes in v3:
 - squash with automation patch
 - tags & minor addition of blanks
---
 automation/gitlab-ci/build.yaml         | 2 +-
 xen/arch/arm/Makefile                   | 2 +-
 xen/arch/arm/configs/tiny64_defconfig   | 2 +-
 xen/arch/arm/include/asm/mem_access.h   | 4 ++--
 xen/arch/ppc/configs/ppc64_defconfig    | 2 +-
 xen/arch/riscv/configs/tiny64_defconfig | 2 +-
 xen/arch/x86/mm/Makefile                | 2 +-
 xen/common/Kconfig                      | 2 +-
 xen/common/Makefile                     | 2 +-
 xen/common/domctl.c                     | 2 +-
 xen/include/xen/mem_access.h            | 6 +++---
 xen/include/xsm/dummy.h                 | 2 +-
 xen/include/xsm/xsm.h                   | 4 ++--
 xen/xsm/dummy.c                         | 2 +-
 xen/xsm/flask/hooks.c                   | 4 ++--
 15 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 034d6d9c3a..b6383c4fc8 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -744,7 +744,7 @@ debian-12-riscv64-gcc:
       CONFIG_EXPERT=y
       CONFIG_GRANT_TABLE=n
       CONFIG_LIVEPATCH=n
-      CONFIG_MEM_ACCESS=n
+      CONFIG_VM_EVENT=n
       CONFIG_QEMU_PLATFORM=y
       CONFIG_XSM=n
 
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index fb0948f067..3bd5125e64 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -38,7 +38,7 @@ obj-y += irq.o
 obj-y += kernel.init.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-$(CONFIG_LLC_COLORING) += llc-coloring.o
-obj-$(CONFIG_MEM_ACCESS) += mem_access.o
+obj-$(CONFIG_VM_EVENT) += mem_access.o
 obj-y += mm.o
 obj-y += monitor.o
 obj-y += p2m.o
diff --git a/xen/arch/arm/configs/tiny64_defconfig b/xen/arch/arm/configs/tiny64_defconfig
index cc6d93f2f8..469a1eb9f9 100644
--- a/xen/arch/arm/configs/tiny64_defconfig
+++ b/xen/arch/arm/configs/tiny64_defconfig
@@ -5,7 +5,7 @@ CONFIG_ARM=y
 # Architecture Features
 #
 # CONFIG_GICV3 is not set
-# CONFIG_MEM_ACCESS is not set
+# CONFIG_VM_EVENT is not set
 # CONFIG_SBSA_VUART_CONSOLE is not set
 
 #
diff --git a/xen/arch/arm/include/asm/mem_access.h b/xen/arch/arm/include/asm/mem_access.h
index abac8032fc..d42f28e8b7 100644
--- a/xen/arch/arm/include/asm/mem_access.h
+++ b/xen/arch/arm/include/asm/mem_access.h
@@ -37,7 +37,7 @@ static inline bool p2m_mem_access_sanity_check(struct domain *d)
  * Send mem event based on the access. Boolean return value indicates if trap
  * needs to be injected into guest.
  */
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
 bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec);
 
 struct page_info*
@@ -58,7 +58,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
     return NULL;
 }
 
-#endif /*CONFIG_MEM_ACCESS*/
+#endif /* CONFIG_VM_EVENT */
 #endif /* _ASM_ARM_MEM_ACCESS_H */
 
 /*
diff --git a/xen/arch/ppc/configs/ppc64_defconfig b/xen/arch/ppc/configs/ppc64_defconfig
index 4924d881a2..d6aaf772e7 100644
--- a/xen/arch/ppc/configs/ppc64_defconfig
+++ b/xen/arch/ppc/configs/ppc64_defconfig
@@ -1,6 +1,6 @@
 # CONFIG_GRANT_TABLE is not set
 # CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
-# CONFIG_MEM_ACCESS is not set
+# CONFIG_VM_EVENT is not set
 
 CONFIG_PPC64=y
 CONFIG_DEBUG=y
diff --git a/xen/arch/riscv/configs/tiny64_defconfig b/xen/arch/riscv/configs/tiny64_defconfig
index bb3ae26a44..2399f7b918 100644
--- a/xen/arch/riscv/configs/tiny64_defconfig
+++ b/xen/arch/riscv/configs/tiny64_defconfig
@@ -1,6 +1,6 @@
 # CONFIG_BOOT_TIME_CPUPOOLS is not set
 # CONFIG_GRANT_TABLE is not set
-# CONFIG_MEM_ACCESS is not set
+# CONFIG_VM_EVENT is not set
 # CONFIG_COVERAGE is not set
 # CONFIG_LIVEPATCH is not set
 # CONFIG_XSM is not set
diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index 0345388359..960f6e8409 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_HVM) += hap/
 obj-$(CONFIG_ALTP2M) += altp2m.o
 obj-$(CONFIG_HVM) += guest_walk_2.o guest_walk_3.o guest_walk_4.o
 obj-$(CONFIG_SHADOW_PAGING) += guest_walk_4.o
-obj-$(CONFIG_MEM_ACCESS) += mem_access.o
+obj-$(CONFIG_VM_EVENT) += mem_access.o
 obj-$(CONFIG_MEM_PAGING) += mem_paging.o
 obj-$(CONFIG_MEM_SHARING) += mem_sharing.o
 obj-$(CONFIG_HVM) += nested.o
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 6166327f4d..a6aa2c5c14 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -92,7 +92,7 @@ config HAS_VMAP
 config MEM_ACCESS_ALWAYS_ON
 	bool
 
-config MEM_ACCESS
+config VM_EVENT
 	def_bool MEM_ACCESS_ALWAYS_ON
 	prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
 	depends on HVM
diff --git a/xen/common/Makefile b/xen/common/Makefile
index cba3b32733..b71d4b3efa 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -26,7 +26,7 @@ obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC) += kimage.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
 obj-$(CONFIG_LLC_COLORING) += llc-coloring.o
-obj-$(CONFIG_MEM_ACCESS) += mem_access.o
+obj-$(CONFIG_VM_EVENT) += mem_access.o
 obj-y += memory.o
 obj-y += multicall.o
 obj-y += notifier.o
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index b897ca8723..bfe2e1f9f0 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -809,7 +809,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             copyback = true;
         break;
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
     case XEN_DOMCTL_set_access_required:
         if ( unlikely(current->domain == d) ) /* no domain_pause() */
             ret = -EPERM;
diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
index 2231341b5d..4de651038d 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -33,7 +33,7 @@
  */
 struct vm_event_st;
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
 #include <asm/mem_access.h>
 #endif
 
@@ -99,7 +99,7 @@ long p2m_set_mem_access_multi(struct domain *d,
 int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access,
                        unsigned int altp2m_idx);
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
 int mem_access_memop(unsigned long cmd,
                      XEN_GUEST_HANDLE_PARAM(xen_mem_access_op_t) arg);
 #else
@@ -109,7 +109,7 @@ int mem_access_memop(unsigned long cmd,
 {
     return -ENOSYS;
 }
-#endif /* CONFIG_MEM_ACCESS */
+#endif /* CONFIG_VM_EVENT */
 
 #endif /* _XEN_MEM_ACCESS_H */
 
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index a8d06de6b0..9227205fcd 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -647,7 +647,7 @@ static XSM_INLINE int cf_check xsm_vm_event_control(
     return xsm_default_action(action, current->domain, d);
 }
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
 static XSM_INLINE int cf_check xsm_mem_access(XSM_DEFAULT_ARG struct domain *d)
 {
     XSM_ASSERT_ACTION(XSM_DM_PRIV);
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 8c33b055fc..24acc16125 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -153,7 +153,7 @@ struct xsm_ops {
 
     int (*vm_event_control)(struct domain *d, int mode, int op);
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
     int (*mem_access)(struct domain *d);
 #endif
 
@@ -637,7 +637,7 @@ static inline int xsm_vm_event_control(
     return alternative_call(xsm_ops.vm_event_control, d, mode, op);
 }
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
 static inline int xsm_mem_access(xsm_default_t def, struct domain *d)
 {
     return alternative_call(xsm_ops.mem_access, d);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index ce6fbdc6c5..93fbfc43cc 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -111,7 +111,7 @@ static const struct xsm_ops __initconst_cf_clobber dummy_ops = {
 
     .vm_event_control              = xsm_vm_event_control,
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
     .mem_access                    = xsm_mem_access,
 #endif
 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 389707a164..6a53487ea4 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1362,7 +1362,7 @@ static int cf_check flask_vm_event_control(struct domain *d, int mode, int op)
     return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__VM_EVENT);
 }
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
 static int cf_check flask_mem_access(struct domain *d)
 {
     return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__MEM_ACCESS);
@@ -1955,7 +1955,7 @@ static const struct xsm_ops __initconst_cf_clobber flask_ops = {
 
     .vm_event_control = flask_vm_event_control,
 
-#ifdef CONFIG_MEM_ACCESS
+#ifdef CONFIG_VM_EVENT
     .mem_access = flask_mem_access,
 #endif
 
-- 
2.25.1

[PATCH v3 2/3] x86:monitor: control monitor.c build with CONFIG_VM_EVENT option

[Sergiy Kibrik]

Replace more general CONFIG_HVM option with CONFIG_VM_EVENT which is more
relevant and specific to monitoring. This is only to clarify at build level
to which subsystem this file belongs.

No functional change here, as VM_EVENT depends on HVM.

Acked-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
 xen/arch/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index c763f80b0b..f59c9665fd 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -49,7 +49,7 @@ obj-$(CONFIG_PV) += ioport_emulate.o
 obj-y += irq.o
 obj-$(CONFIG_KEXEC) += machine_kexec.o
 obj-y += mm.o x86_64/mm.o
-obj-$(CONFIG_HVM) += monitor.o
+obj-$(CONFIG_VM_EVENT) += monitor.o
 obj-y += mpparse.o
 obj-y += nmi.o
 obj-y += numa.o
-- 
2.25.1

[PATCH v3 3/3] xen: mem_access: conditionally compile vm_event.c & monitor.c

[Sergiy Kibrik]

From: Stefano Stabellini <stefano.stabellini@amd.com>

Extend coverage of CONFIG_VM_EVENT option and make the build of VM events
and monitoring support optional. Also make MEM_PAGING option depend on VM_EVENT
to document that mem_paging is relying on vm_event.
This is to reduce code size on Arm when this option isn't enabled.

CC: Jan Beulich <jbeulich@suse.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
changes in v3:
 - add dependency MEM_PAGING -> VM_EVENT
 - monitor_domctl() stub returns -EOPNOTSUPP
changes in v2:
 - rename CONFIG_MEM_ACCESS -> CONFIG_VM_EVENT
 - tags
---
 xen/arch/arm/Makefile      |  4 ++--
 xen/arch/arm/vsmc.c        |  3 ++-
 xen/arch/x86/Kconfig       |  2 +-
 xen/common/Makefile        |  4 ++--
 xen/include/xen/monitor.h  |  9 +++++++++
 xen/include/xen/vm_event.h | 14 +++++++++++---
 6 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 3bd5125e64..4837ad467a 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,7 +40,7 @@ obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-$(CONFIG_LLC_COLORING) += llc-coloring.o
 obj-$(CONFIG_VM_EVENT) += mem_access.o
 obj-y += mm.o
-obj-y += monitor.o
+obj-$(CONFIG_VM_EVENT) += monitor.o
 obj-y += p2m.o
 obj-y += platform.o
 obj-y += platform_hypercall.o
@@ -66,7 +66,7 @@ obj-$(CONFIG_VGICV2) += vgic-v2.o
 obj-$(CONFIG_GICV3) += vgic-v3.o
 obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
 endif
-obj-y += vm_event.o
+obj-$(CONFIG_VM_EVENT) += vm_event.o
 obj-y += vtimer.o
 obj-$(CONFIG_SBSA_VUART_CONSOLE) += vpl011.o
 obj-y += vsmc.o
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index e253865b6c..6081f14ed0 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -330,7 +330,8 @@ void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
     }
 
     /* If monitor is enabled, let it handle the call. */
-    if ( current->domain->arch.monitor.privileged_call_enabled )
+    if ( IS_ENABLED(CONFIG_VM_EVENT) &&
+         current->domain->arch.monitor.privileged_call_enabled )
         rc = monitor_smc();
 
     if ( rc == 1 )
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 6e41bc0fb4..f086799594 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -350,7 +350,7 @@ endif
 
 config MEM_PAGING
 	bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
-	depends on HVM
+	depends on HVM && VM_EVENT
 
 config MEM_SHARING
 	bool "Xen memory sharing support (UNSUPPORTED)" if UNSUPPORTED
diff --git a/xen/common/Makefile b/xen/common/Makefile
index b71d4b3efa..ac23120d7d 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -54,7 +54,7 @@ obj-y += timer.o
 obj-$(CONFIG_TRACEBUFFER) += trace.o
 obj-y += version.o
 obj-y += virtual_region.o
-obj-y += vm_event.o
+obj-$(CONFIG_VM_EVENT) += vm_event.o
 obj-$(CONFIG_HAS_VMAP) += vmap.o
 obj-y += vsprintf.o
 obj-y += wait.o
@@ -68,7 +68,7 @@ obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o memory.o multicall.o xlat.o
 
 ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
 obj-y += domctl.o
-obj-y += monitor.o
+obj-$(CONFIG_VM_EVENT) += monitor.o
 obj-y += sysctl.o
 endif
 
diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
index 713d54f7c1..634b6cd2a1 100644
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -27,8 +27,17 @@
 struct domain;
 struct xen_domctl_monitor_op;
 
+#ifdef CONFIG_VM_EVENT
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop);
 void monitor_guest_request(void);
+#else
+static inline int monitor_domctl(struct domain *d,
+                                 struct xen_domctl_monitor_op *mop)
+{
+    return -EOPNOTSUPP;
+}
+static inline void monitor_guest_request(void) {}
+#endif
 
 int monitor_traps(struct vcpu *v, bool sync, vm_event_request_t *req);
 
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index 9a86358b42..268c85fc4f 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -50,9 +50,6 @@ struct vm_event_domain
     unsigned int last_vcpu_wake_up;
 };
 
-/* Clean up on domain destruction */
-void vm_event_cleanup(struct domain *d);
-
 /* Returns whether a ring has been set up */
 bool vm_event_check_ring(struct vm_event_domain *ved);
 
@@ -88,7 +85,18 @@ void vm_event_cancel_slot(struct domain *d, struct vm_event_domain *ved);
 void vm_event_put_request(struct domain *d, struct vm_event_domain *ved,
                           vm_event_request_t *req);
 
+#ifdef CONFIG_VM_EVENT
+/* Clean up on domain destruction */
+void vm_event_cleanup(struct domain *d);
 int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec);
+#else
+static inline void vm_event_cleanup(struct domain *d) {}
+static inline int vm_event_domctl(struct domain *d,
+                                  struct xen_domctl_vm_event_op *vec)
+{
+    return -EINVAL;
+}
+#endif
 
 void vm_event_vcpu_pause(struct vcpu *v);
 void vm_event_vcpu_unpause(struct vcpu *v);
-- 
2.25.1

Re: [PATCH v3 3/3] xen: mem_access: conditionally compile vm_event.c & monitor.c

[Jan Beulich]

On 11.03.2025 11:27, Sergiy Kibrik wrote:
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> Extend coverage of CONFIG_VM_EVENT option and make the build of VM events
> and monitoring support optional. Also make MEM_PAGING option depend on VM_EVENT
> to document that mem_paging is relying on vm_event.
> This is to reduce code size on Arm when this option isn't enabled.
> 
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>

Please can tags be kept in chronological order? It's impossible to review
a patch that wasn't first signed-off on by the author(s).

> ---
> changes in v3:
>  - add dependency MEM_PAGING -> VM_EVENT
>  - monitor_domctl() stub returns -EOPNOTSUPP

Seeing this, i.e. ...

> --- a/xen/include/xen/monitor.h
> +++ b/xen/include/xen/monitor.h
> @@ -27,8 +27,17 @@
>  struct domain;
>  struct xen_domctl_monitor_op;
>  
> +#ifdef CONFIG_VM_EVENT
>  int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop);
>  void monitor_guest_request(void);
> +#else
> +static inline int monitor_domctl(struct domain *d,
> +                                 struct xen_domctl_monitor_op *mop)
> +{
> +    return -EOPNOTSUPP;

... this, why ...

> @@ -88,7 +85,18 @@ void vm_event_cancel_slot(struct domain *d, struct vm_event_domain *ved);
>  void vm_event_put_request(struct domain *d, struct vm_event_domain *ved,
>                            vm_event_request_t *req);
>  
> +#ifdef CONFIG_VM_EVENT
> +/* Clean up on domain destruction */
> +void vm_event_cleanup(struct domain *d);
>  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec);
> +#else
> +static inline void vm_event_cleanup(struct domain *d) {}
> +static inline int vm_event_domctl(struct domain *d,
> +                                  struct xen_domctl_vm_event_op *vec)
> +{
> +    return -EINVAL;

... is it EINVAL here?

Jan

Re: [PATCH v3 1/3] xen: kconfig: rename MEM_ACCESS -> VM_EVENT

[Jan Beulich]

On 11.03.2025 11:23, Sergiy Kibrik wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -92,7 +92,7 @@ config HAS_VMAP
>  config MEM_ACCESS_ALWAYS_ON
>  	bool
>  
> -config MEM_ACCESS
> +config VM_EVENT
>  	def_bool MEM_ACCESS_ALWAYS_ON
>  	prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
>  	depends on HVM

I still don't see why we would then stick to MEM_ACCESS_ALWAYS_ON as a name
for the sibling option.

Jan

Re: [PATCH v3 1/3] xen: kconfig: rename MEM_ACCESS -> VM_EVENT

[Tamas K Lengyel]

On Tue, Mar 11, 2025 at 8:01 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 11.03.2025 11:23, Sergiy Kibrik wrote:
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -92,7 +92,7 @@ config HAS_VMAP
> >  config MEM_ACCESS_ALWAYS_ON
> >       bool
> >
> > -config MEM_ACCESS
> > +config VM_EVENT
> >       def_bool MEM_ACCESS_ALWAYS_ON
> >       prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
> >       depends on HVM
>
> I still don't see why we would then stick to MEM_ACCESS_ALWAYS_ON as a name
> for the sibling option.

I'm not opposed to renaming it but it's also not something I see much
value in doing. It's not used anywhere in the code, it's purely used
in the config selector to mark that on x86 the
vm_event/mem_access/monitor bits are not setup to be compiled-out.

Tamas

Re: [PATCH v3 3/3] xen: mem_access: conditionally compile vm_event.c & monitor.c

[Tamas K Lengyel]

On Tue, Mar 11, 2025 at 7:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 11.03.2025 11:27, Sergiy Kibrik wrote:
> > From: Stefano Stabellini <stefano.stabellini@amd.com>
> >
> > Extend coverage of CONFIG_VM_EVENT option and make the build of VM events
> > and monitoring support optional. Also make MEM_PAGING option depend on VM_EVENT
> > to document that mem_paging is relying on vm_event.
> > This is to reduce code size on Arm when this option isn't enabled.
> >
> > CC: Jan Beulich <jbeulich@suse.com>
> > CC: Tamas K Lengyel <tamas@tklengyel.com>
> > Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>
> Please can tags be kept in chronological order? It's impossible to review
> a patch that wasn't first signed-off on by the author(s).
>
> > ---
> > changes in v3:
> >  - add dependency MEM_PAGING -> VM_EVENT

This seems to be largely unnecessary since on x86 selecting HVM
already selects it but I guess it also doesn't hurt to explicitly mark
it like this either.

> >  - monitor_domctl() stub returns -EOPNOTSUPP
>
> Seeing this, i.e. ...
>
> > --- a/xen/include/xen/monitor.h
> > +++ b/xen/include/xen/monitor.h
> > @@ -27,8 +27,17 @@
> >  struct domain;
> >  struct xen_domctl_monitor_op;
> >
> > +#ifdef CONFIG_VM_EVENT
> >  int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop);
> >  void monitor_guest_request(void);
> > +#else
> > +static inline int monitor_domctl(struct domain *d,
> > +                                 struct xen_domctl_monitor_op *mop)
> > +{
> > +    return -EOPNOTSUPP;
>
> ... this, why ...
>
> > @@ -88,7 +85,18 @@ void vm_event_cancel_slot(struct domain *d, struct vm_event_domain *ved);
> >  void vm_event_put_request(struct domain *d, struct vm_event_domain *ved,
> >                            vm_event_request_t *req);
> >
> > +#ifdef CONFIG_VM_EVENT
> > +/* Clean up on domain destruction */
> > +void vm_event_cleanup(struct domain *d);
> >  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec);
> > +#else
> > +static inline void vm_event_cleanup(struct domain *d) {}
> > +static inline int vm_event_domctl(struct domain *d,
> > +                                  struct xen_domctl_vm_event_op *vec)
> > +{
> > +    return -EINVAL;
>
> ... is it EINVAL here?

I would prefer if it was consistent too with -EOPNOTSUPP for both when
the subsystems are compiled out.

Thanks,
Tamas

Re: [PATCH v3 1/3] xen: kconfig: rename MEM_ACCESS -> VM_EVENT

[Sergiy Kibrik]

11.03.25 14:01, Jan Beulich:
> On 11.03.2025 11:23, Sergiy Kibrik wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -92,7 +92,7 @@ config HAS_VMAP
>>   config MEM_ACCESS_ALWAYS_ON
>>   	bool
>>   
>> -config MEM_ACCESS
>> +config VM_EVENT
>>   	def_bool MEM_ACCESS_ALWAYS_ON
>>   	prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
>>   	depends on HVM
> 
> I still don't see why we would then stick to MEM_ACCESS_ALWAYS_ON as a name
> for the sibling option.
> 

as HVM requires mem-access to be enabled I felt like it should be kept 
as is -- to hint that it is mem-access that ties vm_event to hvm.

  -Sergiy

Re: [PATCH v3 1/3] xen: kconfig: rename MEM_ACCESS -> VM_EVENT

[Jan Beulich]

On 11.03.2025 14:41, Sergiy Kibrik wrote:
> 11.03.25 14:01, Jan Beulich:
>> On 11.03.2025 11:23, Sergiy Kibrik wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -92,7 +92,7 @@ config HAS_VMAP
>>>   config MEM_ACCESS_ALWAYS_ON
>>>   	bool
>>>   
>>> -config MEM_ACCESS
>>> +config VM_EVENT
>>>   	def_bool MEM_ACCESS_ALWAYS_ON
>>>   	prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
>>>   	depends on HVM
>>
>> I still don't see why we would then stick to MEM_ACCESS_ALWAYS_ON as a name
>> for the sibling option.
> 
> as HVM requires mem-access to be enabled

If that was indeed the case, ...

> I felt like it should be kept 
> as is -- to hint that it is mem-access that ties vm_event to hvm.

... I'd agree. It was my understanding though that MEM_ACCESS_ALWAYS_ON
only exists because when Arm support was added no-one wanted to take the
time to make mem-access properly optional on x86.

Jan

Re: [PATCH v3 3/3] xen: mem_access: conditionally compile vm_event.c & monitor.c

[Sergiy Kibrik]

11.03.25 15:39, Tamas K Lengyel:
> On Tue, Mar 11, 2025 at 7:59 AM Jan Beulich<jbeulich@suse.com>  wrote:
>> On 11.03.2025 11:27, Sergiy Kibrik wrote:
>>> From: Stefano Stabellini<stefano.stabellini@amd.com>
>>>
>>> Extend coverage of CONFIG_VM_EVENT option and make the build of VM events
>>> and monitoring support optional. Also make MEM_PAGING option depend on VM_EVENT
>>> to document that mem_paging is relying on vm_event.
>>> This is to reduce code size on Arm when this option isn't enabled.
>>>
>>> CC: Jan Beulich<jbeulich@suse.com>
>>> CC: Tamas K Lengyel<tamas@tklengyel.com>
>>> Reviewed-by: Ayan Kumar Halder<ayan.kumar.halder@amd.com>
>>> Signed-off-by: Stefano Stabellini<stefano.stabellini@amd.com>
>>> Signed-off-by: Sergiy Kibrik<Sergiy_Kibrik@epam.com>
>> Please can tags be kept in chronological order? It's impossible to review
>> a patch that wasn't first signed-off on by the author(s).
>>
>>> ---
>>> changes in v3:
>>>   - add dependency MEM_PAGING -> VM_EVENT
> This seems to be largely unnecessary since on x86 selecting HVM
> already selects it but I guess it also doesn't hurt to explicitly mark
> it like this either.

It's done just to highlight this dependency, more like a documentation 
hint (as Jan suggested in previous discussion).

   -Sergiy