[git pull] drm udl fixes

[git pull] drm udl fixes

[Dave Airlie]

Pull request to myself just so it's logged and linked in right place,
but this is a set of Mikulas's udl kms patches I've looked over and am
happy with.

Dave.

The following changes since commit acb1872577b346bd15ab3a3f8dff780d6cca4b70:

  Linux 4.18-rc7 (2018-07-29 14:44:52 -0700)

are available in the Git repository at:

  git://people.freedesktop.org/~airlied/linux drm-udl-next

for you to fetch changes up to 90991209837ab619555a46a97a88dead7a960d2d:

  udl-kms: dont spam the syslog with debug messages (2018-07-31 08:11:12 +1000)

----------------------------------------------------------------
Mikulas Patocka (7):
      udl-kms: change down_interruptible to down
      udl-kms: handle allocation failure
      udl-kms: fix crash due to uninitialized memory
      udl-kms: avoid division
      udl-kms: avoid prefetch
      udl-kms: use spin_lock_irq instead of spin_lock_irqsave
      udl-kms: dont spam the syslog with debug messages

 drivers/gpu/drm/udl/udl_drv.h      |  2 +-
 drivers/gpu/drm/udl/udl_fb.c       | 23 ++++++++++---------
 drivers/gpu/drm/udl/udl_main.c     | 45 +++++++++++++++++++------------------
 drivers/gpu/drm/udl/udl_modeset.c  |  7 +++---
 drivers/gpu/drm/udl/udl_transfer.c | 46 +++++++++++++++++---------------------
 5 files changed, 60 insertions(+), 63 deletions(-)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Mikulas Patocka]

On Tue, 31 Jul 2018, Dave Airlie wrote:

> Pull request to myself just so it's logged and linked in right place,
> but this is a set of Mikulas's udl kms patches I've looked over and am
> happy with.
> 
> Dave.

I've seen that you dropped this patch: 
https://patchwork.kernel.org/patch/10445393/

Is that patch correct or incorrect? In case it is incorrect, do you have 
an idea how should fbdefio be used properly on KMS drivers?

Mikulas

> The following changes since commit acb1872577b346bd15ab3a3f8dff780d6cca4b70:
> 
>   Linux 4.18-rc7 (2018-07-29 14:44:52 -0700)
> 
> are available in the Git repository at:
> 
>   git://people.freedesktop.org/~airlied/linux drm-udl-next
> 
> for you to fetch changes up to 90991209837ab619555a46a97a88dead7a960d2d:
> 
>   udl-kms: dont spam the syslog with debug messages (2018-07-31 08:11:12 +1000)
> 
> ----------------------------------------------------------------
> Mikulas Patocka (7):
>       udl-kms: change down_interruptible to down
>       udl-kms: handle allocation failure
>       udl-kms: fix crash due to uninitialized memory
>       udl-kms: avoid division
>       udl-kms: avoid prefetch
>       udl-kms: use spin_lock_irq instead of spin_lock_irqsave
>       udl-kms: dont spam the syslog with debug messages
> 
>  drivers/gpu/drm/udl/udl_drv.h      |  2 +-
>  drivers/gpu/drm/udl/udl_fb.c       | 23 ++++++++++---------
>  drivers/gpu/drm/udl/udl_main.c     | 45 +++++++++++++++++++------------------
>  drivers/gpu/drm/udl/udl_modeset.c  |  7 +++---
>  drivers/gpu/drm/udl/udl_transfer.c | 46 +++++++++++++++++---------------------
>  5 files changed, 60 insertions(+), 63 deletions(-)
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Dave Airlie]

>
> I've seen that you dropped this patch:
> https://patchwork.kernel.org/patch/10445393/
>
> Is that patch correct or incorrect? In case it is incorrect, do you have
> an idea how should fbdefio be used properly on KMS drivers?

I suppose I was wondering what use fbdefio really has, modern code
should be using KMS interface and the KMS dirty handling should be
able to cover those cases.

I don't really like the maintainability decrease moving to in-driver
page handling causes vs using shmem.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Daniel Vetter]

On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie <airlied@gmail.com> wrote:
>>
>> I've seen that you dropped this patch:
>> https://patchwork.kernel.org/patch/10445393/
>>
>> Is that patch correct or incorrect? In case it is incorrect, do you have
>> an idea how should fbdefio be used properly on KMS drivers?
>
> I suppose I was wondering what use fbdefio really has, modern code
> should be using KMS interface and the KMS dirty handling should be
> able to cover those cases.
>
> I don't really like the maintainability decrease moving to in-driver
> page handling causes vs using shmem.

I think tinydrm has the same problem of shmem+fbdefio, could be worth
it to chat with Noralf. Adding him.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Noralf Trønnes]

Den 04.09.2018 08.18, skrev Daniel Vetter:
> On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie <airlied@gmail.com> wrote:
>>> I've seen that you dropped this patch:
>>> https://patchwork.kernel.org/patch/10445393/
>>>
>>> Is that patch correct or incorrect? In case it is incorrect, do you have
>>> an idea how should fbdefio be used properly on KMS drivers?
>> I suppose I was wondering what use fbdefio really has, modern code
>> should be using KMS interface and the KMS dirty handling should be
>> able to cover those cases.
>>
>> I don't really like the maintainability decrease moving to in-driver
>> page handling causes vs using shmem.
> I think tinydrm has the same problem of shmem+fbdefio, could be worth
> it to chat with Noralf. Adding him.

fbdev deferred I/O is used to catch page faults when fbdev has been
mmap'ed. The faults are queued up and a worker runs after a certain time.
The problem as mentioned is that both shmem and defio use page->lru
(and page->mapping).

In the generic fbdev emulation, drm_fb_helper.c:drm_fbdev_generic_setup(),
I solved this by using a shadow vmalloc buffer that is blitted on the
real one and then dirtyfb is called. The driver needs to support
drm_driver->gem_prime_vmap and ->gem_prime_mmap to use it though and udl
has it's own dma-buf handling.

I've just posted a shmem library that maybe udl could use:
https://patchwork.freedesktop.org/series/27184/

With that udl could use the generic fbdev emulation.

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Mikulas Patocka]

On Tue, 4 Sep 2018, Daniel Vetter wrote:

> On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie <airlied@gmail.com> wrote:
> >>
> >> I've seen that you dropped this patch:
> >> https://patchwork.kernel.org/patch/10445393/
> >>
> >> Is that patch correct or incorrect? In case it is incorrect, do you have
> >> an idea how should fbdefio be used properly on KMS drivers?
> >
> > I suppose I was wondering what use fbdefio really has, modern code
> > should be using KMS interface and the KMS dirty handling should be
> > able to cover those cases.

I use fbdefio with the links 2 web browser and with fbi and fbgs image
viewers.

I tried to look into modifying the links browser to work without fbdefio - 
but the DRM_IOCTL_MODE_DIRTYFB ioctl requires master mode, which means 
that it is unuseable by an unprivileged process. If you suggest how should 
console applications use the framebuffer without fbdefio, I can implement 
it.

> > I don't really like the maintainability decrease moving to in-driver 
> > page handling causes vs using shmem.
> 
> I think tinydrm has the same problem of shmem+fbdefio, could be worth
> it to chat with Noralf. Adding him.

Perhaps we could allocate tiny structures that point to pages and 
construct the list from them - but it would require rewriting all the 
fbdefio drivers, and I wonder if anyone has all the hardware that uses 
fbdefio to test it.

Mikulas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Daniel Vetter]

On Tue, Sep 4, 2018 at 7:04 PM, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
> On Tue, 4 Sep 2018, Daniel Vetter wrote:
>
>> On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie <airlied@gmail.com> wrote:
>> >>
>> >> I've seen that you dropped this patch:
>> >> https://patchwork.kernel.org/patch/10445393/
>> >>
>> >> Is that patch correct or incorrect? In case it is incorrect, do you have
>> >> an idea how should fbdefio be used properly on KMS drivers?
>> >
>> > I suppose I was wondering what use fbdefio really has, modern code
>> > should be using KMS interface and the KMS dirty handling should be
>> > able to cover those cases.
>
> I use fbdefio with the links 2 web browser and with fbi and fbgs image
> viewers.
>
> I tried to look into modifying the links browser to work without fbdefio -
> but the DRM_IOCTL_MODE_DIRTYFB ioctl requires master mode, which means
> that it is unuseable by an unprivileged process. If you suggest how should
> console applications use the framebuffer without fbdefio, I can implement
> it.

Well with the console there's really no security. Either you disallow
touching fbdev, or everyone can read&write whatever they want, even if
they're not the currently active vt. So security just doesn't exist
with fbdev.

With kms you need logind or someone like that who orchestrates the vt
switching and makes sure you can read/write other people's stuff.

>> > I don't really like the maintainability decrease moving to in-driver
>> > page handling causes vs using shmem.
>>
>> I think tinydrm has the same problem of shmem+fbdefio, could be worth
>> it to chat with Noralf. Adding him.
>
> Perhaps we could allocate tiny structures that point to pages and
> construct the list from them - but it would require rewriting all the
> fbdefio drivers, and I wonder if anyone has all the hardware that uses
> fbdefio to test it.

We could just entirely implement our own defio copy in drm_fb_helpers.
Not sure that's worth it against the coast of yet another cpu copy.
Anyone still stuck using fbcon will expect performance from the 90s,
and you can do lots of cpu copies and still be fast enough :-)

I'd recommend you use the generic defio stuff Noralf has created and
see how that goes. Best case it's a good cleanup for drm/udl by
switching over to the gem helpers and generic fbdev code.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Mikulas Patocka]

On Tue, 4 Sep 2018, Daniel Vetter wrote:

> On Tue, Sep 4, 2018 at 7:04 PM, Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> >
> > On Tue, 4 Sep 2018, Daniel Vetter wrote:
> >
> >> On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie <airlied@gmail.com> wrote:
> >> >>
> >> >> I've seen that you dropped this patch:
> >> >> https://patchwork.kernel.org/patch/10445393/
> >> >>
> >> >> Is that patch correct or incorrect? In case it is incorrect, do you have
> >> >> an idea how should fbdefio be used properly on KMS drivers?
> >> >
> >> > I suppose I was wondering what use fbdefio really has, modern code
> >> > should be using KMS interface and the KMS dirty handling should be
> >> > able to cover those cases.
> >
> > I use fbdefio with the links 2 web browser and with fbi and fbgs image
> > viewers.
> >
> > I tried to look into modifying the links browser to work without fbdefio -
> > but the DRM_IOCTL_MODE_DIRTYFB ioctl requires master mode, which means
> > that it is unuseable by an unprivileged process. If you suggest how should
> > console applications use the framebuffer without fbdefio, I can implement
> > it.
> 
> Well with the console there's really no security. Either you disallow
> touching fbdev, or everyone can read&write whatever they want, even if
> they're not the currently active vt. So security just doesn't exist
> with fbdev.

I added myself to the video group, so that I can open /dev/fb0 and run 
graphics console applications. But I still can't set master mode, so I 
can't issue DRM_IOCTL_MODE_DIRTYFB to update the screen when fbdefio is 
turned off.

> With kms you need logind or someone like that who orchestrates the vt
> switching and makes sure you can read/write other people's stuff.

The Links browser registers two signals via vt_mode.acqsig and 
vt_mode.relsig and repaints the screen or stops painting when they are 
received. If it should do something else - then what?

It would be nice if the kms developers provided a simple graphics console 
application that describes how to use it.

Mikulas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Mikulas Patocka]

On Tue, 4 Sep 2018, Daniel Vetter wrote:

> With kms you need logind or someone like that who orchestrates the vt
> switching and makes sure you can read/write other people's stuff.

BTW. I'm just wondering how is this 'master mode' security working at all.

The user start Xserver under the user's UID and the Xserver asks logind to 
set master mode on the DRM file descriptor.

There are plenty of ways how the user can steal a file descriptor from the
Xserver that is running under the same UID - for example:
- setting LD_PRELOAD to inject a library into the Xserver
- calling ptrace on the Xserver process
- opening /proc/`pidof Xorg`/fd

When one of the user's processes has a handle in 'master mode', any other 
user's process can steal it. So what does these 'master mode' restrictions 
really protect against?

Mikulas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [git pull] drm udl fixes

[Daniel Vetter]

On Tue, Sep 4, 2018 at 9:05 PM, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
> On Tue, 4 Sep 2018, Daniel Vetter wrote:
>
>> With kms you need logind or someone like that who orchestrates the vt
>> switching and makes sure you can read/write other people's stuff.
>
> BTW. I'm just wondering how is this 'master mode' security working at all.
>
> The user start Xserver under the user's UID and the Xserver asks logind to
> set master mode on the DRM file descriptor.
>
> There are plenty of ways how the user can steal a file descriptor from the
> Xserver that is running under the same UID - for example:
> - setting LD_PRELOAD to inject a library into the Xserver
> - calling ptrace on the Xserver process
> - opening /proc/`pidof Xorg`/fd
>
> When one of the user's processes has a handle in 'master mode', any other
> user's process can steal it. So what does these 'master mode' restrictions
> really protect against?

Other users.

And there _are_ people sufficiently paranoid who disable ptrace and
all that stuff you brought up, e.g. through selinux policies.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel