Re: [PATCH v2 32/32] hw/display: mark most display adapters as insecure

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Aug 19, 2026 at 01:18:18PM +0200, BALATON Zoltan wrote:
> On Wed, 19 Aug 2026, marcandre.lureau@redhat.com wrote:
> > > Most of thte display adapters are emulating old hardware which is not
> > > relevant to virtualization use cases.
> > > 
> > > The exceptions that should be considered secure are Cirrus (PCI, not
> > > ISA), Bochs, QXL, RAMFB, VGA (PCI, MMIO, not ISA) and VMWare VGA.
> > > 
> > > The Cirrus PCI decision is borderline. It has been heavily used with
> > > virtualization in the past, but these days VGA / RAMFB are strongly
> > > recommended instead. Due to its historical usage though, we can
> > > consider the code fairly mature, even if no longer hugely relevant
> > > to virtualization use cases.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Message-ID: <20250926140144.1998694-33-berrange@redhat.com>
> > > 
> > > diff --git a/hw/display/artist.c b/hw/display/artist.c
> > > index 3c884c92437c..caab4d1d4c52 100644
> > > --- a/hw/display/artist.c
> > > +++ b/hw/display/artist.c
> > > @@ -1504,6 +1504,7 @@ static const TypeInfo artist_info = {
> > >      .instance_size = sizeof(ARTISTState),
> > >      .instance_init = artist_initfn,
> > >      .class_init    = artist_class_init,
> > > +    .secure        = false,
> > >  };
> > > 
> > >  static void artist_register_types(void)
> > > diff --git a/hw/display/ati.c b/hw/display/ati.c
> > > index f7c0006a879d..6e332e02d2fb 100644
> > > --- a/hw/display/ati.c
> > > +++ b/hw/display/ati.c
> > > @@ -1080,6 +1080,7 @@ static const TypeInfo ati_vga_info = {
> > >      .instance_size = sizeof(ATIVGAState),
> > >      .class_init = ati_vga_class_init,
> > >      .instance_init = ati_vga_init,
> > > +    .secure = false,
> 
> I've missed the original series but why isn't the default false and devices
> that are secured set it to true? That way a new device would not be set
> secure unless the author explicitly sets it which seems better than assuming
> devices are secure when most of them aren't. Or if false is already the
> default why is it set everywhere? Isn't it enough to mark secure devices?

In the first version I was explicitly tracking "secure" and "insecure"
and "undefined".

Markus made the point that from an external consumer POV we should
treat "insecure" and "undefined" as the same.

Internally, however, I still wanted to keep track of what devices we
had put some intelligent thought into for classification purposes.
IOW, the absence of ".secure = " is effectively our TODO list.

Perhaps this is overkill though. What really matters is that we get
an accurate list for ".secure = true" as quickly as possible, and
once that's more or less complete, the '.secure = false' markers
probably cease to retain any value.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|