All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Banack <banackm@vmware.com>
To: Pekka Paalanen <ppaalanen@gmail.com>
Cc: "mripard@kernel.org" <mripard@kernel.org>,
	"airlied@linux.ie" <airlied@linux.ie>,
	"javierm@redhat.com" <javierm@redhat.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Martin Krastev <krastevm@vmware.com>,
	"tzimmermann@suse.de" <tzimmermann@suse.de>,
	Ian Forbes <iforbes@vmware.com>,
	Maaz Mombasawala <mombasawalam@vmware.com>,
	"zack@kde.org" <zack@kde.org>
Subject: Re: [PATCH v4 2/8] drm/atomic: Add support for mouse hotspots
Date: Wed, 5 Jul 2023 09:08:07 -0700	[thread overview]
Message-ID: <ce7754be-8085-ffd1-93f3-a774f7fcdb87@vmware.com> (raw)
In-Reply-To: <20230704110845.490344f5@eldfell>



On 7/4/23 01:08, Pekka Paalanen wrote:
> On Mon, 3 Jul 2023 14:06:56 -0700
> Michael Banack <banackm@vmware.com> wrote:
>
>> Hi, I can speak to the virtual mouse/console half of this from the
>> VMware-side.
>>
>> I believe Zack's preparing a new set of comments here that can speak to
>> most of your concerns, but I'll answer some of the other questions directly.
>>
>> On 6/29/23 01:03, Pekka Paalanen wrote:
>>> Is it really required that the hotspot coordinates fall inside the
>>> cursor plane? Will the atomic commit be rejected otherwise?
>> Most console systems require the hotspot to get within the cursor image,
>> but in theory it's semantically meaningful to have it extend outside the
>> image.
>>
>> VMware's clients in particular will clamp the hotspot to the dimension
>> of the cursor image if we receive one that's out of bounds.
>>
>> So I would assume the right thing to do here would be to allow it and
>> let the clients figure out how to best handle it.
> Hi,
>
> if it is normal that clients clamp the hotspot to inside the cursor
> image, then I would come to the opposite conclusion: KMS UAPI needs to
> require the hotspot to be within the cursor image. Otherwise the
> results would be unpredictable, if clients still continue to clamp it
> anyway. I would assume that clients in use today are not prepared to
> handle hotspot outside the cursor image.
>
> It is also not a big deal to require that. I think it would be very rare
> to not have hotspot inside the cursor image, and even if it happened,
> the only consequence would be that the guest display server falls back
> to rendered cursor instead of a cursor plane. That may happen any time
> anyway, if an application sets e.g. a huge cursor that exceeds cursor
> plane size limits.
Hypervisors are normally more privileged than the kernel, so any 
hypervisor/remoting client here really should be dealing with this case 
rather than trusting the kernel to handle it for them.

 From that perspective, we would normally try to preserve the 
application/guest semantics as much as possible, and then that gives us 
the ability to deal with this on the hypervisor side if it turns out 
that there's a critical case with the hotspot outside the image that we 
need to handle.

But that said, while I've seen real Guests do this in the past, I don't 
recall seeing this from any modern operating systems, so I don't think 
it's a big deal for us either way.

>
> What I'm after with the question is that the requirement must be spelled
> out clearly if it is there, or not even hinted at if it's not there.
Agreed.

>>> The question of which input device corresponds to which cursor plane
>>> might be good to answer too. I presume the VM runner is configured to
>>> expose exactly one of each, so there can be only one association?
>> As far as I know, all of the VM consoles are written as though they
>> taking the place of what would the the physical monitors and input
>> devices on a native machine.  So they assume that there is one user,
>> sitting in front of one console, and all monitors/input devices are
>> being used by that user.
> Ok, but having a single user does not mean that there cannot be
> multiple independent pointers, especially on Wayland. The same with
> keyboards.

True, and if the userspace is doing anything complicated here, the 
hypervisor has to be responsible for ensuring that whatever it's doing 
works with that, or else this system won't work.  I don't know that the 
kernel is in a good position to police that.

>
>> Any more complicated multi-user/multi-cursor setup would have to be
>> coordinated through a lot of layers (ie from the VM's userspace/kernel
>> and then through hypervisor/client-consoles), and as far as I know
>> nobody has tried to plumb that all the way through.  Even physical
>> multi-user/multi-console configurations like that are rare.
> Right.
>
> So if there a VM viewer client running on a Wayland system, that viewer
> client may be presented with an arbitrary number of independent
> pointer/keyboard/touchscreen input devices. Then it is up to the client
> to pick one at a time to pass through to the VM.
>
> That's fine.
>
> I just think it would be good to document, that VM/viewer systems
> expect to expose just a single pointer to the guest, hence it is
> obvious which input device in the guest is associated with all the
> cursor planes in the guest.

I don't have a problem adding something that suggests what we think the 
hypervisors are doing, but I would be a little cautious trying to 
prescribe what the hypervisors should be doing here.

I certainly can't speak for all of them, but we at least do a lot of odd 
tricks to keep this coordinated that violate what would normally be 
abstraction layers in a physical system such as having the mouse and the 
display adapter collude.  Ultimately it's the hypervisor that is 
responsible for doing the synchronization correctly, and the kernel 
really isn't involved there besides ferrying the right information down.

>
> Btw. what do you do if a guest display server simultaneously uses
> multiple cursor planes, assuming there are multiple outputs each with a
> cursor plane? Or does the VM/viewer system limit the number of outputs
> to one for the guest?

Zack would have to confirm what the vmwgfx driver does, but the VMware 
virtual display hardware at least only has one cursor position.  So I 
would assume that vmwgfx tries to only expose one plane and the rest get 
emulated, or else it just picks one to set live, but I'm not an expert 
on vmwgfx.

Normally we try to run a userspace agent in the Guest that also helps 
coordinate screen positions/resolutions to match what the user wanted on 
their client.  So when a user connects and requests from our UI that 
they want the screens to be a particular configuration, we then send a 
message to the userspace agent which coordinates with the display 
manager to request that setup.  You can certainly manually configure 
modes with things like rotation/topologies that break the console mouse, 
but we try not to put the user into that state as much as possible.  
Multiple cursors in the Guest display manager probably fall into that 
category.


--Michael Banack

  reply	other threads:[~2023-07-05 16:08 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-28  5:21 [PATCH v4 0/8] Fix cursor planes with virtualized drivers Zack Rusin
2023-06-28  5:21 ` [PATCH v4 1/8] drm: Disable the cursor plane on atomic contexts " Zack Rusin
2023-06-28  5:21   ` Zack Rusin
2023-06-28  5:21 ` [PATCH v4 2/8] drm/atomic: Add support for mouse hotspots Zack Rusin
2023-06-28  7:41   ` Pekka Paalanen
2023-06-28  7:54     ` Pekka Paalanen
2023-06-28  8:30       ` Javier Martinez Canillas
2023-06-28 19:54       ` Zack Rusin
2023-06-29  8:03         ` Pekka Paalanen
2023-07-03 21:06           ` Michael Banack
2023-07-04  8:08             ` Pekka Paalanen
2023-07-05 16:08               ` Michael Banack [this message]
2023-07-06  8:01                 ` Pekka Paalanen
2023-07-06 16:23                   ` Michael Banack
2023-07-07  8:38                     ` Pekka Paalanen
2023-07-07 20:54                       ` Michael Banack
2023-07-10  8:17                         ` Pekka Paalanen
2023-07-10 17:46                           ` Michael Banack
2023-07-11  7:14                             ` Pekka Paalanen
2023-07-18  8:41                               ` Javier Martinez Canillas
2023-07-18  9:27                                 ` Javier Martinez Canillas
2023-07-18  9:56                                 ` Pekka Paalanen
2023-06-28 14:15   ` Simon Ser
2023-06-28 16:26     ` Zack Rusin
2023-06-29  8:16       ` Pekka Paalanen
2023-07-03 21:15         ` Michael Banack
2023-06-28  5:21 ` [PATCH v4 3/8] drm/vmwgfx: Use the hotspot properties from cursor planes Zack Rusin
2023-06-28  5:21 ` [PATCH v4 4/8] drm/qxl: " Zack Rusin
2023-06-28  5:21 ` [PATCH v4 5/8] drm/vboxvideo: " Zack Rusin
2023-06-28  5:21 ` [PATCH v4 6/8] drm/virtio: " Zack Rusin
2023-06-28  5:21 ` [PATCH v4 7/8] drm: Remove legacy cursor hotspot code Zack Rusin
2023-06-28  5:21 ` [PATCH v4 8/8] drm: Introduce DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT Zack Rusin

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=ce7754be-8085-ffd1-93f3-a774f7fcdb87@vmware.com \
    --to=banackm@vmware.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=iforbes@vmware.com \
    --cc=javierm@redhat.com \
    --cc=krastevm@vmware.com \
    --cc=mombasawalam@vmware.com \
    --cc=mripard@kernel.org \
    --cc=ppaalanen@gmail.com \
    --cc=tzimmermann@suse.de \
    --cc=zack@kde.org \
    /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.