Re: [PATCH v2 1/2] x86/hvm: make stdvga support optional

[Denis Mukhin <dmkhn@xxxxxxxxx>]

On Thursday, February 20th, 2025 at 1:53 AM, Jiqian Chen <Jiqian.Chen@xxxxxxx> 
wrote:

>
>
> From: Sergiy Kibrik Sergiy_Kibrik@xxxxxxxx
>
>
> Introduce config option X86_STDVGA so that stdvga driver can be
> disabled on systems that don't need it.
>
> What's more, in function emulation_flags_ok, to check if toolstack
> pass any emulation flag that disabled in building time.
>
> Signed-off-by: Sergiy Kibrik Sergiy_Kibrik@xxxxxxxx
>
> Signed-off-by: Jiqian Chen Jiqian.Chen@xxxxxxx
>
> ---
> Hi all,
> this is a rework for 
> https://lore.kernel.org/xen-devel/20240912085709.858052-1-Sergiy_Kibrik@xxxxxxxx/T/#u.
>
> v1->v2 changes:
>
> * For emulation flags, added a new file "arch/x86/hvm/Kconfig.emu" to be a 
> separate seletion,
> and moved definition of "config X86_STDVGA" into it.
> * Added a new macro "#define DISABLED_EMU_MASK 
> (!IS_ENABLED(CONFIG_X86_STDVGA) ? X86_EMU_VGA : 0)",
> and checked it in function emulation_flags_ok.
> * Adjusted macro "has_vvga".
>
> Best regards,
> Jiqian Chen.
> ---
> xen/arch/x86/Kconfig | 2 ++
> xen/arch/x86/domain.c | 2 ++
> xen/arch/x86/hvm/Kconfig.emu | 14 ++++++++++++++
> xen/arch/x86/hvm/Makefile | 2 +-
> xen/arch/x86/include/asm/domain.h | 6 +++++-
> xen/arch/x86/include/asm/hvm/io.h | 4 ++++
> 6 files changed, 28 insertions(+), 2 deletions(-)
> create mode 100644 xen/arch/x86/hvm/Kconfig.emu
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 9cdd04721afa..e4fedf7e54d8 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -123,6 +123,8 @@ config HVM
>
> If unsure, say Y.
>
> +source "arch/x86/hvm/Kconfig.emu"

JFYI, there's this patch:
  https://lore.kernel.org/xen-devel/20250207220302.4190210-1-dmukhin@xxxxxxxx/

I think having one Kconfig under arch/x86/hvm is enough.
Thoughts?

> +
> config AMD_SVM
> bool "AMD-V" if EXPERT
> depends on HVM
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 78a13e6812c9..289c91459470 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -758,6 +758,8 @@ static bool emulation_flags_ok(const struct domain *d, 
> uint32_t emflags)
> (X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ)) &&
> emflags != X86_EMU_LAPIC )
> return false;
> + if ( emflags & DISABLED_EMU_MASK )
> + return false;
> }
> else if ( emflags != 0 && emflags != X86_EMU_PIT )
> {
> diff --git a/xen/arch/x86/hvm/Kconfig.emu b/xen/arch/x86/hvm/Kconfig.emu
> new file mode 100644
> index 000000000000..aa60b6227036
> --- /dev/null
> +++ b/xen/arch/x86/hvm/Kconfig.emu
> @@ -0,0 +1,14 @@
> +menu "Emulated device support"
> + visible if EXPERT
> +
> +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
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 4c1fa5c6c2bf..4d1f8e00eb68 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/domain.h 
> b/xen/arch/x86/include/asm/domain.h
> index b79d6badd71c..68be23bf3bf4 100644
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -494,13 +494,17 @@ struct arch_domain
> X86_EMU_PIT | X86_EMU_USE_PIRQ | \
> X86_EMU_VPCI)
>
> +#define DISABLED_EMU_MASK \
> + (!IS_ENABLED(CONFIG_X86_STDVGA) ? X86_EMU_VGA : 0)
> +
> #define has_vlapic(d) (!!((d)->arch.emulation_flags & X86_EMU_LAPIC))
>
> #define has_vhpet(d) (!!((d)->arch.emulation_flags & X86_EMU_HPET))
>
> #define has_vpm(d) (!!((d)->arch.emulation_flags & X86_EMU_PM))
>
> #define has_vrtc(d) (!!((d)->arch.emulation_flags & X86_EMU_RTC))
>
> #define has_vioapic(d) (!!((d)->arch.emulation_flags & X86_EMU_IOAPIC))
>
> #define has_vpic(d) (!!((d)->arch.emulation_flags & X86_EMU_PIC))
>
> -#define has_vvga(d) (!!((d)->arch.emulation_flags & X86_EMU_VGA))
>
> +#define has_vvga(d) (IS_ENABLED(CONFIG_X86_STDVGA) && \
> + !!((d)->arch.emulation_flags & X86_EMU_VGA))
>
> #define has_viommu(d) (!!((d)->arch.emulation_flags & X86_EMU_IOMMU))
>
> #define has_vpit(d) (!!((d)->arch.emulation_flags & X86_EMU_PIT))
>
> #define has_pirq(d) (!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
>
> diff --git a/xen/arch/x86/include/asm/hvm/io.h 
> b/xen/arch/x86/include/asm/hvm/io.h
> index f2b8431facb0..32a2490fbcb2 100644
> --- a/xen/arch/x86/include/asm/hvm/io.h
> +++ b/xen/arch/x86/include/asm/hvm/io.h
> @@ -108,7 +108,11 @@ struct vpci_arch_msix_entry {
> int pirq;
> };
>
> +#ifdef CONFIG_X86_STDVGA
> void stdvga_init(struct domain *d);
> +#else
> +static inline void stdvga_init(struct domain *d) {}
> +#endif
>
> extern void hvm_dpci_msi_eoi(struct domain *d, int vector);
>
> --
> 2.34.1