diff mbox series

[XEN,v1] arm: introduce kconfig options to disable hypercalls

Message ID 20241216114358.2845447-1-Sergiy_Kibrik@epam.com (mailing list archive)
State New
Headers show
Series [XEN,v1] arm: introduce kconfig options to disable hypercalls | expand

Commit Message

Sergiy Kibrik Dec. 16, 2024, 11:43 a.m. UTC 
From: Stefano Stabellini <stefano.stabellini@amd.com>

It can be beneficial for some dom0less systems to further reduce Xen footprint
and disable some hypercalls handling code, which may not to be used & required
in such systems. Each hypercall has a separate option to keep configuration
flexible.

Options to disable hypercalls:
- domctl, sysctl
- hvm
- physdev
- platform

Some of these options are forced to be configurable only when DOM0LESS is
enabled, so that system won't become accidentally un-bootable when any hypercall
is disabled.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v1:
 - incorporated PV_SHIM_EXCLUSIVE check in Kconfig
 - drop excessive ifeq from Makefile & #ifdef from code
 - drop checks for CONFIG_HVM_OP & CONFIG_PLATFORM_HYP being off when !DOM0LESS
 - description updated

RFC patch here: https://lore.kernel.org/xen-devel/20240926095806.67149-1-Sergiy_Kibrik@epam.com/
---
 xen/arch/arm/Makefile        | 10 +++++-----
 xen/common/Kconfig           | 27 +++++++++++++++++++++++++++
 xen/common/Makefile          |  4 ++--
 xen/common/domain.c          |  2 ++
 xen/include/hypercall-defs.c | 14 ++++++++++++--
 xen/include/xen/hypercall.h  | 12 ++++++++++++
 6 files changed, 60 insertions(+), 9 deletions(-)

Comments

Jan Beulich Dec. 17, 2024, 1 p.m. UTC | #1 
On 16.12.2024 12:43, Sergiy Kibrik wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -516,4 +516,31 @@ config TRACEBUFFER
>  	  to be collected at run time for debugging or performance analysis.
>  	  Memory and execution overhead when not active is minimal.
>  
> +menu "Supported hypercall interfaces"
> +	visible if DOM0LESS_BOOT && EXPERT
> +
> +config SYSCTL
> +	bool "Enable sysctl hypercall"
> +	default y
> +
> +config DOMCTL
> +	bool "Enable domctl hypercalls"
> +	default y
> +
> +config HVM_OP
> +	bool "Enable HVM hypercalls"
> +	depends on HVM
> +	default y
> +
> +config PLATFORM_HYP
> +	bool "Enable platform hypercalls"
> +	depends on !PV_SHIM_EXCLUSIVE

Any reason you don't do the shim related conversion also for domctl and
sysctl?

Much like you have HVM_OP, may I suggest PLATFORM_OP here and ...

> +	default y
> +
> +config PHYSDEVOP
> +	bool "Enable physdev hypercall"
> +	default y

... PHYSDEV_OP here?

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1053,7 +1053,9 @@ int domain_kill(struct domain *d)
>          d->is_dying = DOMDYING_dying;
>          rspin_barrier(&d->domain_lock);
>          argo_destroy(d);
> +#ifdef CONFIG_DOMCTL
>          vnuma_destroy(d->vnuma);
> +#endif

There is a stub already for this, just that right now it's shim-specific.

> --- a/xen/include/hypercall-defs.c
> +++ b/xen/include/hypercall-defs.c
> @@ -234,7 +234,7 @@ stack_switch                       do:2     do:2     -        -        -
>  set_callbacks                      compat   do       -        -        -
>  fpu_taskswitch                     do       do       -        -        -
>  sched_op_compat                    do       do       -        -        dep
> -#ifndef CONFIG_PV_SHIM_EXCLUSIVE
> +#if defined(CONFIG_PLATFORM_HYP)

Nit: Why not #ifdef, like it was, and like you have it ...

> @@ -247,7 +247,9 @@ set_timer_op                       compat   do       compat   do       -
>  event_channel_op_compat            do       do       -        -        dep
>  xen_version                        do       do       do       do       do
>  console_io                         do       do       do       do       do
> +#ifdef CONFIG_PHYSDEV
>  physdev_op_compat                  compat   do       -        -        dep
> +#endif
>  #if defined(CONFIG_GRANT_TABLE)
>  grant_table_op                     compat   do       hvm      hvm      do
>  #elif defined(CONFIG_PV_SHIM)
> @@ -269,14 +271,20 @@ callback_op                        compat   do       -        -        -
>  xenoprof_op                        compat   do       -        -        -
>  #endif
>  event_channel_op                   do       do       do:1     do:1     do:1
> +#ifdef CONFIG_PHYSDEVOP
>  physdev_op                         compat   do       hvm      hvm      do_arm
> -#ifdef CONFIG_HVM
> +#endif
> +#ifdef CONFIG_HVM_OP
>  hvm_op                             do       do       do       do       do
>  #endif
>  #ifndef CONFIG_PV_SHIM_EXCLUSIVE
> +#ifdef CONFIG_SYSCTL
>  sysctl                             do       do       do       do       do
> +#endif
> +#ifdef CONFIG_DOMCTL
>  domctl                             do       do       do       do       do
>  #endif
> +#endif
>  #ifdef CONFIG_KEXEC
>  kexec_op                           compat   do       -        -        -
>  #endif
> @@ -293,7 +301,9 @@ hypfs_op                           do       do       do       do       do
>  #endif
>  mca                                do       do       -        -        -
>  #ifndef CONFIG_PV_SHIM_EXCLUSIVE
> +#ifdef CONFIG_DOMCTL
>  paging_domctl_cont                 do       do       do       do       -
>  #endif
> +#endif

... everywhere else?

> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -24,6 +24,18 @@
>  /* Needs to be after asm/hypercall.h. */
>  #include <xen/hypercall-defs.h>
>  
> +#if !defined(CONFIG_DOMCTL) && !defined(CONFIG_DOM0LESS_BOOT)
> +#error "domctl and dom0less can't be disabled simultaneously"
> +#endif
> +
> +#if !defined(CONFIG_PHYSDEVOP) && !defined(CONFIG_DOM0LESS_BOOT)
> +#error "physdevop and dom0less can't be disabled simultaneously"
> +#endif
> +
> +#if !defined(CONFIG_SYSCTL) && !defined(CONFIG_DOM0LESS_BOOT)
> +#error "sysctl and dom0less can't be disabled simultaneously"
> +#endif

I'm puzzled by this: It covers only 3 of the 5, and it really only
re-checks what Kconfig already enforces.

Jan
Sergiy Kibrik Dec. 18, 2024, 9:04 a.m. UTC | #2 
17.12.24 15:00, Jan Beulich:
> On 16.12.2024 12:43, Sergiy Kibrik wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -516,4 +516,31 @@ config TRACEBUFFER
>>   	  to be collected at run time for debugging or performance analysis.
>>   	  Memory and execution overhead when not active is minimal.
>>   
>> +menu "Supported hypercall interfaces"
>> +	visible if DOM0LESS_BOOT && EXPERT
>> +
>> +config SYSCTL
>> +	bool "Enable sysctl hypercall"
>> +	default y
>> +
>> +config DOMCTL
>> +	bool "Enable domctl hypercalls"
>> +	default y
>> +
>> +config HVM_OP
>> +	bool "Enable HVM hypercalls"
>> +	depends on HVM
>> +	default y
>> +
>> +config PLATFORM_HYP
>> +	bool "Enable platform hypercalls"
>> +	depends on !PV_SHIM_EXCLUSIVE
> 
> Any reason you don't do the shim related conversion also for domctl and
> sysctl?
> 

you're right, I'll do it in v2

> Much like you have HVM_OP, may I suggest PLATFORM_OP here and ...
> 
>> +	default y
>> +
>> +config PHYSDEVOP
>> +	bool "Enable physdev hypercall"
>> +	default y
> 
> ... PHYSDEV_OP here?
> 

yes, sure

>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1053,7 +1053,9 @@ int domain_kill(struct domain *d)
>>           d->is_dying = DOMDYING_dying;
>>           rspin_barrier(&d->domain_lock);
>>           argo_destroy(d);
>> +#ifdef CONFIG_DOMCTL
>>           vnuma_destroy(d->vnuma);
>> +#endif
> 
> There is a stub already for this, just that right now it's shim-specific.
> >> --- a/xen/include/hypercall-defs.c
>> +++ b/xen/include/hypercall-defs.c
>> @@ -234,7 +234,7 @@ stack_switch                       do:2     do:2     -        -        -
>>   set_callbacks                      compat   do       -        -        -
>>   fpu_taskswitch                     do       do       -        -        -
>>   sched_op_compat                    do       do       -        -        dep
>> -#ifndef CONFIG_PV_SHIM_EXCLUSIVE
>> +#if defined(CONFIG_PLATFORM_HYP)
> 
> Nit: Why not #ifdef, like it was, and like you have it ...
> 
>> @@ -247,7 +247,9 @@ set_timer_op                       compat   do       compat   do       -
>>   event_channel_op_compat            do       do       -        -        dep
>>   xen_version                        do       do       do       do       do
>>   console_io                         do       do       do       do       do
>> +#ifdef CONFIG_PHYSDEV
>>   physdev_op_compat                  compat   do       -        -        dep
>> +#endif
>>   #if defined(CONFIG_GRANT_TABLE)
>>   grant_table_op                     compat   do       hvm      hvm      do
>>   #elif defined(CONFIG_PV_SHIM)
>> @@ -269,14 +271,20 @@ callback_op                        compat   do       -        -        -
>>   xenoprof_op                        compat   do       -        -        -
>>   #endif
>>   event_channel_op                   do       do       do:1     do:1     do:1
>> +#ifdef CONFIG_PHYSDEVOP
>>   physdev_op                         compat   do       hvm      hvm      do_arm
>> -#ifdef CONFIG_HVM
>> +#endif
>> +#ifdef CONFIG_HVM_OP
>>   hvm_op                             do       do       do       do       do
>>   #endif
>>   #ifndef CONFIG_PV_SHIM_EXCLUSIVE
>> +#ifdef CONFIG_SYSCTL
>>   sysctl                             do       do       do       do       do
>> +#endif
>> +#ifdef CONFIG_DOMCTL
>>   domctl                             do       do       do       do       do
>>   #endif
>> +#endif
>>   #ifdef CONFIG_KEXEC
>>   kexec_op                           compat   do       -        -        -
>>   #endif
>> @@ -293,7 +301,9 @@ hypfs_op                           do       do       do       do       do
>>   #endif
>>   mca                                do       do       -        -        -
>>   #ifndef CONFIG_PV_SHIM_EXCLUSIVE
>> +#ifdef CONFIG_DOMCTL
>>   paging_domctl_cont                 do       do       do       do       -
>>   #endif
>> +#endif
> 
> ... everywhere else?
> 

yes, will fix that

>> --- a/xen/include/xen/hypercall.h
>> +++ b/xen/include/xen/hypercall.h
>> @@ -24,6 +24,18 @@
>>   /* Needs to be after asm/hypercall.h. */
>>   #include <xen/hypercall-defs.h>
>>   
>> +#if !defined(CONFIG_DOMCTL) && !defined(CONFIG_DOM0LESS_BOOT)
>> +#error "domctl and dom0less can't be disabled simultaneously"
>> +#endif
>> +
>> +#if !defined(CONFIG_PHYSDEVOP) && !defined(CONFIG_DOM0LESS_BOOT)
>> +#error "physdevop and dom0less can't be disabled simultaneously"
>> +#endif
>> +
>> +#if !defined(CONFIG_SYSCTL) && !defined(CONFIG_DOM0LESS_BOOT)
>> +#error "sysctl and dom0less can't be disabled simultaneously"
>> +#endif
> 
> I'm puzzled by this: It covers only 3 of the 5, and it really only
> re-checks what Kconfig already enforces.
> 

At some point I wasn't sure that kconfig will enforce this, because 
somehow I made kconfig produce configuration with both DOMCTL & 
DOM0LESS_BOOT being off. Anyway I can't reproduce it now, so will drop 
these checks in v2.

   -Sergiy
diff mbox series

Patch

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index e4ad1ce851..b910ce3726 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -18,7 +18,7 @@  obj-$(CONFIG_IOREQ_SERVER) += dm.o
 obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
 obj-y += domain.o
 obj-y += domain_build.init.o
-obj-y += domctl.o
+obj-$(CONFIG_DOMCTL) += domctl.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += efi/
 obj-y += gic.o
@@ -29,7 +29,7 @@  obj-$(CONFIG_HAS_ITS) += gic-v3-lpi.o
 obj-y += guestcopy.o
 obj-y += guest_atomics.o
 obj-y += guest_walk.o
-obj-y += hvm.o
+obj-$(CONFIG_HVM_OP) += hvm.o
 obj-y += io.o
 obj-$(CONFIG_IOREQ_SERVER) += ioreq.o
 obj-y += irq.o
@@ -40,8 +40,8 @@  obj-y += mm.o
 obj-y += monitor.o
 obj-y += p2m.o
 obj-y += platform.o
-obj-y += platform_hypercall.o
-obj-y += physdev.o
+obj-$(CONFIG_PLATFORM_HYP) += platform_hypercall.o
+obj-$(CONFIG_PHYSDEVOP) += physdev.o
 obj-y += processor.o
 obj-y += psci.o
 obj-y += setup.o
@@ -51,7 +51,7 @@  obj-y += smpboot.o
 obj-$(CONFIG_STATIC_EVTCHN) += static-evtchn.init.o
 obj-$(CONFIG_STATIC_MEMORY) += static-memory.init.o
 obj-$(CONFIG_STATIC_SHM) += static-shmem.init.o
-obj-y += sysctl.o
+obj-$(CONFIG_SYSCTL) += sysctl.o
 obj-y += time.o
 obj-y += traps.o
 obj-y += vcpreg.o
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 90268d9249..22b1b10700 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -516,4 +516,31 @@  config TRACEBUFFER
 	  to be collected at run time for debugging or performance analysis.
 	  Memory and execution overhead when not active is minimal.
 
+menu "Supported hypercall interfaces"
+	visible if DOM0LESS_BOOT && EXPERT
+
+config SYSCTL
+	bool "Enable sysctl hypercall"
+	default y
+
+config DOMCTL
+	bool "Enable domctl hypercalls"
+	default y
+
+config HVM_OP
+	bool "Enable HVM hypercalls"
+	depends on HVM
+	default y
+
+config PLATFORM_HYP
+	bool "Enable platform hypercalls"
+	depends on !PV_SHIM_EXCLUSIVE
+	default y
+
+config PHYSDEVOP
+	bool "Enable physdev hypercall"
+	default y
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index b279b09bfb..26de84d122 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -66,9 +66,9 @@  obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma lzo unlzo un
 obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o memory.o multicall.o xlat.o)
 
 ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
-obj-y += domctl.o
+obj-$(CONFIG_DOMCTL) += domctl.o
 obj-y += monitor.o
-obj-y += sysctl.o
+obj-$(CONFIG_SYSCTL) += sysctl.o
 endif
 
 extra-y := symbols-dummy.o
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 92263a4fbd..77d15a317f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1053,7 +1053,9 @@  int domain_kill(struct domain *d)
         d->is_dying = DOMDYING_dying;
         rspin_barrier(&d->domain_lock);
         argo_destroy(d);
+#ifdef CONFIG_DOMCTL
         vnuma_destroy(d->vnuma);
+#endif
         domain_set_outstanding_pages(d, 0);
         /* fallthrough */
     case DOMDYING_dying:
diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
index 7720a29ade..16b4c795e4 100644
--- a/xen/include/hypercall-defs.c
+++ b/xen/include/hypercall-defs.c
@@ -234,7 +234,7 @@  stack_switch                       do:2     do:2     -        -        -
 set_callbacks                      compat   do       -        -        -
 fpu_taskswitch                     do       do       -        -        -
 sched_op_compat                    do       do       -        -        dep
-#ifndef CONFIG_PV_SHIM_EXCLUSIVE
+#if defined(CONFIG_PLATFORM_HYP)
 platform_op                        compat   do       compat   do       do
 #endif
 set_debugreg                       do       do       -        -        -
@@ -247,7 +247,9 @@  set_timer_op                       compat   do       compat   do       -
 event_channel_op_compat            do       do       -        -        dep
 xen_version                        do       do       do       do       do
 console_io                         do       do       do       do       do
+#ifdef CONFIG_PHYSDEV
 physdev_op_compat                  compat   do       -        -        dep
+#endif
 #if defined(CONFIG_GRANT_TABLE)
 grant_table_op                     compat   do       hvm      hvm      do
 #elif defined(CONFIG_PV_SHIM)
@@ -269,14 +271,20 @@  callback_op                        compat   do       -        -        -
 xenoprof_op                        compat   do       -        -        -
 #endif
 event_channel_op                   do       do       do:1     do:1     do:1
+#ifdef CONFIG_PHYSDEVOP
 physdev_op                         compat   do       hvm      hvm      do_arm
-#ifdef CONFIG_HVM
+#endif
+#ifdef CONFIG_HVM_OP
 hvm_op                             do       do       do       do       do
 #endif
 #ifndef CONFIG_PV_SHIM_EXCLUSIVE
+#ifdef CONFIG_SYSCTL
 sysctl                             do       do       do       do       do
+#endif
+#ifdef CONFIG_DOMCTL
 domctl                             do       do       do       do       do
 #endif
+#endif
 #ifdef CONFIG_KEXEC
 kexec_op                           compat   do       -        -        -
 #endif
@@ -293,7 +301,9 @@  hypfs_op                           do       do       do       do       do
 #endif
 mca                                do       do       -        -        -
 #ifndef CONFIG_PV_SHIM_EXCLUSIVE
+#ifdef CONFIG_DOMCTL
 paging_domctl_cont                 do       do       do       do       -
 #endif
+#endif
 
 #endif /* !CPPCHECK */
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index f307dfb597..e47c6ddfd2 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -24,6 +24,18 @@ 
 /* Needs to be after asm/hypercall.h. */
 #include <xen/hypercall-defs.h>
 
+#if !defined(CONFIG_DOMCTL) && !defined(CONFIG_DOM0LESS_BOOT)
+#error "domctl and dom0less can't be disabled simultaneously"
+#endif
+
+#if !defined(CONFIG_PHYSDEVOP) && !defined(CONFIG_DOM0LESS_BOOT)
+#error "physdevop and dom0less can't be disabled simultaneously"
+#endif
+
+#if !defined(CONFIG_SYSCTL) && !defined(CONFIG_DOM0LESS_BOOT)
+#error "sysctl and dom0less can't be disabled simultaneously"
+#endif
+
 extern long
 arch_do_domctl(
     struct xen_domctl *domctl, struct domain *d,