On Tue, Jun 04, 2024 at 07:50:38AM +0100, Mark Cave-Ayland wrote:
> On 03/06/2024 12:40, Daniel P. Berrangé wrote:
> 
> > On Thu, May 30, 2024 at 01:22:11PM +0100, Mark Cave-Ayland wrote:
> > > On 30/05/2024 12:40, BALATON Zoltan wrote:
> > > 
> > > > On Thu, 30 May 2024, Gerd Hoffmann wrote:
> > > > > stdvga is the much better option.
> > > > > 
> > > > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> > > > > ---
> > > > > hw/display/cirrus_vga.c     | 1 +
> > > > > hw/display/cirrus_vga_isa.c | 1 +
> > > > > hw/display/Kconfig          | 1 -
> > > > > 3 files changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> > > > > index 150883a97166..81421be1f89d 100644
> > > > > --- a/hw/display/cirrus_vga.c
> > > > > +++ b/hw/display/cirrus_vga.c
> > > > > @@ -3007,6 +3007,7 @@ static void cirrus_vga_class_init(ObjectClass
> > > > > *klass, void *data)
> > > > >      dc->vmsd = &vmstate_pci_cirrus_vga;
> > > > >      device_class_set_props(dc, pci_vga_cirrus_properties);
> > > > >      dc->hotpluggable = false;
> > > > > +    klass->deprecation_note = "use stdvga instead";
> > > > > }
> > > > > 
> > > > > static const TypeInfo cirrus_vga_info = {
> > > > > diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
> > > > > index 84be51670ed8..3abbf4dddd90 100644
> > > > > --- a/hw/display/cirrus_vga_isa.c
> > > > > +++ b/hw/display/cirrus_vga_isa.c
> > > > > @@ -85,6 +85,7 @@ static void isa_cirrus_vga_class_init(ObjectClass
> > > > > *klass, void *data)
> > > > >      dc->realize = isa_cirrus_vga_realizefn;
> > > > >      device_class_set_props(dc, isa_cirrus_vga_properties);
> > > > >      set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
> > > > > +    klass->deprecation_note = "use stdvga instead";
> > > > 
> > > > Excepr some old OSes work better with this than stdvga so could this be
> > > > left and not removed? Does it cause a lot of work to keep this device? I
> > > > thought it's stable already and were not many changes for it lately. If
> > > > something works why drop it?
> > > 
> > > Seconded: whilst stdvga is preferred, there are a lot of older OSs that 
> > > work
> > > well in QEMU using the Cirrus emulation. I appreciate that the code could 
> > > do
> > > with a bit of work, but is there a more specific reason that it should be
> > > deprecated?
> > 
> > I think there's different answers here for upstream vs downstream.
> > 
> > Upstream QEMU's scope is to emulate pretty much arbitrary hardware that
> > may have existed at any point in time. Emulating Cirrus is very much
> > in scope upstream, and even if there are other better VGA devices, that
> > doesn't make emulation of Cirrus redundant.
> > 
> > Downstream is a different matter - if a downstream vendor wants to be
> > opinionated and limit the scope of devices they ship to customers, it
> > is totally valid to cull Cirrus.
> 
> The concern for me is that if someone such as RedHat decided not to ship
> Cirrus then we'd end up in a place where command lines for some legacy OSs
> would work on an upstream build, but if someone was using RHEL then they
> would fail due to the device not being present. I can see this causing
> confusion for users since they would report that a command line doesn't work
> whilst others would shrug and report back that it works for them.

That ship sailed in RHEL over a decade and a half ago.

There is already a mountain of devices and other QEMU features that are
/not/ shipped in RHEL, disabled at build time. Essentially RHEL is only
targetting virtualization use cases, not emulation, so the priority is
virtio devices, paired with a tiny handful of emulated devices to fill
some gaps. Historically RHEL included Cirrus, but it was marked deprecated
in RHEL quite a few years ago now, and will likely be removed entirely
in RHEL-10. The combo of stdvga and virtio-vga/gpu is sufficient for the
virtualization use case.

Yes, this does cause confusion and annoyance for users who want to try
to use RHEL with QEMU cli configs they find around the web, but that's
considered acceptable pain.

> I do wonder if a solution for this would be to add a blocklist file in /etc
> that prevents the listed QOM types from being instantiated. The file could
> contain also contain a machine regex to match and a reason that can be
> reported to the user e.g.
> 
> # QEMU QOM type blocklist
> #
> # QOM type regex, machine regex list, reason
> "cirrus*","pc*,machine*","contains insecure blitter routines"
> "fdc","pc*","may not be completely secure"
> 
> This feels like a better solution because:
> 
> - It's easy to add a message that reports "The requested QOM type <foo>
> cannot be instantiated because <reason>" for specific machine types. The
> machine regex also fixes the problem where usb-ohci should be allowed for
> PPC machines, but not generally for PC machines.
> 
> - Downstream can ship with a secure policy for production environments but
> also a less restrictive policy for more casual users
>
> - If someone really needs a device that is not enabled by default, a
> privileged user can simply edit the blocklist file and allow it
> 
> - It should work both with or without modules

I don't see a user edittable config being useful in RHEL or Fedora.
For RHEL we have a strict policy of only shipping what we want to
support, and everything else must be fully disabled at build time.
Conversely in Fedora we aim to ship everything that QEMU provides.

I would be nice to have a tagging of "quality" status for devices
upstream, but that's not something that needs to be turned into a
user edittable matrix against machine types. A device impl is either
good or not - the code impl quality doesn't vary per machine usage,
nor should upstream try to artificially block usage per machine.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Reply via email to