stdvga is the much better option.
Signed-off-by: Gerd Hoffmann <kraxel@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";
}
static const TypeInfo isa_cirrus_vga_info = {
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index a4552c8ed78d..cd0779396890 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -11,7 +11,6 @@ config FW_CFG_DMA
config VGA_CIRRUS
bool
- default y if PCI_DEVICES
depends on PCI
select VGA
--
2.45.1On Thu, 30 May 2024, Gerd Hoffmann wrote:
> stdvga is the much better option.
>
> Signed-off-by: Gerd Hoffmann <kraxel@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?
Regards,
BALATON Zoltan
> }
>
> static const TypeInfo isa_cirrus_vga_info = {
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index a4552c8ed78d..cd0779396890 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -11,7 +11,6 @@ config FW_CFG_DMA
>
> config VGA_CIRRUS
> bool
> - default y if PCI_DEVICES
> depends on PCI
> select VGA
>
>
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 <kraxel@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?
ATB,
Mark.
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 <kraxel@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.
IOW, I think device deprecation *framework* is relevant to include
upstream, but most actual usage of it will be downstream.
Upstream might use *object* deprecation, if we replace an backend
implementation with a different one.
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 :|
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 <kraxel@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.
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
ATB,
Mark.
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 <kraxel@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 :|
On 3/6/24 13: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 <kraxel@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.
Few years ago I suggested qemu_security_policy_taint() "which allows
unsafe (read "not very maintained") code to 'taint' QEMU security
policy." (Gerd FYI):
https://lore.kernel.org/qemu-devel/20210908232024.2399215-1-philmd@redhat.com/
Upstream we could add a boolean in DeviceClass about device security
status / maintenance (or enum or bitfield); then downstreams could
use it to be sure unsafe devices aren't linked in.
> IOW, I think device deprecation *framework* is relevant to include
> upstream, but most actual usage of it will be downstream.
>
> Upstream might use *object* deprecation, if we replace an backend
> implementation with a different one.
>
> With regards,
> Daniel
Hi,
> > 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.
>
> Few years ago I suggested qemu_security_policy_taint() "which allows
> unsafe (read "not very maintained") code to 'taint' QEMU security
> policy." (Gerd FYI):
> https://lore.kernel.org/qemu-devel/20210908232024.2399215-1-philmd@redhat.com/
>
> Upstream we could add a boolean in DeviceClass about device security
> status / maintenance (or enum or bitfield); then downstreams could
> use it to be sure unsafe devices aren't linked in.
Yes, I think it makes sense to maintain that information upstream. It
is useful information to have. Even if upstream isn't going to enforce
something qemu could print useful hints.
So, the question is whenever we want go for a simple bool, or go for a
bitfield giving more detailed information. Bits I think could be
useful:
(1) OBJECT_STATUS_DEPRECATED
Stuff we actually want remove. Very few cases, maybe usb-hub.
(2) OBJECT_STATUS_UNSAFE
"not very maintained". Probably need a better name for this.
(3) OBJECT_STATUS_OUTDATED
Old device where newer / better alternatives exist.
Looking at the USB host adapters I'd attach 2+3 to ohci and 3 to uhci
and ehci. In general there is a lot of overlap for (2) + (3) though.
We might also have recommendation bits such as:
(4) OBJECT_STATUS_VIRTUALIZATON
Devices recommended for virtualization use cases
(all virtio, xhci, ...).
> > IOW, I think device deprecation *framework* is relevant to include
> > upstream, but most actual usage of it will be downstream.
When doing it at ObjectClass not DeviceClass level we get introspection
support for (almost) free, so I'd do it for ObjectClass even if the vast
majority of users will be devices.
> > Upstream might use *object* deprecation, if we replace an backend
> > implementation with a different one.
Sounds like having OBJECT_STATUS_DEPRECATED makes sense even if we don't
have any device actually using that.
take care,
Gerd
On Wed, Jun 05, 2024 at 09:32:01AM +0200, Gerd Hoffmann wrote: > Hi, > > > > 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. > > > > Few years ago I suggested qemu_security_policy_taint() "which allows > > unsafe (read "not very maintained") code to 'taint' QEMU security > > policy." (Gerd FYI): > > https://lore.kernel.org/qemu-devel/20210908232024.2399215-1-philmd@redhat.com/ > > > > Upstream we could add a boolean in DeviceClass about device security > > status / maintenance (or enum or bitfield); then downstreams could > > use it to be sure unsafe devices aren't linked in. > > Yes, I think it makes sense to maintain that information upstream. It > is useful information to have. Even if upstream isn't going to enforce > something qemu could print useful hints. > > So, the question is whenever we want go for a simple bool, or go for a > bitfield giving more detailed information. Bits I think could be > useful: > > (1) OBJECT_STATUS_DEPRECATED > Stuff we actually want remove. Very few cases, maybe usb-hub. > > (2) OBJECT_STATUS_UNSAFE > "not very maintained". Probably need a better name for this. If it reflects maintainer status, then we're obvioulsy overlapping with the MAINTAINERS file info, but "UNSAFE" suggests something different.... So what I think would be valuable is marking whether a device is considered to provide a guest security boundary. This would in turn indicate whether we would treat flaws in its impl as being worthy of triaging as CVEs, or merely be normal bugs. We already document in security.rst that we consider virtualization use cases only to provide a security boundary, but on every bug report we have to decide whether the device in question is considered relevant to a "virtualization" use case. This was a known limitation, because we had no metadata to track which devices were considered "secure". It was always expected that long term we should be tagging devices/machines/accelerators/etc with their security status. It would be nice from an end user POV to be able to have a way to tell QEMU to enforce that a given VM provides a guest security boundary, and get a hard error at startup, or hot-plug if any device cannot satisfy that requirement. > (3) OBJECT_STATUS_OUTDATED > Old device where newer / better alternatives exist. I think (3) is pretty hard to define, as "better" is very much a use case specific decision. You could say that everything, for which a virtio device exists, has a "better" alternative. That's only true if running a modern OS though. If running a vintage OS, the "outdated" thing is liley "better". > Looking at the USB host adapters I'd attach 2+3 to ohci and 3 to uhci > and ehci. In general there is a lot of overlap for (2) + (3) though. > > We might also have recommendation bits such as: > > (4) OBJECT_STATUS_VIRTUALIZATON > Devices recommended for virtualization use cases > (all virtio, xhci, ...). Even that has the same problem as (3) - the recommended devices differ depending on what OS you're intending to use. This problem is pretty much why libosinfo exists to provide more guided hardware recommendations tailored to OS. 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 :|
On Mon, 3 Jun 2024 at 12:42, Daniel P. Berrangé <berrange@redhat.com> wrote: > 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. > > IOW, I think device deprecation *framework* is relevant to include > upstream, but most actual usage of it will be downstream. Right; for upstream we should expect that we use deprecation mostly as part of the "deprecate and then drop in a few releases" cycle. Deprecating something we don't want to drop doesn't seem to me to make much sense in an upstream context. thanks -- PMM
Hi,
> > > 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?
Well, the cirrus has a 2d blitter implementation which I'd rate
problematic from both security and correctness point of view.
Also any guest new enough to still receive security updates can surely
use stdvga instead. The "operating system museum" is IMHO pretty much
the only use case where it possibly might make sense to continue using
cirrus.
Having sayed that maybe the boolean classification -- be deprecated
or not -- is too simple. The number of devices we can actually
deprecate and remove is probably relatively small. But there also is
a large number of old-ish devices which only make sense in very few use
cases. When running an old OS. Or when emulating an old board. Which
also tend to be not maintained very well because there are not many
users. Maybe we need an "unsupported" state for them, with some
infrastructure like an easy way to compile qemu without unsupported
devices and an option to get warnings from qemu in case an unsupported
device is used.
take care,
Gerd
© 2016 - 2024 Red Hat, Inc.