All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pekka Paalanen <ppaalanen@gmail.com>
To: Zack Rusin <zackr@vmware.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	David Airlie <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org,
	Gurchetan Singh <gurchetansingh@chromium.org>,
	krastevm@vmware.com, Gerd Hoffmann <kraxel@redhat.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	wayland-devel <wayland-devel@lists.freedesktop.org>,
	mombasawalam@vmware.com
Subject: Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
Date: Tue, 7 Jun 2022 11:07:07 +0300	[thread overview]
Message-ID: <20220607110707.02eccda5@eldfell> (raw)
In-Reply-To: <wRnf-Lm5zz6v1e-NlnFPteyARuLl-R98mOZZVjePHD5ue7QQNR_TSU7RwYBssgUa7xM5hf7Fe59-gMEj81ESrHY3mu_H7yE0dtGhFHFPTnc=@emersion.fr>

[-- Attachment #1: Type: text/plain, Size: 7346 bytes --]

On Fri, 03 Jun 2022 14:14:59 +0000
Simon Ser <contact@emersion.fr> wrote:

> Hi,
> 
> Please, read this thread:
> https://lists.freedesktop.org/archives/dri-devel/2020-March/thread.html#259615
> 
> It has a lot of information about the pitfalls of cursor hotspot and
> other things done by VM software.
> 
> In particular: since the driver will ignore the KMS cursor plane
> position set by user-space, I don't think it's okay to just expose
> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
> 
> cc wayland-devel and Pekka for user-space feedback.
> 
> On Thursday, June 2nd, 2022 at 17:42, Zack Rusin <zack@kde.org> wrote:
> 
> > - all userspace code needs to hardcore a list of drivers which require
> > hotspots because there's no way to query from drm "does this driver
> > require hotspot"  
> 
> Can you elaborate? I'm not sure I understand what you mean here.
> 

Hi Zack and everyone,

I would like to try to reboot this discussion and explain where I come
from. Maybe I have misunderstood something.

I have no fundamental objection to adding KMS properties for cursor
hotspot, but I do want to make sure the design is fully thought out and
not simply copied from legacy KMS, because atomic KMS with universal
planes is not like legacy KMS.

To my understanding from the past discussions, the fundamental reason
why you are proposing hotspot properties is that when one is running a
desktop inside a VM and looking at it through a VM viewer application,
the pointer cursor is misplaced: the hotspot is not where the end user
sees it. This you never mentioned in any of the patches nor in the
cover letter. I can only guess that this misplacement could be the
reason why some display servers have deny-listed paravirtualized
drivers. While your goal is to get paravirtualized drivers out of the
deny-lists, we must first understand why they got there in the first
place.

Why are pointer cursors misplaced on paravirtualized drivers?

It is because the paravirtualized drivers or VM viewers do *not* place
the cursor plane at the CRTC_X, CRTC_Y position in the guest CRTC area.
This is obvious: if CRTC_X, CRTC_Y were honoured, there would be no
misplacement.

Instead, the VM stack plays clever tricks with cursor planes. I have
understood only one of those tricks, and it goes something like this.
To improve hand-eye coordination, that is to reduce the hand-to-eye
response time a.k.a latency, the VM guest KMS driver relays the cursor
plane separately to the VM viewer application. The VM viewer
application presents the cursor plane content by pushing them to the
host window system as the pointer cursor. This means the host window
system will be autonomously moving the cursor plane image around,
completely disregarding what the guest KMS client programmed into
CRTC_X, CRTC_Y. When this works, it is a huge improvement in user
experience. I believe this is called "seamless pointer" or such.

When it doesn't work, the cursor is misplaced or even completely
arbitrary guest windows start flying around as if they were the pointer
cursor.

In legacy KMS, cursors have always been very special and had their own
special UAPI with even hotspot information. There paravirtualized
drivers got away with these clever tricks because no-one bothered
putting anything but actual cursor images on the (one) cursor plane.

Then comes KMS universal planes concept. All planes are assumed roughly
equal, apart from what KMS properties tell userspace about them. The
plane type primary/overlay/cursor is still kept, but only because it
helps userspace find a KMS configuration that the driver accepts at
all. Hardware may not be able to light up a CRTC without at least one
primary plane, for example. Atomic KMS requires universal planes.

The atomic KMS UAPI contract says, that a plane is positioned at
CRTC_X, CRTC_Y inside the respective CRTC. I do not know of any
documented exceptions to this. Therefore, an atomic driver that does
not show a cursor plane at the programmed CRTC_X, CRTC_Y is violating
the UAPI contract.

See https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-plane-properties
and how "cursor" plane type makes no exceptions, and how CRTC_X and
CRTC_Y are defined. Also note that all hardware drivers and VKMS
apparently follow the contract.

Given this UAPI contract, it is very easy for userspace to make the
conclusion that a cursor plane is just another plane it can use for
whatever it wants. Weston and wlroots indeed leverage this, putting
also normal windows and other stuff to the cursor plane when they
happen to fit.

If a paravirtualized driver commandeers the cursor plane display
position, a possible result is that arbitrary windows start flying
around unexpectedly.

Therefore we have two problems:

- paravirtualized drivers commandeering the cursor plane
- VM software not able to implement "seamless pointer" correctly

To my understanding, this patch series concerns only the latter problem
but not the former problem. I believe the solutions to both are related
and need to be considered together.

How do we stop paravirtualized drivers from commandeering their cursor
plane when guest userspace does not expect it?

How do we still make "seamless pointer" possible when the guest
userspace is prepared for cursor plane commandeering?

These are the questions that need answers. To me, getting
paravirtualized drivers off display server deny-lists is a consequence,
a secondary goal. The primary goal must be to fix why the drivers ended
up in deny-lists in the first place (and I am only assuming that this
is he reason, so maybe there are other reasons?).

I believe the solution has two parts:

- The guest KMS driver needs to know whether the guest userspace is
  prepared for the cursor plane being commandeered. If userspace does
  not indicate it is prepared for it, commandeering must not happen.

- Cursor hotspot needs new KMS properties, and a KMS client must set
  them if the KMS client indicates it is prepared for cursor plane
  commandeering.

How to exactly do those can be discussed after we can agree on what
problems we are solving.

There are further problems with cursor plane commandeering. The 2020
email thread Simon linked to discusses the problem of pointer devices:
if VM guest userspace takes pointer input from multiple sources, how
will the VM stack know which virtual input device, if any, should drive
the cursor plane position?

To me the answer to this question seems it could be intimately tied
to the first problem: commandeering the cursor plane is allowed only
if guest userspace tells the guest KMS driver which input device the
cursor plane shall be related to. If no input device is indicated, then
commandeering must not happen. I can understand if people do not want
to tackle this question, because it probably has not been a problem
yet. Ignoring it would be unfortunate though, because that would seem
to imply that VM software still needs to keep some heuristics for when
commandeering the cursor plane is desired.

I could also talk about multiple cursors, but I guess that goes really
too far.

Which root problems do you want to solve exactly?


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2022-06-07  8:07 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-02 15:42 [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS Zack Rusin
2022-06-02 15:42 ` [PATCH 1/6] drm/atomic: Add support for mouse hotspots Zack Rusin
2022-06-02 15:42 ` [PATCH 2/6] drm/vmwgfx: Create mouse hotspot properties on cursor planes Zack Rusin
2022-06-03 13:11   ` Martin Krastev (VMware)
2022-06-02 15:42 ` [PATCH 3/6] drm/qxl: " Zack Rusin
2022-06-02 22:05   ` kernel test robot
2022-06-02 22:05     ` kernel test robot
2022-06-02 23:26   ` kernel test robot
2022-06-02 23:26     ` kernel test robot
2022-06-02 23:26     ` kernel test robot
2022-06-02 15:42 ` [PATCH 4/6] drm/vboxvideo: " Zack Rusin
2022-06-02 15:42 ` [PATCH 5/6] drm/virtio: " Zack Rusin
2022-06-02 15:42 ` [PATCH 6/6] drm: Remove legacy cursor hotspot code Zack Rusin
2022-06-03 10:28 ` [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS Gerd Hoffmann
2022-06-03 14:43   ` Zack Rusin
2022-06-03 14:14 ` Simon Ser
2022-06-03 14:27   ` Zack Rusin
2022-06-03 14:32     ` Simon Ser
2022-06-03 14:38       ` Zack Rusin
2022-06-03 14:56         ` Simon Ser
2022-06-03 15:17           ` Zack Rusin
2022-06-03 15:22             ` Simon Ser
2022-06-03 15:32               ` Zack Rusin
2022-06-03 15:49                 ` Simon Ser
2022-06-03 18:31                   ` Zack Rusin
2022-06-05  7:30                     ` Simon Ser
2022-06-05 15:47                       ` Zack Rusin
2022-06-05 16:26                         ` Simon Ser
2022-06-05 18:16                           ` Zack Rusin
2022-06-06  8:11                             ` Simon Ser
2022-06-04 21:19               ` Hans de Goede
2022-06-05  7:34                 ` Simon Ser
2022-06-07 11:25             ` Gerd Hoffmann
2022-06-06  9:13         ` Pekka Paalanen
2022-06-07  8:07   ` Pekka Paalanen [this message]
2022-06-07 14:30     ` Gerd Hoffmann
2022-06-08  7:53       ` Pekka Paalanen
2022-06-08 14:52         ` Gerd Hoffmann
2022-06-07 17:50     ` Zack Rusin
2022-06-08  7:53       ` Pekka Paalanen
2022-06-09 19:39         ` Zack Rusin
2022-06-10  7:49           ` Pekka Paalanen
2022-06-10  8:22             ` Jonas Ådahl
2022-06-10  8:54           ` Simon Ser
2022-06-10  9:01             ` Daniel Vetter
2022-06-10  8:41   ` Daniel Vetter
2022-06-10  8:56     ` Pekka Paalanen
2022-06-10  8:59     ` Daniel Vetter
2022-06-10 12:03       ` Gerd Hoffmann
2022-06-10 14:24       ` Zack Rusin
2022-06-13  7:33         ` Pekka Paalanen
2022-06-13 13:14           ` Zack Rusin
2022-06-13 14:25             ` Pekka Paalanen
2022-06-13 14:54               ` Zack Rusin
2022-06-14  7:36                 ` Pekka Paalanen
2022-06-14 14:40                   ` Zack Rusin
2022-06-14 14:54                     ` Daniel Stone
2023-06-09 15:20       ` Albert Esteve
2023-06-21  7:10         ` Javier Martinez Canillas
2023-06-22  4:29           ` Zack Rusin
2023-06-22  6:20             ` Javier Martinez Canillas
2022-06-10  9:15     ` Simon Ser
2022-06-10  9:49       ` Daniel Vetter
2022-06-10 12:36         ` Gerd Hoffmann
2022-06-10 12:53           ` Simon Ser
2022-06-11 15:34             ` Hans de Goede
2022-06-13  7:45               ` Pekka Paalanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220607110707.02eccda5@eldfell \
    --to=ppaalanen@gmail.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gurchetansingh@chromium.org \
    --cc=hdegoede@redhat.com \
    --cc=krastevm@vmware.com \
    --cc=kraxel@redhat.com \
    --cc=mombasawalam@vmware.com \
    --cc=tzimmermann@suse.de \
    --cc=wayland-devel@lists.freedesktop.org \
    --cc=zackr@vmware.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.