Re: [PATCH v3] drm/virtio: Add option to disable KMS support

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Am 28.02.23 um 13:34 schrieb Thomas Zimmermann:
> Hi
>
> Am 27.02.23 um 19:15 schrieb Rob Clark:
> > On Mon, Feb 27, 2023 at 9:57 AM Dmitry Osipenko
> > <dmitry.osipenko@xxxxxxxxxxxxx> wrote:
> > > On 2/27/23 20:38, Rob Clark wrote:
> > > ...
> > > > +     if (IS_ENABLED(CONFIG_DRM_VIRTIO_GPU_KMS)) {
> > > > +             /* get display info */
> > > > +             virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
> > > > +                             num_scanouts, &num_scanouts);
> > > > +             vgdev->num_scanouts = min_t(uint32_t, num_scanouts,
> > > > +                                         VIRTIO_GPU_MAX_SCANOUTS);
> > > > +             if (!vgdev->num_scanouts) {
> > > > +                     /*
> > > > +                      * Having an EDID but no scanouts is 
> > > > non-sensical,
> > > > +                      * but it is permitted to have no scanouts and no
> > > > +                      * EDID (in which case DRIVER_MODESET and
> > > > +                      * DRIVER_ATOMIC are not advertised)
> > > > +                      */
> > > > +                     if (vgdev->has_edid) {
> > > > +                             DRM_ERROR("num_scanouts is zero\n");
> > > > +                             ret = -EINVAL;
> > > > +                             goto err_scanouts;
> > > > +                     }
> > > > +                     dev->driver_features &= ~(DRIVER_MODESET | 
> > > > DRIVER_ATOMIC);
> > >
> > > If it's now configurable by host, why do we need the
> > > CONFIG_DRM_VIRTIO_GPU_KMS?
> >
> > Because a kernel config option makes it more obvious that
> > modeset/atomic ioctls are blocked.  Which makes it more obvious about
> > where any potential security issues apply and where fixes need to get
> > backported to.  The config option is the only thing _I_ want,
> > everything else is just a bonus to help other people's use-cases.
>
> I find this very vague. What's the security thread?
>
> And if the config option is useful, shouldn't it be DRM-wide? The 
> modesetting ioctl calls are shared among all drivers.

For reference, here's an older discussion about render-only devices.

https://lore.kernel.org/dri-devel/20221011110437.15258-1-christian.koenig@xxxxxxx/

> Best regards
> Thomas
>
> > BR,
> > -R

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature