[XEN PATCH] x86/hvm: make stdvga support optional

[XEN PATCH] x86/hvm: make stdvga support optional

[Sergiy Kibrik]

Introduce config option X86_STDVGA so that stdvga driver can be disabled on
systems that don't need it.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
 xen/arch/x86/Kconfig              | 10 ++++++++++
 xen/arch/x86/hvm/Makefile         |  2 +-
 xen/arch/x86/include/asm/hvm/io.h |  5 +++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 62f0b5e0f4..2ba25e6906 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -385,6 +385,16 @@ config ALTP2M
 
 	  If unsure, stay with defaults.
 
+config X86_STDVGA
+	bool "Standard VGA card emulation support" if EXPERT
+	default y
+	depends on HVM
+	help
+	  Build stdvga driver that emulates standard VGA card with VESA BIOS
+          Extensions for HVM guests.
+
+	  If unsure, say Y.
+
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 4c1fa5c6c2..4d1f8e00eb 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -22,7 +22,7 @@ obj-y += pmtimer.o
 obj-y += quirks.o
 obj-y += rtc.o
 obj-y += save.o
-obj-y += stdvga.o
+obj-$(CONFIG_X86_STDVGA) += stdvga.o
 obj-y += vioapic.o
 obj-y += vlapic.o
 obj-y += vm_event.o
diff --git a/xen/arch/x86/include/asm/hvm/io.h b/xen/arch/x86/include/asm/hvm/io.h
index 24d1b6134f..9b8d4f6b7a 100644
--- a/xen/arch/x86/include/asm/hvm/io.h
+++ b/xen/arch/x86/include/asm/hvm/io.h
@@ -128,8 +128,13 @@ struct hvm_hw_stdvga {
     spinlock_t lock;
 };
 
+#ifdef CONFIG_X86_STDVGA
 void stdvga_init(struct domain *d);
 void stdvga_deinit(struct domain *d);
+#else
+static inline void stdvga_init(struct domain *d) {}
+static inline void stdvga_deinit(struct domain *d) {}
+#endif
 
 extern void hvm_dpci_msi_eoi(struct domain *d, int vector);
 
-- 
2.25.1

Re: [XEN PATCH] x86/hvm: make stdvga support optional

[Roger Pau Monné]

On Thu, Sep 12, 2024 at 11:57:09AM +0300, Sergiy Kibrik wrote:
> Introduce config option X86_STDVGA so that stdvga driver can be disabled on
> systems that don't need it.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> ---
>  xen/arch/x86/Kconfig              | 10 ++++++++++
>  xen/arch/x86/hvm/Makefile         |  2 +-
>  xen/arch/x86/include/asm/hvm/io.h |  5 +++++
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 62f0b5e0f4..2ba25e6906 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -385,6 +385,16 @@ config ALTP2M
>  
>  	  If unsure, stay with defaults.
>  
> +config X86_STDVGA
> +	bool "Standard VGA card emulation support" if EXPERT
> +	default y
> +	depends on HVM
> +	help
> +	  Build stdvga driver that emulates standard VGA card with VESA BIOS
> +          Extensions for HVM guests.
> +
> +	  If unsure, say Y.
> +
>  endmenu
>  
>  source "common/Kconfig"
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 4c1fa5c6c2..4d1f8e00eb 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -22,7 +22,7 @@ obj-y += pmtimer.o
>  obj-y += quirks.o
>  obj-y += rtc.o
>  obj-y += save.o
> -obj-y += stdvga.o
> +obj-$(CONFIG_X86_STDVGA) += stdvga.o
>  obj-y += vioapic.o
>  obj-y += vlapic.o
>  obj-y += vm_event.o
> diff --git a/xen/arch/x86/include/asm/hvm/io.h b/xen/arch/x86/include/asm/hvm/io.h
> index 24d1b6134f..9b8d4f6b7a 100644
> --- a/xen/arch/x86/include/asm/hvm/io.h
> +++ b/xen/arch/x86/include/asm/hvm/io.h
> @@ -128,8 +128,13 @@ struct hvm_hw_stdvga {
>      spinlock_t lock;
>  };
>  
> +#ifdef CONFIG_X86_STDVGA
>  void stdvga_init(struct domain *d);
>  void stdvga_deinit(struct domain *d);
> +#else
> +static inline void stdvga_init(struct domain *d) {}
> +static inline void stdvga_deinit(struct domain *d) {}
> +#endif

Shouldn't Xen report an error if a user attempts to create a domain
with X86_EMU_VGA set in emulation_flags, but stdvga has been built
time disabled?

Thanks, Roger.

Re: [XEN PATCH] x86/hvm: make stdvga support optional

[Sergiy Kibrik]

12.09.24 12:14, Roger Pau Monné:
> Shouldn't Xen report an error if a user attempts to create a domain
> with X86_EMU_VGA set in emulation_flags, but stdvga has been built
> time disabled?

I'm afraid this can accidentally render the system unbootable, because 
it looks like toolstack always sets X86_EMU_VGA flag.

   -Sergiy

Re: [XEN PATCH] x86/hvm: make stdvga support optional

[Jan Beulich]

On 16.09.2024 08:37, Sergiy Kibrik wrote:
> 12.09.24 12:14, Roger Pau Monné:
>> Shouldn't Xen report an error if a user attempts to create a domain
>> with X86_EMU_VGA set in emulation_flags, but stdvga has been built
>> time disabled?
> 
> I'm afraid this can accidentally render the system unbootable, because 
> it looks like toolstack always sets X86_EMU_VGA flag.

Which would mean that further toolstack side changes are needed, if you
really want to pursue this route. The toolstack ought to be put in a
position to figure out whether the hypervisor has stdvga support for
HVM guests.

Jan

Re: [XEN PATCH] x86/hvm: make stdvga support optional

[Roger Pau Monné]

On Mon, Sep 16, 2024 at 09:37:16AM +0300, Sergiy Kibrik wrote:
> 12.09.24 12:14, Roger Pau Monné:
> > Shouldn't Xen report an error if a user attempts to create a domain
> > with X86_EMU_VGA set in emulation_flags, but stdvga has been built
> > time disabled?
> 
> I'm afraid this can accidentally render the system unbootable, because it
> looks like toolstack always sets X86_EMU_VGA flag.

Not for PV or PVH guests.  It won't render the system unbootable, it
would just leave it unable to create HVM guests.  dom0 however, and PV
or PVH guests don't use the X86_EMU_VGA flag.

As pointed out by Jan, we need slightly better integration with the
toolstack.  IMO if we want to pursue this route we need a way for Xen
to advertise which X86_EMU_* are supported at least.

Thanks, Roger.

Re: [XEN PATCH] x86/hvm: make stdvga support optional

[Jan Beulich]

On 12.09.2024 10:57, Sergiy Kibrik wrote:
> Introduce config option X86_STDVGA so that stdvga driver can be disabled on
> systems that don't need it.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>

Considering what was committed earlier in the day (and what you would have
seen on the list already before that) - is this really a worthwhile saving
anymore?

Jan

Re: [XEN PATCH] x86/hvm: make stdvga support optional

[Stefano Stabellini]

On Thu, 12 Sep 2024, Jan Beulich wrote:
> On 12.09.2024 10:57, Sergiy Kibrik wrote:
> > Introduce config option X86_STDVGA so that stdvga driver can be disabled on
> > systems that don't need it.
> > 
> > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> 
> Considering what was committed earlier in the day (and what you would have
> seen on the list already before that) - is this really a worthwhile saving
> anymore?

Although it is smaller than before, and it can certainly be said that
there are other bigger areas of concern in terms of code size, it is
still good to make stdvga selectable so that it can be removed when not
needed.