[v4,9/9] drm/vkms: convert to use faux_device

Commit Message

Greg KH Feb. 10, 2025, 12:30 p.m. UTC

The vkms driver does not need to create a platform device, as there is
no real platform resources associated it,  it only did so because it was
simple to do that in order to get a device to use for resource
management of drm resources.  Change the driver to use the faux device
instead as this is NOT a real platform device.

Cc: Louis Chauvet <[e-mail]>
Cc: Haneen Mohammed <[e-mail]>
Cc: Simona Vetter <[e-mail]>
Cc: Melissa Wen <[e-mail]>
Cc: Maarten Lankhorst <[e-mail]>
Cc: Maxime Ripard <[e-mail]>
Cc: Thomas Zimmermann <[e-mail]>
Cc: David Airlie <[e-mail]>
Cc: [e-mail]
Reviewed-by: Lyude Paul <[e-mail]>
Signed-off-by: Greg Kroah-Hartman <[e-mail]>
---
v4: - api tweaked due to parent pointer added to faux_device create
      function.
    - fixed space I removed in the .h file
 v3: new patch in the series.  For an example of the api working, does
     not have to be merged at this time, but I can take it if the
     maintainers give an ack.

 drivers/gpu/drm/vkms/vkms_drv.c | 28 ++++++++++++++--------------
 drivers/gpu/drm/vkms/vkms_drv.h |  4 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)

Comments

Louis Chauvet Feb. 10, 2025, 2:37 p.m. UTC | #1

On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> The vkms driver does not need to create a platform device, as there is
> no real platform resources associated it,  it only did so because it was
> simple to do that in order to get a device to use for resource
> management of drm resources.  Change the driver to use the faux device
> instead as this is NOT a real platform device.
> 
> Cc: Louis Chauvet <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Simona Vetter <[e-mail]>
> Cc: Melissa Wen <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Thomas Zimmermann <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: [e-mail]
> Reviewed-by: Lyude Paul <[e-mail]>
> Signed-off-by: Greg Kroah-Hartman <[e-mail]>

Tested-by: Louis Chauvet <[e-mail]>
Reviewed-by: Louis Chauvet <[e-mail]>

Thanks for the modification, it seems to work.

Louis chauvet

Greg KH Feb. 10, 2025, 2:49 p.m. UTC | #2

On Mon, Feb 10, 2025 at 03:37:42PM +0100, Louis Chauvet wrote:
> On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > The vkms driver does not need to create a platform device, as there is
> > no real platform resources associated it,  it only did so because it was
> > simple to do that in order to get a device to use for resource
> > management of drm resources.  Change the driver to use the faux device
> > instead as this is NOT a real platform device.
> > 
> > Cc: Louis Chauvet <[e-mail]>
> > Cc: Haneen Mohammed <[e-mail]>
> > Cc: Simona Vetter <[e-mail]>
> > Cc: Melissa Wen <[e-mail]>
> > Cc: Maarten Lankhorst <[e-mail]>
> > Cc: Maxime Ripard <[e-mail]>
> > Cc: Thomas Zimmermann <[e-mail]>
> > Cc: David Airlie <[e-mail]>
> > Cc: [e-mail]
> > Reviewed-by: Lyude Paul <[e-mail]>
> > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> 
> Tested-by: Louis Chauvet <[e-mail]>
> Reviewed-by: Louis Chauvet <[e-mail]>
> 
> Thanks for the modification, it seems to work.

Great, thanks for testing!

greg k-h

Thomas Zimmermann Feb. 25, 2025, 11:41 a.m. UTC | #3

Hi

Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
>> The vkms driver does not need to create a platform device, as there is
>> no real platform resources associated it,  it only did so because it was
>> simple to do that in order to get a device to use for resource
>> management of drm resources.  Change the driver to use the faux device
>> instead as this is NOT a real platform device.
>>
>> Cc: Louis Chauvet <[e-mail]>
>> Cc: Haneen Mohammed <[e-mail]>
>> Cc: Simona Vetter <[e-mail]>
>> Cc: Melissa Wen <[e-mail]>
>> Cc: Maarten Lankhorst <[e-mail]>
>> Cc: Maxime Ripard <[e-mail]>
>> Cc: Thomas Zimmermann <[e-mail]>
>> Cc: David Airlie <[e-mail]>
>> Cc: [e-mail]
>> Reviewed-by: Lyude Paul <[e-mail]>
>> Signed-off-by: Greg Kroah-Hartman <[e-mail]>

Reviewed-by: Thomas Zimmermann <[e-mail]>

> Tested-by: Louis Chauvet <[e-mail]>
> Reviewed-by: Louis Chauvet <[e-mail]>
>
> Thanks for the modification, it seems to work.

Should this patch be merged through DRM trees? drm-misc-next is at 
v6.14-rc4 and has struct faux_device.

Best regards
Thomas



>
> Louis chauvet
>

Louis Chauvet Feb. 25, 2025, 1:51 p.m. UTC | #4

Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> Hi
> 
> Am 10.02.25 um 15:37 schrieb Louis Chauvet:
>> On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
>>> The vkms driver does not need to create a platform device, as there is
>>> no real platform resources associated it,  it only did so because it was
>>> simple to do that in order to get a device to use for resource
>>> management of drm resources.  Change the driver to use the faux device
>>> instead as this is NOT a real platform device.
>>>
>>> Cc: Louis Chauvet <[e-mail]>
>>> Cc: Haneen Mohammed <[e-mail]>
>>> Cc: Simona Vetter <[e-mail]>
>>> Cc: Melissa Wen <[e-mail]>
>>> Cc: Maarten Lankhorst <[e-mail]>
>>> Cc: Maxime Ripard <[e-mail]>
>>> Cc: Thomas Zimmermann <[e-mail]>
>>> Cc: David Airlie <[e-mail]>
>>> Cc: [e-mail]
>>> Reviewed-by: Lyude Paul <[e-mail]>
>>> Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> 
> Reviewed-by: Thomas Zimmermann <[e-mail]>
> 
>> Tested-by: Louis Chauvet <[e-mail]>
>> Reviewed-by: Louis Chauvet <[e-mail]>
>>
>> Thanks for the modification, it seems to work.
> 
> Should this patch be merged through DRM trees? drm-misc-next is at
> v6.14-rc4 and has struct faux_device.

Hi,

I was not aware the faux-device was merged, as it is a new feature, I 
expected it to reach drm-misc-next on 6.15-rc1.

I plan to merge [1] today/tomorrow (well tested with platform_device), 
and then I will submit an updated version of this patch (only trivial 
conflicts, but never tested with multiple VKMS devices).

[1]:https://lore.kernel.org/all/[e-mail]/

Thanks,
Louis Chauvet

> Best regards
> Thomas
> 
> 
> 
>>
>> Louis chauvet
>>
>

Greg KH Feb. 26, 2025, 10:07 a.m. UTC | #5

On Tue, Feb 25, 2025 at 02:51:40PM +0100, Louis Chauvet wrote:
> 
> 
> Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> > Hi
> > 
> > Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> > > On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > > > The vkms driver does not need to create a platform device, as there is
> > > > no real platform resources associated it,  it only did so because it was
> > > > simple to do that in order to get a device to use for resource
> > > > management of drm resources.  Change the driver to use the faux device
> > > > instead as this is NOT a real platform device.
> > > > 
> > > > Cc: Louis Chauvet <[e-mail]>
> > > > Cc: Haneen Mohammed <[e-mail]>
> > > > Cc: Simona Vetter <[e-mail]>
> > > > Cc: Melissa Wen <[e-mail]>
> > > > Cc: Maarten Lankhorst <[e-mail]>
> > > > Cc: Maxime Ripard <[e-mail]>
> > > > Cc: Thomas Zimmermann <[e-mail]>
> > > > Cc: David Airlie <[e-mail]>
> > > > Cc: [e-mail]
> > > > Reviewed-by: Lyude Paul <[e-mail]>
> > > > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> > 
> > Reviewed-by: Thomas Zimmermann <[e-mail]>
> > 
> > > Tested-by: Louis Chauvet <[e-mail]>
> > > Reviewed-by: Louis Chauvet <[e-mail]>
> > > 
> > > Thanks for the modification, it seems to work.
> > 
> > Should this patch be merged through DRM trees? drm-misc-next is at
> > v6.14-rc4 and has struct faux_device.
> 
> Hi,
> 
> I was not aware the faux-device was merged, as it is a new feature, I
> expected it to reach drm-misc-next on 6.15-rc1.

I added it to Linus's tree just so that DRM could get these changes into
their tree now :)

> I plan to merge [1] today/tomorrow (well tested with platform_device), and
> then I will submit an updated version of this patch (only trivial conflicts,
> but never tested with multiple VKMS devices).
> 
> [1]:https://lore.kernel.org/all/[e-mail]/

Great, thanks!

greg k-h

José Expósito March 11, 2025, 5:20 p.m. UTC | #6

Hi everyone,

> On Tue, Feb 25, 2025 at 02:51:40PM +0100, Louis Chauvet wrote:
> > 
> > 
> > Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> > > Hi
> > > 
> > > Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> > > > On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > > > > The vkms driver does not need to create a platform device, as there is
> > > > > no real platform resources associated it,  it only did so because it was
> > > > > simple to do that in order to get a device to use for resource
> > > > > management of drm resources.  Change the driver to use the faux device
> > > > > instead as this is NOT a real platform device.
> > > > > 
> > > > > Cc: Louis Chauvet <[e-mail]>
> > > > > Cc: Haneen Mohammed <[e-mail]>
> > > > > Cc: Simona Vetter <[e-mail]>
> > > > > Cc: Melissa Wen <[e-mail]>
> > > > > Cc: Maarten Lankhorst <[e-mail]>
> > > > > Cc: Maxime Ripard <[e-mail]>
> > > > > Cc: Thomas Zimmermann <[e-mail]>
> > > > > Cc: David Airlie <[e-mail]>
> > > > > Cc: [e-mail]
> > > > > Reviewed-by: Lyude Paul <[e-mail]>
> > > > > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> > > 
> > > Reviewed-by: Thomas Zimmermann <[e-mail]>
> > > 
> > > > Tested-by: Louis Chauvet <[e-mail]>
> > > > Reviewed-by: Louis Chauvet <[e-mail]>
> > > > 
> > > > Thanks for the modification, it seems to work.
> > > 
> > > Should this patch be merged through DRM trees? drm-misc-next is at
> > > v6.14-rc4 and has struct faux_device.
> > 
> > Hi,
> > 
> > I was not aware the faux-device was merged, as it is a new feature, I
> > expected it to reach drm-misc-next on 6.15-rc1.
> 
> I added it to Linus's tree just so that DRM could get these changes into
> their tree now :)
> 
> > I plan to merge [1] today/tomorrow (well tested with platform_device), and
> > then I will submit an updated version of this patch (only trivial conflicts,
> > but never tested with multiple VKMS devices).
> > 
> > [1]:https://lore.kernel.org/all/[e-mail]/
> 
> Great, thanks!
> 
> greg k-h

Testing this patch again as part of some IGT tests I'm working on,
I noticed that, applying this patch on top of the latest drm-misc-next
triggers a warning at drivers/gpu/drm/drm_gem.c:571, in
drm_gem_get_pages():

    if (WARN_ON(!obj->filp))
            return ERR_PTR(-EINVAL);

To double check that the cause were not my local changes, I checked
out commit 2d4d775d11d3 ("drm: pl111: fix inconsistent indenting
warning"), which doesn't contain any vkms config changes, applied this
patch and the result is the same.

Louis, do you also see this warning?

For reference, the log is at the end of this email.
Jose

---

[  110.126952] [drm] Initialized vkms 1.0.0 for vkms on minor 1
[  110.129193] faux_driver vkms: [drm] fb1: vkmsdrmfb frame buffer device
[  110.326759] ------------[ cut here ]------------
[  110.326769] WARNING: CPU: 5 PID: 1106 at drivers/gpu/drm/drm_gem.c:571 drm_gem_get_pages+0x5e9/0x780
[  110.326795] Modules linked in: vkms snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables qrtr sunrpc binfmt_misc ppdev pktcdvd parport_pc e1000 parport i2c_piix4 pcspkr i2c_smbus joydev loop nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vmw_vmci zram vsock bochs serio_raw ata_generic pata_acpi fuse qemu_fw_cfg
[  110.326892] CPU: 5 UID: 42 PID: 1106 Comm: KMS thread Not tainted 6.14.0-rc4+ #2
[  110.326898] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-3.fc41 04/01/2014
[  110.326902] RIP: 0010:drm_gem_get_pages+0x5e9/0x780
[  110.326909] Code: 8d bd 78 fe ff ff e8 76 ca 8c fe 48 8d bd 78 fe ff ff e8 ea d9 8a fe e8 75 1a 17 01 e9 4b ff ff ff 48 8d 58 ff e9 6a fe ff ff <0f> 0b 49 c7 c7 ea ff ff ff e9 40 ff ff ff 0f 0b e9 0f fb ff ff 4c
[  110.326914] RSP: 0018:ffffc9000572f550 EFLAGS: 00010246
[  110.326919] RAX: 1ffff11024531402 RBX: ffff88812298a000 RCX: 0000000000000000
[  110.326923] RDX: dffffc0000000000 RSI: ffff8881125d54c8 RDI: ffff88812298a010
[  110.326927] RBP: ffffc9000572f750 R08: 8000000000000063 R09: fffff52000ae5ef0
[  110.326929] R10: ffffc9000572f787 R11: ffff888104638528 R12: 0000000000000000
[  110.326932] R13: ffff88812298a168 R14: ffff88812298a0e8 R15: ffff88812298a190
[  110.326936] FS:  00007f3cf4e536c0(0000) GS:ffff8883af080000(0000) knlGS:0000000000000000
[  110.326940] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  110.326944] CR2: 00007f3cd40337f8 CR3: 00000001394e4000 CR4: 00000000000006f0
[  110.326951] Call Trace:
[  110.326954]  <TASK>
[  110.326957]  ? show_regs.cold+0x19/0x1e
[  110.326965]  ? __warn.cold+0xbd/0x17f
[  110.326970]  ? drm_gem_get_pages+0x5e9/0x780
[  110.326975]  ? report_bug+0x20b/0x2d0
[  110.326982]  ? handle_bug+0x60/0xa0
[  110.326988]  ? exc_invalid_op+0x1c/0x50
[  110.326993]  ? asm_exc_invalid_op+0x1f/0x30
[  110.327000]  ? drm_gem_get_pages+0x5e9/0x780
[  110.327005]  ? kasan_save_stack+0x4d/0x60
[  110.327011]  ? kasan_save_stack+0x3d/0x60
[  110.327016]  ? kasan_save_track+0x1c/0x70
[  110.327020]  ? kasan_save_alloc_info+0x3b/0x50
[  110.327025]  ? __pfx_drm_gem_get_pages+0x10/0x10
[  110.327029]  ? drm_atomic_helper_commit+0x7c/0x290
[  110.327037]  ? dma_resv_get_singleton+0x99/0x2b0
[  110.327043]  drm_gem_shmem_get_pages+0x69/0x1f0
[  110.327048]  drm_gem_shmem_vmap+0x1f8/0x670
[  110.327054]  drm_gem_shmem_object_vmap+0xd/0x20
[  110.327058]  drm_gem_vmap+0x70/0xd0
[  110.327063]  drm_gem_vmap_unlocked+0x4f/0xa0
[  110.327067]  drm_gem_fb_vmap+0xaf/0x3c0
[  110.327073]  vkms_prepare_fb+0x6e/0x80 [vkms]
[  110.327081]  drm_atomic_helper_prepare_planes+0x1a5/0xba0
[  110.327088]  drm_atomic_helper_commit+0x128/0x290
[  110.327092]  ? __pfx_drm_atomic_helper_commit+0x10/0x10
[  110.327097]  drm_atomic_commit+0x203/0x2d0
[  110.327101]  ? _raw_spin_lock_irqsave+0x97/0xf0
[  110.327106]  ? __pfx_drm_atomic_commit+0x10/0x10
[  110.327110]  ? __pfx___drm_printfn_info+0x10/0x10
[  110.327116]  ? drm_event_reserve_init+0x1cd/0x260
[  110.327123]  drm_mode_atomic_ioctl+0x1d2b/0x2d30
[  110.327129]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
[  110.327134]  ? __kasan_check_write+0x18/0x20
[  110.327140]  drm_ioctl_kernel+0x177/0x2f0
[  110.327145]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
[  110.327149]  ? __pfx_drm_ioctl_kernel+0x10/0x10
[  110.327155]  ? __kasan_check_write+0x18/0x20
[  110.327160]  drm_ioctl+0x557/0xc90
[  110.327165]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
[  110.327170]  ? __pfx_drm_ioctl+0x10/0x10
[  110.327176]  ? selinux_file_ioctl+0x106/0x250
[  110.327181]  ? fdget+0x2ca/0x3d0
[  110.327185]  ? selinux_file_ioctl+0x106/0x250
[  110.327191]  __x64_sys_ioctl+0x13c/0x1a0
[  110.327198]  x64_sys_call+0xf3b/0x1d70
[  110.327202]  do_syscall_64+0x82/0x160
[  110.327208]  ? irqentry_exit+0x3f/0x50
[  110.327213]  ? exc_page_fault+0x94/0x110
[  110.327218]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  110.327223] RIP: 0033:0x7f3d100fda6d
[  110.327234] Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8 64 48 2b 04 25 28 00 00 00
[  110.327238] RSP: 002b:00007f3cf4e519b0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  110.327244] RAX: ffffffffffffffda RBX: 00007f3cd402a440 RCX: 00007f3d100fda6d
[  110.327247] RDX: 00007f3cf4e51a50 RSI: 00000000c03864bc RDI: 0000000000000035
[  110.327250] RBP: 00007f3cf4e51a00 R08: 00000000000000d0 R09: 0000000000000001
[  110.327254] R10: 0000000000000003 R11: 0000000000000246 R12: 00007f3cf4e51a50
[  110.327257] R13: 00000000c03864bc R14: 0000000000000035 R15: 00007f3cd4029f20
[  110.327262]  </TASK>
[  110.327264] ---[ end trace 0000000000000000 ]---

José Expósito March 11, 2025, 5:24 p.m. UTC | #7

On Tue, Mar 11, 2025 at 06:20:53PM +0100, José Expósito wrote:
> Hi everyone,
> 
> > On Tue, Feb 25, 2025 at 02:51:40PM +0100, Louis Chauvet wrote:
> > > 
> > > 
> > > Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> > > > Hi
> > > > 
> > > > Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> > > > > On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > > > > > The vkms driver does not need to create a platform device, as there is
> > > > > > no real platform resources associated it,  it only did so because it was
> > > > > > simple to do that in order to get a device to use for resource
> > > > > > management of drm resources.  Change the driver to use the faux device
> > > > > > instead as this is NOT a real platform device.
> > > > > > 
> > > > > > Cc: Louis Chauvet <[e-mail]>
> > > > > > Cc: Haneen Mohammed <[e-mail]>
> > > > > > Cc: Simona Vetter <[e-mail]>
> > > > > > Cc: Melissa Wen <[e-mail]>
> > > > > > Cc: Maarten Lankhorst <[e-mail]>
> > > > > > Cc: Maxime Ripard <[e-mail]>
> > > > > > Cc: Thomas Zimmermann <[e-mail]>
> > > > > > Cc: David Airlie <[e-mail]>
> > > > > > Cc: [e-mail]
> > > > > > Reviewed-by: Lyude Paul <[e-mail]>
> > > > > > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> > > > 
> > > > Reviewed-by: Thomas Zimmermann <[e-mail]>
> > > > 
> > > > > Tested-by: Louis Chauvet <[e-mail]>
> > > > > Reviewed-by: Louis Chauvet <[e-mail]>
> > > > > 
> > > > > Thanks for the modification, it seems to work.
> > > > 
> > > > Should this patch be merged through DRM trees? drm-misc-next is at
> > > > v6.14-rc4 and has struct faux_device.
> > > 
> > > Hi,
> > > 
> > > I was not aware the faux-device was merged, as it is a new feature, I
> > > expected it to reach drm-misc-next on 6.15-rc1.
> > 
> > I added it to Linus's tree just so that DRM could get these changes into
> > their tree now :)
> > 
> > > I plan to merge [1] today/tomorrow (well tested with platform_device), and
> > > then I will submit an updated version of this patch (only trivial conflicts,
> > > but never tested with multiple VKMS devices).
> > > 
> > > [1]:https://lore.kernel.org/all/[e-mail]/
> > 
> > Great, thanks!
> > 
> > greg k-h
> 
> Testing this patch again as part of some IGT tests I'm working on,
> I noticed that, applying this patch on top of the latest drm-misc-next
> triggers a warning at drivers/gpu/drm/drm_gem.c:571, in
> drm_gem_get_pages():
> 
>     if (WARN_ON(!obj->filp))
>             return ERR_PTR(-EINVAL);

I forgot to mention that this warning is triggered just by `sudo modprobe vkms`.

Sorry for the additional email.
Jose

> To double check that the cause were not my local changes, I checked
> out commit 2d4d775d11d3 ("drm: pl111: fix inconsistent indenting
> warning"), which doesn't contain any vkms config changes, applied this
> patch and the result is the same.
> 
> Louis, do you also see this warning?
> 
> For reference, the log is at the end of this email.
> Jose
> 
> ---
> 
> [  110.126952] [drm] Initialized vkms 1.0.0 for vkms on minor 1
> [  110.129193] faux_driver vkms: [drm] fb1: vkmsdrmfb frame buffer device
> [  110.326759] ------------[ cut here ]------------
> [  110.326769] WARNING: CPU: 5 PID: 1106 at drivers/gpu/drm/drm_gem.c:571 drm_gem_get_pages+0x5e9/0x780
> [  110.326795] Modules linked in: vkms snd_seq_dummy snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables qrtr sunrpc binfmt_misc ppdev pktcdvd parport_pc e1000 parport i2c_piix4 pcspkr i2c_smbus joydev loop nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vmw_vmci zram vsock bochs serio_raw ata_generic pata_acpi fuse qemu_fw_cfg
> [  110.326892] CPU: 5 UID: 42 PID: 1106 Comm: KMS thread Not tainted 6.14.0-rc4+ #2
> [  110.326898] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-3.fc41 04/01/2014
> [  110.326902] RIP: 0010:drm_gem_get_pages+0x5e9/0x780
> [  110.326909] Code: 8d bd 78 fe ff ff e8 76 ca 8c fe 48 8d bd 78 fe ff ff e8 ea d9 8a fe e8 75 1a 17 01 e9 4b ff ff ff 48 8d 58 ff e9 6a fe ff ff <0f> 0b 49 c7 c7 ea ff ff ff e9 40 ff ff ff 0f 0b e9 0f fb ff ff 4c
> [  110.326914] RSP: 0018:ffffc9000572f550 EFLAGS: 00010246
> [  110.326919] RAX: 1ffff11024531402 RBX: ffff88812298a000 RCX: 0000000000000000
> [  110.326923] RDX: dffffc0000000000 RSI: ffff8881125d54c8 RDI: ffff88812298a010
> [  110.326927] RBP: ffffc9000572f750 R08: 8000000000000063 R09: fffff52000ae5ef0
> [  110.326929] R10: ffffc9000572f787 R11: ffff888104638528 R12: 0000000000000000
> [  110.326932] R13: ffff88812298a168 R14: ffff88812298a0e8 R15: ffff88812298a190
> [  110.326936] FS:  00007f3cf4e536c0(0000) GS:ffff8883af080000(0000) knlGS:0000000000000000
> [  110.326940] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  110.326944] CR2: 00007f3cd40337f8 CR3: 00000001394e4000 CR4: 00000000000006f0
> [  110.326951] Call Trace:
> [  110.326954]  <TASK>
> [  110.326957]  ? show_regs.cold+0x19/0x1e
> [  110.326965]  ? __warn.cold+0xbd/0x17f
> [  110.326970]  ? drm_gem_get_pages+0x5e9/0x780
> [  110.326975]  ? report_bug+0x20b/0x2d0
> [  110.326982]  ? handle_bug+0x60/0xa0
> [  110.326988]  ? exc_invalid_op+0x1c/0x50
> [  110.326993]  ? asm_exc_invalid_op+0x1f/0x30
> [  110.327000]  ? drm_gem_get_pages+0x5e9/0x780
> [  110.327005]  ? kasan_save_stack+0x4d/0x60
> [  110.327011]  ? kasan_save_stack+0x3d/0x60
> [  110.327016]  ? kasan_save_track+0x1c/0x70
> [  110.327020]  ? kasan_save_alloc_info+0x3b/0x50
> [  110.327025]  ? __pfx_drm_gem_get_pages+0x10/0x10
> [  110.327029]  ? drm_atomic_helper_commit+0x7c/0x290
> [  110.327037]  ? dma_resv_get_singleton+0x99/0x2b0
> [  110.327043]  drm_gem_shmem_get_pages+0x69/0x1f0
> [  110.327048]  drm_gem_shmem_vmap+0x1f8/0x670
> [  110.327054]  drm_gem_shmem_object_vmap+0xd/0x20
> [  110.327058]  drm_gem_vmap+0x70/0xd0
> [  110.327063]  drm_gem_vmap_unlocked+0x4f/0xa0
> [  110.327067]  drm_gem_fb_vmap+0xaf/0x3c0
> [  110.327073]  vkms_prepare_fb+0x6e/0x80 [vkms]
> [  110.327081]  drm_atomic_helper_prepare_planes+0x1a5/0xba0
> [  110.327088]  drm_atomic_helper_commit+0x128/0x290
> [  110.327092]  ? __pfx_drm_atomic_helper_commit+0x10/0x10
> [  110.327097]  drm_atomic_commit+0x203/0x2d0
> [  110.327101]  ? _raw_spin_lock_irqsave+0x97/0xf0
> [  110.327106]  ? __pfx_drm_atomic_commit+0x10/0x10
> [  110.327110]  ? __pfx___drm_printfn_info+0x10/0x10
> [  110.327116]  ? drm_event_reserve_init+0x1cd/0x260
> [  110.327123]  drm_mode_atomic_ioctl+0x1d2b/0x2d30
> [  110.327129]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [  110.327134]  ? __kasan_check_write+0x18/0x20
> [  110.327140]  drm_ioctl_kernel+0x177/0x2f0
> [  110.327145]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [  110.327149]  ? __pfx_drm_ioctl_kernel+0x10/0x10
> [  110.327155]  ? __kasan_check_write+0x18/0x20
> [  110.327160]  drm_ioctl+0x557/0xc90
> [  110.327165]  ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
> [  110.327170]  ? __pfx_drm_ioctl+0x10/0x10
> [  110.327176]  ? selinux_file_ioctl+0x106/0x250
> [  110.327181]  ? fdget+0x2ca/0x3d0
> [  110.327185]  ? selinux_file_ioctl+0x106/0x250
> [  110.327191]  __x64_sys_ioctl+0x13c/0x1a0
> [  110.327198]  x64_sys_call+0xf3b/0x1d70
> [  110.327202]  do_syscall_64+0x82/0x160
> [  110.327208]  ? irqentry_exit+0x3f/0x50
> [  110.327213]  ? exc_page_fault+0x94/0x110
> [  110.327218]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [  110.327223] RIP: 0033:0x7f3d100fda6d
> [  110.327234] Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8 64 48 2b 04 25 28 00 00 00
> [  110.327238] RSP: 002b:00007f3cf4e519b0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  110.327244] RAX: ffffffffffffffda RBX: 00007f3cd402a440 RCX: 00007f3d100fda6d
> [  110.327247] RDX: 00007f3cf4e51a50 RSI: 00000000c03864bc RDI: 0000000000000035
> [  110.327250] RBP: 00007f3cf4e51a00 R08: 00000000000000d0 R09: 0000000000000001
> [  110.327254] R10: 0000000000000003 R11: 0000000000000246 R12: 00007f3cf4e51a50
> [  110.327257] R13: 00000000c03864bc R14: 0000000000000035 R15: 00007f3cd4029f20
> [  110.327262]  </TASK>
> [  110.327264] ---[ end trace 0000000000000000 ]---

Greg KH March 12, 2025, 6:22 a.m. UTC | #8

On Tue, Mar 11, 2025 at 06:20:53PM +0100, José Expósito wrote:
> Hi everyone,
> 
> > On Tue, Feb 25, 2025 at 02:51:40PM +0100, Louis Chauvet wrote:
> > > 
> > > 
> > > Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> > > > Hi
> > > > 
> > > > Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> > > > > On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > > > > > The vkms driver does not need to create a platform device, as there is
> > > > > > no real platform resources associated it,  it only did so because it was
> > > > > > simple to do that in order to get a device to use for resource
> > > > > > management of drm resources.  Change the driver to use the faux device
> > > > > > instead as this is NOT a real platform device.
> > > > > > 
> > > > > > Cc: Louis Chauvet <[e-mail]>
> > > > > > Cc: Haneen Mohammed <[e-mail]>
> > > > > > Cc: Simona Vetter <[e-mail]>
> > > > > > Cc: Melissa Wen <[e-mail]>
> > > > > > Cc: Maarten Lankhorst <[e-mail]>
> > > > > > Cc: Maxime Ripard <[e-mail]>
> > > > > > Cc: Thomas Zimmermann <[e-mail]>
> > > > > > Cc: David Airlie <[e-mail]>
> > > > > > Cc: [e-mail]
> > > > > > Reviewed-by: Lyude Paul <[e-mail]>
> > > > > > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> > > > 
> > > > Reviewed-by: Thomas Zimmermann <[e-mail]>
> > > > 
> > > > > Tested-by: Louis Chauvet <[e-mail]>
> > > > > Reviewed-by: Louis Chauvet <[e-mail]>
> > > > > 
> > > > > Thanks for the modification, it seems to work.
> > > > 
> > > > Should this patch be merged through DRM trees? drm-misc-next is at
> > > > v6.14-rc4 and has struct faux_device.
> > > 
> > > Hi,
> > > 
> > > I was not aware the faux-device was merged, as it is a new feature, I
> > > expected it to reach drm-misc-next on 6.15-rc1.
> > 
> > I added it to Linus's tree just so that DRM could get these changes into
> > their tree now :)
> > 
> > > I plan to merge [1] today/tomorrow (well tested with platform_device), and
> > > then I will submit an updated version of this patch (only trivial conflicts,
> > > but never tested with multiple VKMS devices).
> > > 
> > > [1]:https://lore.kernel.org/all/[e-mail]/
> > 
> > Great, thanks!
> > 
> > greg k-h
> 
> Testing this patch again as part of some IGT tests I'm working on,
> I noticed that, applying this patch on top of the latest drm-misc-next
> triggers a warning at drivers/gpu/drm/drm_gem.c:571, in
> drm_gem_get_pages():
> 
>     if (WARN_ON(!obj->filp))
>             return ERR_PTR(-EINVAL);

I don't see how the faux bus change would have anything to do with a
filp as that's not related as far as I can tell.  But I don't know the
drm layer at all, where does that filp come from?

thanks,

greg k-h

Simona Vetter March 13, 2025, 2:22 p.m. UTC | #9

On Wed, Mar 12, 2025 at 07:22:07AM +0100, Greg KH wrote:
> On Tue, Mar 11, 2025 at 06:20:53PM +0100, José Expósito wrote:
> > Hi everyone,
> > 
> > > On Tue, Feb 25, 2025 at 02:51:40PM +0100, Louis Chauvet wrote:
> > > > 
> > > > 
> > > > Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> > > > > Hi
> > > > > 
> > > > > Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> > > > > > On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > > > > > > The vkms driver does not need to create a platform device, as there is
> > > > > > > no real platform resources associated it,  it only did so because it was
> > > > > > > simple to do that in order to get a device to use for resource
> > > > > > > management of drm resources.  Change the driver to use the faux device
> > > > > > > instead as this is NOT a real platform device.
> > > > > > > 
> > > > > > > Cc: Louis Chauvet <[e-mail]>
> > > > > > > Cc: Haneen Mohammed <[e-mail]>
> > > > > > > Cc: Simona Vetter <[e-mail]>
> > > > > > > Cc: Melissa Wen <[e-mail]>
> > > > > > > Cc: Maarten Lankhorst <[e-mail]>
> > > > > > > Cc: Maxime Ripard <[e-mail]>
> > > > > > > Cc: Thomas Zimmermann <[e-mail]>
> > > > > > > Cc: David Airlie <[e-mail]>
> > > > > > > Cc: [e-mail]
> > > > > > > Reviewed-by: Lyude Paul <[e-mail]>
> > > > > > > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> > > > > 
> > > > > Reviewed-by: Thomas Zimmermann <[e-mail]>
> > > > > 
> > > > > > Tested-by: Louis Chauvet <[e-mail]>
> > > > > > Reviewed-by: Louis Chauvet <[e-mail]>
> > > > > > 
> > > > > > Thanks for the modification, it seems to work.
> > > > > 
> > > > > Should this patch be merged through DRM trees? drm-misc-next is at
> > > > > v6.14-rc4 and has struct faux_device.
> > > > 
> > > > Hi,
> > > > 
> > > > I was not aware the faux-device was merged, as it is a new feature, I
> > > > expected it to reach drm-misc-next on 6.15-rc1.
> > > 
> > > I added it to Linus's tree just so that DRM could get these changes into
> > > their tree now :)
> > > 
> > > > I plan to merge [1] today/tomorrow (well tested with platform_device), and
> > > > then I will submit an updated version of this patch (only trivial conflicts,
> > > > but never tested with multiple VKMS devices).
> > > > 
> > > > [1]:https://lore.kernel.org/all/[e-mail]/
> > > 
> > > Great, thanks!
> > > 
> > > greg k-h
> > 
> > Testing this patch again as part of some IGT tests I'm working on,
> > I noticed that, applying this patch on top of the latest drm-misc-next
> > triggers a warning at drivers/gpu/drm/drm_gem.c:571, in
> > drm_gem_get_pages():
> > 
> >     if (WARN_ON(!obj->filp))
> >             return ERR_PTR(-EINVAL);
> 
> I don't see how the faux bus change would have anything to do with a
> filp as that's not related as far as I can tell.  But I don't know the
> drm layer at all, where does that filp come from?

Yeah that filp is the shmem file that backs gem bo. That's very far away
from anything device/driver related datastrctures. If this is a new
failure due to the aux bux conversion then it would be really surprising.
-Sima

José Expósito March 13, 2025, 5:20 p.m. UTC | #10

On Thu, Mar 13, 2025 at 03:22:21PM +0100, Simona Vetter wrote:
> On Wed, Mar 12, 2025 at 07:22:07AM +0100, Greg KH wrote:
> > On Tue, Mar 11, 2025 at 06:20:53PM +0100, José Expósito wrote:
> > > Hi everyone,
> > > 
> > > > On Tue, Feb 25, 2025 at 02:51:40PM +0100, Louis Chauvet wrote:
> > > > > 
> > > > > 
> > > > > Le 25/02/2025 à 12:41, Thomas Zimmermann a écrit :
> > > > > > Hi
> > > > > > 
> > > > > > Am 10.02.25 um 15:37 schrieb Louis Chauvet:
> > > > > > > On 10/02/25 - 13:30, Greg Kroah-Hartman wrote:
> > > > > > > > The vkms driver does not need to create a platform device, as there is
> > > > > > > > no real platform resources associated it,  it only did so because it was
> > > > > > > > simple to do that in order to get a device to use for resource
> > > > > > > > management of drm resources.  Change the driver to use the faux device
> > > > > > > > instead as this is NOT a real platform device.
> > > > > > > > 
> > > > > > > > Cc: Louis Chauvet <[e-mail]>
> > > > > > > > Cc: Haneen Mohammed <[e-mail]>
> > > > > > > > Cc: Simona Vetter <[e-mail]>
> > > > > > > > Cc: Melissa Wen <[e-mail]>
> > > > > > > > Cc: Maarten Lankhorst <[e-mail]>
> > > > > > > > Cc: Maxime Ripard <[e-mail]>
> > > > > > > > Cc: Thomas Zimmermann <[e-mail]>
> > > > > > > > Cc: David Airlie <[e-mail]>
> > > > > > > > Cc: [e-mail]
> > > > > > > > Reviewed-by: Lyude Paul <[e-mail]>
> > > > > > > > Signed-off-by: Greg Kroah-Hartman <[e-mail]>
> > > > > > 
> > > > > > Reviewed-by: Thomas Zimmermann <[e-mail]>
> > > > > > 
> > > > > > > Tested-by: Louis Chauvet <[e-mail]>
> > > > > > > Reviewed-by: Louis Chauvet <[e-mail]>
> > > > > > > 
> > > > > > > Thanks for the modification, it seems to work.
> > > > > > 
> > > > > > Should this patch be merged through DRM trees? drm-misc-next is at
> > > > > > v6.14-rc4 and has struct faux_device.
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > I was not aware the faux-device was merged, as it is a new feature, I
> > > > > expected it to reach drm-misc-next on 6.15-rc1.
> > > > 
> > > > I added it to Linus's tree just so that DRM could get these changes into
> > > > their tree now :)
> > > > 
> > > > > I plan to merge [1] today/tomorrow (well tested with platform_device), and
> > > > > then I will submit an updated version of this patch (only trivial conflicts,
> > > > > but never tested with multiple VKMS devices).
> > > > > 
> > > > > [1]:https://lore.kernel.org/all/[e-mail]/
> > > > 
> > > > Great, thanks!
> > > > 
> > > > greg k-h
> > > 
> > > Testing this patch again as part of some IGT tests I'm working on,
> > > I noticed that, applying this patch on top of the latest drm-misc-next
> > > triggers a warning at drivers/gpu/drm/drm_gem.c:571, in
> > > drm_gem_get_pages():
> > > 
> > >     if (WARN_ON(!obj->filp))
> > >             return ERR_PTR(-EINVAL);
> > 
> > I don't see how the faux bus change would have anything to do with a
> > filp as that's not related as far as I can tell.  But I don't know the
> > drm layer at all, where does that filp come from?
> 
> Yeah that filp is the shmem file that backs gem bo. That's very far away
> from anything device/driver related datastrctures. If this is a new
> failure due to the aux bux conversion then it would be really surprising.

Agreed, I find it surprising, but reverting the patch removes the warning.

It's most likely an issue on my side, but I decided to double check just
in case someone else is also seeing this warning.

Jose

> -Sima
> 
> -- 
> Simona Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Patch

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index e0409aba9349..b3fa0e7c7751 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -10,7 +10,7 @@ 
  */
 
 #include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/device/faux.h>
 #include <linux/dma-mapping.h>
 
 #include <drm/clients/drm_client_setup.h>
@@ -177,25 +177,25 @@  static int vkms_modeset_init(struct vkms_device *vkmsdev)
 static int vkms_create(struct vkms_config *config)
 {
 	int ret;
-	struct platform_device *pdev;
+	struct faux_device *fdev;
 	struct vkms_device *vkms_device;
 
-	pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	fdev = faux_device_create(DRIVER_NAME, NULL, NULL);
+	if (!fdev)
+		return -ENODEV;
 
-	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
+	if (!devres_open_group(&fdev->dev, NULL, GFP_KERNEL)) {
 		ret = -ENOMEM;
 		goto out_unregister;
 	}
 
-	vkms_device = devm_drm_dev_alloc(&pdev->dev, &vkms_driver,
+	vkms_device = devm_drm_dev_alloc(&fdev->dev, &vkms_driver,
 					 struct vkms_device, drm);
 	if (IS_ERR(vkms_device)) {
 		ret = PTR_ERR(vkms_device);
 		goto out_devres;
 	}
-	vkms_device->platform = pdev;
+	vkms_device->faux_dev = fdev;
 	vkms_device->config = config;
 	config->dev = vkms_device;
 
@@ -229,9 +229,9 @@  static int vkms_create(struct vkms_config *config)
 	return 0;
 
 out_devres:
-	devres_release_group(&pdev->dev, NULL);
+	devres_release_group(&fdev->dev, NULL);
 out_unregister:
-	platform_device_unregister(pdev);
+	faux_device_destroy(fdev);
 	return ret;
 }
 
@@ -259,19 +259,19 @@  static int __init vkms_init(void)
 
 static void vkms_destroy(struct vkms_config *config)
 {
-	struct platform_device *pdev;
+	struct faux_device *fdev;
 
 	if (!config->dev) {
 		DRM_INFO("vkms_device is NULL.\n");
 		return;
 	}
 
-	pdev = config->dev->platform;
+	fdev = config->dev->faux_dev;
 
 	drm_dev_unregister(&config->dev->drm);
 	drm_atomic_helper_shutdown(&config->dev->drm);
-	devres_release_group(&pdev->dev, NULL);
-	platform_device_unregister(pdev);
+	devres_release_group(&fdev->dev, NULL);
+	faux_device_destroy(fdev);
 
 	config->dev = NULL;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 00541eff3d1b..f56af53856f7 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -209,13 +209,13 @@  struct vkms_config {
  * struct vkms_device - Description of a VKMS device
  *
  * @drm - Base device in DRM
- * @platform - Associated platform device
+ * @faux_dev - Associated faux device
  * @output - Configuration and sub-components of the VKMS device
  * @config: Configuration used in this VKMS device
  */
 struct vkms_device {
 	struct drm_device drm;
-	struct platform_device *platform;
+	struct faux_device *faux_dev;
 	struct vkms_output output;
 	const struct vkms_config *config;
 };