drivers: hv: dxgkrnl: Driver overview
Subsystems (2)
cover drivers: hv: dxgkrnl: Driver overview
This is a follow-up on the changes we sent a few months back[1].
[1] https://lore.kernel.org/lkml/[email protected]/
The patches address the feedback, given by Greg KH and other reviewers, contain bug fixes, the implementation of asynchronous VM bus messages to the host and contain the remaining implementation of our vGPU / Compute hardware virtualization support that powers the Windows Subsystem for Linux (WSL) and will soon power the Windows Subsystem for Android (WSA).
This set of patches is rebuilt from ground up and organized in logical layers to better understand how the driver is built, making it easier for reviewers to understand and follow. The first patch introduces headers and driver initialization. The subsequent patches add additional functionality to the driver.
Per earlier feedback, the driver is now located under the Hyper-V node as it is not a classic Linux GPU (KMS/DRM) driver and really only make sense when running under a Windows host under Hyper-V. Although we refer to this driver as vGPU for shorthand, in reality this is a generic virtualization infrastructure for various class of compute accelerators, the most popular and ubiquitous being the GPU. We support virtualization of non-GPU devices through this infrastructure. These devices are exposed to user-space and used by various API and framework, such as CUDA, OpenCL, OpenVINO, OneAPI, DX12, etc...
One critical piece of feedback, provided by the community in our earlier submission, was the lack of an open source user-space to go along with our kernel compute driver. At the time we only had CUDA and DX12 user-space APIs, both of which being closed source. This is a divisive issue in the community and is heavily debated (https://lwn.net/Articles/821817/).
We took this feedback to heart and we spent the last year working on a way to address this key piece of feedback. Working closely with our partner Intel, we're happy to say that we now have a fully open source user-space for the OpenCL, OpenVINO and OneAPI compute family of APIs on Intel GPU platforms. This is supported by this open source project
(https://github.com/intel/compute-runtime).
To make it easy for our partners to build compute drivers, which are usable in both Windows and WSL environment, we provide a library, that provides a stable interface to our compute device abstraction. This was originally part of the libdxcore closed source API, but have now spawn that off into it's own open source project (https://github.com/microsoft/libdxg).
Between the Intel compute runtime project and libdxg, we now have a fully open source implementation of our virtualized compute stack inside of WSL. We will continue to support both open source user-space API against our compute abstraction as well as closed source one (CUDA, DX12), leaving it to the API owners and partners to decide what makes the most sense for them.
A lot of efforts went into addressing community feedback in this revised set of patches and we hope this is getting closer to what the community would like to see.
We're looking forward additional feedback.
Overview
-------------------------------------------------------
dxgkrnl is a driver for Hyper-V virtual compute devices, such as vGPU devices, which are projected to a Linux virtual machine (VM) by a Windows host. dxgkrnl works in context of WDDM (Windows Display Driver Model) for GPU or MCDM (Microsoft Compute Driver Model) for non-GPU devices. WDDM/MCDM consists of the following components:
- Graphics or Compute applications
- A graphics or compute user mode API (for example OpenGL, Vulkan, OpenCL, OpenVINO, OneAPI, CUDA, DX12, ...)
- User Mode Driver (UMD), written by a hardware vendor
- optional libdxg library helping UMD portability across Windows and Linux
- dxgkrnl Linux kernel driver (this driver)
- Kernel mode port driver on the Windows host (dxgkrnl.sys / dxgmms*.sys)
- Kernel Mode miniport driver (KMD) on the Windows host, written by a hardware vendor running on the Windows host and interfacing with the hardware device.
dxgkrnl exposes a subset the WDDM/MCDM D3DKMT interface to user space. See
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/
This interface provides user space with an abstracted view and control of compute devices in a portable way across Windows and WSL. It is used for devices such as GPU or AI/ML processors. This interface is mapped to custom IOCTLs on Linux (see d3dkmthk.h).
A more detailed overview of this architecture is available here:
https://devblogs.microsoft.com/directx/directx-heart-linux/
Compute devices are paravirtualized, meaning that the actual communication with the corresponding device happens on the host. The version of dxgkrnl inside of the Linux kernel coordinates with the version of dxgkrnl running on Windows to provide a consistent and portable abstraction for the device that the various APIs and UMD can rely on across Windows and Linux.
Dxgkrnl creates the /dev/dxg device, which can be used to enumerate devices. UMD or an API runtime open the device and send ioctls to create various device objects (dxgdevice, dxgcontext, dxgallocation, etc., defined in dxgkrnl.h) and to submit work to the device. The WDDM objects are represented in user mode as opaque handles (struct d3dkmthandle). Dxgkrnl creates a dxgprocess object for each process, which opens the /dev/dxg device. This object has a handle table, which is used to translate d3dkmt handles to kernel objects. Handle table definitions are in hmgr.h. There is also a global handle table for objects, which do not belong to a particular process.
Driver initialization
-------------------------------------------------------
When dxgkrnl is loaded, dxgkrnl registers for virtual PCI device arrival notifications and VM bus channel device notifications (see dxgmodule.c). When the first virtual device is started, dxgkrnl creates a misc device (/dev/dxg). A user mode client can open the /dev/dxg device and send IOCTLs to enumerate and control virtual compute devices.
Virtual device initialization
-------------------------------------------------------
A virtual device is represented by a dxgadapter object. It is created when the corresponding device arrives on the virtual PCI bus. The device vendor is PCI_VENDOR_ID_MICROSOFT and the device id is PCI_DEVICE_ID_VIRTUAL_RENDER. The adapter is started when the corresponding VM bus channel and the global VM bus channel are initialized.
Dynamic arrival/removal of devices is supported.
Internal objects
-------------------------------------------------------
Dxgkrnl creates various internal objects in response to IOCTL calls. The corresponsing objects are also created on the host. Each object is placed to a process or global handle table. Object handles (d3dkmthandle) are returned to user mode. The object handles are also used to reference objects on the host. Corresponding objects in the guest and the host have the same handle value to avoid handle translation. The internal objects are:
- dxgadapter
Represents a virtual device object. It is created for every device projected by the host to the VM.
- dxgprocess
The object is created for each Linux process, which opens /dxg/dev. It has the object handle table, which holds pointers to all internal objects, which are created by this process.
- dxgcontext
Represents a device execution thread in the packet scheduling mode (as oppose to the hardware scheduling mode).
- dxghwqueue
Represents a device execution thread in the hardware scheduling mode.
- dxgdevice
A collection of device contexts, allocations, resources, sync objects, etc.
- dxgallocation
Represents a device accessible memory allocation.
- dxgresource
A collection of dxgallocation objects. This object could be shared between devices and processes.
- dxgsharedresource
Represents a dxgresource object, which is sharable between processes.
- dxgsyncobject
Represents a device synchronization object, which is used to synchronize execution of the device execution contexts.
- dxgsharedsyncobject
Represent a device synchronization object, which is sharable between processes.
- dxgpagingqueue
Represents a queue, which is used to manage residency of the device allocation objects.
Communications with the host
-------------------------------------------------------
Dxgkrnl communicates with the host via Hyper-V VM bus channels. There is a global channel and a per device channel. The VM bus messages are defined in dxgvmbus.h and the implementation is in dxgvmbus.c.
Most VM bus messages to the host are synchronous. When the host enables the asynchronous mode, some high frequency VM bus messages are sent asynchronously to improve performance. When async messages are enabled, all VM bus messages are sent only via the global channel to maintain the order of messages on the host.
The host could send asynchronous messages to dxgkrnl via the global VM bus channel. The host messages are handled by dxgvmbuschannel_receive().
PCI config space of the device is used to exchange information between the host and the guest during dxgkrnl initialization. Look at dxg_pci_probe_device().
CPU access to device accessible allocations
-------------------------------------------------------
D3DKMT API allows creation of allocations, which are accessible by the device and the CPU. The global VM bus channels has associated IO space, which is used to implement CPU access to CPU visible allocations. For each such allocation the host allocates a portion of the guest IO space and maps it to the allocation memory (it could be in system memory or in device local memory). A user mode application calls the LX_DXLOCK2 ioctl to get the allocation CPU address. Dxgkrnl uses vm_mmap to allocate a user space VA range and maps it to the allocation IO space using io_remap_pfn_range(). This way Linux user mode virtual addresses point to the host system memory or device local memory.
Sharing objects between processes
-------------------------------------------------------
Some dxgkrnl objects could be shared between processes. This includes resources (dxgresource) and synchronization objects (dxgsyncobject).
The WDDM API provides a way to share objects using so called "NT handles". "NT handle" on Windows is a native Windows process handle (HANDLE). "NT handles" are implemented as file descriptors (FD) on Linux.
The LX_DXSHAREOBJECTS ioctl is used to get an FD for a shared object. Before a shared object can be used, it needs to be "opened" to get the "local" d3dkmthandle handle. The LX_DXOPENRESOURCEFROMNTHANDLE and LX_DXOPENSYNCOBJECTFROMNTHANDLE2 aioctls re used to open a shared object.
Iouri Tarassov (9):
drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
drivers: hv: dxgkrnl: Open device object, adapter enumeration,
dxgdevice, dxgcontext creation
drivers: hv: dxgkrnl: Implement creation/destruction of GPU
allocations/resources
drivers: hv: dxgkrnl: Implement operations with GPU sync objects
drivers: hv: dxgkrnl: Implement sharing resources and sync objects
drivers: hv: dxgkrnl: Seal the shared resource object when
dxgk_share_objects is called.
drivers: hv: dxgkrnl: Implementation of submit command, paging and
hardware queue.
drivers: hv: dxgkrnl: Implement various WDDM ioctls
drivers: hv: dxgkrnl: Implement DXGSYNCFILE
MAINTAINERS | 7 + drivers/hv/Kconfig | 2 + drivers/hv/Makefile | 1 + drivers/hv/dxgkrnl/Kconfig | 28 + drivers/hv/dxgkrnl/Makefile | 5 + drivers/hv/dxgkrnl/dxgadapter.c | 1375 ++++++++ drivers/hv/dxgkrnl/dxgkrnl.h | 953 ++++++ drivers/hv/dxgkrnl/dxgmodule.c | 924 +++++ drivers/hv/dxgkrnl/dxgprocess.c | 343 ++ drivers/hv/dxgkrnl/dxgsyncfile.c | 210 ++ drivers/hv/dxgkrnl/dxgsyncfile.h | 30 + drivers/hv/dxgkrnl/dxgvmbus.c | 3725 ++++++++++++++++++++ drivers/hv/dxgkrnl/dxgvmbus.h | 928 +++++ drivers/hv/dxgkrnl/hmgr.c | 597 ++++ drivers/hv/dxgkrnl/hmgr.h | 112 + drivers/hv/dxgkrnl/ioctl.c | 5458 ++++++++++++++++++++++++++++++ drivers/hv/dxgkrnl/misc.c | 43 + drivers/hv/dxgkrnl/misc.h | 97 + include/linux/hyperv.h | 16 + include/uapi/misc/d3dkmthk.h | 1954 +++++++++++ 20 files changed, 16808 insertions(+) create mode 100644 drivers/hv/dxgkrnl/Kconfig create mode 100644 drivers/hv/dxgkrnl/Makefile create mode 100644 drivers/hv/dxgkrnl/dxgadapter.c create mode 100644 drivers/hv/dxgkrnl/dxgkrnl.h create mode 100644 drivers/hv/dxgkrnl/dxgmodule.c create mode 100644 drivers/hv/dxgkrnl/dxgprocess.c create mode 100644 drivers/hv/dxgkrnl/dxgsyncfile.c create mode 100644 drivers/hv/dxgkrnl/dxgsyncfile.h create mode 100644 drivers/hv/dxgkrnl/dxgvmbus.c create mode 100644 drivers/hv/dxgkrnl/dxgvmbus.h create mode 100644 drivers/hv/dxgkrnl/hmgr.c create mode 100644 drivers/hv/dxgkrnl/hmgr.h create mode 100644 drivers/hv/dxgkrnl/ioctl.c create mode 100644 drivers/hv/dxgkrnl/misc.c create mode 100644 drivers/hv/dxgkrnl/misc.h create mode 100644 include/uapi/misc/d3dkmthk.h
Signature · 1 line
2.32.0
PATCH 2/9 drivers: hv: dxgkrnl: Open device object, adapter enumeration, dxgdevice, dxgcontext creation
- Handle opening of the device (/dev/dxg) file object and creation of dxgprocess. dxgprocess is created for each process, which opens /dev/dxg. dxgprocess is ref counted, so the exicting dxgprocess objects is used for a process, which opens the device object multiple time. dxgprocess is destroyed when the device object is closed.
- Implement ioctls for virtual GPU adapter enumeration:LX_DXENUMADAPTERS2, LX_DXENUMADAPTERS3
The IOCTLs return a d3dkmt handle for each enumerated adapter.
- Implement ioctl to query adapter information:LX_DXQUERYADAPTERINFO.
- Implement ioctls to open and closet a dxgadapter object:LX_DXOPENADAPTERFROMLUID, LX_DXCLOSEADAPTER
Note that dxgadapter is opened when LX_DXENUMADAPTERS2, LX_DXENUMADAPTERS3 are called.
- Implement ioctls for dxgdevice and dxgcontext creation/destruction:LX_DXCREATEDEVICE, LX_DXCREATECONTEXT, LX_DXCREATECONTEXTVIRTUAL, LX_DXDESTROYCONTEXT, LX_DXCREATEHWCONTEXT, LX_DXDESTROYDEVICE, LX_DXDESTROYHWCONTEXT
--- drivers/hv/dxgkrnl/dxgadapter.c | 494 +++++++++++++++++++ drivers/hv/dxgkrnl/dxgkrnl.h | 4 - drivers/hv/dxgkrnl/dxgprocess.c | 316 ++++++++++++- drivers/hv/dxgkrnl/dxgvmbus.c | 314 ++++++++++++ drivers/hv/dxgkrnl/hmgr.c | 509 ++++++++++++++++++++ drivers/hv/dxgkrnl/ioctl.c | 812 +++++++++++++++++++++++++++++++- 6 files changed, 2436 insertions(+), 13 deletions(-)
drivers/hv/dxgkrnl/dxgadapter.c +494 -0
@@ -149,6 +149,24 @@ bool dxgadapter_is_active(struct dxgadapter *adapter)return adapter->adapter_state ==DXGADAPTER_STATE_ACTIVE ; } +/* Protected by dxgglobal_acquire_process_adapter_lock */ +void dxgadapter_add_process (struct dxgadapter *adapter, +struct dxgprocess_adapter *process_info) +{ +dev_dbg (dxgglobaldev,"%s %p %p" , __func__, adapter, process_info); +list_add_tail (&process_info->adapter_process_list_entry, + &adapter->adapter_process_list_head); +} + +void dxgadapter_remove_process (struct dxgprocess_adapter *process_info) +{ +dev_dbg (dxgglobaldev,"%s %p %p" , __func__, + process_info->adapter, process_info); +list_del (&process_info->adapter_process_list_entry); + process_info->adapter_process_list_entry.next =NULL ; + process_info->adapter_process_list_entry.prev =NULL ; +} +int dxgadapter_acquire_lock_exclusive (struct dxgadapter *adapter) {down_write (&adapter->core_lock);
@@ -183,7 +201,483 @@ void dxgadapter_release_lock_shared(struct dxgadapter *adapter)up_read (&adapter->core_lock); } +struct dxgdevice *dxgdevice_create (struct dxgadapter *adapter, +struct dxgprocess *process) +{ +struct dxgdevice *device =vzalloc (sizeof (struct dxgdevice )); +int ret; + +if (device) { +kref_init (&device->device_kref); + device->adapter = adapter; + device->process = process; +kref_get (&adapter->adapter_kref); +INIT_LIST_HEAD (&device->context_list_head); +INIT_LIST_HEAD (&device->alloc_list_head); +INIT_LIST_HEAD (&device->resource_list_head); +init_rwsem (&device->device_lock); +init_rwsem (&device->context_list_lock); +init_rwsem (&device->alloc_list_lock); +INIT_LIST_HEAD (&device->pqueue_list_head); +INIT_LIST_HEAD (&device->syncobj_list_head); + device->object_state =DXGOBJECTSTATE_CREATED ; + device->execution_state = _D3DKMT_DEVICEEXECUTION_ACTIVE; + + ret =dxgprocess_adapter_add_device (process, adapter, device); +if (ret <0 ) { +kref_put (&device->device_kref, dxgdevice_release); + device =NULL ; + } + } +return device; +} + +void dxgdevice_stop (struct dxgdevice *device) +{ +struct dxgallocation *alloc; +struct dxgpagingqueue *pqueue; +struct dxgsyncobject *syncobj; + +dev_dbg (dxgglobaldev,"%s: %p" , __func__, device); +dxgdevice_acquire_alloc_list_lock (device); +list_for_each_entry (alloc, &device->alloc_list_head, alloc_list_entry) { +dxgallocation_stop (alloc); + } +dxgdevice_release_alloc_list_lock (device); + +hmgrtable_lock (&device->process->handle_table,DXGLOCK_EXCL ); +list_for_each_entry (pqueue, &device->pqueue_list_head, + pqueue_list_entry) { +dxgpagingqueue_stop (pqueue); + } +list_for_each_entry (syncobj, &device->syncobj_list_head, + syncobj_list_entry) { +dxgsyncobject_stop (syncobj); + } +hmgrtable_unlock (&device->process->handle_table,DXGLOCK_EXCL ); +dev_dbg (dxgglobaldev,"%s: end %p\n" , __func__, device); +} + +void dxgdevice_mark_destroyed (struct dxgdevice *device) +{ +down_write (&device->device_lock); + device->object_state =DXGOBJECTSTATE_DESTROYED ; +up_write (&device->device_lock); +} + +void dxgdevice_destroy (struct dxgdevice *device) +{ +struct dxgprocess *process = device->process; +struct dxgadapter *adapter = device->adapter; +struct d3dkmthandle device_handle = {}; + +dev_dbg (dxgglobaldev,"%s: %p\n" , __func__, device); + +down_write (&device->device_lock); + +if (device->object_state !=DXGOBJECTSTATE_ACTIVE ) + goto cleanup; + + device->object_state =DXGOBJECTSTATE_DESTROYED ; + +dxgdevice_stop (device); + +dxgdevice_acquire_alloc_list_lock (device); + +while (!list_empty (&device->syncobj_list_head)) { +struct dxgsyncobject *syncobj = +list_first_entry (&device->syncobj_list_head, + struct dxgsyncobject, + syncobj_list_entry); +list_del (&syncobj->syncobj_list_entry); + syncobj->syncobj_list_entry.next =NULL ; +dxgdevice_release_alloc_list_lock (device); + +dxgsyncobject_destroy (process, syncobj); + +dxgdevice_acquire_alloc_list_lock (device); + } + + { +struct dxgallocation *alloc; +struct dxgallocation *tmp; + +dev_dbg (dxgglobaldev,"destroying allocations\n" ); +list_for_each_entry_safe (alloc, tmp, &device->alloc_list_head, + alloc_list_entry) { +dxgallocation_destroy (alloc); + } + } + + { +struct dxgresource *resource; +struct dxgresource *tmp; + +dev_dbg (dxgglobaldev,"destroying resources\n" ); +list_for_each_entry_safe (resource, tmp, + &device->resource_list_head, + resource_list_entry) { +dxgresource_destroy (resource); + } + } + +dxgdevice_release_alloc_list_lock (device); + + { +struct dxgcontext *context; +struct dxgcontext *tmp; + +dev_dbg (dxgglobaldev,"destroying contexts\n" ); +dxgdevice_acquire_context_list_lock (device); +list_for_each_entry_safe (context, tmp, + &device->context_list_head, + context_list_entry) { +dxgcontext_destroy (process, context); + } +dxgdevice_release_context_list_lock (device); + } + + { +struct dxgpagingqueue *tmp; +struct dxgpagingqueue *pqueue; + +dev_dbg (dxgglobaldev,"destroying paging queues\n" ); +list_for_each_entry_safe (pqueue, tmp, &device->pqueue_list_head, + pqueue_list_entry) { +dxgpagingqueue_destroy (pqueue); + } + } + +/* Guest handles need to be released before the host handles */ +hmgrtable_lock (&process->handle_table,DXGLOCK_EXCL ); +if (device->handle_valid) { +hmgrtable_free_handle (&process->handle_table, +HMGRENTRY_TYPE_DXGDEVICE , device->handle); + device_handle = device->handle; + device->handle_valid =0 ; + } +hmgrtable_unlock (&process->handle_table,DXGLOCK_EXCL ); + +if (device_handle.v) { +up_write (&device->device_lock); +if (dxgadapter_acquire_lock_shared (adapter) ==0 ) { +dxgvmb_send_destroy_device (adapter, process, + device_handle); +dxgadapter_release_lock_shared (adapter); + } +down_write (&device->device_lock); + } + +cleanup: + +if (device->adapter) { +dxgprocess_adapter_remove_device (device); +kref_put (&device->adapter->adapter_kref, dxgadapter_release); + } + +up_write (&device->device_lock); + +kref_put (&device->device_kref, dxgdevice_release); +dev_dbg (dxgglobaldev,"dxgdevice_destroy_end\n" ); +} + +int dxgdevice_acquire_lock_shared (struct dxgdevice *device) +{ +down_read (&device->device_lock); +if (!dxgdevice_is_active (device)) { +up_read (&device->device_lock); +return -ENODEV ; + } +return 0 ; +} + +void dxgdevice_release_lock_shared (struct dxgdevice *device) +{ +up_read (&device->device_lock); +} + +bool dxgdevice_is_active (struct dxgdevice *device) +{ +return device->object_state ==DXGOBJECTSTATE_ACTIVE ; +} + +void dxgdevice_acquire_context_list_lock (struct dxgdevice *device) +{ +down_write (&device->context_list_lock); +} + +void dxgdevice_release_context_list_lock (struct dxgdevice *device) +{ +up_write (&device->context_list_lock); +} + +void dxgdevice_acquire_alloc_list_lock (struct dxgdevice *device) +{ +down_write (&device->alloc_list_lock); +} + +void dxgdevice_release_alloc_list_lock (struct dxgdevice *device) +{ +up_write (&device->alloc_list_lock); +} + +void dxgdevice_acquire_alloc_list_lock_shared (struct dxgdevice *device) +{ +down_read (&device->alloc_list_lock); +} + +void dxgdevice_release_alloc_list_lock_shared (struct dxgdevice *device) +{ +up_read (&device->alloc_list_lock); +} + +void dxgdevice_add_context (struct dxgdevice *device,struct dxgcontext *context) +{ +down_write (&device->context_list_lock); +list_add_tail (&context->context_list_entry, &device->context_list_head); +up_write (&device->context_list_lock); +} + +void dxgdevice_remove_context (struct dxgdevice *device, +struct dxgcontext *context) +{ +if (context->context_list_entry.next) { +list_del (&context->context_list_entry); + context->context_list_entry.next =NULL ; + } +} + +void dxgdevice_release (struct kref *refcount) +{ +struct dxgdevice *device; + + device = container_of(refcount, struct dxgdevice, device_kref); +vfree (device); +} + +struct dxgcontext *dxgcontext_create (struct dxgdevice *device) +{ +struct dxgcontext *context =vzalloc (sizeof (struct dxgcontext )); + +if (context) { +kref_init (&context->context_kref); + context->device = device; + context->process = device->process; + context->device_handle = device->handle; +kref_get (&device->device_kref); +INIT_LIST_HEAD (&context->hwqueue_list_head); +init_rwsem (&context->hwqueue_list_lock); +dxgdevice_add_context (device, context); + context->object_state =DXGOBJECTSTATE_ACTIVE ; + } +return context; +} + +/* +* Called when the device context list lock is held +*/ +void dxgcontext_destroy (struct dxgprocess *process,struct dxgcontext *context) +{ +struct dxghwqueue *hwqueue; +struct dxghwqueue *tmp; + +dev_dbg (dxgglobaldev,"%s %p\n" , __func__, context); + context->object_state =DXGOBJECTSTATE_DESTROYED ; +if (context->device) { +if (context->handle.v) { +hmgrtable_free_handle_safe (&process->handle_table, +HMGRENTRY_TYPE_DXGCONTEXT , + context->handle); + } +dxgdevice_remove_context (context->device, context); +kref_put (&context->device->device_kref, dxgdevice_release); + } +list_for_each_entry_safe (hwqueue, tmp, &context->hwqueue_list_head, + hwqueue_list_entry) { +dxghwqueue_destroy (process, hwqueue); + } +kref_put (&context->context_kref, dxgcontext_release); +} + +void dxgcontext_destroy_safe (struct dxgprocess *process, +struct dxgcontext *context) +{ +struct dxgdevice *device = context->device; + +dxgdevice_acquire_context_list_lock (device); +dxgcontext_destroy (process, context); +dxgdevice_release_context_list_lock (device); +} + +bool dxgcontext_is_active (struct dxgcontext *context) +{ +return context->object_state ==DXGOBJECTSTATE_ACTIVE ; +} + +void dxgcontext_release (struct kref *refcount) +{ +struct dxgcontext *context; + + context = container_of(refcount, struct dxgcontext, context_kref); +vfree (context); +} + +struct dxgprocess_adapter *dxgprocess_adapter_create (struct dxgprocess *process, +struct dxgadapter *adapter) +{ +struct dxgprocess_adapter *adapter_info; + + adapter_info =vzalloc (sizeof (*adapter_info)); +if (adapter_info) { +if (kref_get_unless_zero (&adapter->adapter_kref) ==0 ) { +pr_err ("failed to acquire adapter reference" ); + goto cleanup; + } + adapter_info->adapter = adapter; + adapter_info->process = process; + adapter_info->refcount =1 ; +mutex_init (&adapter_info->device_list_mutex); +INIT_LIST_HEAD (&adapter_info->device_list_head); +list_add_tail (&adapter_info->process_adapter_list_entry, + &process->process_adapter_list_head); +dxgadapter_add_process (adapter, adapter_info); + } +return adapter_info; +cleanup: +if (adapter_info) +vfree (adapter_info); +return NULL ; +} +void dxgprocess_adapter_stop (struct dxgprocess_adapter *adapter_info) +{ +struct dxgdevice *device; + +mutex_lock (&adapter_info->device_list_mutex); +list_for_each_entry (device, &adapter_info->device_list_head, + device_list_entry) { +dxgdevice_stop (device); + } +mutex_unlock (&adapter_info->device_list_mutex); +} + +void dxgprocess_adapter_destroy (struct dxgprocess_adapter *adapter_info) +{ +struct dxgdevice *device; + +mutex_lock (&adapter_info->device_list_mutex); +while (!list_empty (&adapter_info->device_list_head)) { + device =list_first_entry (&adapter_info->device_list_head, + struct dxgdevice, device_list_entry); +list_del (&device->device_list_entry); + device->device_list_entry.next =NULL ; +mutex_unlock (&adapter_info->device_list_mutex); +dxgdevice_destroy (device); +mutex_lock (&adapter_info->device_list_mutex); + } +mutex_unlock (&adapter_info->device_list_mutex); + +dxgadapter_remove_process (adapter_info); +kref_put (&adapter_info->adapter->adapter_kref, dxgadapter_release); +list_del (&adapter_info->process_adapter_list_entry); +vfree (adapter_info); +} + +/* +* Must be called when dxgglobal::process_adapter_mutex is held +*/ +void dxgprocess_adapter_release (struct dxgprocess_adapter *adapter_info) +{ +dev_dbg (dxgglobaldev,"%s %p %d" , + __func__, adapter_info, adapter_info->refcount); + adapter_info->refcount--; +if (adapter_info->refcount ==0 ) +dxgprocess_adapter_destroy (adapter_info); +} + +int dxgprocess_adapter_add_device (struct dxgprocess *process, +struct dxgadapter *adapter, +struct dxgdevice *device) +{ +struct dxgprocess_adapter *entry; +struct dxgprocess_adapter *adapter_info =NULL ; +int ret =0 ; + +dxgglobal_acquire_process_adapter_lock (); + +list_for_each_entry (entry, &process->process_adapter_list_head, + process_adapter_list_entry) { +if (entry->adapter == adapter) { + adapter_info = entry; +break ; + } + } +if (adapter_info ==NULL ) { +pr_err ("failed to find process adapter info\n" ); + ret = -EINVAL ; + goto cleanup; + } +mutex_lock (&adapter_info->device_list_mutex); +list_add_tail (&device->device_list_entry, + &adapter_info->device_list_head); + device->adapter_info = adapter_info; +mutex_unlock (&adapter_info->device_list_mutex); + +cleanup: + +dxgglobal_release_process_adapter_lock (); +return ret; +} + +void dxgprocess_adapter_remove_device (struct dxgdevice *device) +{ +dev_dbg (dxgglobaldev,"%s %p\n" , __func__, device); +mutex_lock (&device->adapter_info->device_list_mutex); +if (device->device_list_entry.next) { +list_del (&device->device_list_entry); + device->device_list_entry.next =NULL ; + } +mutex_unlock (&device->adapter_info->device_list_mutex); +} + +void dxghwqueue_destroy (struct dxgprocess *process,struct dxghwqueue *hwqueue) +{ +/* Placeholder */ +} + +void dxgpagingqueue_destroy (struct dxgpagingqueue *pqueue) {/* Placeholder */ } + +void dxgpagingqueue_stop (struct dxgpagingqueue *pqueue) +{ +/* Placeholder */ +} + +void dxgallocation_destroy (struct dxgallocation *alloc) +{ +/* Placeholder */ +} + +void dxgallocation_stop (struct dxgallocation *alloc) +{ +/* Placeholder */ +} + +void dxgresource_destroy (struct dxgresource *resource) +{ +/* Placeholder */ +} + +void dxgsyncobject_destroy (struct dxgprocess *process, +struct dxgsyncobject *syncobj) +{ +/* Placeholder */ +} + +void dxgsyncobject_stop (struct dxgsyncobject *syncobj) +{ +/* Placeholder */ +} +
drivers/hv/dxgkrnl/dxgkrnl.h +0 -4
@@ -723,10 +723,6 @@ long dxgk_compat_ioctl(struct file *f, unsigned int p1, unsigned long p2);long dxgk_unlocked_ioctl (struct file *f,unsigned int p1,unsigned long p2);int dxg_unmap_iospace (void *va,u32 size); -int dxg_copy_from_user (void *to,const void __user *from, -unsigned long len); -int dxg_copy_to_user (void __user *to,const void *from, -unsigned long len);static inline void guid_to_luid (guid_t *guid,struct winluid *luid) { *luid = *(struct winluid *)&guid->b[0 ];
drivers/hv/dxgkrnl/dxgprocess.c +311 -5
334 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgvmbus.c +314 -0
323 lines not on this page · raw message
drivers/hv/dxgkrnl/hmgr.c +509 -0
519 lines not on this page · raw message
drivers/hv/dxgkrnl/ioctl.c +808 -4
829 lines not on this page · raw message
Signature · 1 line
2.32.0
PATCH 3/9 drivers: hv: dxgkrnl: Implement creation/destruction of GPU allocations/resources
In reply to Iouri TarassovImplemented the following ioctls:
- Creation and destruction of GPU accessible allocations:
LX_DXCREATEALLOCATION, LX_DXDESTROYALLOCATION2,
- LX_DXGETSHAREDRESOURCEADAPTERLUID
- LX_DXRENDER
At this moment GPU paravirtualization does not support D3DKMTRender API.
--- drivers/hv/dxgkrnl/dxgadapter.c | 310 ++++++++++++- drivers/hv/dxgkrnl/dxgvmbus.c | 665 ++++++++++++++++++++++++++++ drivers/hv/dxgkrnl/ioctl.c | 745 ++++++++++++++++++++++++++++++++ 3 files changed, 1705 insertions(+), 15 deletions(-)
drivers/hv/dxgkrnl/dxgadapter.c +295 -15
@@ -447,6 +447,230 @@ void dxgdevice_remove_context(struct dxgdevice *device, } } +void dxgdevice_add_alloc (struct dxgdevice *device,struct dxgallocation *alloc) +{ +dxgdevice_acquire_alloc_list_lock (device); +list_add_tail (&alloc->alloc_list_entry, &device->alloc_list_head); +kref_get (&device->device_kref); + alloc->owner.device = device; +dxgdevice_release_alloc_list_lock (device); +} + +void dxgdevice_remove_alloc (struct dxgdevice *device, +struct dxgallocation *alloc) +{ +if (alloc->alloc_list_entry.next) { +list_del (&alloc->alloc_list_entry); + alloc->alloc_list_entry.next =NULL ; +kref_put (&device->device_kref, dxgdevice_release); + } +} + +void dxgdevice_remove_alloc_safe (struct dxgdevice *device, +struct dxgallocation *alloc) +{ +dxgdevice_acquire_alloc_list_lock (device); +dxgdevice_remove_alloc (device, alloc); +dxgdevice_release_alloc_list_lock (device); +} + +void dxgdevice_add_resource (struct dxgdevice *device,struct dxgresource *res) +{ +dxgdevice_acquire_alloc_list_lock (device); +list_add_tail (&res->resource_list_entry, &device->resource_list_head); +kref_get (&device->device_kref); +dxgdevice_release_alloc_list_lock (device); +} + +void dxgdevice_remove_resource (struct dxgdevice *device, +struct dxgresource *res) +{ +if (res->resource_list_entry.next) { +list_del (&res->resource_list_entry); + res->resource_list_entry.next =NULL ; +kref_put (&device->device_kref, dxgdevice_release); + } +} + +struct dxgsharedresource *dxgsharedresource_create (struct dxgadapter *adapter) +{ +struct dxgsharedresource *resource; + + resource =vzalloc (sizeof (*resource)); +if (resource) { +INIT_LIST_HEAD (&resource->resource_list_head); +kref_init (&resource->sresource_kref); +mutex_init (&resource->fd_mutex); + resource->adapter = adapter; + } +return resource; +} + +void dxgsharedresource_destroy (struct kref *refcount) +{ +struct dxgsharedresource *resource; + + resource = container_of(refcount, struct dxgsharedresource, + sresource_kref); +if (resource->runtime_private_data) +vfree (resource->runtime_private_data); +if (resource->resource_private_data) +vfree (resource->resource_private_data); +if (resource->alloc_private_data_sizes) +vfree (resource->alloc_private_data_sizes); +if (resource->alloc_private_data) +vfree (resource->alloc_private_data); +vfree (resource); +} + +void dxgsharedresource_add_resource (struct dxgsharedresource *shared_resource, +struct dxgresource *resource) +{ +down_write (&shared_resource->adapter->shared_resource_list_lock); +dev_dbg (dxgglobaldev,"%s: %p %p" , __func__, shared_resource, resource); +list_add_tail (&resource->shared_resource_list_entry, + &shared_resource->resource_list_head); +kref_get (&shared_resource->sresource_kref); +kref_get (&resource->resource_kref); + resource->shared_owner = shared_resource; +up_write (&shared_resource->adapter->shared_resource_list_lock); +} + +void dxgsharedresource_remove_resource (struct dxgsharedresource + *shared_resource, +struct dxgresource *resource) +{ +struct dxgadapter *adapter = shared_resource->adapter; + +down_write (&adapter->shared_resource_list_lock); +dev_dbg (dxgglobaldev,"%s: %p %p" , __func__, shared_resource, resource); +if (resource->shared_resource_list_entry.next) { +list_del (&resource->shared_resource_list_entry); + resource->shared_resource_list_entry.next =NULL ; +kref_put (&shared_resource->sresource_kref, + dxgsharedresource_destroy); + resource->shared_owner =NULL ; +kref_put (&resource->resource_kref, dxgresource_release); + } +up_write (&adapter->shared_resource_list_lock); +} + +struct dxgresource *dxgresource_create (struct dxgdevice *device) +{ +struct dxgresource *resource =vzalloc (sizeof (struct dxgresource )); + +if (resource) { +kref_init (&resource->resource_kref); + resource->device = device; + resource->process = device->process; + resource->object_state =DXGOBJECTSTATE_ACTIVE ; +mutex_init (&resource->resource_mutex); +INIT_LIST_HEAD (&resource->alloc_list_head); +dxgdevice_add_resource (device, resource); + } +return resource; +} + +void dxgresource_free_handle (struct dxgresource *resource) +{ +struct dxgallocation *alloc; +struct dxgprocess *process; + +if (resource->handle_valid) { + process = resource->device->process; +hmgrtable_free_handle_safe (&process->handle_table, +HMGRENTRY_TYPE_DXGRESOURCE , + resource->handle); + resource->handle_valid =0 ; + } +list_for_each_entry (alloc, &resource->alloc_list_head, + alloc_list_entry) { +dxgallocation_free_handle (alloc); + } +} + +void dxgresource_destroy (struct dxgresource *resource) +{ +/* device->alloc_list_lock is held */ +struct dxgallocation *alloc; +struct dxgallocation *tmp; +struct d3dkmt_destroyallocation2 args = { }; +int destroyed =test_and_set_bit (0 , &resource->flags); +struct dxgdevice *device = resource->device; +struct dxgsharedresource *shared_resource; + +if (!destroyed) { +dxgresource_free_handle (resource); +if (resource->handle.v) { + args.device = device->handle; + args.resource = resource->handle; +dxgvmb_send_destroy_allocation (device->process, + device, &args,NULL ); + resource->handle.v =0 ; + } +list_for_each_entry_safe (alloc, tmp, &resource->alloc_list_head, + alloc_list_entry) { +dxgallocation_destroy (alloc); + } +dxgdevice_remove_resource (device, resource); + shared_resource = resource->shared_owner; +if (shared_resource) { +dxgsharedresource_remove_resource (shared_resource, + resource); + resource->shared_owner =NULL ; + } + } +kref_put (&resource->resource_kref, dxgresource_release); +} + +void dxgresource_release (struct kref *refcount) +{ +struct dxgresource *resource; + + resource = container_of(refcount, struct dxgresource, resource_kref); +vfree (resource); +} + +bool dxgresource_is_active (struct dxgresource *resource) +{ +return resource->object_state ==DXGOBJECTSTATE_ACTIVE ; +} + +int dxgresource_add_alloc (struct dxgresource *resource, +struct dxgallocation *alloc) +{ +int ret = -ENODEV ; +struct dxgdevice *device = resource->device; + +dxgdevice_acquire_alloc_list_lock (device); +if (dxgresource_is_active (resource)) { +list_add_tail (&alloc->alloc_list_entry, + &resource->alloc_list_head); + alloc->owner.resource = resource; + ret =0 ; + } + alloc->resource_owner =1 ; +dxgdevice_release_alloc_list_lock (device); +return ret; +} + +void dxgresource_remove_alloc (struct dxgresource *resource, +struct dxgallocation *alloc) +{ +if (alloc->alloc_list_entry.next) { +list_del (&alloc->alloc_list_entry); + alloc->alloc_list_entry.next =NULL ; + } +} + +void dxgresource_remove_alloc_safe (struct dxgresource *resource, +struct dxgallocation *alloc) +{ +dxgdevice_acquire_alloc_list_lock (resource->device); +dxgresource_remove_alloc (resource, alloc); +dxgdevice_release_alloc_list_lock (resource->device); +} +void dxgdevice_release (struct kref *refcount) {struct dxgdevice *device;
@@ -522,6 +746,77 @@ void dxgcontext_release(struct kref *refcount)vfree (context); } +struct dxgallocation *dxgallocation_create (struct dxgprocess *process) +{ +struct dxgallocation *alloc =vzalloc (sizeof (struct dxgallocation )); + +if (alloc) + alloc->process = process; +return alloc; +} + +void dxgallocation_stop (struct dxgallocation *alloc) +{ +if (alloc->pages) { +release_pages (alloc->pages, alloc->num_pages); +vfree (alloc->pages); + alloc->pages =NULL ; + } +dxgprocess_ht_lock_exclusive_down (alloc->process); +if (alloc->cpu_address_mapped) { +dxg_unmap_iospace (alloc->cpu_address, + alloc->num_pages <<PAGE_SHIFT ); + alloc->cpu_address_mapped = false; + alloc->cpu_address =NULL ; + alloc->cpu_address_refcount =0 ; + } +dxgprocess_ht_lock_exclusive_up (alloc->process); +} + +void dxgallocation_free_handle (struct dxgallocation *alloc) +{ +dxgprocess_ht_lock_exclusive_down (alloc->process); +if (alloc->handle_valid) { +hmgrtable_free_handle (&alloc->process->handle_table, +HMGRENTRY_TYPE_DXGALLOCATION , + alloc->alloc_handle); + alloc->handle_valid =0 ; + } +dxgprocess_ht_lock_exclusive_up (alloc->process); +} + +void dxgallocation_destroy (struct dxgallocation *alloc) +{ +struct dxgprocess *process = alloc->process; +struct d3dkmt_destroyallocation2 args = { }; + +dxgallocation_stop (alloc); +if (alloc->resource_owner) +dxgresource_remove_alloc (alloc->owner.resource, alloc); +else if (alloc->owner.device) +dxgdevice_remove_alloc (alloc->owner.device, alloc); +dxgallocation_free_handle (alloc); +if (alloc->alloc_handle.v && !alloc->resource_owner) { + args.device = alloc->owner.device->handle; + args.alloc_count =1 ; +dxgvmb_send_destroy_allocation (process, + alloc->owner.device, + &args, &alloc->alloc_handle); + } +if (alloc->gpadl.gpadl_handle) { +dev_dbg (dxgglobaldev,"Teardown gpadl %d" , + alloc->gpadl.gpadl_handle); +vmbus_teardown_gpadl (dxgglobal_get_vmbus (), &alloc->gpadl); +dev_dbg (dxgglobaldev,"Teardown gpadl end" ); + alloc->gpadl.gpadl_handle =0 ; + } +if (alloc->priv_drv_data) +vfree (alloc->priv_drv_data); +if (alloc->cpu_address_mapped) +pr_err ("Alloc IO space is mapped: %p" , alloc); +vfree (alloc); +} +struct dxgprocess_adapter *dxgprocess_adapter_create (struct dxgprocess *process,struct dxgadapter *adapter) {
@@ -655,21 +950,6 @@ void dxgpagingqueue_stop(struct dxgpagingqueue *pqueue)/* Placeholder */ } -void dxgallocation_destroy (struct dxgallocation *alloc) -{ -/* Placeholder */ -} - -void dxgallocation_stop (struct dxgallocation *alloc) -{ -/* Placeholder */ -} - -void dxgresource_destroy (struct dxgresource *resource) -{ -/* Placeholder */ -} -void dxgsyncobject_destroy (struct dxgprocess *process,struct dxgsyncobject *syncobj) {
drivers/hv/dxgkrnl/dxgvmbus.c +665 -0
689 lines not on this page · raw message
drivers/hv/dxgkrnl/ioctl.c +745 -0
771 lines not on this page · raw message
Signature · 1 line
2.32.0
PATCH 4/9 drivers: hv: dxgkrnl: Implement operations with GPU sync objects
In reply to Iouri TarassovImplement ioctls for using GPU synchronization objects:
LX_DXCREATESYNCHRONIZATIONOBJECT, LX_DXSIGNALSYNCHRONIZATIONOBJECT, LX_DXWAITFORSYNCHRONIZATIONOBJECT, LX_DXDESTROYSYNCHRONIZATIONOBJECT, LX_DXSIGNALSYNCHRONIZATIONOBJECTFROMCPU, LX_DXSIGNALSYNCHRONIZATIONOBJECTFROMGPU, LX_DXSIGNALSYNCHRONIZATIONOBJECTFROMGPU2, LX_DXWAITFORSYNCHRONIZATIONOBJECTFROMCPU, LX_DXWAITFORSYNCHRONIZATIONOBJECTFROMGPU.
GPU synchronization objects are used to synchornize GPU command execution between different execution contexts. A wait for a sync object could be submitted to a GPU context (thread) or could be done on CPU. The wait is satisfied when the sync object is signaled. The signal operation could be submitted to a GPU context (thread) or could be signaled by a CPU thread.
The driver creates the corresponsing tracking structures and sends VM bus messages to the host to do the corresponding operation.
When a caller needs to wait for a sync object on CPU, an event structure is added to the global list (dxgglobal->host_event_list_head). Each list entry has an ID and a pointer to the event to signal. When the sync object is signaled on the host, the host sends a message to the guest with the event ID to signal. dxgglobal_signal_host_event() processes this message and signals the corresponding CPU event.
--- drivers/hv/dxgkrnl/dxgadapter.c | 280 ++++++++++- drivers/hv/dxgkrnl/dxgkrnl.h | 4 +- drivers/hv/dxgkrnl/dxgvmbus.c | 429 +++++++++++++++- drivers/hv/dxgkrnl/ioctl.c | 840 ++++++++++++++++++++++++++++++++ 4 files changed, 1541 insertions(+), 12 deletions(-)
drivers/hv/dxgkrnl/dxgadapter.c +273 -7
@@ -167,6 +167,55 @@ void dxgadapter_remove_process(struct dxgprocess_adapter *process_info) process_info->adapter_process_list_entry.prev =NULL ; } +void dxgadapter_remove_shared_resource (struct dxgadapter *adapter, +struct dxgsharedresource *object) +{ +down_write (&adapter->shared_resource_list_lock); +if (object->shared_resource_list_entry.next) { +list_del (&object->shared_resource_list_entry); + object->shared_resource_list_entry.next =NULL ; + } +up_write (&adapter->shared_resource_list_lock); +} + +void dxgadapter_add_shared_syncobj (struct dxgadapter *adapter, +struct dxgsharedsyncobject *object) +{ +down_write (&adapter->shared_resource_list_lock); +list_add_tail (&object->adapter_shared_syncobj_list_entry, + &adapter->adapter_shared_syncobj_list_head); +up_write (&adapter->shared_resource_list_lock); +} + +void dxgadapter_remove_shared_syncobj (struct dxgadapter *adapter, +struct dxgsharedsyncobject *object) +{ +down_write (&adapter->shared_resource_list_lock); +if (object->adapter_shared_syncobj_list_entry.next) { +list_del (&object->adapter_shared_syncobj_list_entry); + object->adapter_shared_syncobj_list_entry.next =NULL ; + } +up_write (&adapter->shared_resource_list_lock); +} + +void dxgadapter_add_syncobj (struct dxgadapter *adapter, +struct dxgsyncobject *object) +{ +down_write (&adapter->shared_resource_list_lock); +list_add_tail (&object->syncobj_list_entry, &adapter->syncobj_list_head); +up_write (&adapter->shared_resource_list_lock); +} + +void dxgadapter_remove_syncobj (struct dxgsyncobject *object) +{ +down_write (&object->adapter->shared_resource_list_lock); +if (object->syncobj_list_entry.next) { +list_del (&object->syncobj_list_entry); + object->syncobj_list_entry.next =NULL ; + } +up_write (&object->adapter->shared_resource_list_lock); +} +int dxgadapter_acquire_lock_exclusive (struct dxgadapter *adapter) {down_write (&adapter->core_lock);
@@ -679,6 +728,30 @@ void dxgdevice_release(struct kref *refcount)vfree (device); } +void dxgdevice_add_syncobj (struct dxgdevice *device, +struct dxgsyncobject *syncobj) +{ +dxgdevice_acquire_alloc_list_lock (device); +list_add_tail (&syncobj->syncobj_list_entry, &device->syncobj_list_head); +kref_get (&syncobj->syncobj_kref); +dxgdevice_release_alloc_list_lock (device); +} + +void dxgdevice_remove_syncobj (struct dxgsyncobject *entry) +{ +struct dxgdevice *device = entry->device; + +dxgdevice_acquire_alloc_list_lock (device); +if (entry->syncobj_list_entry.next) { +list_del (&entry->syncobj_list_entry); + entry->syncobj_list_entry.next =NULL ; +kref_put (&entry->syncobj_kref, dxgsyncobject_release); + } +dxgdevice_release_alloc_list_lock (device); +kref_put (&device->device_kref, dxgdevice_release); + entry->device =NULL ; +} +struct dxgcontext *dxgcontext_create (struct dxgdevice *device) {struct dxgcontext *context =vzalloc (sizeof (struct dxgcontext ));
@@ -935,28 +1008,221 @@ void dxgprocess_adapter_remove_device(struct dxgdevice *device)mutex_unlock (&device->adapter_info->device_list_mutex); } -void dxghwqueue_destroy (struct dxgprocess *process,struct dxghwqueue *hwqueue) +struct dxgsharedsyncobject *dxgsharedsyncobj_create (struct dxgadapter *adapter, +struct dxgsyncobject *so) { -/* Placeholder */ +struct dxgsharedsyncobject *syncobj; + + syncobj =vzalloc (sizeof (*syncobj)); +if (syncobj) { +kref_init (&syncobj->ssyncobj_kref); +INIT_LIST_HEAD (&syncobj->shared_syncobj_list_head); + syncobj->adapter = adapter; + syncobj->type = so->type; + syncobj->monitored_fence = so->monitored_fence; +dxgadapter_add_shared_syncobj (adapter, syncobj); +kref_get (&adapter->adapter_kref); +init_rwsem (&syncobj->syncobj_list_lock); +mutex_init (&syncobj->fd_mutex); + } +return syncobj; } -void dxgpagingqueue_destroy (struct dxgpagingqueue *pqueue) +void dxgsharedsyncobj_release (struct kref *refcount) { -/* Placeholder */ +struct dxgsharedsyncobject *syncobj; + + syncobj = container_of(refcount, struct dxgsharedsyncobject, + ssyncobj_kref); +dev_dbg (dxgglobaldev,"Destroying shared sync object %p" , syncobj); +if (syncobj->adapter) { +dxgadapter_remove_shared_syncobj (syncobj->adapter, + syncobj); +kref_put (&syncobj->adapter->adapter_kref, + dxgadapter_release); + } +vfree (syncobj); } -void dxgpagingqueue_stop (struct dxgpagingqueue *pqueue) +void dxgsharedsyncobj_add_syncobj (struct dxgsharedsyncobject *shared, +struct dxgsyncobject *syncobj) { -/* Placeholder */ +dev_dbg (dxgglobaldev,"%s 0x%p 0x%p" , __func__, shared, syncobj); +kref_get (&shared->ssyncobj_kref); +down_write (&shared->syncobj_list_lock); +list_add (&syncobj->shared_syncobj_list_entry, + &shared->shared_syncobj_list_head); + syncobj->shared_owner = shared; +up_write (&shared->syncobj_list_lock); +} + +void dxgsharedsyncobj_remove_syncobj (struct dxgsharedsyncobject *shared, +struct dxgsyncobject *syncobj) +{ +dev_dbg (dxgglobaldev,"%s 0x%p" , __func__, shared); +down_write (&shared->syncobj_list_lock); +list_del (&syncobj->shared_syncobj_list_entry); +up_write (&shared->syncobj_list_lock); +} + +struct dxgsyncobject *dxgsyncobject_create (struct dxgprocess *process, +struct dxgdevice *device, +struct dxgadapter *adapter, +enum +d3dddi_synchronizationobject_type + type, +struct +d3dddi_synchronizationobject_flags + flags) +{ +struct dxgsyncobject *syncobj; + + syncobj =vzalloc (sizeof (*syncobj)); +if (syncobj ==NULL ) + goto cleanup; + syncobj->type = type; + syncobj->process = process; +switch (type) { +case _D3DDDI_MONITORED_FENCE: +case _D3DDDI_PERIODIC_MONITORED_FENCE: + syncobj->monitored_fence =1 ; +break ; +case _D3DDDI_CPU_NOTIFICATION: + syncobj->cpu_event =1 ; + syncobj->host_event =vzalloc (sizeof (struct dxghostevent )); +if (syncobj->host_event ==NULL ) + goto cleanup; +break ; +default : +break ; + } +if (flags.shared) { + syncobj->shared =1 ; +if (!flags.nt_security_sharing) { +dev_err (dxgglobaldev, +"%s: nt_security_sharing must be set" , + __func__); + goto cleanup; + } + } + +kref_init (&syncobj->syncobj_kref); + +if (syncobj->monitored_fence) { + syncobj->device = device; + syncobj->device_handle = device->handle; +kref_get (&device->device_kref); +dxgdevice_add_syncobj (device, syncobj); + }else { +dxgadapter_add_syncobj (adapter, syncobj); + } + syncobj->adapter = adapter; +kref_get (&adapter->adapter_kref); + +dev_dbg (dxgglobaldev,"%s 0x%p\n" , __func__, syncobj); +return syncobj; +cleanup: +if (syncobj->host_event) +vfree (syncobj->host_event); +if (syncobj) +vfree (syncobj); +return NULL ; }void dxgsyncobject_destroy (struct dxgprocess *process,struct dxgsyncobject *syncobj) { -/* Placeholder */ +int destroyed; +struct dxghosteventcpu *host_event; + +dev_dbg (dxgglobaldev,"%s 0x%p" , __func__, syncobj); + +dxgsyncobject_stop (syncobj); + + destroyed =test_and_set_bit (0 , &syncobj->flags); +if (!destroyed) { +dev_dbg (dxgglobaldev,"Deleting handle: %x" , syncobj->handle.v); +hmgrtable_lock (&process->handle_table,DXGLOCK_EXCL ); +if (syncobj->handle.v) { +hmgrtable_free_handle (&process->handle_table, +HMGRENTRY_TYPE_DXGSYNCOBJECT , + syncobj->handle); + syncobj->handle.v =0 ; +kref_put (&syncobj->syncobj_kref, dxgsyncobject_release); + } +hmgrtable_unlock (&process->handle_table,DXGLOCK_EXCL ); + +if (syncobj->cpu_event) { + host_event = syncobj->host_event; +if (host_event->cpu_event) { +eventfd_ctx_put (host_event->cpu_event); +if (host_event->hdr.event_id) +dxgglobal_remove_host_event ( + &host_event->hdr); + host_event->cpu_event =NULL ; + } + } +if (syncobj->monitored_fence) +dxgdevice_remove_syncobj (syncobj); +else +dxgadapter_remove_syncobj (syncobj); +if (syncobj->adapter) { +kref_put (&syncobj->adapter->adapter_kref, + dxgadapter_release); + syncobj->adapter =NULL ; + } + } +kref_put (&syncobj->syncobj_kref, dxgsyncobject_release); }void dxgsyncobject_stop (struct dxgsyncobject *syncobj) +{ +int stopped =test_and_set_bit (1 , &syncobj->flags); + +if (!stopped) { +dev_dbg (dxgglobaldev,"stopping" ); +if (syncobj->monitored_fence) { +if (syncobj->mapped_address) { +int ret = +dxg_unmap_iospace (syncobj->mapped_address, +PAGE_SIZE ); + + (void )ret; +dev_dbg (dxgglobaldev,"fence is unmapped %d %p\n" , + ret, syncobj->mapped_address); + syncobj->mapped_address =NULL ; + } + } + } +} + +void dxgsyncobject_release (struct kref *refcount) +{ +struct dxgsyncobject *syncobj; + + syncobj = container_of(refcount, struct dxgsyncobject, syncobj_kref); +if (syncobj->shared_owner) { +dxgsharedsyncobj_remove_syncobj (syncobj->shared_owner, + syncobj); +kref_put (&syncobj->shared_owner->ssyncobj_kref, + dxgsharedsyncobj_release); + } +if (syncobj->host_event) +vfree (syncobj->host_event); +vfree (syncobj); +} + +void dxghwqueue_destroy (struct dxgprocess *process,struct dxghwqueue *hwqueue) +{ +/* Placeholder */ +} + +void dxgpagingqueue_destroy (struct dxgpagingqueue *pqueue) +{ +/* Placeholder */ +} + +void dxgpagingqueue_stop (struct dxgpagingqueue *pqueue) {/* Placeholder */ }
drivers/hv/dxgkrnl/dxgkrnl.h +2 -2
@@ -100,8 +100,8 @@ struct dxgpagingqueue {* a message from host. */ enum dxghosteventtype { - dxghostevent_cpu_event, - dxghostevent_dma_fence + dxghostevent_cpu_event =1 , + dxghostevent_dma_fence =2 };struct dxghostevent {
drivers/hv/dxgkrnl/dxgvmbus.c +426 -3
468 lines not on this page · raw message
drivers/hv/dxgkrnl/ioctl.c +840 -0
862 lines not on this page · raw message
Signature · 1 line
2.32.0
PATCH 5/9 drivers: hv: dxgkrnl: Implement sharing resources and sync objects
In reply to Iouri TarassovImplement IOCTLs for sharing dxgresource and dxgsyncobject objects between processes in the virtual machine.
Resources and sync objects are shared using FD (file descriptor) handles. The name "NT handle" is used to be compatible with Windows implementation.
An FD handle is created by the LX_DXSHAREOBJECTS ioctl. The given FD handle could be sent to another process using any Linux API.
To use a shared object in WDDM API calls, the object needs to be opened using its FD handle. An object could be opened by the following ioctls:LX_DXOPENRESOURCEFROMNTHANDLE, LX_DXOPENSYNCOBJECTFROMNTHANDLE2.
LX_DXQUERYRESOURCEINFOFROMNTHANDLE is used to query private driver data of a shared resource object. This private data needs to be used to actually open the object using LX_DXOPENRESOURCEFROMNTHANDLE.
--- drivers/hv/dxgkrnl/dxgvmbus.c | 190 +++++++ drivers/hv/dxgkrnl/ioctl.c | 936 +++++++++++++++++++++++++++++++++- 2 files changed, 1123 insertions(+), 3 deletions(-)
drivers/hv/dxgkrnl/dxgvmbus.c +190 -0
@@ -690,6 +690,142 @@ int dxgvmb_send_destroy_process(struct d3dkmthandle process)return ret; } +int dxgvmb_send_open_sync_object_nt (struct dxgprocess *process, +struct dxgvmbuschannel *channel, +struct d3dkmt_opensyncobjectfromnthandle2 + *args, +struct dxgsyncobject *syncobj) +{ +struct dxgkvmb_command_opensyncobject *command; +struct dxgkvmb_command_opensyncobject_return result = { }; +int ret; +struct dxgvmbusmsg msg; + + ret =init_message (&msg,NULL , process,sizeof (*command)); +if (ret) +return ret; + command = (void *)msg.msg; + +command_vm_to_host_init2 (&command->hdr,DXGK_VMBCOMMAND_OPENSYNCOBJECT , + process->host_handle); + command->device = args->device; + command->global_sync_object = syncobj->shared_owner->host_shared_handle; + command->flags = args->flags; +if (syncobj->monitored_fence) + command->engine_affinity = + args->monitored_fence.engine_affinity; + + ret =dxgglobal_acquire_channel_lock (); +if (ret <0 ) + goto cleanup; + + ret =dxgvmb_send_sync_msg (channel, msg.hdr, msg.size, + &result,sizeof (result)); + +dxgglobal_release_channel_lock (); + +if (ret <0 ) + goto cleanup; + + ret =ntstatus2int (result.status); +if (ret <0 ) + goto cleanup; + + args->sync_object = result.sync_object; +if (syncobj->monitored_fence) { +void *va =dxg_map_iospace (result.guest_cpu_physical_address, +PAGE_SIZE ,PROT_READ |PROT_WRITE , + true); +if (va ==NULL ) { + ret = -ENOMEM ; + goto cleanup; + } + args->monitored_fence.fence_value_cpu_va = va; + args->monitored_fence.fence_value_gpu_va = + result.gpu_virtual_address; + syncobj->mapped_address = va; + } + +cleanup: +free_message (&msg, process); +if (ret) +dev_dbg (dxgglobaldev,"err: %s %d" , __func__, ret); +return ret; +} + +int dxgvmb_send_create_nt_shared_object (struct dxgprocess *process, +struct d3dkmthandle object, +struct d3dkmthandle *shared_handle) +{ +struct dxgkvmb_command_createntsharedobject *command; +int ret; +struct dxgvmbusmsg msg; + + ret =init_message (&msg,NULL , process,sizeof (*command)); +if (ret) +return ret; + command = (void *)msg.msg; + +command_vm_to_host_init2 (&command->hdr, +DXGK_VMBCOMMAND_CREATENTSHAREDOBJECT , + process->host_handle); + command->object = object; + + ret =dxgglobal_acquire_channel_lock (); +if (ret <0 ) + goto cleanup; + + ret =dxgvmb_send_sync_msg (dxgglobal_get_dxgvmbuschannel (), + msg.hdr, msg.size, shared_handle, +sizeof (*shared_handle)); + +dxgglobal_release_channel_lock (); + +if (ret <0 ) + goto cleanup; +if (shared_handle->v ==0 ) { +pr_err ("failed to create NT shared object" ); + ret = -ENOTRECOVERABLE ; + } + +cleanup: +free_message (&msg, process); +if (ret) +dev_dbg (dxgglobaldev,"err: %s %d" , __func__, ret); +return ret; +} + +int dxgvmb_send_destroy_nt_shared_object (struct d3dkmthandle shared_handle) +{ +struct dxgkvmb_command_destroyntsharedobject *command; +int ret; +struct dxgvmbusmsg msg; + + ret =init_message (&msg,NULL ,NULL ,sizeof (*command)); +if (ret) +return ret; + command = (void *)msg.msg; + +command_vm_to_host_init1 (&command->hdr, +DXGK_VMBCOMMAND_DESTROYNTSHAREDOBJECT ); + command->shared_handle = shared_handle; + + ret =dxgglobal_acquire_channel_lock (); +if (ret <0 ) + goto cleanup; + + ret =dxgvmb_send_sync_msg_ntstatus (dxgglobal_get_dxgvmbuschannel (), + msg.hdr, msg.size); + +dxgglobal_release_channel_lock (); + +cleanup: +free_message (&msg,NULL ); +if (ret) +dev_dbg (dxgglobaldev,"err: %s %d" , __func__, ret); +return ret; +} +int dxgvmb_send_destroy_sync_object (struct dxgprocess *process,struct d3dkmthandle sync_object) {
@@ -1495,6 +1631,60 @@ int dxgvmb_send_destroy_allocation(struct dxgprocess *process,return ret; } +int dxgvmb_send_open_resource (struct dxgprocess *process, +struct dxgadapter *adapter, +struct d3dkmthandle device, +struct d3dkmthandle global_share, +u32 allocation_count, +u32 total_priv_drv_data_size, +struct d3dkmthandle *resource_handle, +struct d3dkmthandle *alloc_handles) +{ +struct dxgkvmb_command_openresource *command; +struct dxgkvmb_command_openresource_return *result; +struct d3dkmthandle *handles; +int ret; +int i; +u32 result_size = allocation_count *sizeof (struct d3dkmthandle ) + +sizeof (*result); +struct dxgvmbusmsgres msg = {.hdr =NULL }; + + ret =init_message_res (&msg, adapter, process,sizeof (*command), + result_size); +if (ret) + goto cleanup; + command = msg.msg; + result = msg.res; + +command_vgpu_to_host_init2 (&command->hdr,DXGK_VMBCOMMAND_OPENRESOURCE , + process->host_handle); + command->device = device; + command->nt_security_sharing =1 ; + command->global_share = global_share; + command->allocation_count = allocation_count; + command->total_priv_drv_data_size = total_priv_drv_data_size; + + ret =dxgvmb_send_sync_msg (msg.channel, msg.hdr, msg.size, + result, msg.res_size); +if (ret <0 ) + goto cleanup; + + ret =ntstatus2int (result->status); +if (ret <0 ) + goto cleanup; + + *resource_handle = result->resource; + handles = (struct d3dkmthandle *) &result[1 ]; +for (i =0 ; i < allocation_count; i++) + alloc_handles[i] = handles[i]; + +cleanup: +free_message ((struct dxgvmbusmsg *)&msg, process); +if (ret) +dev_dbg (dxgglobaldev,"err: %s %d" , __func__, ret); +return ret; +} + int dxgvmb_send_get_stdalloc_data(struct dxgdevice *device, enum d3dkmdt_standardallocationtype alloctype, struct d3dkmdt_gdisurfacedata *alloc_data,
drivers/hv/dxgkrnl/ioctl.c +933 -3
977 lines not on this page · raw message
Signature · 1 line
2.32.0
PATCH 6/9 drivers: hv: dxgkrnl: Seal the shared resource object when dxgk_share_objects is called.
In reply to Iouri TarassovA dxgresource object is a collection of dxgallocation objects. The WDDM API allows addition/removal allocations to a resource. The WDDM API has limitations on addition/removal of allocations to a shared resource. When a resource is "sealed", addition/removal of allocations is not allowed.
--- drivers/hv/dxgkrnl/ioctl.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-)
drivers/hv/dxgkrnl/ioctl.c +28 -8
@@ -2767,18 +2767,38 @@ dxgk_share_objects(struct dxgprocess *process, void *__user inargs)case HMGRENTRY_TYPE_DXGSYNCOBJECT : ret =get_object_fd (DXG_SHARED_SYNCOBJECT , shared_syncobj, &object_fd); -if (ret >=0 ) - ret = -dxgsharedsyncobj_get_host_nt_handle (shared_syncobj, - process, - handles[0 ]); +if (ret <0 ) { +pr_err ("%s get_object_fd failed for sync object" , + __func__); + goto cleanup; + } + ret =dxgsharedsyncobj_get_host_nt_handle (shared_syncobj, + process, + handles[0 ]); +if (ret <0 ) { +pr_err ("%s get_host_nt_handle failed" , __func__); + goto cleanup; + }break ;case HMGRENTRY_TYPE_DXGRESOURCE : ret =get_object_fd (DXG_SHARED_RESOURCE , shared_resource, &object_fd); -if (ret >=0 ) - ret =dxgsharedresource_get_host_nt_handle ( - shared_resource, process, handles[0 ]); +if (ret <0 ) { +pr_err ("%s get_object_fd failed for resource" , + __func__); + goto cleanup; + } + ret =dxgsharedresource_get_host_nt_handle (shared_resource, + process, handles[0 ]); +if (ret <0 ) { +pr_err ("%s get_host_res_nt_handle failed" , __func__); + goto cleanup; + } + ret =dxgsharedresource_seal (shared_resource); +if (ret <0 ) { +pr_err ("%s dxgsharedresource_seal failed" , __func__); + goto cleanup; + }break ;default : ret = -EINVAL ;
Signature · 1 line
2.32.0
PATCH 7/9 drivers: hv: dxgkrnl: Implementation of submit command, paging and hardware queue.
In reply to Iouri TarassovImplement various IOCTLs to deal with hardware queues and paging queues. Hardware queues are used when a compute device supports "hardware scheduling". This means that the compute device itself schedules execution of DMA buffers from hardware queues without CPU intervention. This is as oppose to the "packet scheduling" mode where the software GPU scheduler on the host schedules DMA buffer execution.
LX_DXSUBMITCOMMAND
This IOCTL is used to submit GPU commands when the device supports the "packet scheduling" mode.
LX_DXSUBMITCOMMANDTOHWQUEUE
This IOCTL is used to submit GPU commands when the device supports the "hardware scheduling" mode.
LX_DXCREATEPAGINGQUEUE, LX_DXDESTROYPAGINGQUEUE
These IOCTLs are used to create/destroy a paging queue. Paging queues are used to handle residency of device accessible allocations. An allocation is resident, when the device has access to it. For example, the allocation resides in local device memory or device page tables point to system memory which is made non-pageable.
LX_DXCREATEHWQUEUE, LX_DXDESTROYHWQUEUE
These IOCTLs are used to create/destroy hardware queue objects. Hardware queues are used when the compute device supports the "hardware scheduling" mode.
--- drivers/hv/dxgkrnl/dxgadapter.c | 164 ++++++++- drivers/hv/dxgkrnl/dxgvmbus.c | 341 +++++++++++++++++ drivers/hv/dxgkrnl/ioctl.c | 634 ++++++++++++++++++++++++++++++++ 3 files changed, 1130 insertions(+), 9 deletions(-)
drivers/hv/dxgkrnl/dxgadapter.c +155 -9
@@ -728,6 +728,26 @@ void dxgdevice_release(struct kref *refcount)vfree (device); } +void dxgdevice_add_paging_queue (struct dxgdevice *device, +struct dxgpagingqueue *entry) +{ +dxgdevice_acquire_alloc_list_lock (device); +list_add_tail (&entry->pqueue_list_entry, &device->pqueue_list_head); +dxgdevice_release_alloc_list_lock (device); +} + +void dxgdevice_remove_paging_queue (struct dxgpagingqueue *pqueue) +{ +struct dxgdevice *device = pqueue->device; + +dxgdevice_acquire_alloc_list_lock (device); +if (pqueue->pqueue_list_entry.next) { +list_del (&pqueue->pqueue_list_entry); + pqueue->pqueue_list_entry.next =NULL ; + } +dxgdevice_release_alloc_list_lock (device); +} +void dxgdevice_add_syncobj (struct dxgdevice *device,struct dxgsyncobject *syncobj) {
@@ -819,6 +839,38 @@ void dxgcontext_release(struct kref *refcount)vfree (context); } +int dxgcontext_add_hwqueue (struct dxgcontext *context, +struct dxghwqueue *hwqueue) +{ +int ret =0 ; + +down_write (&context->hwqueue_list_lock); +if (dxgcontext_is_active (context)) +list_add_tail (&hwqueue->hwqueue_list_entry, + &context->hwqueue_list_head); +else + ret = -ENODEV ; +up_write (&context->hwqueue_list_lock); +return ret; +} + +void dxgcontext_remove_hwqueue (struct dxgcontext *context, +struct dxghwqueue *hwqueue) +{ +if (hwqueue->hwqueue_list_entry.next) { +list_del (&hwqueue->hwqueue_list_entry); + hwqueue->hwqueue_list_entry.next =NULL ; + } +} + +void dxgcontext_remove_hwqueue_safe (struct dxgcontext *context, +struct dxghwqueue *hwqueue) +{ +down_write (&context->hwqueue_list_lock); +dxgcontext_remove_hwqueue (context, hwqueue); +up_write (&context->hwqueue_list_lock); +} +struct dxgallocation *dxgallocation_create (struct dxgprocess *process) {struct dxgallocation *alloc =vzalloc (sizeof (struct dxgallocation ));
@@ -890,6 +942,59 @@ void dxgallocation_destroy(struct dxgallocation *alloc)vfree (alloc); } +struct dxgpagingqueue *dxgpagingqueue_create (struct dxgdevice *device) +{ +struct dxgpagingqueue *pqueue; + + pqueue =vzalloc (sizeof (*pqueue)); +if (pqueue) { + pqueue->device = device; + pqueue->process = device->process; + pqueue->device_handle = device->handle; +dxgdevice_add_paging_queue (device, pqueue); + } +return pqueue; +} + +void dxgpagingqueue_stop (struct dxgpagingqueue *pqueue) +{ +int ret; + +if (pqueue->mapped_address) { + ret =dxg_unmap_iospace (pqueue->mapped_address,PAGE_SIZE ); +dev_dbg (dxgglobaldev,"fence is unmapped %d %p" , + ret, pqueue->mapped_address); + pqueue->mapped_address =NULL ; + } +} + +void dxgpagingqueue_destroy (struct dxgpagingqueue *pqueue) +{ +struct dxgprocess *process = pqueue->process; + +dev_dbg (dxgglobaldev,"%s %p %x\n" , __func__, pqueue, pqueue->handle.v); + +dxgpagingqueue_stop (pqueue); + +hmgrtable_lock (&process->handle_table,DXGLOCK_EXCL ); +if (pqueue->handle.v) { +hmgrtable_free_handle (&process->handle_table, +HMGRENTRY_TYPE_DXGPAGINGQUEUE , + pqueue->handle); + pqueue->handle.v =0 ; + } +if (pqueue->syncobj_handle.v) { +hmgrtable_free_handle (&process->handle_table, +HMGRENTRY_TYPE_MONITOREDFENCE , + pqueue->syncobj_handle); + pqueue->syncobj_handle.v =0 ; + } +hmgrtable_unlock (&process->handle_table,DXGLOCK_EXCL ); +if (pqueue->device) +dxgdevice_remove_paging_queue (pqueue); +vfree (pqueue); +} +struct dxgprocess_adapter *dxgprocess_adapter_create (struct dxgprocess *process,struct dxgadapter *adapter) {
@@ -1188,8 +1293,8 @@ void dxgsyncobject_stop(struct dxgsyncobject *syncobj) PAGE_SIZE); (void )ret; -dev_dbg (dxgglobaldev,"fence is unmapped %d %p\n" , - ret, syncobj->mapped_address); +dev_dbg (dxgglobaldev,"unmap fence %d %p\n" , + ret, syncobj->mapped_address); syncobj->mapped_address =NULL ; } }
@@ -1212,18 +1317,59 @@ void dxgsyncobject_release(struct kref *refcount)vfree (syncobj); } -void dxghwqueue_destroy (struct dxgprocess *process,struct dxghwqueue *hwqueue) +struct dxghwqueue *dxghwqueue_create (struct dxgcontext *context) { -/* Placeholder */ +struct dxgprocess *process = context->device->process; +struct dxghwqueue *hwqueue =vzalloc (sizeof (*hwqueue)); + +if (hwqueue) { +kref_init (&hwqueue->hwqueue_kref); + hwqueue->context = context; + hwqueue->process = process; + hwqueue->device_handle = context->device->handle; +if (dxgcontext_add_hwqueue (context, hwqueue) <0 ) { +kref_put (&hwqueue->hwqueue_kref, dxghwqueue_release); + hwqueue =NULL ; + }else { +kref_get (&context->context_kref); + } + } +return hwqueue; } -void dxgpagingqueue_destroy (struct dxgpagingqueue *pqueue) +void dxghwqueue_destroy (struct dxgprocess *process,struct dxghwqueue *hwqueue) { -/* Placeholder */ +dev_dbg (dxgglobaldev,"%s %p\n" , __func__, hwqueue); +hmgrtable_lock (&process->handle_table,DXGLOCK_EXCL ); +if (hwqueue->handle.v) { +hmgrtable_free_handle (&process->handle_table, +HMGRENTRY_TYPE_DXGHWQUEUE , + hwqueue->handle); + hwqueue->handle.v =0 ; + } +if (hwqueue->progress_fence_sync_object.v) { +hmgrtable_free_handle (&process->handle_table, +HMGRENTRY_TYPE_MONITOREDFENCE , + hwqueue->progress_fence_sync_object); + hwqueue->progress_fence_sync_object.v =0 ; + } +hmgrtable_unlock (&process->handle_table,DXGLOCK_EXCL ); + +if (hwqueue->progress_fence_mapped_address) { +dxg_unmap_iospace (hwqueue->progress_fence_mapped_address, +PAGE_SIZE ); + hwqueue->progress_fence_mapped_address =NULL ; + } +dxgcontext_remove_hwqueue_safe (hwqueue->context, hwqueue); + +kref_put (&hwqueue->context->context_kref, dxgcontext_release); +kref_put (&hwqueue->hwqueue_kref, dxghwqueue_release); } -void dxgpagingqueue_stop (struct dxgpagingqueue *pqueue) +void dxghwqueue_release (struct kref *refcount) { -/* Placeholder */ -} +struct dxghwqueue *hwqueue; + hwqueue = container_of(refcount, struct dxghwqueue, hwqueue_kref); +vfree (hwqueue); +}
drivers/hv/dxgkrnl/dxgvmbus.c +341 -0
362 lines not on this page · raw message
drivers/hv/dxgkrnl/ioctl.c +634 -0
670 lines not on this page · raw message
Signature · 1 line
2.32.0
PATCH 8/9 drivers: hv: dxgkrnl: Implement various WDDM ioctls
In reply to Iouri TarassovImplement various WDDM IOCTLs.
- IOCTLs to handle GPU virtual addressing (VA):
LX_DXRESERVEGPUVIRTUALADDRESS (D3DKMTReserveGpuVertualAddress) LX_DXFREEGPUVIRTUALADDRESS (D3DKMTFreeGpuVertualAddress) LX_DXMAPGPUVIRTUALADDRESS (D3DKMTMapGpuVertualAddress) LX_DXUPDATEGPUVIRTUALADDRESS (D3DKMTUpdateGpuVertualAddress)
WDDM supports a compute device to use GPU virtual addresses when accessing allocation memory. A GPU VA could be reserved or mapped to a GPU allocation. The video memory manager on the host updates GPU page tables for the virtual addresses.
- IOCTLs to manage residency of GPU accessing allocations:
LX_DXMAKERESIDENT (D3DKMTMakeResident) LX_DXEVICT (D3DKMTEvict)
An allocation is resident when GPU is setup to access it. The current WDDM design does not support on demand GPU page faulting. An allocation must be resident (be in the local device memory or in non-pageable system memory) before GPU is allowed to access it.
- IOCTLs to offer/reclaim alloctions:
LX_DXOFFERALLOCATIONS {D3DKMTOfferAllocations) LX_DXRECLAIMALLOCATIONS2 (D3DKMTReclaimAllocations)
When a user mode driver does not need an allocation, it can "offer" it. This means that the allocation is not in use and it local device memory could be reclaimed and given to another allocation. When the allocation is again needed, the caller can "reclaim" the allocations. If the allocation is still in the device local memory, the reclaim operation succeeds. If not the called must restore the content of the allocation before it can be used by the device.
- LX_DXESCAPE (D3DKMTEscape)
This IOCTL is used to send/receive private data between user mode driver and kernel mode driver. This is an extension of the WDDM APIs.
- LX_DXGETDEVICESTATE (D3DKMTGetDeviceState)
The IOCTL is used to get the current execution state of the dxgdevice object.
- LX_DXMARKDEVICEASERROR (D3DKMTMarkDeviceAsError)
The IOCTL is used to bring the dxgdevice object to the error state. Subsequent calls to use the device object will fail.
- LX_DXQUERYSTATISTICS (D3DKMTQuerystatistics)
The IOCTL is used to query various statistics from the compute device on the host.
- IOCTLs to deal with execution context priorities LX_DXGETCONTEXTINPROCESSSCHEDULINGPRIORITY LX_DXGETCONTEXTSCHEDULINGPRIORITY LX_DXSETCONTEXTINPROCESSSCHEDULINGPRIORITY LX_DXSETCONTEXTSCHEDULINGPRIORITY
--- drivers/hv/dxgkrnl/dxgkrnl.h | 2 + drivers/hv/dxgkrnl/dxgvmbus.c | 1466 ++++++++++++++++++++++++++++--- drivers/hv/dxgkrnl/dxgvmbus.h | 15 + drivers/hv/dxgkrnl/ioctl.c | 1524 ++++++++++++++++++++++++++++++++- 4 files changed, 2831 insertions(+), 176 deletions(-)
drivers/hv/dxgkrnl/dxgkrnl.h +2 -0
@@ -945,5 +945,7 @@ int dxgvmb_send_get_stdalloc_data(struct dxgdevice *device,int dxgvmb_send_query_statistics (struct dxgprocess *process,struct dxgadapter *adapter,struct d3dkmt_querystatistics *args); +int dxgvmb_send_share_object_with_host (struct dxgprocess *process, +struct d3dkmt_shareobjectwithhost *args);#endif
drivers/hv/dxgkrnl/dxgvmbus.c +1327 -139
1,590 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgvmbus.h +15 -0
@@ -63,6 +63,7 @@ enum dxgkvmb_commandtype_global {DXGK_VMBCOMMAND_QUERYETWSESSION =1009 ,DXGK_VMBCOMMAND_SETIOSPACEREGION =1010 ,DXGK_VMBCOMMAND_COMPLETETRANSACTION =1011 , +DXGK_VMBCOMMAND_SHAREOBJECTWITHHOST =1021 , DXGK_VMBCOMMAND_INVALID_VM_TO_HOST };
@@ -520,6 +521,7 @@ struct dxgkvmb_command_querystatistics {struct dxgkvmb_command_querystatistics_return {struct ntstatus status; +u32 reserved;struct d3dkmt_querystatistics_result result; };
@@ -847,6 +849,19 @@ struct dxgkvmb_command_getdevicestate_return {struct ntstatus status; }; +struct dxgkvmb_command_shareobjectwithhost { +struct dxgkvmb_command_vm_to_host hdr; +struct d3dkmthandle device_handle; +struct d3dkmthandle object_handle; +u64 reserved; +}; + +struct dxgkvmb_command_shareobjectwithhost_return { +struct ntstatus status; +u32 alignment; +u64 vail_nt_handle; +}; +/* * Helper functions */
drivers/hv/dxgkrnl/ioctl.c +1487 -37
1,370 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.32.0
Part of the diff is not shown here. View the raw message.
PATCH 9/9 drivers: hv: dxgkrnl: Implement DXGSYNCFILE
In reply to Iouri TarassovImplement the LX_DXCREATESYNCFILE IOCTL (D3DKMTCreateSyncFile).
dxgsyncfile is built on top of the Linux sync_file object and provides a way for the user mode to synchronize with the execution of the device DMA packets.
The IOCTL creates a dxgsyncfile object for the given GPU synchronization object and a fence value. A sync_object file descriptor is returned to the caller. The caller could wait for the object by using poll(). When the GPU synchronization object is signaled on the host, the host sends a message to the virtual machine and the sync_file object is signaled.
--- drivers/hv/dxgkrnl/Kconfig | 2 + drivers/hv/dxgkrnl/Makefile | 2 +- drivers/hv/dxgkrnl/dxgkrnl.h | 2 + drivers/hv/dxgkrnl/dxgmodule.c | 44 ++++++- drivers/hv/dxgkrnl/dxgsyncfile.c | 210 +++++++++++++++++++++++++++++++ drivers/hv/dxgkrnl/dxgsyncfile.h | 30 +++++ drivers/hv/dxgkrnl/dxgvmbus.c | 145 ++++++++++++++------- drivers/hv/dxgkrnl/dxgvmbus.h | 12 ++ drivers/hv/dxgkrnl/ioctl.c | 10 +- drivers/hv/dxgkrnl/misc.c | 6 + drivers/hv/dxgkrnl/misc.h | 1 + 11 files changed, 414 insertions(+), 50 deletions(-) create mode 100644 drivers/hv/dxgkrnl/dxgsyncfile.c create mode 100644 drivers/hv/dxgkrnl/dxgsyncfile.h
drivers/hv/dxgkrnl/Kconfig +2 -0
@@ -6,6 +6,8 @@ config DXGKRNL tristate "Microsoft Paravirtualized GPU support" depends on HYPERV depends on 64BIT || COMPILE_TEST + select DMA_SHARED_BUFFER + select SYNC_FILE help This driver supports paravirtualized virtual compute devices, exposed by Microsoft Hyper-V when Linux is running inside of a virtual machine
drivers/hv/dxgkrnl/Makefile +1 -1
@@ -2,4 +2,4 @@ # Makefile for the Linux video drivers. obj-$(CONFIG_DXGKRNL) += dxgkrnl.o -dxgkrnl-y := dxgmodule.o hmgr.o misc.o dxgadapter.o ioctl.o dxgvmbus.o dxgprocess.o +dxgkrnl-y := dxgmodule.o hmgr.o misc.o dxgadapter.o ioctl.o dxgvmbus.o dxgprocess.o dxgsyncfile.o
drivers/hv/dxgkrnl/dxgkrnl.h +2 -0
@@ -307,6 +307,7 @@ struct dxgglobal {bool pci_registered;bool global_channel_initialized;bool async_msg_enabled; +bool map_guest_pages_enabled; };extern struct dxgglobal *dxgglobal;
@@ -830,6 +831,7 @@ int dxgvmb_send_wait_sync_object_cpu(struct dxgprocess *process,struct d3dkmt_waitforsynchronizationobjectfromcpu *args, + bool user_address, u64 cpu_event); int dxgvmb_send_lock2(struct dxgprocess *process, struct dxgadapter *adapter,
drivers/hv/dxgkrnl/dxgmodule.c +43 -1
94 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgsyncfile.c new file +210 -0
210 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgsyncfile.h new file +30 -0
@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* +* Copyright (c) 2019, Microsoft Corporation. +* +* Author: +* Iouri Tarassov <[email protected]> +* +* Dxgkrnl Graphics Driver +* Headers for sync file objects +* +*/ + +#ifndef _DXGSYNCFILE_H +#define _DXGSYNCFILE_H + +#include <linux/sync_file.h> + +int dxgk_create_sync_file (struct dxgprocess *process,void *__user inargs); + +struct dxgsyncpoint { +struct dxghostevent hdr; +struct dma_fence base; +u64 fence_value; +u64 context; +spinlock_t lock; +u64 u64; +}; + +#endif /* _DXGSYNCFILE_H */
drivers/hv/dxgkrnl/dxgvmbus.c +103 -42
179 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgvmbus.h +12 -0
@@ -139,6 +139,8 @@ enum dxgkvmb_commandtype {DXGK_VMBCOMMAND_GETCONTEXTSCHEDULINGPRIORITY =61 ,DXGK_VMBCOMMAND_QUERYCLOCKCALIBRATION =62 ,DXGK_VMBCOMMAND_QUERYRESOURCEINFO =64 , +DXGK_VMBCOMMAND_LOGEVENT =65 , +DXGK_VMBCOMMAND_SETEXISTINGSYSMEMPAGES =66 , DXGK_VMBCOMMAND_INVALID };
@@ -245,6 +247,16 @@ struct dxgkvmb_command_setexistingsysmemstore {u32 gpadl; }; +/* Returns ntstatus */ +struct dxgkvmb_command_setexistingsysmempages { +struct dxgkvmb_command_vgpu_to_host hdr; +struct d3dkmthandle device; +struct d3dkmthandle allocation; +u32 num_pages; +u32 alloc_offset_in_pages; +/* u64 pfn_array[num_pages] */ +}; +struct dxgkvmb_command_createprocess {struct dxgkvmb_command_vm_to_host hdr;void *process;
drivers/hv/dxgkrnl/ioctl.c +4 -6
@@ -19,6 +19,7 @@#include "dxgkrnl.h" #include "dxgvmbus.h" +#include "dxgsyncfile.h" #undef pr_fmt#define pr_fmt (fmt) "dxgk:err: " fmt
@@ -32,11 +33,6 @@ struct ioctl_desc { };static struct ioctl_desc ioctls[LX_IO_MAX +1 ]; -static char *errorstr (int ret) -{ -return ret <0 ?"err" :"" ; -} -static int dxgsyncobj_release (struct inode *inode,struct file *file) {struct dxgsharedsyncobject *syncobj = file->private_data;
@@ -3561,7 +3557,7 @@ dxgk_wait_sync_object_cpu(struct dxgprocess *process, void *__user inargs) } ret =dxgvmb_send_wait_sync_object_cpu (process, adapter, - &args, event_id); + &args, true, event_id);if (ret <0 ) goto cleanup;
@@ -5457,4 +5453,6 @@ void init_ioctls(void) LX_DXQUERYSTATISTICS);SET_IOCTL (/*0x44 */ dxgk_share_object_with_host,LX_DXSHAREOBJECTWITHHOST ); +SET_IOCTL (/*0x45 */ dxgk_create_sync_file, +LX_DXCREATESYNCFILE ); }
drivers/hv/dxgkrnl/misc.c +6 -0
@@ -35,3 +35,9 @@ u16 *wcsncpy(u16 *dest, const u16 *src, size_t n) dest[i -1 ] =0 ;return dest; } + +char *errorstr (int ret) +{ +return ret <0 ?"err" :"" ; +} +
drivers/hv/dxgkrnl/misc.h +1 -0
@@ -44,6 +44,7 @@ extern const struct d3dkmthandle zerohandle;*/ u16 *wcsncpy (u16 *dest,const u16 *src,size_t n); +char *errorstr (int ret);enum dxglockstate {DXGLOCK_SHARED ,
Signature · 1 line
2.32.0
PATCH 1/9 drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Iouri Tarassov- Add support for a Hyper-V based vGPU implementation that exposes the DirectX API to Linux userspace.
- Handle driver loading, registration for the PCI and VM bus device notifications
- Add headers for user mode interfaces, internal driver objects and VM bus communication interface
- Handle initialization of VM bus channels and creation of the dxgadapter object
- Removed dxg_copy_from_user and dxg_copy_to_user
- Connect the dxgkrnl module to the drivers/hv/ makefile and Kconfig.
- Create a MAINTAINERS entry
PCI driver registration
A PCI device is created for each virtual GPU (vGPU) device, projected by the host. The device vendor is PCI_VENDOR_ID_MICROSOFT and device id is PCI_DEVICE_ID_VIRTUAL_RENDER. dxg_pci_probe_device handles arrival of such devices and it creates dxgadapter objects. The PCI config space of the vGPU device has luid of the corresponding per GPU VM bus channel. This is how the adapters are linked to VM bus channels.
dxgadapter initialization
A dxgadapter object represents a virtual GPU, projected to the VM by the host. This object can start functioning only when the global VM bus channel and the corresponding per vGPU VM bus channel are initialized in the guest. Notifications about arrival of vGPU PCI device and VM bus channels can happen in any order. Therefore, the initial dxgadapter object state is DXGADAPTER_STATE_WAITING_VMBUS. A list of VM bus channels and a list of dxgadapter objects are created. When dxgkrnl is notified about a VM bus channel arrival, if tries to start all adapters, which are not started yet.
VM bus interface version is exchanged by reading/writing the PCI config space of the vGPU device.
--- MAINTAINERS | 7 + drivers/hv/Kconfig | 2 + drivers/hv/Makefile | 1 + drivers/hv/dxgkrnl/Kconfig | 26 + drivers/hv/dxgkrnl/Makefile | 5 + drivers/hv/dxgkrnl/dxgadapter.c | 189 +++ drivers/hv/dxgkrnl/dxgkrnl.h | 953 +++++++++++++++ drivers/hv/dxgkrnl/dxgmodule.c | 882 ++++++++++++++ drivers/hv/dxgkrnl/dxgprocess.c | 37 + drivers/hv/dxgkrnl/dxgvmbus.c | 543 +++++++++ drivers/hv/dxgkrnl/dxgvmbus.h | 901 ++++++++++++++ drivers/hv/dxgkrnl/hmgr.c | 88 ++ drivers/hv/dxgkrnl/hmgr.h | 112 ++ drivers/hv/dxgkrnl/ioctl.c | 37 + drivers/hv/dxgkrnl/misc.c | 37 + drivers/hv/dxgkrnl/misc.h | 96 ++ include/linux/hyperv.h | 16 + include/uapi/misc/d3dkmthk.h | 1954 +++++++++++++++++++++++++++++++ 18 files changed, 5886 insertions(+) create mode 100644 drivers/hv/dxgkrnl/Kconfig create mode 100644 drivers/hv/dxgkrnl/Makefile create mode 100644 drivers/hv/dxgkrnl/dxgadapter.c create mode 100644 drivers/hv/dxgkrnl/dxgkrnl.h create mode 100644 drivers/hv/dxgkrnl/dxgmodule.c create mode 100644 drivers/hv/dxgkrnl/dxgprocess.c create mode 100644 drivers/hv/dxgkrnl/dxgvmbus.c create mode 100644 drivers/hv/dxgkrnl/dxgvmbus.h create mode 100644 drivers/hv/dxgkrnl/hmgr.c create mode 100644 drivers/hv/dxgkrnl/hmgr.h create mode 100644 drivers/hv/dxgkrnl/ioctl.c create mode 100644 drivers/hv/dxgkrnl/misc.c create mode 100644 drivers/hv/dxgkrnl/misc.h create mode 100644 include/uapi/misc/d3dkmthk.h
MAINTAINERS +7 -0
@@ -8841,6 +8841,13 @@ F: Documentation/devicetree/bindings/mtd/ti,am654-hbmc.yaml F: drivers/mtd/hyperbus/ F: include/linux/mtd/hyperbus.h +Hyper-V vGPU DRIVER +M: Iouri Tarassov <[email protected]> +L: [email protected] +S: Supported +F: drivers/hv/dxgkrnl/ +F: include/uapi/misc/d3dkmthk.h + HYPERVISOR VIRTUAL CONSOLE DRIVER L: [email protected] S: Odd Fixes
drivers/hv/Kconfig +2 -0
@@ -29,4 +29,6 @@ config HYPERV_BALLOON help Select this option to enable Hyper-V Balloon driver. +source "drivers/hv/dxgkrnl/Kconfig" + endmenu
drivers/hv/Makefile +1 -0
@@ -2,6 +2,7 @@ obj-$(CONFIG_HYPERV) += hv_vmbus.o obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o +obj-$(CONFIG_DXGKRNL) += dxgkrnl/ CFLAGS_hv_trace.o = -I$(src) CFLAGS_hv_balloon.o = -I$(src)
drivers/hv/dxgkrnl/Kconfig new file +26 -0
@@ -0,0 +1,26 @@ +# +# dxgkrnl configuration +# + +config DXGKRNL + tristate "Microsoft Paravirtualized GPU support" + depends on HYPERV + depends on 64BIT || COMPILE_TEST + help + This driver supports paravirtualized virtual compute devices, exposed + by Microsoft Hyper-V when Linux is running inside of a virtual machine + hosted by Windows. The virtual machines needs to be configured to use + host compute adapters. The driver name is dxgkrnl. + + An example of such virtual machine is a Windows Subsystem for + Linux container. When such container is instantiated, the Windows host + assigns compatible host GPU adapters to the container. The corresponding + virtual GPU devices appear on the PCI bus in the container. These + devices are enumerated and accessed by this driver. + + Communications with the driver are done by using the Microsoft libdxcore + library, which translates the D3DKMT interface + <https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/> + to the driver IOCTLs. The virtual GPU devices are paravirtualized, + which means that access to the hardware is done in the host. The driver + communicates with the host using Hyper-V VM bus communication channels.
drivers/hv/dxgkrnl/Makefile new file +5 -0
@@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +# Makefile for the Linux video drivers. + +obj-$(CONFIG_DXGKRNL) += dxgkrnl.o +dxgkrnl-y := dxgmodule.o hmgr.o misc.o dxgadapter.o ioctl.o dxgvmbus.o dxgprocess.o
drivers/hv/dxgkrnl/dxgadapter.c new file +189 -0
189 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgkrnl.h new file +953 -0
953 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgmodule.c new file +882 -0
882 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgprocess.c new file +37 -0
@@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Copyright (c) 2019, Microsoft Corporation. +* +* Author: +* Iouri Tarassov <[email protected]> +* +* Dxgkrnl Graphics Driver +* DXGPROCESS implementation +* +*/ + +#include "dxgkrnl.h" + +#undef pr_fmt +#define pr_fmt (fmt) "dxgk:err: " fmt + +/* +* Creates a new dxgprocess object +* Must be called when dxgglobal->plistmutex is held +*/ +struct dxgprocess *dxgprocess_create (void ) +{ +/* Placeholder */ +return NULL ; +} + +void dxgprocess_destroy (struct dxgprocess *process) +{ +/* Placeholder */ +} + +void dxgprocess_release (struct kref *refcount) +{ +/* Placeholder */ +}
drivers/hv/dxgkrnl/dxgvmbus.c new file +543 -0
543 lines not on this page · raw message
drivers/hv/dxgkrnl/dxgvmbus.h new file +901 -0
339 lines not on this page · raw message
drivers/hv/dxgkrnl/hmgr.c new file +88 -0
88 lines not on this page · raw message
drivers/hv/dxgkrnl/hmgr.h new file +112 -0
112 lines not on this page · raw message
drivers/hv/dxgkrnl/misc.h new file +96 -0
96 lines not on this page · raw message
include/uapi/misc/d3dkmthk.h new file +1954 -0
1,954 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.32.0
Part of the diff is not shown here. View the raw message.
Hi Iouri,
Quoted 9 lines · Iouri Tarassov
This is a follow-up on the changes we sent a few months back[1].
[1] https://lore.kernel.org/lkml/[email protected]/
The patches address the feedback, given by Greg KH and other reviewers, contain bug fixes, the implementation of asynchronous VM bus messages to the host and contain the remaining implementation of our vGPU / Compute hardware virtualization support that powers the Windows Subsystem for Linux (WSL) and will soon power the Windows Subsystem for Android (WSA).
<snip>
Quoted 1 line
We're looking forward additional feedback.
I have been including this patch set into my downstream WSL2 kernel that I build with clang and I noticed an instance of -Wenum-conversion that is still present in this revision:
In file included from drivers/hv/dxgkrnl/dxgsyncfile.c:21:drivers/hv/dxgkrnl/dxgvmbus.h:894:26: warning: implicit conversion from enumeration type 'enum dxgkvmb_commandtype' to different enumeration type 'enum dxgkvmb_commandtype_global' [-Wenum-conversion]
command->command_type = DXGK_VMBCOMMAND_INVALID;
~ ^~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from drivers/hv/dxgkrnl/dxgvmbus.c:23:
drivers/hv/dxgkrnl/dxgvmbus.h:894:26: warning: implicit conversion from enumeration type 'enum dxgkvmb_commandtype' to different enumeration type 'enum dxgkvmb_commandtype_global' [-Wenum-conversion]
command->command_type = DXGK_VMBCOMMAND_INVALID;
~ ^~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from drivers/hv/dxgkrnl/ioctl.c:21:
drivers/hv/dxgkrnl/dxgvmbus.h:894:26: warning: implicit conversion from enumeration type 'enum dxgkvmb_commandtype' to different enumeration type 'enum dxgkvmb_commandtype_global' [-Wenum-conversion]
command->command_type = DXGK_VMBCOMMAND_INVALID;
~ ^~~~~~~~~~~~~~~~~~~~~~~1 warning generated.
This comes from command_vgpu_to_host_init0(), which does not actually appear to be used anywhere:
$ rg command_vgpu_to_host_init0 drivers/hv/dxgkrnl/dxgvmbus.h 891:static inline void command_vgpu_to_host_init0(struct dxgkvmb_command_vm_to_host
Please consider cleaning this up in a future revision so that clang builds stay clean :)
I happened to notice there was another function that looks very similar to command_vgpu_to_host_init0(), command_vm_to_host_init0(), which is also unused. This was hidden because it is marked as "static inline" in a .c file, which should generally be avoided; I would recommend replacing all instances of "static inline" with just "static". The compiler will still inline it if it feels it is worthwhile. Doing this reveals one other unused function, is_empty():
$ sed -i 's/static inline /static /g' drivers/hv/dxgkrnl/*.c
$ make -skj"$(nproc)" LLVM=1 drivers/hv/dxgkrnl/
drivers/hv/dxgkrnl/hmgr.c:167:13: warning: unused function 'is_empty' [-Wunused-function]
static bool is_empty(struct hmgrtable *table)
^
1 warning generated.
drivers/hv/dxgkrnl/dxgvmbus.c:234:13: warning: unused function 'command_vm_to_host_init0' [-Wunused-function]
static void command_vm_to_host_init0(struct dxgkvmb_command_vm_to_host
^
1 warning generated.Cheers,
Nathan
Quoted 19 lines · Nathan Chancellor
Hi Iouri,
Quoted 1 line
We're looking forward additional feedback.
I have been including this patch set into my downstream WSL2 kernel that
I build with clang and I noticed an instance of -Wenum-conversion that
is still present in this revision:Please consider cleaning this up in a future revision so that clang builds stay clean :)
...
I happened to notice there was another function that looks very similar
tocommand_vgpu_to_host_init0(),command_vm_to_host_init0(), which is
also unused. This was hidden because it is marked as "static inline" in
a .c file, which should generally be avoided; I would recommend
replacing all instances of "static inline" with just "static". The
compiler will still inline it if it feels it is worthwhile. Doing this
reveals one other unused function,is_empty():$ sed -i 's/static inline /static /g' drivers/hv/dxgkrnl/*.c
Quoted 9 lines
$ make -skj"$(nproc)" LLVM=1 drivers/hv/dxgkrnl/
drivers/hv/dxgkrnl/hmgr.c:167:13: warning: unused function 'is_empty' [-Wunused-function]
static bool is_empty(struct hmgrtable *table)
^
1 warning generated.
drivers/hv/dxgkrnl/dxgvmbus.c:234:13: warning: unused function 'command_vm_to_host_init0' [-Wunused-function]
static void command_vm_to_host_init0(struct dxgkvmb_command_vm_to_host
^
1 warning generated.Hi Nathan,
Thanks a lot! We will get this fixed
Iouri
Quoted 4 lines
Cheers,
Nathan
kernel test robot
build botRe: [PATCH v1 1/9] drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Iouri TarassovI love your patch! Perhaps something to improve:
Hi Iouri,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16 next-20220112]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e3084ed48fd6b661fe434da0cb36d7d6706cf27f config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220113/202201130941.ZVnyqikS-lkp(a)intel.com/config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/00f97c12e2cf0ba4ba1108e2fce9a3d0e287cc8c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 git checkout 00f97c12e2cf0ba4ba1108e2fce9a3d0e287cc8c
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/hv/dxgkrnl/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> drivers/hv/dxgkrnl/dxgmodule.c:79:20: warning: no previous prototype for 'find_pci_adapter' [-Wmissing-prototypes]
79 | struct dxgadapter *find_pci_adapter(struct pci_dev *dev)
| ^~~~~~~~~~~~~~~~
>> drivers/hv/dxgkrnl/dxgmodule.c:135:6: warning: no previous prototype for 'signal_host_cpu_event' [-Wmissing-prototypes]
135 | void signal_host_cpu_event(struct dxghostevent *eventhdr)
| ^~~~~~~~~~~~~~~~~~~~~
>> drivers/hv/dxgkrnl/dxgmodule.c:219:5: warning: no previous prototype for 'dxgglobal_create_adapter' [-Wmissing-prototypes]
219 | int dxgglobal_create_adapter(struct pci_dev *dev, guid_t *guid,
| ^~~~~~~~~~~~~~~~~~~~~~~~
--
>> drivers/hv/dxgkrnl/dxgvmbus.c:116:5: warning: no previous prototype for 'ntstatus2int' [-Wmissing-prototypes]
116 | int ntstatus2int(struct ntstatus status)
| ^~~~~~~~~~~~
>> drivers/hv/dxgkrnl/dxgvmbus.c:219:6: warning: no previous prototype for 'process_inband_packet' [-Wmissing-prototypes]
219 | void process_inband_packet(struct dxgvmbuschannel *channel,
| ^~~~~~~~~~~~~~~~~~~~~
>> drivers/hv/dxgkrnl/dxgvmbus.c:237:6: warning: no previous prototype for 'process_completion_packet' [-Wmissing-prototypes]
237 | void process_completion_packet(struct dxgvmbuschannel *channel,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hv/dxgkrnl/dxgvmbus.c:363:5: warning: no previous prototype for 'dxgvmb_send_async_msg' [-Wmissing-prototypes] 363 | int dxgvmb_send_async_msg(struct dxgvmbuschannel *channel,
| ^~~~~~~~~~~~~~~~~~~~~
vim +/find_pci_adapter +79 drivers/hv/dxgkrnl/dxgmodule.c
78
> 79 struct dxgadapter *find_pci_adapter(struct pci_dev *dev)
80 {
81 struct dxgadapter *entry;
82 struct dxgadapter *adapter = NULL;
83
84 dxgglobal_acquire_adapter_list_lock(DXGLOCK_EXCL);
85
86 list_for_each_entry(entry, &dxgglobal->adapter_list_head,
87 adapter_list_entry) {
88 if (dev == entry->pci_dev) {
89 adapter = entry;
90 break;
91 }
92 }
93
94 dxgglobal_release_adapter_list_lock(DXGLOCK_EXCL);
95 return adapter;
96 }
97
98 static struct dxgadapter *find_adapter(struct winluid *luid)
99 {
100 struct dxgadapter *entry;
101 struct dxgadapter *adapter = NULL;
102
103 dxgglobal_acquire_adapter_list_lock(DXGLOCK_EXCL);
104
105 list_for_each_entry(entry, &dxgglobal->adapter_list_head,
106 adapter_list_entry) {
107 if (memcmp(luid, &entry->luid, sizeof(struct winluid)) == 0) {
108 adapter = entry;
109 break;
110 }
111 }
112
113 dxgglobal_release_adapter_list_lock(DXGLOCK_EXCL);
114 return adapter;
115 }
116
117 void dxgglobal_add_host_event(struct dxghostevent *event)
118 {
119 spin_lock_irq(&dxgglobal->host_event_list_mutex);
120 list_add_tail(&event->host_event_list_entry,
121 &dxgglobal->host_event_list_head);
122 spin_unlock_irq(&dxgglobal->host_event_list_mutex);
123 }
124
125 void dxgglobal_remove_host_event(struct dxghostevent *event)
126 {
127 spin_lock_irq(&dxgglobal->host_event_list_mutex);
128 if (event->host_event_list_entry.next != NULL) {
129 list_del(&event->host_event_list_entry);
130 event->host_event_list_entry.next = NULL;
131 }
132 spin_unlock_irq(&dxgglobal->host_event_list_mutex);
133 }
134
> 135 void signal_host_cpu_event(struct dxghostevent *eventhdr)
136 {
137 struct dxghosteventcpu *event = (struct dxghosteventcpu *)eventhdr;
138
139 if (event->remove_from_list ||
140 event->destroy_after_signal) {
141 list_del(&eventhdr->host_event_list_entry);
142 eventhdr->host_event_list_entry.next = NULL;
143 }
144 if (event->cpu_event) {
145 dev_dbg(dxgglobaldev, "signal cpu event\n");
146 eventfd_signal(event->cpu_event, 1);
147 if (event->destroy_after_signal)
148 eventfd_ctx_put(event->cpu_event);
149 } else {
150 dev_dbg(dxgglobaldev, "signal completion\n");
151 complete(event->completion_event);
152 }
153 if (event->destroy_after_signal) {
154 dev_dbg(dxgglobaldev, "destroying event %p\n",
155 event);
156 vfree(event);
157 }
158 }
159
160 void dxgglobal_signal_host_event(u64 event_id)
161 {
162 struct dxghostevent *event;
163 unsigned long flags;
164
165 dev_dbg(dxgglobaldev, "%s %lld\n", __func__, event_id);
166
167 spin_lock_irqsave(&dxgglobal->host_event_list_mutex, flags);
168 list_for_each_entry(event, &dxgglobal->host_event_list_head,
169 host_event_list_entry) {
170 if (event->event_id == event_id) {
171 dev_dbg(dxgglobaldev, "found event to signal %lld\n",
172 event_id);
173 if (event->event_type == dxghostevent_cpu_event)
174 signal_host_cpu_event(event);
175 else
176 pr_err("Unknown host event type");
177 break;
178 }
179 }
180 spin_unlock_irqrestore(&dxgglobal->host_event_list_mutex, flags);
181 dev_dbg(dxgglobaldev, "dxgglobal_signal_host_event_end %lld\n",
182 event_id);
183 }
184
185 struct dxghostevent *dxgglobal_get_host_event(u64 event_id)
186 {
187 struct dxghostevent *entry;
188 struct dxghostevent *event = NULL;
189
190 spin_lock_irq(&dxgglobal->host_event_list_mutex);
191 list_for_each_entry(entry, &dxgglobal->host_event_list_head,
192 host_event_list_entry) {
193 if (entry->event_id == event_id) {
194 list_del(&entry->host_event_list_entry);
195 entry->host_event_list_entry.next = NULL;
196 event = entry;
197 break;
198 }
199 }
200 spin_unlock_irq(&dxgglobal->host_event_list_mutex);
201 return event;
202 }
203
204 u64 dxgglobal_new_host_event_id(void)
205 {
206 return atomic64_inc_return(&dxgglobal->host_event_id);
207 }
208
209 void dxgglobal_acquire_process_adapter_lock(void)
210 {
211 mutex_lock(&dxgglobal->process_adapter_mutex);
212 }
213
214 void dxgglobal_release_process_adapter_lock(void)
215 {
216 mutex_unlock(&dxgglobal->process_adapter_mutex);
217 }
218
> 219 int dxgglobal_create_adapter(struct pci_dev *dev, guid_t *guid, 220 struct winluid host_vgpu_luid)
221 {
222 struct dxgadapter *adapter;
223 int ret = 0;
224
225 adapter = vzalloc(sizeof(struct dxgadapter));
226 if (adapter == NULL) {
227 ret = -ENOMEM;
228 goto cleanup;
229 }
230
231 adapter->adapter_state = DXGADAPTER_STATE_WAITING_VMBUS;
232 adapter->host_vgpu_luid = host_vgpu_luid;
233 kref_init(&adapter->adapter_kref);
234 init_rwsem(&adapter->core_lock);
235
236 INIT_LIST_HEAD(&adapter->adapter_process_list_head);
237 INIT_LIST_HEAD(&adapter->shared_resource_list_head);
238 INIT_LIST_HEAD(&adapter->adapter_shared_syncobj_list_head);
239 INIT_LIST_HEAD(&adapter->syncobj_list_head);
240 init_rwsem(&adapter->shared_resource_list_lock);
241 adapter->pci_dev = dev;
242 guid_to_luid(guid, &adapter->luid);
243
244 dxgglobal_acquire_adapter_list_lock(DXGLOCK_EXCL);
245
246 list_add_tail(&adapter->adapter_list_entry,
247 &dxgglobal->adapter_list_head);
248 dxgglobal->num_adapters++;
249 dxgglobal_release_adapter_list_lock(DXGLOCK_EXCL);
250
251 dev_dbg(dxgglobaldev, "new adapter added %p %x-%x\n", adapter,
252 adapter->luid.a, adapter->luid.b);
253 cleanup:
254 dev_dbg(dxgglobaldev, "%s end: %d", __func__, ret);
255 return ret;
256 }
257
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [PATCH v1 1/9] drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Iouri TarassovI love your patch! Perhaps something to improve:
Hi Iouri,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16 next-20220113]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e3084ed48fd6b661fe434da0cb36d7d6706cf27f config: arm64-randconfig-r032-20220113 (https://download.01.org/0day-ci/archive/20220113/202201131405.tgtilUdq-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d1021978b8e7e35dcc30201ca1731d64b5a602a8)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/00f97c12e2cf0ba4ba1108e2fce9a3d0e287cc8c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 git checkout 00f97c12e2cf0ba4ba1108e2fce9a3d0e287cc8c
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hv/dxgkrnl/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> drivers/hv/dxgkrnl/dxgmodule.c:79:20: warning: no previous prototype for function 'find_pci_adapter' [-Wmissing-prototypes]
struct dxgadapter *find_pci_adapter(struct pci_dev *dev)
^
drivers/hv/dxgkrnl/dxgmodule.c:79:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct dxgadapter *find_pci_adapter(struct pci_dev *dev)
^
static
>> drivers/hv/dxgkrnl/dxgmodule.c:135:6: warning: no previous prototype for function 'signal_host_cpu_event' [-Wmissing-prototypes]
void signal_host_cpu_event(struct dxghostevent *eventhdr)
^
drivers/hv/dxgkrnl/dxgmodule.c:135:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void signal_host_cpu_event(struct dxghostevent *eventhdr)
^
static
>> drivers/hv/dxgkrnl/dxgmodule.c:219:5: warning: no previous prototype for function 'dxgglobal_create_adapter' [-Wmissing-prototypes]
int dxgglobal_create_adapter(struct pci_dev *dev, guid_t *guid,
^ drivers/hv/dxgkrnl/dxgmodule.c:219:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int dxgglobal_create_adapter(struct pci_dev *dev, guid_t *guid,
^
static
3 warnings generated.
--
In file included from drivers/hv/dxgkrnl/dxgvmbus.c:22:
drivers/hv/dxgkrnl/dxgvmbus.h:867:26: warning: implicit conversion from enumeration type 'enum dxgkvmb_commandtype' to different enumeration type 'enum dxgkvmb_commandtype_global' [-Wenum-conversion]
command->command_type = DXGK_VMBCOMMAND_INVALID;
~ ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hv/dxgkrnl/dxgvmbus.c:116:5: warning: no previous prototype for function 'ntstatus2int' [-Wmissing-prototypes]
int ntstatus2int(struct ntstatus status)
^
drivers/hv/dxgkrnl/dxgvmbus.c:116:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int ntstatus2int(struct ntstatus status)
^
static
>> drivers/hv/dxgkrnl/dxgvmbus.c:219:6: warning: no previous prototype for function 'process_inband_packet' [-Wmissing-prototypes]
void process_inband_packet(struct dxgvmbuschannel *channel,
^
drivers/hv/dxgkrnl/dxgvmbus.c:219:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void process_inband_packet(struct dxgvmbuschannel *channel,
^
static
>> drivers/hv/dxgkrnl/dxgvmbus.c:237:6: warning: no previous prototype for function 'process_completion_packet' [-Wmissing-prototypes]
void process_completion_packet(struct dxgvmbuschannel *channel,
^
drivers/hv/dxgkrnl/dxgvmbus.c:237:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void process_completion_packet(struct dxgvmbuschannel *channel,
^
static
>> drivers/hv/dxgkrnl/dxgvmbus.c:363:5: warning: no previous prototype for function 'dxgvmb_send_async_msg' [-Wmissing-prototypes]
int dxgvmb_send_async_msg(struct dxgvmbuschannel *channel,
^
drivers/hv/dxgkrnl/dxgvmbus.c:363:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int dxgvmb_send_async_msg(struct dxgvmbuschannel *channel,
^
static
drivers/hv/dxgkrnl/dxgvmbus.c:199:20: warning: unused function 'command_vm_to_host_init0' [-Wunused-function]
static inline void command_vm_to_host_init0(struct dxgkvmb_command_vm_to_host
^6 warnings generated.
vim +/find_pci_adapter +79 drivers/hv/dxgkrnl/dxgmodule.c
78
> 79 struct dxgadapter *find_pci_adapter(struct pci_dev *dev)
80 {
81 struct dxgadapter *entry;
82 struct dxgadapter *adapter = NULL;
83
84 dxgglobal_acquire_adapter_list_lock(DXGLOCK_EXCL);
85
86 list_for_each_entry(entry, &dxgglobal->adapter_list_head,
87 adapter_list_entry) {
88 if (dev == entry->pci_dev) {
89 adapter = entry;
90 break;
91 }
92 }
93
94 dxgglobal_release_adapter_list_lock(DXGLOCK_EXCL);
95 return adapter;
96 }
97
98 static struct dxgadapter *find_adapter(struct winluid *luid)
99 {
100 struct dxgadapter *entry;
101 struct dxgadapter *adapter = NULL;
102
103 dxgglobal_acquire_adapter_list_lock(DXGLOCK_EXCL);
104
105 list_for_each_entry(entry, &dxgglobal->adapter_list_head,
106 adapter_list_entry) {
107 if (memcmp(luid, &entry->luid, sizeof(struct winluid)) == 0) {
108 adapter = entry;
109 break;
110 }
111 }
112
113 dxgglobal_release_adapter_list_lock(DXGLOCK_EXCL);
114 return adapter;
115 }
116
117 void dxgglobal_add_host_event(struct dxghostevent *event)
118 {
119 spin_lock_irq(&dxgglobal->host_event_list_mutex);
120 list_add_tail(&event->host_event_list_entry,
121 &dxgglobal->host_event_list_head);
122 spin_unlock_irq(&dxgglobal->host_event_list_mutex);
123 }
124
125 void dxgglobal_remove_host_event(struct dxghostevent *event)
126 {
127 spin_lock_irq(&dxgglobal->host_event_list_mutex);
128 if (event->host_event_list_entry.next != NULL) {
129 list_del(&event->host_event_list_entry);
130 event->host_event_list_entry.next = NULL;
131 }
132 spin_unlock_irq(&dxgglobal->host_event_list_mutex);
133 }
134
> 135 void signal_host_cpu_event(struct dxghostevent *eventhdr)
136 {
137 struct dxghosteventcpu *event = (struct dxghosteventcpu *)eventhdr;
138
139 if (event->remove_from_list ||
140 event->destroy_after_signal) {
141 list_del(&eventhdr->host_event_list_entry);
142 eventhdr->host_event_list_entry.next = NULL;
143 }
144 if (event->cpu_event) {
145 dev_dbg(dxgglobaldev, "signal cpu event\n");
146 eventfd_signal(event->cpu_event, 1);
147 if (event->destroy_after_signal)
148 eventfd_ctx_put(event->cpu_event);
149 } else {
150 dev_dbg(dxgglobaldev, "signal completion\n");
151 complete(event->completion_event);
152 }
153 if (event->destroy_after_signal) {
154 dev_dbg(dxgglobaldev, "destroying event %p\n",
155 event);
156 vfree(event);
157 }
158 }
159
160 void dxgglobal_signal_host_event(u64 event_id)
161 {
162 struct dxghostevent *event;
163 unsigned long flags;
164
165 dev_dbg(dxgglobaldev, "%s %lld\n", __func__, event_id);
166
167 spin_lock_irqsave(&dxgglobal->host_event_list_mutex, flags);
168 list_for_each_entry(event, &dxgglobal->host_event_list_head,
169 host_event_list_entry) {
170 if (event->event_id == event_id) {
171 dev_dbg(dxgglobaldev, "found event to signal %lld\n",
172 event_id);
173 if (event->event_type == dxghostevent_cpu_event)
174 signal_host_cpu_event(event);
175 else
176 pr_err("Unknown host event type");
177 break;
178 }
179 }
180 spin_unlock_irqrestore(&dxgglobal->host_event_list_mutex, flags);
181 dev_dbg(dxgglobaldev, "dxgglobal_signal_host_event_end %lld\n",
182 event_id);
183 }
184
185 struct dxghostevent *dxgglobal_get_host_event(u64 event_id)
186 {
187 struct dxghostevent *entry;
188 struct dxghostevent *event = NULL;
189
190 spin_lock_irq(&dxgglobal->host_event_list_mutex);
191 list_for_each_entry(entry, &dxgglobal->host_event_list_head,
192 host_event_list_entry) {
193 if (entry->event_id == event_id) {
194 list_del(&entry->host_event_list_entry);
195 entry->host_event_list_entry.next = NULL;
196 event = entry;
197 break;
198 }
199 }
200 spin_unlock_irq(&dxgglobal->host_event_list_mutex);
201 return event;
202 }
203
204 u64 dxgglobal_new_host_event_id(void)
205 {
206 return atomic64_inc_return(&dxgglobal->host_event_id);
207 }
208
209 void dxgglobal_acquire_process_adapter_lock(void)
210 {
211 mutex_lock(&dxgglobal->process_adapter_mutex);
212 }
213
214 void dxgglobal_release_process_adapter_lock(void)
215 {
216 mutex_unlock(&dxgglobal->process_adapter_mutex);
217 }
218
> 219 int dxgglobal_create_adapter(struct pci_dev *dev, guid_t *guid, 220 struct winluid host_vgpu_luid)
221 {
222 struct dxgadapter *adapter;
223 int ret = 0;
224
225 adapter = vzalloc(sizeof(struct dxgadapter));
226 if (adapter == NULL) {
227 ret = -ENOMEM;
228 goto cleanup;
229 }
230
231 adapter->adapter_state = DXGADAPTER_STATE_WAITING_VMBUS;
232 adapter->host_vgpu_luid = host_vgpu_luid;
233 kref_init(&adapter->adapter_kref);
234 init_rwsem(&adapter->core_lock);
235
236 INIT_LIST_HEAD(&adapter->adapter_process_list_head);
237 INIT_LIST_HEAD(&adapter->shared_resource_list_head);
238 INIT_LIST_HEAD(&adapter->adapter_shared_syncobj_list_head);
239 INIT_LIST_HEAD(&adapter->syncobj_list_head);
240 init_rwsem(&adapter->shared_resource_list_lock);
241 adapter->pci_dev = dev;
242 guid_to_luid(guid, &adapter->luid);
243
244 dxgglobal_acquire_adapter_list_lock(DXGLOCK_EXCL);
245
246 list_add_tail(&adapter->adapter_list_entry,
247 &dxgglobal->adapter_list_head);
248 dxgglobal->num_adapters++;
249 dxgglobal_release_adapter_list_lock(DXGLOCK_EXCL);
250
251 dev_dbg(dxgglobaldev, "new adapter added %p %x-%x\n", adapter,
252 adapter->luid.a, adapter->luid.b);
253 cleanup:
254 dev_dbg(dxgglobaldev, "%s end: %d", __func__, ret);
255 return ret;
256 }
257
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Re: [PATCH v1 2/9] drivers: hv: dxgkrnl: Open device object, adapter enumeration, dxgdevice, dxgcontext creation
In reply to Iouri TarassovQuoted 18 lines · Iouri Tarassov
- Handle opening of the device (
/dev/dxg) file object and creation of dxgprocess. dxgprocess is created for each process, which opens/dev/dxg. dxgprocess is ref counted, so the exicting dxgprocess objects is used for a process, which opens the device object multiple time. dxgprocess is destroyed when the device object is closed.- Implement ioctls for virtual GPU adapter enumeration:
LX_DXENUMADAPTERS2,LX_DXENUMADAPTERS3
The IOCTLs return a d3dkmt handle for each enumerated adapter.- Implement ioctl to query adapter information:
LX_DXQUERYADAPTERINFO.- Implement ioctls to open and closet a dxgadapter object:
LX_DXOPENADAPTERFROMLUID,LX_DXCLOSEADAPTER
Note that dxgadapter is opened whenLX_DXENUMADAPTERS2,LX_DXENUMADAPTERS3are called.
Quoted 4 lines
- Implement ioctls for dxgdevice and dxgcontext creation/destruction:
LX_DXCREATEDEVICE,LX_DXCREATECONTEXT,LX_DXCREATECONTEXTVIRTUAL,LX_DXDESTROYCONTEXT,LX_DXCREATEHWCONTEXT,LX_DXDESTROYDEVICE,LX_DXDESTROYHWCONTEXT
Hint, when you have to list all of the different things you are doing in a single commit, that is a good clue that you should be splitting up the commit into "one per thing".
This commit has way too many things happening all at once, and is very difficult to review because of that. What would you do if you were confronted with a single patch like this and you had to review it?
Make it trivial to review, please break this up into smaller pieces.
thanks,
greg k-h
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
In reply to Iouri TarassovQuoted 1 line · Iouri Tarassov
Implement the
LX_DXCREATESYNCFILEIOCTL (D3DKMTCreateSyncFile).
Your subject line does not describe what this is doing at all, as we have no clue what DXGSYNCFILE is.
thanks,
greg k-h
Re: [PATCH v1 1/9] drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Iouri TarassovQuoted 11 lines · Iouri Tarassov
- Add support for a Hyper-V based vGPU implementation that exposes the DirectX API to Linux userspace.
- Handle driver loading, registration for the PCI and VM bus device notifications
- Add headers for user mode interfaces, internal driver objects and VM bus communication interface
- Handle initialization of VM bus channels and creation of the dxgadapter object
- Removeddxg_copy_from_useranddxg_copy_to_user
- Connect the dxgkrnl module to thedrivers/hv/makefile andKconfig.
- Create aMAINTAINERSentry
This looks like a history list?
What exactly is this?
Quoted 37 lines
PCI driver registration
A PCI device is created for each virtual GPU (vGPU) device, projected by the host. The device vendor is
PCI_VENDOR_ID_MICROSOFTand device id isPCI_DEVICE_ID_VIRTUAL_RENDER.dxg_pci_probe_devicehandles arrival of such devices and it creates dxgadapter objects. The PCI config space of the vGPU device has luid of the corresponding per GPU VM bus channel. This is how the adapters are linked to VM bus channels.dxgadapter initialization
A dxgadapter object represents a virtual GPU, projected to the VM by the host. This object can start functioning only when the global VM bus channel and the corresponding per vGPU VM bus channel are initialized in the guest. Notifications about arrival of vGPU PCI device and VM bus channels can happen in any order. Therefore, the initial dxgadapter object state is
DXGADAPTER_STATE_WAITING_VMBUS. A list of VM bus channels and a list of dxgadapter objects are created. When dxgkrnl is notified about a VM bus channel arrival, if tries to start all adapters, which are not started yet.VM bus interface version is exchanged by reading/writing the PCI config space of the vGPU device.
Signed-off-by Iouri Tarassov <[email protected]>--- MAINTAINERS | 7 + drivers/hv/Kconfig | 2 + drivers/hv/Makefile | 1 + drivers/hv/dxgkrnl/Kconfig | 26 + drivers/hv/dxgkrnl/Makefile | 5 + drivers/hv/dxgkrnl/dxgadapter.c | 189 +++ drivers/hv/dxgkrnl/dxgkrnl.h | 953 +++++++++++++++ drivers/hv/dxgkrnl/dxgmodule.c | 882 ++++++++++++++ drivers/hv/dxgkrnl/dxgprocess.c | 37 + drivers/hv/dxgkrnl/dxgvmbus.c | 543 +++++++++
Quoted 8 lines
drivers/hv/dxgkrnl/dxgvmbus.h | 901 ++++++++++++++ drivers/hv/dxgkrnl/hmgr.c | 88 ++ drivers/hv/dxgkrnl/hmgr.h | 112 ++ drivers/hv/dxgkrnl/ioctl.c | 37 + drivers/hv/dxgkrnl/misc.c | 37 + drivers/hv/dxgkrnl/misc.h | 96 ++ include/linux/hyperv.h | 16 + include/uapi/misc/d3dkmthk.h | 1954 +++++++++++++++++++++++++++++++
Why are you adding apis and structures that you are not actually using in this commit? Please only add the ones that you use, when you use them.
This commit should be much smaller, and in more different parts.
thanks,
greg k-h
kernel test robot
build botRe: [PATCH v1 2/9] drivers: hv: dxgkrnl: Open device object, adapter enumeration, dxgdevice, dxgcontext creation
In reply to Iouri TarassovI love your patch! Perhaps something to improve:
Hi Iouri,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16 next-20220113]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e3084ed48fd6b661fe434da0cb36d7d6706cf27f config: arm64-randconfig-r032-20220113 (https://download.01.org/0day-ci/archive/20220113/202201131549.w952i85x-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d1021978b8e7e35dcc30201ca1731d64b5a602a8)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/91a8d0866d1c0efc52ca8e1cb504e0ec15c979e2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 git checkout 91a8d0866d1c0efc52ca8e1cb504e0ec15c979e2
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hv/dxgkrnl/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> drivers/hv/dxgkrnl/hmgr.c:85:5: warning: no previous prototype for function 'get_instance' [-Wmissing-prototypes]
u32 get_instance(struct d3dkmthandle h)
^
drivers/hv/dxgkrnl/hmgr.c:85:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
u32 get_instance(struct d3dkmthandle h)
^
static
>> drivers/hv/dxgkrnl/hmgr.c:172:6: warning: no previous prototype for function 'print_status' [-Wmissing-prototypes]
void print_status(struct hmgrtable *table)
^
drivers/hv/dxgkrnl/hmgr.c:172:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void print_status(struct hmgrtable *table)
^
static
>> drivers/hv/dxgkrnl/hmgr.c:551:21: warning: no previous prototype for function 'hmgrtable_get_entry_type' [-Wmissing-prototypes]
enum hmgrentry_type hmgrtable_get_entry_type(struct hmgrtable *table,
^
drivers/hv/dxgkrnl/hmgr.c:551:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
enum hmgrentry_type hmgrtable_get_entry_type(struct hmgrtable *table,
^
static
drivers/hv/dxgkrnl/hmgr.c:167:20: warning: unused function 'is_empty' [-Wunused-function]
static inline bool is_empty(struct hmgrtable *table)
^
4 warnings generated.
--
>> drivers/hv/dxgkrnl/dxgadapter.c:261:6: warning: no previous prototype for function 'dxgdevice_mark_destroyed' [-Wmissing-prototypes]
void dxgdevice_mark_destroyed(struct dxgdevice *device)
^ drivers/hv/dxgkrnl/dxgadapter.c:261:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void dxgdevice_mark_destroyed(struct dxgdevice *device)
^
static
1 warning generated.
vim +/get_instance +85 drivers/hv/dxgkrnl/hmgr.c
84
> 85 u32 get_instance(struct d3dkmthandle h)
86 {
87 return (h.v & HMGRHANDLE_INSTANCE_MASK) >> HMGRHANDLE_INSTANCE_SHIFT;
88 }
89
90 static bool is_handle_valid(struct hmgrtable *table, struct d3dkmthandle h,
91 bool ignore_destroyed, enum hmgrentry_type t)
92 {
93 u32 index = get_index(h);
94 u32 unique = get_unique(h);
95 struct hmgrentry *entry;
96
97 if (index >= table->table_size) {
98 pr_err("%s Invalid index %x %d\n", __func__, h.v, index);
99 return false;
100 }
101
102 entry = &table->entry_table[index];
103 if (unique != entry->unique) {
104 pr_err("%s Invalid unique %x %d %d %d %p",
105 __func__, h.v, unique, entry->unique,
106 index, entry->object);
107 return false;
108 }
109
110 if (entry->destroyed && !ignore_destroyed) {
111 pr_err("%s Invalid destroyed", __func__);
112 return false;
113 }
114
115 if (entry->type == HMGRENTRY_TYPE_FREE) {
116 pr_err("%s Entry is freed %x %d", __func__, h.v, index);
117 return false;
118 }
119
120 if (t != HMGRENTRY_TYPE_FREE && t != entry->type) {
121 pr_err("%s type mismatch %x %d %d", __func__, h.v,
122 t, entry->type);
123 return false;
124 }
125
126 return true;
127 }
128
129 static struct d3dkmthandle build_handle(u32 index, u32 unique, u32 instance)
130 {
131 struct d3dkmthandle handle;
132
133 handle.v = (index << HMGRHANDLE_INDEX_SHIFT) & HMGRHANDLE_INDEX_MASK;
134 handle.v |= (unique << HMGRHANDLE_UNIQUE_SHIFT) &
135 HMGRHANDLE_UNIQUE_MASK;
136 handle.v |= (instance << HMGRHANDLE_INSTANCE_SHIFT) &
137 HMGRHANDLE_INSTANCE_MASK;
138
139 return handle;
140 }
141
142 inline u32 hmgrtable_get_used_entry_count(struct hmgrtable *table)
143 {
144 DXGKRNL_ASSERT(table->table_size >= table->free_count);
145 return (table->table_size - table->free_count);
146 }
147
148 bool hmgrtable_mark_destroyed(struct hmgrtable *table, struct d3dkmthandle h)
149 {
150 if (!is_handle_valid(table, h, false, HMGRENTRY_TYPE_FREE))
151 return false;
152
153 table->entry_table[get_index(h)].destroyed = true;
154 return true;
155 }
156
157 bool hmgrtable_unmark_destroyed(struct hmgrtable *table, struct d3dkmthandle h)
158 {
159 if (!is_handle_valid(table, h, true, HMGRENTRY_TYPE_FREE))
160 return true;
161
162 DXGKRNL_ASSERT(table->entry_table[get_index(h)].destroyed);
163 table->entry_table[get_index(h)].destroyed = 0;
164 return true;
165 }
166
167 static inline bool is_empty(struct hmgrtable *table)
168 {
169 return (table->free_count == table->table_size);
170 }
171
> 172 void print_status(struct hmgrtable *table)
173 {
174 int i;
175
176 dev_dbg(dxgglobaldev, "hmgrtable head, tail %p %d %d\n",
177 table, table->free_handle_list_head,
178 table->free_handle_list_tail);
179 if (table->entry_table == NULL)
180 return;
181 for (i = 0; i < 3; i++) {
182 if (table->entry_table[i].type != HMGRENTRY_TYPE_FREE)
183 dev_dbg(dxgglobaldev, "hmgrtable entry %p %d %p\n",
184 table, i, table->entry_table[i].object);
185 else
186 dev_dbg(dxgglobaldev, "hmgrtable entry %p %d %d %d\n",
187 table, i,
188 table->entry_table[i].next_free_index,
189 table->entry_table[i].prev_free_index); 190 }
191 }
192
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Re: [PATCH v1 1/9] drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Iouri Tarassovdrivers/hv/dxgkrnl/dxgadapter.c line 34
Quoted 3 lines · Iouri Tarassov
drivers/hv/dxgkrnl/dxgadapter.c new file +3
-0+dev_dbg (dxgglobaldev,"%s: %x:%x %p %pUb\n" , + __func__, adapter->luid.b, adapter->luid.a, hdev->channel, + &hdev->channel->offermsg.offer.if_instance);
When I see something like "global device pointer", that is a HUGE red flag.
No driver should ever have anything that is static to the driver like this, it should always be per-device. Please use the correct device model here, which does not include a global pointer, but rather unique ones that are given to you by the driver core. That way you are never tied to only "one device per system" as that is a constraint that you will have to fix eventually, might as well do it all correctly the first time as it is not any extra effort to do so.
thanks,
greg k-h
Re: [PATCH v1 8/9] drivers: hv: dxgkrnl: Implement various WDDM ioctls
In reply to Iouri TarassovQuoted 56 lines · Iouri Tarassov
Implement various WDDM IOCTLs.
- IOCTLs to handle GPU virtual addressing (VA):
LX_DXRESERVEGPUVIRTUALADDRESS(D3DKMTReserveGpuVertualAddress)LX_DXFREEGPUVIRTUALADDRESS(D3DKMTFreeGpuVertualAddress)LX_DXMAPGPUVIRTUALADDRESS(D3DKMTMapGpuVertualAddress)LX_DXUPDATEGPUVIRTUALADDRESS(D3DKMTUpdateGpuVertualAddress)WDDM supports a compute device to use GPU virtual addresses when accessing allocation memory. A GPU VA could be reserved or mapped to a GPU allocation. The video memory manager on the host updates GPU page tables for the virtual addresses.
- IOCTLs to manage residency of GPU accessing allocations:
LX_DXMAKERESIDENT(D3DKMTMakeResident)LX_DXEVICT(D3DKMTEvict)An allocation is resident when GPU is setup to access it. The current WDDM design does not support on demand GPU page faulting. An allocation must be resident (be in the local device memory or in non-pageable system memory) before GPU is allowed to access it.
- IOCTLs to offer/reclaim alloctions:
LX_DXOFFERALLOCATIONS{D3DKMTOfferAllocations)LX_DXRECLAIMALLOCATIONS2(D3DKMTReclaimAllocations)When a user mode driver does not need an allocation, it can "offer" it. This means that the allocation is not in use and it local device memory could be reclaimed and given to another allocation. When the allocation is again needed, the caller can "reclaim" the allocations. If the allocation is still in the device local memory, the reclaim operation succeeds. If not the called must restore the content of the allocation before it can be used by the device.
-
LX_DXESCAPE(D3DKMTEscape)
This IOCTL is used to send/receive private data between user mode driver and kernel mode driver. This is an extension of the WDDM APIs.-
LX_DXGETDEVICESTATE(D3DKMTGetDeviceState)
The IOCTL is used to get the current execution state of the dxgdevice object.-
LX_DXMARKDEVICEASERROR(D3DKMTMarkDeviceAsError)
The IOCTL is used to bring the dxgdevice object to the error state. Subsequent calls to use the device object will fail.-
LX_DXQUERYSTATISTICS(D3DKMTQuerystatistics)
The IOCTL is used to query various statistics from the compute device on the host.- IOCTLs to deal with execution context priorities
LX_DXGETCONTEXTINPROCESSSCHEDULINGPRIORITYLX_DXGETCONTEXTSCHEDULINGPRIORITYLX_DXSETCONTEXTINPROCESSSCHEDULINGPRIORITYLX_DXSETCONTEXTSCHEDULINGPRIORITY
Quoted 7 lines
Signed-off-by Iouri Tarassov <[email protected]>--- drivers/hv/dxgkrnl/dxgkrnl.h | 2 + drivers/hv/dxgkrnl/dxgvmbus.c | 1466 ++++++++++++++++++++++++++++--- drivers/hv/dxgkrnl/dxgvmbus.h | 15 + drivers/hv/dxgkrnl/ioctl.c | 1524 ++++++++++++++++++++++++++++++++- 4 files changed, 2831 insertions(+), 176 deletions(-)
Again, break this up into smaller pieces. Would you want to review all of these at the same time?
Remember, you write code for people to review and understand first, and the compiler second. With large changes like this, you are making it difficult for people to review, which is your target audience.
I'll stop here, please fix up this patch series into something that is reviewable.
thanks,
greg k-h
kernel test robot
build botRe: [PATCH v1 3/9] drivers: hv: dxgkrnl: Implement creation/destruction of GPU allocations/resources
In reply to Iouri TarassovI love your patch! Perhaps something to improve:
Hi Iouri,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16 next-20220113]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e3084ed48fd6b661fe434da0cb36d7d6706cf27f config: arm64-randconfig-r032-20220113 (https://download.01.org/0day-ci/archive/20220113/202201131642.1nercfCr-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d1021978b8e7e35dcc30201ca1731d64b5a602a8)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/a2aa8c606c48a4e6bf8a7a51e2e4e5738e35da32
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Iouri-Tarassov/drivers-hv-dxgkrnl-Driver-overview/20220113-035836 git checkout a2aa8c606c48a4e6bf8a7a51e2e4e5738e35da32
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/hv/dxgkrnl/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
In file included from drivers/hv/dxgkrnl/ioctl.c:21:
drivers/hv/dxgkrnl/dxgvmbus.h:867:26: warning: implicit conversion from enumeration type 'enum dxgkvmb_commandtype' to different enumeration type 'enum dxgkvmb_commandtype_global' [-Wenum-conversion]
command->command_type = DXGK_VMBCOMMAND_INVALID;
~ ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hv/dxgkrnl/ioctl.c:1245:5: warning: no previous prototype for function 'validate_alloc' [-Wmissing-prototypes]
int validate_alloc(struct dxgallocation *alloc0,
^ drivers/hv/dxgkrnl/ioctl.c:1245:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int validate_alloc(struct dxgallocation *alloc0,
^
static
2 warnings generated.
--
In file included from drivers/hv/dxgkrnl/dxgvmbus.c:23:
drivers/hv/dxgkrnl/dxgvmbus.h:867:26: warning: implicit conversion from enumeration type 'enum dxgkvmb_commandtype' to different enumeration type 'enum dxgkvmb_commandtype_global' [-Wenum-conversion]
command->command_type = DXGK_VMBCOMMAND_INVALID;
~ ^~~~~~~~~~~~~~~~~~~~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:151:5: warning: no previous prototype for function 'ntstatus2int' [-Wmissing-prototypes]
int ntstatus2int(struct ntstatus status)
^
drivers/hv/dxgkrnl/dxgvmbus.c:151:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int ntstatus2int(struct ntstatus status)
^
static
drivers/hv/dxgkrnl/dxgvmbus.c:254:6: warning: no previous prototype for function 'process_inband_packet' [-Wmissing-prototypes]
void process_inband_packet(struct dxgvmbuschannel *channel,
^
drivers/hv/dxgkrnl/dxgvmbus.c:254:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void process_inband_packet(struct dxgvmbuschannel *channel,
^
static
drivers/hv/dxgkrnl/dxgvmbus.c:272:6: warning: no previous prototype for function 'process_completion_packet' [-Wmissing-prototypes]
void process_completion_packet(struct dxgvmbuschannel *channel,
^
drivers/hv/dxgkrnl/dxgvmbus.c:272:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void process_completion_packet(struct dxgvmbuschannel *channel,
^
static
drivers/hv/dxgkrnl/dxgvmbus.c:398:5: warning: no previous prototype for function 'dxgvmb_send_async_msg' [-Wmissing-prototypes]
int dxgvmb_send_async_msg(struct dxgvmbuschannel *channel,
^
drivers/hv/dxgkrnl/dxgvmbus.c:398:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int dxgvmb_send_async_msg(struct dxgvmbuschannel *channel,
^
static
>> drivers/hv/dxgkrnl/dxgvmbus.c:909:5: warning: no previous prototype for function 'create_existing_sysmem' [-Wmissing-prototypes]
int create_existing_sysmem(struct dxgdevice *device,
^
drivers/hv/dxgkrnl/dxgvmbus.c:909:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int create_existing_sysmem(struct dxgdevice *device,
^
static
drivers/hv/dxgkrnl/dxgvmbus.c:234:20: warning: unused function 'command_vm_to_host_init0' [-Wunused-function]
static inline void command_vm_to_host_init0(struct dxgkvmb_command_vm_to_host
^7 warnings generated.
vim +/validate_alloc +1245 drivers/hv/dxgkrnl/ioctl.c
1244
> 1245 int validate_alloc(struct dxgallocation *alloc0, 1246 struct dxgallocation *alloc, 1247 struct dxgdevice *device, 1248 struct d3dkmthandle alloc_handle)
1249 {
1250 u32 fail_reason;
1251
1252 if (alloc == NULL) {
1253 fail_reason = 1;
1254 goto cleanup;
1255 }
1256 if (alloc->resource_owner != alloc0->resource_owner) {
1257 fail_reason = 2;
1258 goto cleanup;
1259 }
1260 if (alloc->resource_owner) {
1261 if (alloc->owner.resource != alloc0->owner.resource) {
1262 fail_reason = 3;
1263 goto cleanup;
1264 }
1265 if (alloc->owner.resource->device != device) {
1266 fail_reason = 4;
1267 goto cleanup;
1268 }
1269 if (alloc->owner.resource->shared_owner) {
1270 fail_reason = 5;
1271 goto cleanup;
1272 }
1273 } else {
1274 if (alloc->owner.device != device) {
1275 fail_reason = 6;
1276 goto cleanup;
1277 }
1278 }
1279 return 0;
1280 cleanup:
1281 pr_err("Alloc validation failed: reason: %d %x",
1282 fail_reason, alloc_handle.v);
1283 return -EINVAL;
1284 }
1285
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Re: [PATCH v1 1/9] drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Greg KHQuoted 8 lines · Greg KH
drivers/hv/dxgkrnl/dxgadapter.c line 34
Quoted 3 lines · Iouri Tarassov
drivers/hv/dxgkrnl/dxgadapter.c new file +3
-0+dev_dbg (dxgglobaldev,"%s: %x:%x %p %pUb\n" , + __func__, adapter->luid.b, adapter->luid.a, hdev->channel, + &hdev->channel->offermsg.offer.if_instance);When I see something like "global device pointer", that is a HUGE red flag.
Quoted 7 lines
No driver should ever have anything that is static to the driver like this, it should always be per-device. Please use the correct device model here, which does not include a global pointer, but rather unique ones that are given to you by the driver core. That way you are never tied to only "one device per system" as that is a constraint that you will have to fix eventually, might as well do it all correctly the first time as it is not any extra effort to do so
Hi Greg,
dxgglobaldev is a pointer to the global driver data. By design there is a single hyper-v VM bus and a single corresponding /dev/dxg device. Virtual GPU adapters are present on the VM bus. /dev/dxg device is used to enumerate all virtual GPUs, which are accessible only via IOCTLs to /dev/dxg. dxgglobaldev has a list of all vGPU adapters and other global driver state. This follows the design on Windows where a single global object in dxgkrnl.sys driver is used to enumerate and access all GPU devices. This is also how the public D3DKMT interface to dxgkrnl is structured.
Thanks
Iouri
Re: [PATCH v1 8/9] drivers: hv: dxgkrnl: Implement various WDDM ioctls
In reply to Greg KHQuoted 6 lines · Greg KH
Quoted 2 lines · Iouri Tarassov
Implement various WDDM IOCTLs.
Again, break this up into smaller pieces. Would you want to review all of these at the same time?
Quoted 6 lines
Remember, you write code for people to review and understand first, and the compiler second. With large changes like this, you are making it difficult for people to review, which is your target audience.
I'll stop here, please fix up this patch series into something that is reviewable.
Hi Greg,
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
states that "only post say 15 [patches] or so at a time and wait for review and integration". The IOCTLs here are simple and I tried to keep the number of patches smaller than 15. Is it ok to have more than 15 patches in a submission, or I need to submit the driver is several chunks (some of which would be not fully functional)?
Thanks
Iouri
Re: [PATCH v1 8/9] drivers: hv: dxgkrnl: Implement various WDDM ioctls
Quoted 17 lines · Iouri Tarassov
Quoted 6 lines · Greg KH
Quoted 2 lines · Iouri Tarassov
Implement various WDDM IOCTLs.
Again, break this up into smaller pieces. Would you want to reviewall
of these at the same time?Quoted 6 lines
Remember, you write code for people to review and understand first, and the compiler second. With large changes like this, you are making it difficult for people to review, which is your target audience.
I'll stop here, please fix up this patch series into something that is reviewable.
Hi Greg,
Quoted 7 lines
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
states that "only post say 15 [patches] or so at a time and wait for review and integration".
The IOCTLs here are simple and I tried to keep the number of patches smaller than 15. Is it ok to have more than 15 patches in a submission, or I need to submit the driver is several chunks (some of which would be not fully functional)?
We get patch series that are much longer all the time, that's fine. How many do you feel would be needed to properly break this out?
Again, create changes to make review easy.
greg k-h
Re: [PATCH v1 1/9] drivers: hv: dxgkrnl: Driver initialization and creation of dxgadapter
In reply to Iouri TarassovQuoted 17 lines · Iouri Tarassov
Quoted 8 lines · Greg KH
drivers/hv/dxgkrnl/dxgadapter.c line 34
Quoted 3 lines · Iouri Tarassov
drivers/hv/dxgkrnl/dxgadapter.c new file +3
-0+dev_dbg (dxgglobaldev,"%s: %x:%x %p %pUb\n" , + __func__, adapter->luid.b, adapter->luid.a, hdev->channel, + &hdev->channel->offermsg.offer.if_instance);When I see something like "global device pointer", that is a HUGE red flag.
Quoted 7 lines
No driver should ever have anything that is static to the driver like this, it should always be per-device. Please use the correct device model here, which does not include a global pointer, but rather unique ones that are given to you by the driver core. That way you are never tied to only "one device per system" as that is a constraint that you will have to fix eventually, might as well do it all correctly the first time as it is not any extra effort to do so
Quoted 4 lines
Hi Greg,
dxgglobaldev is a pointer to the global driver data. By design there is a single hyper-v VM bus and a single corresponding
/dev/dxgdevice.
That's fine, but use the pointer that you create based on your bus device, not some static pointer please.
Quoted 7 lines
Virtual GPU adapters are present on the VM bus.
/dev/dxgdevice is used to enumerate all virtual GPUs, which are accessible only via IOCTLs to/dev/dxg. dxgglobaldev has a list of all vGPU adapters and other global driver state. This follows the design on Windows where a single global object in dxgkrnl.sys driver is used to enumerate and access all GPU devices. This is also how the public D3DKMT interface to dxgkrnl is structured.
First off, remember this isn't Windows, let's not make the same mistakes they have made there please :)
Secondly, this isn't the problem, the issue is that you have a non-dynamic device here, which is not how Linux drivers should ever work. It's fine to have a "mux" device like this, but create it properly, based on the device that the driver core gives you as you must respect that lifetime, not the lifetime of a static pointer embedded in a module.
thanks,
greg k-h
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
In reply to Iouri TarassovHi all,
Quoted 6 lines · Iouri Tarassov
Implement the
LX_DXCREATESYNCFILEIOCTL (D3DKMTCreateSyncFile).dxgsyncfile is built on top of the Linux
sync_fileobject and provides a way for the user mode to synchronize with the execution of the device DMA packets.
Quoted 8 lines
The IOCTL creates a dxgsyncfile object for the given GPU synchronization object and a fence value. A
sync_objectfile descriptor is returned to the caller. The caller could wait for the object by usingpoll(). When the GPU synchronization object is signaled on the host, the host sends a message to the virtual machine and thesync_fileobject is signaled.Signed-off-by Iouri Tarassov <[email protected]>
Adding dri-devel, which get_maintainers.pl should have done automatically with the dma_fence wildcard match. Not sure why that didn't happen.
drivers/hv/dxgkrnl/dxgsyncfile.h line 23
Quoted 3 lines
drivers/hv/dxgkrnl/dxgsyncfile.h new file +3
-0+struct dxgsyncpoint { +struct dxghostevent hdr; +struct dma_fence base;
This doesn't work unfortuntately. For better or worse memory fences like monitored fences from wddm have completely different semantics from dma_fence. You could probably hack this to be self-consistent for hyper-v, but the problem is that then hv would have incompatible locking/nesting rules compared to everything else, and dma_fence matter for memory management so this includes whether you're allowed to kmalloc(GFP_KERNEL) or not, and that's just a bit too much.
I discussed this quickly with Jesse on irc and it sounds like the reason you want the dma_fence is just to emulate the sync_file interface for android. I think the correct solution here is to create a hv_dxg_sync_file fd, which emulates the exact ioctls that Android needs, but with a wddm monitored fence underneath instead of a dma_fence underneath.
This way we guarantee that no one ever accidentally mixes these incompatible concepts up in the kernel, and Android should still be able to happily run under hyperv.
Thoughts?
Also pls cc me on this sync work since even if you drop dma_fence use completely I'd like to follow this a bit.
Cheers, Daniel
Quoted 294 lines
drivers/hv/dxgkrnl/dxgsyncfile.h new file +7
-0+u64 fence_value; +u64 context; +spinlock_t lock; + u64 u64; +}; + +#endif /* _DXGSYNCFILE_H */ drivers/hv/dxgkrnl/dxgvmbus.c +103
-42@@ -1311,15 +1311,18 @@ int create_existing_sysmem(struct dxgdevice *device,void *kmem =NULL ;int ret =0 ;struct dxgkvmb_command_setexistingsysmemstore *set_store_command; +struct dxgkvmb_command_setexistingsysmempages *set_pages_command;u64 alloc_size = host_alloc->allocation_size;u32 npages = alloc_size >>PAGE_SHIFT ;struct dxgvmbusmsg msg = {.hdr =NULL }; - - ret =init_message (&msg, device->adapter, device->process, -sizeof (*set_store_command)); -if (ret) - goto cleanup; - set_store_command = (void *)msg.msg; +const u32 max_pfns_in_message = + (DXG_MAX_VM_BUS_PACKET_SIZE -sizeof (*set_pages_command) - +PAGE_SIZE ) /sizeof (__u64); +u32 alloc_offset_in_pages =0 ; +struct page **page_in; +u64 *pfn; +u32 pages_to_send; +u32 i;/* * Create a guest physical address list and set it as the allocation @@ -1330,6 +1333,7 @@ int create_existing_sysmem(struct dxgdevice *device,dev_dbg (dxgglobaldev," Alloc size: %lld" , alloc_size); dxgalloc->cpu_address = (void *)sysmem; + dxgalloc->pages =vzalloc (npages *sizeof (void *));if (dxgalloc->pages ==NULL ) {pr_err ("failed to allocate pages" );@@ -1347,31 +1351,80 @@ int create_existing_sysmem(struct dxgdevice *device, ret = -ENOMEM ; goto cleanup; } - kmem =vmap (dxgalloc->pages, npages,VM_MAP ,PAGE_KERNEL ); -if (kmem ==NULL ) { -pr_err ("vmap failed" ); - ret = -ENOMEM ; - goto cleanup; - } - ret1 =vmbus_establish_gpadl (dxgglobal_get_vmbus (), kmem, - alloc_size, &dxgalloc->gpadl); -if (ret1) { -pr_err ("establish_gpadl failed: %d" , ret1); - ret = -ENOMEM ; - goto cleanup; - } -dev_dbg (dxgglobaldev,"New gpadl %d" , dxgalloc->gpadl.gpadl_handle); +if (!dxgglobal->map_guest_pages_enabled) { + ret =init_message (&msg, device->adapter, device->process, +sizeof (*set_store_command)); +if (ret) + goto cleanup; + set_store_command = (void *)msg.msg; -command_vgpu_to_host_init2 (&set_store_command->hdr, -DXGK_VMBCOMMAND_SETEXISTINGSYSMEMSTORE , - device->process->host_handle); - set_store_command->device = device->handle; - set_store_command->device = device->handle; - set_store_command->allocation = host_alloc->allocation; - set_store_command->gpadl = dxgalloc->gpadl.gpadl_handle; - ret =dxgvmb_send_sync_msg_ntstatus (msg.channel, msg.hdr, msg.size); -if (ret <0 ) -pr_err ("failed to set existing store: %x" , ret); + kmem =vmap (dxgalloc->pages, npages,VM_MAP ,PAGE_KERNEL ); +if (kmem ==NULL ) { +pr_err ("vmap failed" ); + ret = -ENOMEM ; + goto cleanup; + } + ret1 =vmbus_establish_gpadl (dxgglobal_get_vmbus (), kmem, + alloc_size, &dxgalloc->gpadl); +if (ret1) { +pr_err ("establish_gpadl failed: %d" , ret1); + ret = -ENOMEM ; + goto cleanup; + } +dev_dbg (dxgglobaldev,"New gpadl %d" , + dxgalloc->gpadl.gpadl_handle); + +command_vgpu_to_host_init2 (&set_store_command->hdr, +DXGK_VMBCOMMAND_SETEXISTINGSYSMEMSTORE , + device->process->host_handle); + set_store_command->device = device->handle; + set_store_command->allocation = host_alloc->allocation; + set_store_command->gpadl = dxgalloc->gpadl.gpadl_handle; + ret =dxgvmb_send_sync_msg_ntstatus (msg.channel, msg.hdr, + msg.size); +if (ret <0 ) +pr_err ("failed to set existing store: %x" , ret); + }else { +/* +* Send the list of the allocation PFNs to the host. The host +* will map the pages for GPU access. +*/ + + ret =init_message (&msg, device->adapter, device->process, +sizeof (*set_pages_command) + + max_pfns_in_message *sizeof (u64)); +if (ret) + goto cleanup; + set_pages_command = (void *)msg.msg; +command_vgpu_to_host_init2 (&set_pages_command->hdr, +DXGK_VMBCOMMAND_SETEXISTINGSYSMEMPAGES , + device->process->host_handle); + set_pages_command->device = device->handle; + set_pages_command->allocation = host_alloc->allocation; + + page_in = dxgalloc->pages; +while (alloc_offset_in_pages < npages) { + pfn = (u64 *)((char *)msg.msg + +sizeof (*set_pages_command)); + pages_to_send =min (npages - alloc_offset_in_pages, + max_pfns_in_message); + set_pages_command->num_pages = pages_to_send; + set_pages_command->alloc_offset_in_pages = + alloc_offset_in_pages; + +for (i =0 ; i < pages_to_send; i++) + *pfn++ =page_to_pfn (*page_in++); + + ret =dxgvmb_send_sync_msg_ntstatus (msg.channel, + msg.hdr, + msg.size); +if (ret <0 ) { +pr_err ("failed to set existing pages: %x" , ret); +break ; + } + alloc_offset_in_pages += pages_to_send; + } + } cleanup:if (kmem)@@ -2685,6 +2738,7 @@ int dxgvmb_send_wait_sync_object_cpu(struct dxgprocess *process,struct d3dkmt_waitforsynchronizationobjectfromcpu *args, + bool user_address, u64 cpu_event) { int ret = -EINVAL;@@ -2708,18 +2762,25 @@ int dxgvmb_send_wait_sync_object_cpu(struct dxgprocess *process, command->object_count = args->object_count; command->guest_event_pointer = (u64 ) cpu_event; current_pos = (u8 *) &command[1 ]; - ret =copy_from_user (current_pos, args->objects, object_size); -if (ret) { -pr_err ("%s failed to copy objects" , __func__); - ret = -EINVAL ; - goto cleanup; - } - current_pos += object_size; - ret =copy_from_user (current_pos, args->fence_values, fence_size); -if (ret) { -pr_err ("%s failed to copy fences" , __func__); - ret = -EINVAL ; - goto cleanup; +if (user_address) { + ret =copy_from_user (current_pos, args->objects, object_size); +if (ret) { +pr_err ("%s failed to copy objects" , __func__); + ret = -EINVAL ; + goto cleanup; + } + current_pos += object_size; + ret =copy_from_user (current_pos, args->fence_values, + fence_size); +if (ret) { +pr_err ("%s failed to copy fences" , __func__); + ret = -EINVAL ; + goto cleanup; + } + }else { +memcpy (current_pos, args->objects, object_size); + current_pos += object_size; +memcpy (current_pos, args->fence_values, fence_size); } ret =dxgvmb_send_sync_msg_ntstatus (msg.channel, msg.hdr, msg.size);drivers/hv/dxgkrnl/dxgvmbus.h +12
-0@@ -139,6 +139,8 @@ enum dxgkvmb_commandtype {DXGK_VMBCOMMAND_GETCONTEXTSCHEDULINGPRIORITY =61 ,DXGK_VMBCOMMAND_QUERYCLOCKCALIBRATION =62 ,DXGK_VMBCOMMAND_QUERYRESOURCEINFO =64 , +DXGK_VMBCOMMAND_LOGEVENT =65 , +DXGK_VMBCOMMAND_SETEXISTINGSYSMEMPAGES =66 , DXGK_VMBCOMMAND_INVALID };@@ -245,6 +247,16 @@ struct dxgkvmb_command_setexistingsysmemstore {u32 gpadl; }; +/* Returns ntstatus */ +struct dxgkvmb_command_setexistingsysmempages { +struct dxgkvmb_command_vgpu_to_host hdr; +struct d3dkmthandle device; +struct d3dkmthandle allocation; +u32 num_pages; +u32 alloc_offset_in_pages; +/* u64 pfn_array[num_pages] */ +}; +struct dxgkvmb_command_createprocess {struct dxgkvmb_command_vm_to_host hdr;void *process;drivers/hv/dxgkrnl/ioctl.c +4
-6@@ -19,6 +19,7 @@#include "dxgkrnl.h" #include "dxgvmbus.h" +#include "dxgsyncfile.h" #undef pr_fmt#define pr_fmt (fmt) "dxgk:err: " fmt@@ -32,11 +33,6 @@ struct ioctl_desc { };static struct ioctl_desc ioctls[LX_IO_MAX +1 ]; -static char *errorstr (int ret) -{ -return ret <0 ?"err" :"" ; -} -static int dxgsyncobj_release (struct inode *inode,struct file *file) {struct dxgsharedsyncobject *syncobj = file->private_data;@@ -3561,7 +3557,7 @@ dxgk_wait_sync_object_cpu(struct dxgprocess *process, void *__user inargs) } ret =dxgvmb_send_wait_sync_object_cpu (process, adapter, - &args, event_id); + &args, true, event_id);if (ret <0 ) goto cleanup;@@ -5457,4 +5453,6 @@ void init_ioctls(void) LX_DXQUERYSTATISTICS);SET_IOCTL (/*0x44 */ dxgk_share_object_with_host,LX_DXSHAREOBJECTWITHHOST ); +SET_IOCTL (/*0x45 */ dxgk_create_sync_file, +LX_DXCREATESYNCFILE ); }drivers/hv/dxgkrnl/misc.c +6
-0@@ -35,3 +35,9 @@ u16 *wcsncpy(u16 *dest, const u16 *src, size_t n) dest[i -1 ] =0 ;return dest; } + +char *errorstr (int ret) +{ +return ret <0 ?"err" :"" ; +} +drivers/hv/dxgkrnl/misc.h +1
-0@@ -44,6 +44,7 @@ extern const struct d3dkmthandle zerohandle;*/ u16 *wcsncpy (u16 *dest,const u16 *src,size_t n); +char *errorstr (int ret);enum dxglockstate {DXGLOCK_SHARED ,Signature · 1 line
2.32.0
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
Quoted 39 lines · Daniel Vetter
Hi all,
Quoted 6 lines · Iouri Tarassov
Implement the
LX_DXCREATESYNCFILEIOCTL (D3DKMTCreateSyncFile).dxgsyncfile is built on top of the Linux
sync_fileobject and provides a way for the user mode to synchronize with the execution of the device DMA packets.Quoted 8 lines
The IOCTL creates a dxgsyncfile object for the given GPU synchronization object and a fence value. A
sync_objectfile descriptor is returned to the caller. The caller could wait for the object by usingpoll(). When the GPU synchronization object is signaled on the host, the host sends a message to the virtual machine and thesync_fileobject is signaled.Signed-off-by Iouri Tarassov <[email protected]>Adding dri-devel, which get_maintainers.pl should have done automatically with the
dma_fencewildcard match. Not sure why that didn't happen.drivers/hv/dxgkrnl/dxgsyncfile.h line 23
Quoted 3 lines
drivers/hv/dxgkrnl/dxgsyncfile.h new file +3
-0+struct dxgsyncpoint { +struct dxghostevent hdr; +struct dma_fence base;This doesn't work unfortuntately. For better or worse memory fences like monitored fences from wddm have completely different semantics from
dma_fence. You could probably hack this to be self-consistent for hyper-v, but the problem is that then hv would have incompatible locking/nesting rules compared to everything else, anddma_fencematter for memory management so this includes whether you're allowed tokmalloc(GFP_KERNEL)or not, and that's just a bit too much.I discussed this quickly with Jesse on irc and it sounds like the reason you want the
dma_fenceis just to emulate thesync_fileinterface for android. I think the correct solution here is to create ahv_dxg_sync_filefd, which emulates the exact ioctls that Android needs, but with a wddm monitored fence underneath instead of adma_fenceunderneath.
Quoted 8 lines
This way we guarantee that no one ever accidentally mixes these incompatible concepts up in the kernel, and Android should still be able to happily run under hyperv.
Thoughts?
Also pls cc me on this sync work since even if you drop
dma_fenceuse completely I'd like to follow this a bit.
Hi Daniel,
Thank you for the review and feedback.
I will get this addressed.
Iouri
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
In reply to Greg KHQuoted 5 lines · Greg KH
Quoted 1 line · Iouri Tarassov
Implement the
LX_DXCREATESYNCFILEIOCTL (D3DKMTCreateSyncFile).Your subject line does not describe what this is doing at all, as we have no clue what DXGSYNCFILE is.
Hi Greg,
I do not understand this comment. The full description is:
"
drivers: hv: dxgkrnl: Implement DXGSYNCFILE
Implement the LX_DXCREATESYNCFILE IOCTL (D3DKMTCreateSyncFile).
dxgsyncfile is built on top of the Linux sync_file object and provides a way for the user mode to synchronize with the execution of the device DMA packets.
The IOCTL creates a dxgsyncfile object for the given GPU synchronization object and a fence value. A sync_object file descriptor is returned to the caller. The caller could wait for the object by using poll(). When the GPU synchronization object is signaled on the host, the host sends a message to the virtual machine and the sync_file object is signaled.
"
Is this not enough?
Thanks
Iouri
Re: [PATCH v1 8/9] drivers: hv: dxgkrnl: Implement various WDDM ioctls
In reply to Greg KHQuoted 26 lines · Greg KH
Quoted 17 lines · Iouri Tarassov
Quoted 6 lines · Greg KH
Quoted 2 lines · Iouri Tarassov
Implement various WDDM IOCTLs.
Again, break this up into smaller pieces. Would you want to reviewall
of these at the same time?Quoted 6 lines
Remember, you write code for people to review and understand first, and the compiler second. With large changes like this, you are making it difficult for people to review, which is your target audience.
I'll stop here, please fix up this patch series into something that is reviewable.
Hi Greg,
Quoted 7 lines
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
states that "only post say 15 [patches] or so at a time and wait for review and integration".
The IOCTLs here are simple and I tried to keep the number of patches smaller than 15. Is it ok to have more than 15 patches in a submission, or I need to submit the driver is several chunks (some of which would be not fully functional)?
Quoted 2 lines
We get patch series that are much longer all the time, that's fine. How many do you feel would be needed to properly break this out?
Hi Greg,
I think there could be 20-25 patches.
Implementation of many IOCTLs follow the same pattern:
- add the IOCTL definition to the ioctl table
- implement a function to send the corresponding VM bus message to the host
- implement a function to handle the IOCTL input data, call the function to send message to the host and copy results back to the caller.
I tried to combine several such implementations to a single patch. I think the patch is logically simple and it would be easy to review.
What is your opinion?
Thanks
Iouri
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
In reply to Iouri TarassovQuoted 48 lines · Iouri Tarassov
Quoted 45 lines · Daniel Vetter
Hi all,
Quoted 6 lines · Iouri Tarassov
Implement the
LX_DXCREATESYNCFILEIOCTL (D3DKMTCreateSyncFile).dxgsyncfile is built on top of the Linux
sync_fileobject and provides a way for the user mode to synchronize with the execution of the device DMA packets.Quoted 8 lines
The IOCTL creates a dxgsyncfile object for the given GPU synchronization object and a fence value. A
sync_objectfile descriptor is returned to the caller. The caller could wait for the object by usingpoll(). When the GPU synchronization object is signaled on the host, the host sends a message to the virtual machine and thesync_fileobject is signaled.Signed-off-by Iouri Tarassov <[email protected]>Adding dri-devel, which get_maintainers.pl should have done automatically with the
dma_fencewildcard match. Not sure why that didn't happen.drivers/hv/dxgkrnl/dxgsyncfile.h line 23
Quoted 3 lines
drivers/hv/dxgkrnl/dxgsyncfile.h new file +3
-0+struct dxgsyncpoint { +struct dxghostevent hdr; +struct dma_fence base;This doesn't work unfortuntately. For better or worse memory fences like monitored fences from wddm have completely different semantics from
dma_fence. You could probably hack this to be self-consistent for hyper-v, but the problem is that then hv would have incompatible locking/nesting rules compared to everything else, anddma_fencematter for memory management so this includes whether you're allowed tokmalloc(GFP_KERNEL)or not, and that's just a bit too much.I discussed this quickly with Jesse on irc and it sounds like the reason you want the
dma_fenceis just to emulate thesync_fileinterface for android. I think the correct solution here is to create ahv_dxg_sync_filefd, which emulates the exact ioctls that Android needs, but with a wddm monitored fence underneath instead of adma_fenceunderneath.This way we guarantee that no one ever accidentally mixes these incompatible concepts up in the kernel, and Android should still be able to happily run under hyperv.
Thoughts?
Quoted 7 lines
Quoted 2 lines
Also pls cc me on this sync work since even if you drop
dma_fenceuse completely I'd like to follow this a bit.Hi Daniel,
Thank you for the review and feedback.
I will get this addressed.
btw another idea I had over the w/e: Another option might be to allow different backends for sync_file, and then making sure that you cannot ever mix dma_fence and hv_dxg_fence type sync_file up (in e.g. the merge ioctl).
The issue is that fundamentally dma_fence and memory fences (or umf for userspace memory fences as we tend to call them) aren't compatible, but some of the interop plans we have is to allow stuffing either of them into fence container objects like sync_file. So going that route for wddm monitored fence support too could be a really future-proof approach, plus it'd allow you to still share the sync_file interface code. Not that it's going to be much code sharing, since all the implementation code needs to be distinct. -Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Hi Iouri,
Quoted 31 lines · Iouri Tarassov
Quoted 19 lines · Nathan Chancellor
Hi Iouri,
Quoted 1 line
We're looking forward additional feedback.
I have been including this patch set into my downstream WSL2 kernel that
I build with clang and I noticed an instance of -Wenum-conversion that
is still present in this revision:Please consider cleaning this up in a future revision so that clang builds stay clean :)
...
I happened to notice there was another function that looks very similar
tocommand_vgpu_to_host_init0(),command_vm_to_host_init0(), which is
also unused. This was hidden because it is marked as "static inline" in
a .c file, which should generally be avoided; I would recommend
replacing all instances of "static inline" with just "static". The
compiler will still inline it if it feels it is worthwhile. Doing this
reveals one other unused function,is_empty():$ sed -i 's/static inline /static /g' drivers/hv/dxgkrnl/*.cQuoted 9 lines
$ make -skj"$(nproc)" LLVM=1 drivers/hv/dxgkrnl/ drivers/hv/dxgkrnl/hmgr.c:167:13: warning: unused function 'is_empty' [-Wunused-function] static bool is_empty(struct hmgrtable *table) ^ 1 warning generated. drivers/hv/dxgkrnl/dxgvmbus.c:234:13: warning: unused function 'command_vm_to_host_init0' [-Wunused-function] static void command_vm_to_host_init0(struct dxgkvmb_command_vm_to_host ^ 1 warning generated.
Quoted 1 line
Thanks a lot! We will get this fixed
In addition to the warnings that I mentioned above, when I apply this patchset on top of next-20220125, which enables -Warray-bounds, I see the following warnings with GCC 11.2.1:
drivers/hv/dxgkrnl/ioctl.c: In function ‘init_ioctls’:drivers/hv/dxgkrnl/ioctl.c:5315:15: warning: array subscript 69 is above array bounds of ‘struct ioctl_desc[69]’ [-Warray-bounds]
5315 | ioctls[_IOC_NR(v)].ioctl_callback = callback; \
| ~~~~~~^~~~~~~~~~~~
drivers/hv/dxgkrnl/ioctl.c:5456:9: note: in expansion of macro ‘SET_IOCTL’
5456 | SET_IOCTL(/*0x45 */ dxgk_create_sync_file,
| ^~~~~~~~~drivers/hv/dxgkrnl/ioctl.c:34:26: note: while referencing ‘ioctls’
34 | static struct ioctl_desc ioctls[LX_IO_MAX + 1];
| ^~~~~~drivers/hv/dxgkrnl/ioctl.c:5316:15: warning: array subscript 69 is above array bounds of ‘struct ioctl_desc[69]’ [-Warray-bounds]
5316 | ioctls[_IOC_NR(v)].ioctl = v
| ~~~~~~^~~~~~~~~~~~
drivers/hv/dxgkrnl/ioctl.c:5456:9: note: in expansion of macro ‘SET_IOCTL’
5456 | SET_IOCTL(/*0x45 */ dxgk_create_sync_file,
| ^~~~~~~~~drivers/hv/dxgkrnl/ioctl.c:34:26: note: while referencing ‘ioctls’
34 | static struct ioctl_desc ioctls[LX_IO_MAX + 1];
| ^~~~~~drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_query_alloc_residency’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr && (char *)msg->hdr != msg->msg_on_stack)
| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:1882:31: note: while referencing ‘msg’
1882 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_open_resource’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr && (char *)msg->hdr != msg->msg_on_stack)
| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:2132:31: note: while referencing ‘msg’
2132 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_get_stdalloc_data’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr && (char *)msg->hdr != msg->msg_on_stack)
| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:2183:31: note: while referencing ‘msg’
2183 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_get_allocation_priority’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr && (char *)msg->hdr != msg->msg_on_stack)
| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:3106:31: note: while referencing ‘msg’
3106 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_reclaim_allocations’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr && (char *)msg->hdr != msg->msg_on_stack)
| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:3298:31: note: while referencing ‘msg’
3298 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_query_statistics’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr && (char *)msg->hdr != msg->msg_on_stack)
| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:3698:31: note: while referencing ‘msg’
3698 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~
The warning in ioctl.c is resolved with the following diff, which appears to be a forward port problem, since the WSL2 tree is fine. I don't see the warning in dxgvmbus.c with clang so I did not look into it.
Cheers,
Nathan
include/uapi/misc/d3dkmthk.h +1 -1
@@ -1949,6 +1949,6 @@ struct d3dkmt_createsyncfile {#define LX_DXCREATESYNCFILE \ _IOWR(0x47, 0x45, struct d3dkmt_createsyncfile) -#define LX_IO_MAX 0x44 +#define LX_IO_MAX 0x45#endif /* _D3DKMTHK_H */
Quoted 11 lines · Nathan Chancellor
Hi Iouri,
Quoted 1 line · Iouri Tarassov
drivers/hv/dxgkrnl/dxgvmbus.c: In function ‘dxgvmb_send_query_alloc_residency’:drivers/hv/dxgkrnl/dxgvmbus.c:147:16: warning: array subscript ‘struct dxgvmbusmsg[0]’ is partly outside array bounds of ‘struct dxgvmbusmsgres[1]’ [-Warray-bounds]
147 | if (msg->hdr&& (char *)msg->hdr !=msg->msg_on_stack)| ~~~^~~~~
drivers/hv/dxgkrnl/dxgvmbus.c:1882:31: note: while referencing ‘msg’
1882 | struct dxgvmbusmsgres msg = {.hdr = NULL};
| ^~~
Hi Nathan,
I fixed the issue with the ioctls array.
Regarding the warning above, it looks like a compiler error. I do not see where the out of bounds access is. I tried GCC-11 version 11.2.0 and this warning is not there.
Thanks
Iouri
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
In reply to Daniel VetterQuoted 9 lines · Daniel Vetter
Quoted 2 lines · Iouri Tarassov
btw another idea I had over the w/e: Another option might be to allow different backends for
sync_file, and then making sure that you cannot ever mixdma_fenceandhv_dxg_fencetypesync_fileup (in e.g. the merge ioctl).
Quoted 9 lines
The issue is that fundamentally
dma_fenceand memory fences (or umf for userspace memory fences as we tend to call them) aren't compatible, but some of the interop plans we have is to allow stuffing either of them into fence container objects likesync_file. So going that route for wddm monitored fence support too could be a really future-proof approach, plus it'd allow you to still share thesync_fileinterface code. Not that it's going to be much code sharing, since all the implementation code needs to be distinct. -Daniel
Thanks Daniel!
I will remove the patch for dxgsyncfile from the next set of upstream patches.
It will be added later after a re-design.
Iouri
Re: [PATCH v1 9/9] drivers: hv: dxgkrnl: Implement DXGSYNCFILE
Quoted 21 lines · Iouri Tarassov
Quoted 9 lines · Daniel Vetter
Quoted 2 lines · Iouri Tarassov
btw another idea I had over the w/e: Another option might be to allow different backends for
sync_file, and then making sure that you cannot ever mixdma_fenceandhv_dxg_fencetypesync_fileup (in e.g. the merge ioctl).Quoted 9 lines
The issue is that fundamentally
dma_fenceand memory fences (or umf for userspace memory fences as we tend to call them) aren't compatible, but some of the interop plans we have is to allow stuffing either of them into fence container objects likesync_file. So going that route for wddm monitored fence support too could be a really future-proof approach, plus it'd allow you to still share thesync_fileinterface code. Not that it's going to be much code sharing, since all the implementation code needs to be distinct. -Daniel
Quoted 6 lines
Thanks Daniel!
I will remove the patch for dxgsyncfile from the next set of upstream patches.
It will be added later after a re-design.
Yeah sounds like a good plan forward to not hold up everything else meanwhile.
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch