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(-)
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(-)
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.
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
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.
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
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
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
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
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
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Dave Airlie -
Mikulas Patocka -
Noralf Trønnes