Re: [XEN PATCH v2 2/2] x86/hvm: introduce config option for stdvga emulation

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 6 Nov 2024, Sergiy Kibrik wrote:
> Introduce config option X86_HVM_STDVGA and make stdvga emulation driver
> configurable so it can be disabled on systems that don't need it.
> 
> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> changes in v2:
>  - updated description
>  - renamed config option X86_STDVGA -> X86_HVM_STDVGA & moved related
>    Kconfig changes to this patch
>  - reverted changes to has_vvga() macro
>  - moved emulation_flags_ok() checks to this patch
> ---
>  xen/arch/x86/Kconfig              | 10 ++++++++++
>  xen/arch/x86/domain.c             |  4 ++--
>  xen/arch/x86/hvm/Makefile         |  2 +-
>  xen/arch/x86/include/asm/domain.h |  8 ++++++--
>  xen/arch/x86/include/asm/hvm/io.h |  4 ++++
>  5 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index ed0ece85c7..35c8ace8ef 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -155,6 +155,16 @@ config X86_HVM_PMTIMER
>         Build pmtimer driver that emulates ACPI PM timer for HVM/PVH guests.
>  
>         If unsure, say Y.
> +
> +config X86_HVM_STDVGA
> +     bool "Standard VGA card emulation support"
> +     default y
> +     help
> +       Build stdvga driver that emulates standard VGA card with VESA BIOS
> +       Extensions for HVM/PVH guests.
> +
> +       If unsure, say Y.
> +
>  endmenu
>  
>  config XEN_SHSTK
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index b340818ee2..aefa1fc136 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -742,9 +742,9 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  
>  static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
>  {
> -    const uint32_t disabled_emu_mask = X86_EMU_PM;
> +    const uint32_t disabled_emu_mask = X86_EMU_PM | X86_EMU_VGA;
>  
> -#if defined(CONFIG_X86_HVM_PMTIMER)
> +#if defined(CONFIG_X86_HVM_PMTIMER) && defined(CONFIG_X86_HVM_STDVGA)
>      /* This doesn't catch !CONFIG_HVM case but it is better than nothing */
>      BUILD_BUG_ON(X86_EMU_ALL != XEN_X86_EMU_ALL);
>  #endif

You need to remove this part, following your reply to the previous patch

With that modification:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 3af8963218..80ec425aa8 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -22,7 +22,7 @@ obj-$(CONFIG_X86_HVM_PMTIMER) += pmtimer.o
>  obj-y += quirks.o
>  obj-y += rtc.o
>  obj-y += save.o
> -obj-y += stdvga.o
> +obj-$(CONFIG_X86_HVM_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 8550473997..106b438779 100644
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -466,7 +466,6 @@ struct arch_domain
>  #define X86_EMU_RTC      XEN_X86_EMU_RTC
>  #define X86_EMU_IOAPIC   XEN_X86_EMU_IOAPIC
>  #define X86_EMU_PIC      XEN_X86_EMU_PIC
> -#define X86_EMU_VGA      XEN_X86_EMU_VGA
>  #define X86_EMU_IOMMU    XEN_X86_EMU_IOMMU
>  #define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
>  #define X86_EMU_VPCI     XEN_X86_EMU_VPCI
> @@ -476,7 +475,6 @@ struct arch_domain
>  #define X86_EMU_RTC      0
>  #define X86_EMU_IOAPIC   0
>  #define X86_EMU_PIC      0
> -#define X86_EMU_VGA      0
>  #define X86_EMU_IOMMU    0
>  #define X86_EMU_USE_PIRQ 0
>  #define X86_EMU_VPCI     0
> @@ -488,6 +486,12 @@ struct arch_domain
>  #define X86_EMU_PM       0
>  #endif
>  
> +#ifdef CONFIG_X86_HVM_STDVGA
> +#define X86_EMU_VGA      XEN_X86_EMU_VGA
> +#else
> +#define X86_EMU_VGA      0
> +#endif
> +
>  #define X86_EMU_PIT     XEN_X86_EMU_PIT
>  
>  /* This must match XEN_X86_EMU_ALL in xen.h */
> diff --git a/xen/arch/x86/include/asm/hvm/io.h 
> b/xen/arch/x86/include/asm/hvm/io.h
> index f2b8431fac..c02fad876c 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_HVM_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.25.1
>