[PATCH 0/2] pvg: clean up Kconfig

[PATCH 0/2] pvg: clean up Kconfig

[Paolo Bonzini]

Do not enable Apple ParavirtualizedGraphics on cross-architecture targets,
where it is not supported by the drivers, and add an option to configure it
out just like most others external dependencies.

Only compile-tested on non-Mac for now, but CI is in progress.

Paolo

Paolo Bonzini (2):
  pvg: do not enable it on cross-architecture targets
  pvg: add option to configure it out

 meson.build                   | 16 +++++++++++++---
 Kconfig.host                  |  3 +++
 hw/display/Kconfig            |  4 ----
 hw/display/meson.build        |  9 +++------
 meson_options.txt             |  2 ++
 scripts/meson-buildoptions.sh |  3 +++
 6 files changed, 24 insertions(+), 13 deletions(-)

-- 
2.48.1

[PATCH 1/2] pvg: do not enable it on cross-architecture targets

[Paolo Bonzini]

PVG is not cross-architecture; the PVG guest drivers with x86-64 macOS do not give
useful results with the aarch64 macOS host PVG framework, and vice versa.
To express this repurpose CONFIG_MAC_PVG, making it true only if the target has
the same architecture as the host.  Furthermore, remove apple-gfx.m unless
one of the devices is actually present.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build            | 6 ++++++
 Kconfig.host           | 3 +++
 hw/display/Kconfig     | 4 ----
 hw/display/meson.build | 9 +++------
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/meson.build b/meson.build
index 0ee79c664d3..ad2c6b61930 100644
--- a/meson.build
+++ b/meson.build
@@ -3367,6 +3367,12 @@ foreach target : target_dirs
     target_kconfig += 'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y'
     target_kconfig += 'CONFIG_TARGET_BIG_ENDIAN=' + config_target['TARGET_BIG_ENDIAN']
 
+    # PVG is not cross-architecture.  Use accelerator_targets as a proxy to
+    # figure out which target can support PVG on this host
+    if pvg.found() and target in accelerator_targets.get('CONFIG_HVF', [])
+      target_kconfig += 'CONFIG_MAC_PVG=y'
+    endif
+
     config_input = meson.get_external_property(target, 'default')
     config_devices_mak = target + '-config-devices.mak'
     config_devices_mak = configure_file(
diff --git a/Kconfig.host b/Kconfig.host
index 842cbe0d6c5..933425c74b4 100644
--- a/Kconfig.host
+++ b/Kconfig.host
@@ -61,3 +61,6 @@ config HV_BALLOON_POSSIBLE
 
 config HAVE_RUST
     bool
+
+config MAC_PVG
+    bool
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 2b53dfd7d26..1e95ab28ef4 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -141,10 +141,6 @@ config XLNX_DISPLAYPORT
 config DM163
     bool
 
-config MAC_PVG
-    bool
-    default y
-
 config MAC_PVG_MMIO
     bool
     depends on MAC_PVG && AARCH64
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 94f4f05d36f..b9bdf219103 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -61,12 +61,9 @@ system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
 
 system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
 
-if host_os == 'darwin'
-  system_ss.add(when: 'CONFIG_MAC_PVG',         if_true: [files('apple-gfx.m'), pvg, metal])
-  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx-pci.m'), pvg, metal])
-  if cpu == 'aarch64'
-    system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',  if_true: [files('apple-gfx-mmio.m'), pvg, metal])
-  endif
+if pvg.found()
+  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx.m', 'apple-gfx-pci.m'), pvg, metal])
+  system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',    if_true: [files('apple-gfx.m', 'apple-gfx-mmio.m'), pvg, metal])
 endif
 
 if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
-- 
2.48.1

[PATCH 2/2] pvg: add option to configure it out

[Paolo Bonzini]

... and also to require it (--enable-pvg).  While at it, unify the dependency()
call for pvg and metal, which simplifies the logic a bit.

Note that all other Apple frameworks are either required or always-present,
therefore do not add them to the summary in the same way as PVG.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                   |  8 +++++---
 hw/display/meson.build        |  4 ++--
 meson_options.txt             |  2 ++
 scripts/meson-buildoptions.sh |  3 +++
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index ad2c6b61930..ad8e7c1890e 100644
--- a/meson.build
+++ b/meson.build
@@ -821,7 +821,6 @@ version_res = []
 coref = []
 iokit = []
 pvg = not_found
-metal = []
 emulator_link_args = []
 midl = not_found
 widl = not_found
@@ -843,8 +842,8 @@ elif host_os == 'darwin'
   coref = dependency('appleframeworks', modules: 'CoreFoundation')
   iokit = dependency('appleframeworks', modules: 'IOKit', required: false)
   host_dsosuf = '.dylib'
-  pvg = dependency('appleframeworks', modules: 'ParavirtualizedGraphics')
-  metal = dependency('appleframeworks', modules: 'Metal')
+  pvg = dependency('appleframeworks', modules: ['ParavirtualizedGraphics', 'Metal'],
+                   required: get_option('pvg'))
 elif host_os == 'sunos'
   socket = [cc.find_library('socket'),
             cc.find_library('nsl'),
@@ -4846,6 +4847,9 @@ summary_info += {'libdw':             libdw}
 if host_os == 'freebsd'
   summary_info += {'libinotify-kqueue': inotify}
 endif
+if host_os == 'darwin'
+  summary_info += {'ParavirtualizedGraphics support': pvg}
+endif
 summary(summary_info, bool_yn: true, section: 'Dependencies')
 
 if host_arch == 'unknown'
diff --git a/hw/display/meson.build b/hw/display/meson.build
index b9bdf219103..9d82fbc9c89 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -62,8 +62,8 @@ system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
 system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
 
 if pvg.found()
-  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx.m', 'apple-gfx-pci.m'), pvg, metal])
-  system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',    if_true: [files('apple-gfx.m', 'apple-gfx-mmio.m'), pvg, metal])
+  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx.m', 'apple-gfx-pci.m'), pvg])
+  system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',    if_true: [files('apple-gfx.m', 'apple-gfx-mmio.m'), pvg])
 endif
 
 if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
diff --git a/meson_options.txt b/meson_options.txt
index 5eeaf3eee5c..59d973bca00 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -198,6 +198,8 @@ option('lzfse', type : 'feature', value : 'auto',
        description: 'lzfse support for DMG images')
 option('lzo', type : 'feature', value : 'auto',
        description: 'lzo compression support')
+option('pvg', type: 'feature', value: 'auto',
+       description: 'macOS paravirtualized graphics support')
 option('rbd', type : 'feature', value : 'auto',
        description: 'Ceph block device driver')
 option('opengl', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index a8066aab037..3e8e00852b2 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -168,6 +168,7 @@ meson_options_help() {
   printf "%s\n" '  pixman          pixman support'
   printf "%s\n" '  plugins         TCG plugins via shared library loading'
   printf "%s\n" '  png             PNG support with libpng'
+  printf "%s\n" '  pvg             macOS paravirtualized graphics support'
   printf "%s\n" '  qatzip          QATzip compression support'
   printf "%s\n" '  qcow1           qcow1 image format support'
   printf "%s\n" '  qed             qed image format support'
@@ -436,6 +437,8 @@ _meson_option_parse() {
     --enable-png) printf "%s" -Dpng=enabled ;;
     --disable-png) printf "%s" -Dpng=disabled ;;
     --prefix=*) quote_sh "-Dprefix=$2" ;;
+    --enable-pvg) printf "%s" -Dpvg=enabled ;;
+    --disable-pvg) printf "%s" -Dpvg=disabled ;;
     --enable-qatzip) printf "%s" -Dqatzip=enabled ;;
     --disable-qatzip) printf "%s" -Dqatzip=disabled ;;
     --enable-qcow1) printf "%s" -Dqcow1=enabled ;;
-- 
2.48.1

Re: [PATCH 1/2] pvg: do not enable it on cross-architecture targets

[Philippe Mathieu-Daudé]

On 20/2/25 14:33, Paolo Bonzini wrote:
> PVG is not cross-architecture; the PVG guest drivers with x86-64 macOS do not give
> useful results with the aarch64 macOS host PVG framework, and vice versa.
> To express this repurpose CONFIG_MAC_PVG, making it true only if the target has
> the same architecture as the host.  Furthermore, remove apple-gfx.m unless
> one of the devices is actually present.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   meson.build            | 6 ++++++
>   Kconfig.host           | 3 +++
>   hw/display/Kconfig     | 4 ----
>   hw/display/meson.build | 9 +++------
>   4 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 0ee79c664d3..ad2c6b61930 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3367,6 +3367,12 @@ foreach target : target_dirs
>       target_kconfig += 'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y'
>       target_kconfig += 'CONFIG_TARGET_BIG_ENDIAN=' + config_target['TARGET_BIG_ENDIAN']
>   
> +    # PVG is not cross-architecture.  Use accelerator_targets as a proxy to
> +    # figure out which target can support PVG on this host
> +    if pvg.found() and target in accelerator_targets.get('CONFIG_HVF', [])
> +      target_kconfig += 'CONFIG_MAC_PVG=y'
> +    endif
> +
>       config_input = meson.get_external_property(target, 'default')
>       config_devices_mak = target + '-config-devices.mak'
>       config_devices_mak = configure_file(
> diff --git a/Kconfig.host b/Kconfig.host
> index 842cbe0d6c5..933425c74b4 100644
> --- a/Kconfig.host
> +++ b/Kconfig.host
> @@ -61,3 +61,6 @@ config HV_BALLOON_POSSIBLE
>   
>   config HAVE_RUST
>       bool
> +
> +config MAC_PVG
> +    bool
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 2b53dfd7d26..1e95ab28ef4 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -141,10 +141,6 @@ config XLNX_DISPLAYPORT
>   config DM163
>       bool
>   
> -config MAC_PVG
> -    bool
> -    default y
> -
>   config MAC_PVG_MMIO
>       bool
>       depends on MAC_PVG && AARCH64

Hmm what about keeping a MAC_PVG_COMMON instead:

-- >8 --
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -141,15 +141,17 @@ config XLNX_DISPLAYPORT
  config DM163
      bool

-config MAC_PVG
+config MAC_PVG_COMMON
      bool
-    default y
+    depends on MAC_PVG

  config MAC_PVG_MMIO
      bool
-    depends on MAC_PVG && AARCH64
+    depends on AARCH64
+    select MAC_PVG_COMMON

  config MAC_PVG_PCI
      bool
-    depends on MAC_PVG && PCI
+    depends on PCI
+    select MAC_PVG_COMMON
      default y if PCI_DEVICES
---

> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index 94f4f05d36f..b9bdf219103 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -61,12 +61,9 @@ system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
>   
>   system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
>   
> -if host_os == 'darwin'
> -  system_ss.add(when: 'CONFIG_MAC_PVG',         if_true: [files('apple-gfx.m'), pvg, metal])
> -  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx-pci.m'), pvg, metal])
> -  if cpu == 'aarch64'
> -    system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',  if_true: [files('apple-gfx-mmio.m'), pvg, metal])
> -  endif
> +if pvg.found()
> +  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx.m', 'apple-gfx-pci.m'), pvg, metal])
> +  system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',    if_true: [files('apple-gfx.m', 'apple-gfx-mmio.m'), pvg, metal])
>   endif

Directly using here:

-- >8 --
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 94f4f05d36f..f636ca0999c 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -61,13 +61,9 @@ system_ss.add(when: 'CONFIG_ARTIST', if_true: 
files('artist.c'))

  system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 
'ati_2d.c', 'ati_dbg.c'), pixman])

-if host_os == 'darwin'
-  system_ss.add(when: 'CONFIG_MAC_PVG',         if_true: 
[files('apple-gfx.m'), pvg, metal])
-  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: 
[files('apple-gfx-pci.m'), pvg, metal])
-  if cpu == 'aarch64'
-    system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',  if_true: 
[files('apple-gfx-mmio.m'), pvg, metal])
-  endif
-endif
+system_ss.add(when: 'CONFIG_MAC_PVG_COMMON', if_true: 
[files('apple-gfx.m'), pvg, metal])
+system_ss.add(when: 'CONFIG_MAC_PVG_PCI',    if_true: 
[files('apple-gfx-pci.m'), pvg, metal])
+system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',   if_true: 
[files('apple-gfx-mmio.m'), pvg, metal])

  if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
    virtio_gpu_ss = ss.source_set()
---

?

Re: [PATCH 1/2] pvg: do not enable it on cross-architecture targets

[Paolo Bonzini]

On 2/20/25 16:19, Philippe Mathieu-Daudé wrote:
> Hmm what about keeping a MAC_PVG_COMMON instead:
> 
> -- >8 --
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -141,15 +141,17 @@ config XLNX_DISPLAYPORT
>   config DM163
>       bool
> 
> -config MAC_PVG
> +config MAC_PVG_COMMON
>       bool
> -    default y
> +    depends on MAC_PVG
> 
>   config MAC_PVG_MMIO
>       bool
> -    depends on MAC_PVG && AARCH64
> +    depends on AARCH64

It's possible, but the two "depends on MAC_PVG" cannot be removed here,
because otherwise MAC_PVG_PCI would be selected by "default y".  The
only reason to do so, in my opinion, was if one wanted to build pvg as
a module, otherwise it's just (a handful) more lines of code with no
particular benefit.

> -if host_os == 'darwin'
> -  system_ss.add(when: 'CONFIG_MAC_PVG',         if_true: [files('apple- 
> gfx.m'), pvg, metal])
> -  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple- 
> gfx-pci.m'), pvg, metal])
> -  if cpu == 'aarch64'
> -    system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',  if_true: [files('apple- 
> gfx-mmio.m'), pvg, metal])
> -  endif
> -endif
> +system_ss.add(when: 'CONFIG_MAC_PVG_COMMON', if_true: [files('apple- 
> gfx.m'), pvg, metal])
> +system_ss.add(when: 'CONFIG_MAC_PVG_PCI',    if_true: [files('apple- 
> gfx-pci.m'), pvg, metal])
> +system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',   if_true: [files('apple- 
> gfx-mmio.m'), pvg, metal])

Removing the "if" is independent of whether you keep CONFIG_MAC_PVG_COMMON.
I can squash that in patch 2:

-if pvg.found()
-  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple-gfx.m', 'apple-gfx-pci.m'), pvg])
-  system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',    if_true: [files('apple-gfx.m', 'apple-gfx-mmio.m'), pvg])
-endif
+system_ss.add(when: [pvg, 'CONFIG_MAC_PVG_PCI'],     if_true: [files('apple-gfx.m', 'apple-gfx-pci.m')])
+system_ss.add(when: [pvg, 'CONFIG_MAC_PVG_MMIO'],    if_true: [files('apple-gfx.m', 'apple-gfx-mmio.m')])

(you need to keep the conditional, whether as "if" or "when:"; otherwise,
because Meson cannot know that the .m files will never be built, it
complains that you don't have an Objective-C compiler).

Paolo

Re: [PATCH 1/2] pvg: do not enable it on cross-architecture targets

[Philippe Mathieu-Daudé]

On 21/2/25 18:01, Paolo Bonzini wrote:
> On 2/20/25 16:19, Philippe Mathieu-Daudé wrote:
>> Hmm what about keeping a MAC_PVG_COMMON instead:
>>
>> -- >8 --
>> --- a/hw/display/Kconfig
>> +++ b/hw/display/Kconfig
>> @@ -141,15 +141,17 @@ config XLNX_DISPLAYPORT
>>   config DM163
>>       bool
>>
>> -config MAC_PVG
>> +config MAC_PVG_COMMON
>>       bool
>> -    default y
>> +    depends on MAC_PVG
>>
>>   config MAC_PVG_MMIO
>>       bool
>> -    depends on MAC_PVG && AARCH64
>> +    depends on AARCH64
> 
> It's possible, but the two "depends on MAC_PVG" cannot be removed here,
> because otherwise MAC_PVG_PCI would be selected by "default y".  The
> only reason to do so, in my opinion, was if one wanted to build pvg as
> a module, otherwise it's just (a handful) more lines of code with no
> particular benefit.
> 
>> -if host_os == 'darwin'
>> -  system_ss.add(when: 'CONFIG_MAC_PVG',         if_true: 
>> [files('apple- gfx.m'), pvg, metal])
>> -  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: 
>> [files('apple- gfx-pci.m'), pvg, metal])
>> -  if cpu == 'aarch64'
>> -    system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',  if_true: 
>> [files('apple- gfx-mmio.m'), pvg, metal])
>> -  endif
>> -endif
>> +system_ss.add(when: 'CONFIG_MAC_PVG_COMMON', if_true: [files('apple- 
>> gfx.m'), pvg, metal])
>> +system_ss.add(when: 'CONFIG_MAC_PVG_PCI',    if_true: [files('apple- 
>> gfx-pci.m'), pvg, metal])
>> +system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',   if_true: [files('apple- 
>> gfx-mmio.m'), pvg, metal])
> 
> Removing the "if" is independent of whether you keep CONFIG_MAC_PVG_COMMON.
> I can squash that in patch 2:
> 
> -if pvg.found()
> -  system_ss.add(when: 'CONFIG_MAC_PVG_PCI',     if_true: [files('apple- 
> gfx.m', 'apple-gfx-pci.m'), pvg])
> -  system_ss.add(when: 'CONFIG_MAC_PVG_MMIO',    if_true: [files('apple- 
> gfx.m', 'apple-gfx-mmio.m'), pvg])
> -endif
> +system_ss.add(when: [pvg, 'CONFIG_MAC_PVG_PCI'],     if_true: 
> [files('apple-gfx.m', 'apple-gfx-pci.m')])
> +system_ss.add(when: [pvg, 'CONFIG_MAC_PVG_MMIO'],    if_true: 
> [files('apple-gfx.m', 'apple-gfx-mmio.m')])
> 
> (you need to keep the conditional, whether as "if" or "when:"; otherwise,
> because Meson cannot know that the .m files will never be built, it
> complains that you don't have an Objective-C compiler).

Ah, I haven't thought of that.

Series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>