[PATCH] qemu: Add support for -device ati-vga (models: rage128p and rv100)

Steven Newbury posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201007125834.224913-1-steve@snewbury.org.uk
docs/schemas/domaincommon.rng                     | 2 ++
src/conf/domain_conf.c                            | 7 +++++++
src/conf/domain_conf.h                            | 2 ++
src/qemu/qemu_capabilities.c                      | 6 ++++++
src/qemu/qemu_capabilities.h                      | 1 +
src/qemu/qemu_command.c                           | 6 ++++++
src/qemu/qemu_domain_address.c                    | 2 ++
src/qemu/qemu_process.c                           | 2 ++
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml   | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml  | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml  | 1 +
12 files changed, 32 insertions(+)
[PATCH] qemu: Add support for -device ati-vga (models: rage128p and rv100)
Posted by Steven Newbury 3 years, 6 months ago
QEMU since 5.0 has two new video devices rage128p and rv100.
These emulate ATI Rage128 Pro and Radeon GPUs respectively,
at least to some extent.  This can be useful for OS without
virtualisation drivers or support for VBE or the old Cirrus
emulation.

Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
---
 docs/schemas/domaincommon.rng                     | 2 ++
 src/conf/domain_conf.c                            | 7 +++++++
 src/conf/domain_conf.h                            | 2 ++
 src/qemu/qemu_capabilities.c                      | 6 ++++++
 src/qemu/qemu_capabilities.h                      | 1 +
 src/qemu/qemu_command.c                           | 6 ++++++
 src/qemu/qemu_domain_address.c                    | 2 ++
 src/qemu/qemu_process.c                           | 2 ++
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml  | 1 +
 12 files changed, 32 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7d4b105981..d52eb293a8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3915,6 +3915,8 @@
                 <value>none</value>
                 <value>bochs</value>
                 <value>ramfb</value>
+                <value>rage128p</value>
+                <value>rv100</value>
               </choice>
             </attribute>
             <group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c003b5c030..5d8b6a7611 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -811,6 +811,8 @@ VIR_ENUM_IMPL(virDomainVideo,
               "none",
               "bochs",
               "ramfb",
+              "rage128p",
+              "rv100",
 );
 
 VIR_ENUM_IMPL(virDomainVideoVGAConf,
@@ -16016,6 +16018,11 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
         /* QEMU use 64M as the minimal video memory for qxl device */
         return 64 * 1024;
 
+    case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
+    case VIR_DOMAIN_VIDEO_TYPE_RV100:
+        /* The real world devices started at 16M */
+        return 16 * 1024;
+
     case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
     case VIR_DOMAIN_VIDEO_TYPE_VBOX:
     case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 450686dfb5..b12c7ab00d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1502,6 +1502,8 @@ typedef enum {
     VIR_DOMAIN_VIDEO_TYPE_NONE,
     VIR_DOMAIN_VIDEO_TYPE_BOCHS,
     VIR_DOMAIN_VIDEO_TYPE_RAMFB,
+    VIR_DOMAIN_VIDEO_TYPE_RAGE128P,
+    VIR_DOMAIN_VIDEO_TYPE_RV100,
 
     VIR_DOMAIN_VIDEO_TYPE_LAST
 } virDomainVideoType;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 38b901a6c4..0b2bd24619 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -600,6 +600,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
               /* 380 */
               "usb-host.hostdevice",
+              "ati-vga",
     );
 
 
@@ -1325,6 +1326,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
     { "tcg-accel", QEMU_CAPS_TCG },
     { "pvscsi", QEMU_CAPS_SCSI_PVSCSI },
     { "spapr-tpm-proxy", QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY },
+    { "ati-vga", QEMU_CAPS_DEVICE_ATI_VGA },
 };
 
 
@@ -6127,6 +6129,10 @@ virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps,
         VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_BOCHS);
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))
         VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RAMFB);
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ATI_VGA)) {
+        VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RAGE128P);
+        VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RV100);
+    }
 }
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 107056ba17..e316bd1532 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -580,6 +580,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
 
     /* 380 */
     QEMU_CAPS_USB_HOST_HOSTDEVICE, /* -device usb-host.hostdevice */
+    QEMU_CAPS_DEVICE_ATI_VGA, /* -device ati-vga */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 476cf6972e..b501b8e3e2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -101,6 +101,8 @@ VIR_ENUM_IMPL(qemuVideo,
               "" /* 'none' doesn't make sense here */,
               "bochs-display",
               "", /* ramfb can't be used with -vga */
+              "rage128p",
+              "rv100",
 );
 
 VIR_ENUM_DECL(qemuDeviceVideo);
@@ -120,6 +122,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo,
               "" /* 'none' doesn't make sense here */,
               "bochs-display",
               "ramfb",
+              "ati-vga" /* (rage128p) */,
+              "ati-vga" /* (rv100) */,
 );
 
 VIR_ENUM_DECL(qemuDeviceVideoSecondary);
@@ -139,6 +143,8 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary,
               "" /* 'none' doesn't make sense here */,
               "" /* no secondary device for bochs */,
               "" /* no secondary device for ramfb */,
+              "" /* no secondary device for ati-vga? (rage128p) */,
+              "" /* no secondary device for ati-vga? (rv100) */,
 );
 
 VIR_ENUM_IMPL(qemuSoundCodec,
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index dd87915a97..d8273af92f 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -964,6 +964,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
         case VIR_DOMAIN_VIDEO_TYPE_VBOX:
         case VIR_DOMAIN_VIDEO_TYPE_QXL:
         case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
+        case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
+        case VIR_DOMAIN_VIDEO_TYPE_RV100:
             return pciFlags;
 
         case VIR_DOMAIN_VIDEO_TYPE_BOCHS:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6b5de29fdb..09aac14430 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3111,6 +3111,8 @@ qemuProcessUpdateVideoRamSize(virQEMUDriverPtr driver,
         case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
         case VIR_DOMAIN_VIDEO_TYPE_XEN:
         case VIR_DOMAIN_VIDEO_TYPE_VBOX:
+        case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
+        case VIR_DOMAIN_VIDEO_TYPE_RV100:
         case VIR_DOMAIN_VIDEO_TYPE_LAST:
             break;
         }
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
index 928af2a01c..30609085e1 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
@@ -199,6 +199,7 @@
   <flag name='migration-param.xbzrle-cache-size'/>
   <flag name='numa.hmat'/>
   <flag name='blockdev-hostdev-scsi'/>
+  <flag name='ati-vga'/>
   <version>5000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>61700241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
index e8668a25a9..254da198b2 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
@@ -208,6 +208,7 @@
   <flag name='spapr-tpm-proxy'/>
   <flag name='numa.hmat'/>
   <flag name='blockdev-hostdev-scsi'/>
+  <flag name='ati-vga'/>
   <version>5000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>42900241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
index 546b9b0422..ae6e66679b 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
@@ -243,6 +243,7 @@
   <flag name='intel-iommu.aw-bits'/>
   <flag name='numa.hmat'/>
   <flag name='blockdev-hostdev-scsi'/>
+  <flag name='ati-vga'/>
   <version>5000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100241</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
index 987beb965e..33929735f5 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
@@ -243,6 +243,7 @@
   <flag name='numa.hmat'/>
   <flag name='blockdev-hostdev-scsi'/>
   <flag name='usb-host.hostdevice'/>
+  <flag name='ati-vga'/>
   <version>5001000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100242</microcodeVersion>
-- 
2.28.0

Re: [PATCH] qemu: Add support for -device ati-vga (models: rage128p and rv100)
Posted by Ján Tomko 3 years, 6 months ago
On a Wednesday in 2020, Steven Newbury wrote:
>QEMU since 5.0 has two new video devices rage128p and rv100.

Are you sure about the QEMU version?

The way you check it (presence of 'ati-vga') says it's available
since 4.0.0.

commit 862b4a291dcf143fdb227e97feb7fd45e6466aca
     hw/display: Add basic ATI VGA emulation
git describe: v3.1.0-2677-g862b4a291d contains: v4.0.0-rc0~39^2~1

If they were present in QEMU before 5.0, but unusable, we
need to find a different way to set the capability.

>These emulate ATI Rage128 Pro and Radeon GPUs respectively,
>at least to some extent.  This can be useful for OS without
>virtualisation drivers or support for VBE or the old Cirrus
>emulation.
>
>Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
>---
> docs/schemas/domaincommon.rng                     | 2 ++
> src/conf/domain_conf.c                            | 7 +++++++
> src/conf/domain_conf.h                            | 2 ++

Ideally, this would be split in multiple patches:
* The QEMU capablity addition:
   src/qemu/qemu_capabilities*
   tests/qemucapabilitiesdata*
* XML parser/formatter:
   docs/schemas/
   src/conf/*
   tests/*xml2xml*
* QEMU command line formatter
   the rest

But this is an enum addition, so our compile warnings require
that the two last steps are squashed together. But having the
capability separate means less changes in the main patch
and easier rebasing if multiple people add a capability.

> src/qemu/qemu_capabilities.c                      | 6 ++++++
> src/qemu/qemu_capabilities.h                      | 1 +
> src/qemu/qemu_command.c                           | 6 ++++++
> src/qemu/qemu_domain_address.c                    | 2 ++
> src/qemu/qemu_process.c                           | 2 ++
> tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
> tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml   | 1 +
> tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml  | 1 +
> tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml  | 1 +

Missing xml2xml test is not such a big deal for just another enum value,
but please take a look at qemuxml2argvtest and add a test case
to verify that it actually creates the commandline you want.

Once you create a domain XML using the device in qemuxml2argvdata,
it will automatically be checked against the schema thans to
virschematest.

By setting VIR_TEST_REGENERATE_OUTPUT=1 before running the test,
it will overwrite the expected output file with what it actually
expects, so you don't have to write the output by hand.

> 12 files changed, 32 insertions(+)
>
>diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>index 7d4b105981..d52eb293a8 100644
>--- a/docs/schemas/domaincommon.rng
>+++ b/docs/schemas/domaincommon.rng
>@@ -3915,6 +3915,8 @@
>                 <value>none</value>
>                 <value>bochs</value>
>                 <value>ramfb</value>
>+                <value>rage128p</value>
>+                <value>rv100</value>
>               </choice>
>             </attribute>
>             <group>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index c003b5c030..5d8b6a7611 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -811,6 +811,8 @@ VIR_ENUM_IMPL(virDomainVideo,
>               "none",
>               "bochs",
>               "ramfb",
>+              "rage128p",
>+              "rv100",
> );
>
> VIR_ENUM_IMPL(virDomainVideoVGAConf,
>@@ -16016,6 +16018,11 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
>         /* QEMU use 64M as the minimal video memory for qxl device */
>         return 64 * 1024;
>
>+    case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
>+    case VIR_DOMAIN_VIDEO_TYPE_RV100:
>+        /* The real world devices started at 16M */
>+        return 16 * 1024;
>+
>     case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
>     case VIR_DOMAIN_VIDEO_TYPE_VBOX:
>     case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index 450686dfb5..b12c7ab00d 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -1502,6 +1502,8 @@ typedef enum {
>     VIR_DOMAIN_VIDEO_TYPE_NONE,
>     VIR_DOMAIN_VIDEO_TYPE_BOCHS,
>     VIR_DOMAIN_VIDEO_TYPE_RAMFB,
>+    VIR_DOMAIN_VIDEO_TYPE_RAGE128P,
>+    VIR_DOMAIN_VIDEO_TYPE_RV100,
>
>     VIR_DOMAIN_VIDEO_TYPE_LAST
> } virDomainVideoType;
>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>index 38b901a6c4..0b2bd24619 100644
>--- a/src/qemu/qemu_capabilities.c
>+++ b/src/qemu/qemu_capabilities.c
>@@ -600,6 +600,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
>
>               /* 380 */
>               "usb-host.hostdevice",
>+              "ati-vga",
>     );
>
>
>@@ -1325,6 +1326,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>     { "tcg-accel", QEMU_CAPS_TCG },
>     { "pvscsi", QEMU_CAPS_SCSI_PVSCSI },
>     { "spapr-tpm-proxy", QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY },
>+    { "ati-vga", QEMU_CAPS_DEVICE_ATI_VGA },
> };
>
>
>@@ -6127,6 +6129,10 @@ virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps,
>         VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_BOCHS);
>     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))
>         VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RAMFB);
>+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ATI_VGA)) {
>+        VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RAGE128P);
>+        VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RV100);
>+    }
> }
>
>
>diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>index 107056ba17..e316bd1532 100644
>--- a/src/qemu/qemu_capabilities.h
>+++ b/src/qemu/qemu_capabilities.h
>@@ -580,6 +580,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>
>     /* 380 */
>     QEMU_CAPS_USB_HOST_HOSTDEVICE, /* -device usb-host.hostdevice */
>+    QEMU_CAPS_DEVICE_ATI_VGA, /* -device ati-vga */
>
>     QEMU_CAPS_LAST /* this must always be the last item */
> } virQEMUCapsFlags;
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 476cf6972e..b501b8e3e2 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -101,6 +101,8 @@ VIR_ENUM_IMPL(qemuVideo,
>               "" /* 'none' doesn't make sense here */,
>               "bochs-display",
>               "", /* ramfb can't be used with -vga */
>+              "rage128p",
>+              "rv100",
> );
>
> VIR_ENUM_DECL(qemuDeviceVideo);
>@@ -120,6 +122,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo,
>               "" /* 'none' doesn't make sense here */,
>               "bochs-display",
>               "ramfb",
>+              "ati-vga" /* (rage128p) */,
>+              "ati-vga" /* (rv100) */,
> );
>
> VIR_ENUM_DECL(qemuDeviceVideoSecondary);
>@@ -139,6 +143,8 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary,
>               "" /* 'none' doesn't make sense here */,
>               "" /* no secondary device for bochs */,
>               "" /* no secondary device for ramfb */,
>+              "" /* no secondary device for ati-vga? (rage128p) */,
>+              "" /* no secondary device for ati-vga? (rv100) */,

IIUC there can only be one vga device, so this seems correct.

> );
>
> VIR_ENUM_IMPL(qemuSoundCodec,
>diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>index dd87915a97..d8273af92f 100644
>--- a/src/qemu/qemu_domain_address.c
>+++ b/src/qemu/qemu_domain_address.c
>@@ -964,6 +964,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>         case VIR_DOMAIN_VIDEO_TYPE_VBOX:
>         case VIR_DOMAIN_VIDEO_TYPE_QXL:
>         case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
>+        case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
>+        case VIR_DOMAIN_VIDEO_TYPE_RV100:
>             return pciFlags;
>
>         case VIR_DOMAIN_VIDEO_TYPE_BOCHS:
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 6b5de29fdb..09aac14430 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -3111,6 +3111,8 @@ qemuProcessUpdateVideoRamSize(virQEMUDriverPtr driver,
>         case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
>         case VIR_DOMAIN_VIDEO_TYPE_XEN:
>         case VIR_DOMAIN_VIDEO_TYPE_VBOX:
>+        case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
>+        case VIR_DOMAIN_VIDEO_TYPE_RV100:
>         case VIR_DOMAIN_VIDEO_TYPE_LAST:
>             break;
>         }
>diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
>index 928af2a01c..30609085e1 100644
>--- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
>+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml

[...]

These capabilities changes are incomplete.

VIR_TEST_REGENERATE_OUTPUT=1 build/tests/qemucapabilitiestest
assumes this is supported since QEMU 4.0

Jano
Re: [PATCH] qemu: Add support for -device ati-vga (models: rage128p and rv100)
Posted by Steven Newbury 3 years, 6 months ago
On Wed, 2020-10-07 at 19:09 +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Steven Newbury wrote:
> > QEMU since 5.0 has two new video devices rage128p and rv100.
> 
> Are you sure about the QEMU version?
> 
> The way you check it (presence of 'ati-vga') says it's available
> since 4.0.0.
> 
> commit 862b4a291dcf143fdb227e97feb7fd45e6466aca
>      hw/display: Add basic ATI VGA emulation
> git describe: v3.1.0-2677-g862b4a291d contains: v4.0.0-rc0~39^2~1
> 
> If they were present in QEMU before 5.0, but unusable, we
> need to find a different way to set the capability.
> 
I will check to see what the status was.  I have only tested it down to
5.0, but did have earlier versions in my patch before I sent it.

> > These emulate ATI Rage128 Pro and Radeon GPUs respectively,
> > at least to some extent.  This can be useful for OS without
> > virtualisation drivers or support for VBE or the old Cirrus
> > emulation.
> > 
> > Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
> > ---
> > docs/schemas/domaincommon.rng                     | 2 ++
> > src/conf/domain_conf.c                            | 7 +++++++
> > src/conf/domain_conf.h                            | 2 ++
> 
> Ideally, this would be split in multiple patches:
> * The QEMU capablity addition:
>    src/qemu/qemu_capabilities*
>    tests/qemucapabilitiesdata*
> * XML parser/formatter:
>    docs/schemas/
>    src/conf/*
>    tests/*xml2xml*
> * QEMU command line formatter
>    the rest
> 
> But this is an enum addition, so our compile warnings require
> that the two last steps are squashed together. But having the
> capability separate means less changes in the main patch
> and easier rebasing if multiple people add a capability.
> 
Okay

> > src/qemu/qemu_capabilities.c                      | 6 ++++++
> > src/qemu/qemu_capabilities.h                      | 1 +
> > src/qemu/qemu_command.c                           | 6 ++++++
> > src/qemu/qemu_domain_address.c                    | 2 ++
> > src/qemu/qemu_process.c                           | 2 ++
> > tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
> > tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml   | 1 +
> > tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml  | 1 +
> > tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml  | 1 +
> 
> Missing xml2xml test is not such a big deal for just another enum
> value,
> but please take a look at qemuxml2argvtest and add a test case
> to verify that it actually creates the commandline you want.
> 
Well it does work, so I guess it does!  But I will do as you say and
make sure all thest tests are working.

> Once you create a domain XML using the device in qemuxml2argvdata,
> it will automatically be checked against the schema thans to
> virschematest.
> 
> By setting VIR_TEST_REGENERATE_OUTPUT=1 before running the test,
> it will overwrite the expected output file with what it actually
> expects, so you don't have to write the output by hand.
> 
Handy

> > 12 files changed, 32 insertions(+)
> > 
> > diff --git a/docs/schemas/domaincommon.rng
> > b/docs/schemas/domaincommon.rng
> > index 7d4b105981..d52eb293a8 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -3915,6 +3915,8 @@
> >                 <value>none</value>
> >                 <value>bochs</value>
> >                 <value>ramfb</value>
> > +                <value>rage128p</value>
> > +                <value>rv100</value>
> >               </choice>
> >             </attribute>
> >             <group>
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index c003b5c030..5d8b6a7611 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -811,6 +811,8 @@ VIR_ENUM_IMPL(virDomainVideo,
> >               "none",
> >               "bochs",
> >               "ramfb",
> > +              "rage128p",
> > +              "rv100",
> > );
> > 
> > VIR_ENUM_IMPL(virDomainVideoVGAConf,
> > @@ -16016,6 +16018,11 @@ virDomainVideoDefaultRAM(const
> > virDomainDef *def,
> >         /* QEMU use 64M as the minimal video memory for qxl device
> > */
> >         return 64 * 1024;
> > 
> > +    case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
> > +    case VIR_DOMAIN_VIDEO_TYPE_RV100:
> > +        /* The real world devices started at 16M */
> > +        return 16 * 1024;
> > +
> >     case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
> >     case VIR_DOMAIN_VIDEO_TYPE_VBOX:
> >     case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 450686dfb5..b12c7ab00d 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -1502,6 +1502,8 @@ typedef enum {
> >     VIR_DOMAIN_VIDEO_TYPE_NONE,
> >     VIR_DOMAIN_VIDEO_TYPE_BOCHS,
> >     VIR_DOMAIN_VIDEO_TYPE_RAMFB,
> > +    VIR_DOMAIN_VIDEO_TYPE_RAGE128P,
> > +    VIR_DOMAIN_VIDEO_TYPE_RV100,
> > 
> >     VIR_DOMAIN_VIDEO_TYPE_LAST
> > } virDomainVideoType;
> > diff --git a/src/qemu/qemu_capabilities.c
> > b/src/qemu/qemu_capabilities.c
> > index 38b901a6c4..0b2bd24619 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -600,6 +600,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> > 
> >               /* 380 */
> >               "usb-host.hostdevice",
> > +              "ati-vga",
> >     );
> > 
> > 
> > @@ -1325,6 +1326,7 @@ struct virQEMUCapsStringFlags
> > virQEMUCapsObjectTypes[] = {
> >     { "tcg-accel", QEMU_CAPS_TCG },
> >     { "pvscsi", QEMU_CAPS_SCSI_PVSCSI },
> >     { "spapr-tpm-proxy", QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY },
> > +    { "ati-vga", QEMU_CAPS_DEVICE_ATI_VGA },
> > };
> > 
> > 
> > @@ -6127,6 +6129,10 @@
> > virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps,
> >         VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType,
> > VIR_DOMAIN_VIDEO_TYPE_BOCHS);
> >     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))
> >         VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType,
> > VIR_DOMAIN_VIDEO_TYPE_RAMFB);
> > +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ATI_VGA)) {
> > +        VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType,
> > VIR_DOMAIN_VIDEO_TYPE_RAGE128P);
> > +        VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType,
> > VIR_DOMAIN_VIDEO_TYPE_RV100);
> > +    }
> > }
> > 
> > 
> > diff --git a/src/qemu/qemu_capabilities.h
> > b/src/qemu/qemu_capabilities.h
> > index 107056ba17..e316bd1532 100644
> > --- a/src/qemu/qemu_capabilities.h
> > +++ b/src/qemu/qemu_capabilities.h
> > @@ -580,6 +580,7 @@ typedef enum { /* virQEMUCapsFlags grouping
> > marker for syntax-check */
> > 
> >     /* 380 */
> >     QEMU_CAPS_USB_HOST_HOSTDEVICE, /* -device usb-host.hostdevice
> > */
> > +    QEMU_CAPS_DEVICE_ATI_VGA, /* -device ati-vga */
> > 
> >     QEMU_CAPS_LAST /* this must always be the last item */
> > } virQEMUCapsFlags;
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 476cf6972e..b501b8e3e2 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -101,6 +101,8 @@ VIR_ENUM_IMPL(qemuVideo,
> >               "" /* 'none' doesn't make sense here */,
> >               "bochs-display",
> >               "", /* ramfb can't be used with -vga */
> > +              "rage128p",
> > +              "rv100",
> > );
> > 
> > VIR_ENUM_DECL(qemuDeviceVideo);
> > @@ -120,6 +122,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo,
> >               "" /* 'none' doesn't make sense here */,
> >               "bochs-display",
> >               "ramfb",
> > +              "ati-vga" /* (rage128p) */,
> > +              "ati-vga" /* (rv100) */,
> > );
> > 
> > VIR_ENUM_DECL(qemuDeviceVideoSecondary);
> > @@ -139,6 +143,8 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary,
> >               "" /* 'none' doesn't make sense here */,
> >               "" /* no secondary device for bochs */,
> >               "" /* no secondary device for ramfb */,
> > +              "" /* no secondary device for ati-vga? (rage128p)
> > */,
> > +              "" /* no secondary device for ati-vga? (rv100) */,
> 
> IIUC there can only be one vga device, so this seems correct.
> 
But there can in theory be a second video device (non-vga).  It
certainly works with passthrough + emulated.  I never tried two
emulated devices, that might be because it can't be done...

> > );
> > 
> > VIR_ENUM_IMPL(qemuSoundCodec,
> > diff --git a/src/qemu/qemu_domain_address.c
> > b/src/qemu/qemu_domain_address.c
> > index dd87915a97..d8273af92f 100644
> > --- a/src/qemu/qemu_domain_address.c
> > +++ b/src/qemu/qemu_domain_address.c
> > @@ -964,6 +964,8 @@
> > qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> >         case VIR_DOMAIN_VIDEO_TYPE_VBOX:
> >         case VIR_DOMAIN_VIDEO_TYPE_QXL:
> >         case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
> > +        case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
> > +        case VIR_DOMAIN_VIDEO_TYPE_RV100:
> >             return pciFlags;
> > 
> >         case VIR_DOMAIN_VIDEO_TYPE_BOCHS:
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 6b5de29fdb..09aac14430 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -3111,6 +3111,8 @@
> > qemuProcessUpdateVideoRamSize(virQEMUDriverPtr driver,
> >         case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
> >         case VIR_DOMAIN_VIDEO_TYPE_XEN:
> >         case VIR_DOMAIN_VIDEO_TYPE_VBOX:
> > +        case VIR_DOMAIN_VIDEO_TYPE_RAGE128P:
> > +        case VIR_DOMAIN_VIDEO_TYPE_RV100:
> >         case VIR_DOMAIN_VIDEO_TYPE_LAST:
> >             break;
> >         }
> > diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
> > b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
> > index 928af2a01c..30609085e1 100644
> > --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
> > +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
> 
> [...]
> 
> These capabilities changes are incomplete.
> 
> VIR_TEST_REGENERATE_OUTPUT=1 build/tests/qemucapabilitiestest
> assumes this is supported since QEMU 4.0
Okay.  As I said, I'll try it and see.

Thanks for the feedback!