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

Steven Newbury steve at snewbury.org.uk
Wed Oct 7 17:22:41 UTC 2020


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 at 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!






More information about the libvir-list mailing list