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

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

Hi all,

On 2025/2/20 17:53, Jiqian Chen 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.
> 
I am sorry.
After sending my series, I just found out that there are v3 for this work.
https://lore.kernel.org/xen-devel/7a0ee883-8542-4e17-adeb-9c1d83f58657@xxxxxxxx/
And it seems that the v3 has no other implementation-related comment, just 
waiting for x86 Maintainers' opinion.

> 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"
> +
>  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);
>  

-- 
Best regards,
Jiqian Chen.