From 738e1d995f64d20fd11ab59a88d12b524acb672e Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 20 Mar 2025 15:24:55 -0700 Subject: [PATCH] nvproxy: Add HasStatus.SetStatus and provide failWithStatus() util functions. This creates a more centralized way for nvproxy to return errors to the the user mode driver via the NvStatus field in ioctl structs. As opposed to failing the ioctl with mysterious EINVALs. Also updated the following structs to NOT implement HasStatus interface: - IoctlRegisterFD - RMAPIVersion - IoctlSysParams These don't have a Status field so it is misleading for them to implement HasStatus. Created frontendIoctlSimpleNoStatus() and frontendIoctlInvokeNoStatus() for such structs to use. PiperOrigin-RevId: 738959856 --- pkg/abi/nvgpu/frontend.go | 124 +++++++++--- pkg/abi/nvgpu/nvgpu.go | 1 + pkg/abi/nvgpu/uvm.go | 180 ++++++++++++++++++ pkg/sentry/devices/nvproxy/frontend.go | 75 +++++--- pkg/sentry/devices/nvproxy/frontend_unsafe.go | 21 +- pkg/sentry/devices/nvproxy/uvm.go | 18 +- pkg/sentry/devices/nvproxy/uvm_unsafe.go | 8 +- pkg/sentry/devices/nvproxy/version.go | 4 +- 8 files changed, 347 insertions(+), 84 deletions(-) diff --git a/pkg/abi/nvgpu/frontend.go b/pkg/abi/nvgpu/frontend.go index 0f8011044..524d544b9 100644 --- a/pkg/abi/nvgpu/frontend.go +++ b/pkg/abi/nvgpu/frontend.go @@ -66,15 +66,6 @@ type IoctlRegisterFD struct { CtlFD int32 } -// GetStatus implements HasStatus.GetStatus. -func (p *IoctlRegisterFD) GetStatus() uint32 { - // nv_ioctl_register_fd_t doesn't have a NvStatus field. Any failures are - // returned from src/nvidia/arch/nvalloc/unix/src/escape.c:nvidia_ioctl()'s - // NV_ESC_REGISTER_FD case to kernel-open/nvidia/nv.c:nvidia_ioctl()'s - // default case, which converts it to an ioctl(2) syscall error. - return NV_OK -} - // IoctlAllocOSEvent is the parameter type for NV_ESC_ALLOC_OS_EVENT. // // +marshal @@ -100,6 +91,11 @@ func (p *IoctlAllocOSEvent) GetStatus() uint32 { return p.Status } +// SetStatus implements HasStatus.SetStatus. +func (p *IoctlAllocOSEvent) SetStatus(status uint32) { + p.Status = status +} + // IoctlFreeOSEvent is the parameter type for NV_ESC_FREE_OS_EVENT. // // +marshal @@ -125,6 +121,11 @@ func (p *IoctlFreeOSEvent) GetStatus() uint32 { return p.Status } +// SetStatus implements HasStatus.SetStatus. +func (p *IoctlFreeOSEvent) SetStatus(status uint32) { + p.Status = status +} + // RMAPIVersion is the parameter type for NV_ESC_CHECK_VERSION_STR. // // +marshal @@ -134,14 +135,6 @@ type RMAPIVersion struct { VersionString [64]byte } -// GetStatus implements HasStatus.GetStatus. -func (p *RMAPIVersion) GetStatus() uint32 { - // nv_ioctl_rm_api_version_t doesn't have a NvStatus field. The driver - // translates the rmStatus to an ioctl(2) failure. See - // kernel-open/nvidia/nv.c:nvidia_ioctl() => case NV_ESC_CHECK_VERSION_STR. - return NV_OK -} - // IoctlSysParams is the parameter type for NV_ESC_SYS_PARAMS. // // +marshal @@ -149,14 +142,6 @@ type IoctlSysParams struct { MemblockSize uint64 } -// GetStatus implements HasStatus.GetStatus. -func (p *IoctlSysParams) GetStatus() uint32 { - // nv_ioctl_sys_params_t doesn't have a NvStatus field. The driver fails the - // ioctl(2) syscall in case of any failure. See - // kernel-open/nvidia/nv.c:nvidia_ioctl() => case NV_ESC_SYS_PARAMS. - return NV_OK -} - // IoctlWaitOpenComplete is the parameter type for NV_ESC_WAIT_OPEN_COMPLETE. // // +marshal @@ -170,6 +155,11 @@ func (p *IoctlWaitOpenComplete) GetStatus() uint32 { return p.AdapterStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *IoctlWaitOpenComplete) SetStatus(status uint32) { + p.AdapterStatus = status +} + // IoctlNVOS02ParametersWithFD is the parameter type for NV_ESC_RM_ALLOC_MEMORY. // // +marshal @@ -184,6 +174,11 @@ func (p *IoctlNVOS02ParametersWithFD) GetStatus() uint32 { return p.Params.Status } +// SetStatus implements HasStatus.SetStatus. +func (p *IoctlNVOS02ParametersWithFD) SetStatus(status uint32) { + p.Params.Status = status +} + // +marshal type NVOS02_PARAMETERS struct { HRoot Handle @@ -224,6 +219,11 @@ func (p *NVOS00_PARAMETERS) GetStatus() uint32 { return p.Status } +// SetStatus implements HasStatus.SetStatus. +func (p *NVOS00_PARAMETERS) SetStatus(status uint32) { + p.Status = status +} + // RmAllocParamType should be implemented by all possible parameter types for // NV_ESC_RM_ALLOC. type RmAllocParamType interface { @@ -308,11 +308,16 @@ func (n *NVOS21_PARAMETERS) ToOS64() NVOS64_PARAMETERS { } } -// GetStatus implements RmAllocParamType.GetStatus. +// GetStatus implements HasStatus.GetStatus. func (n *NVOS21_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS21_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS55_PARAMETERS is the parameter type for NV_ESC_RM_DUP_OBJECT. // // +marshal @@ -331,6 +336,11 @@ func (n *NVOS55_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS55_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS57_PARAMETERS is the parameter type for NV_ESC_RM_SHARE. // // +marshal @@ -346,6 +356,11 @@ func (n *NVOS57_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS57_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS30_PARAMETERS is the parameter type for NV_ESC_RM_IDLE_CHANNELS. // // +marshal @@ -370,6 +385,11 @@ func (n *NVOS30_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS30_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS32_PARAMETERS is the parameter type for NV_ESC_RM_VID_HEAP_CONTROL. // // +marshal @@ -391,6 +411,11 @@ func (n *NVOS32_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS32_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // Possible values for NVOS32Parameters.Function: const ( NVOS32_FUNCTION_ALLOC_SIZE = 2 @@ -457,6 +482,11 @@ func (p *IoctlNVOS33ParametersWithFD) GetStatus() uint32 { return p.Params.Status } +// SetStatus implements HasStatus.SetStatus. +func (p *IoctlNVOS33ParametersWithFD) SetStatus(status uint32) { + p.Params.Status = status +} + // +marshal type NVOS33_PARAMETERS struct { HClient Handle @@ -500,6 +530,11 @@ func (n *NVOS34_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS34_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS39_PARAMETERS is the parameter type for NV_ESC_RM_ALLOC_CONTEXT_DMA2. // // +marshal @@ -523,6 +558,11 @@ func (n *NVOS39_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS39_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS46_PARAMETERS is the parameter type for NV_ESC_RM_MAP_MEMORY_DMA. // // +marshal @@ -545,6 +585,11 @@ func (n *NVOS46_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS46_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS47_PARAMETERS is the parameter type for NV_ESC_RM_UNMAP_MEMORY_DMA. // // +marshal @@ -565,6 +610,11 @@ func (n *NVOS47_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS47_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS47_PARAMETERS_V550 is the updated version of NVOS47_PARAMETERS since // 550.54.04. // @@ -587,6 +637,11 @@ func (n *NVOS47_PARAMETERS_V550) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS47_PARAMETERS_V550) SetStatus(status uint32) { + n.Status = status +} + // NVOS54_PARAMETERS is the parameter type for NV_ESC_RM_CONTROL. // // +marshal @@ -605,6 +660,11 @@ func (n *NVOS54_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS54_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS56_PARAMETERS is the parameter type for NV_ESC_RM_UPDATE_DEVICE_MAPPING_INFO. // // +marshal @@ -624,6 +684,11 @@ func (n *NVOS56_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS56_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // NVOS64_PARAMETERS is one possible parameter type for NV_ESC_RM_ALLOC. // // +marshal @@ -668,11 +733,16 @@ func (n *NVOS64_PARAMETERS) FromOS64(other NVOS64_PARAMETERS) { *n = other } // ToOS64 implements RmAllocParamType.ToOS64. func (n *NVOS64_PARAMETERS) ToOS64() NVOS64_PARAMETERS { return *n } -// GetStatus implements RmAllocParamType.GetStatus. +// GetStatus implements HasStatus.GetStatus. func (n *NVOS64_PARAMETERS) GetStatus() uint32 { return n.Status } +// SetStatus implements HasStatus.SetStatus. +func (n *NVOS64_PARAMETERS) SetStatus(status uint32) { + n.Status = status +} + // HasFrontendFD is a type constraint for parameter structs containing a // frontend FD field. This is necessary because, as of this writing (Go 1.20), // there is no way to enable field access using a Go type constraint. diff --git a/pkg/abi/nvgpu/nvgpu.go b/pkg/abi/nvgpu/nvgpu.go index 9aa930421..5d4df11ad 100644 --- a/pkg/abi/nvgpu/nvgpu.go +++ b/pkg/abi/nvgpu/nvgpu.go @@ -88,4 +88,5 @@ type NvUUID [16]uint8 // HasStatus is an interface for parameter structs that have a Status field. type HasStatus interface { GetStatus() uint32 + SetStatus(status uint32) } diff --git a/pkg/abi/nvgpu/uvm.go b/pkg/abi/nvgpu/uvm.go index e73dc6264..0226a4ee9 100644 --- a/pkg/abi/nvgpu/uvm.go +++ b/pkg/abi/nvgpu/uvm.go @@ -64,6 +64,11 @@ func (p *UVM_INITIALIZE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_INITIALIZE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // UVM_INITIALIZE_PARAMS flags, from kernel-open/nvidia-uvm/uvm_types.h. const ( UVM_INIT_FLAGS_MULTI_PROCESS_SHARING_MODE = 0x2 @@ -81,6 +86,11 @@ func (p *UVM_CREATE_RANGE_GROUP_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_CREATE_RANGE_GROUP_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_DESTROY_RANGE_GROUP_PARAMS struct { RangeGroupID uint64 @@ -93,6 +103,11 @@ func (p *UVM_DESTROY_RANGE_GROUP_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_DESTROY_RANGE_GROUP_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_REGISTER_GPU_VASPACE_PARAMS struct { GPUUUID NvUUID @@ -117,6 +132,11 @@ func (p *UVM_REGISTER_GPU_VASPACE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_REGISTER_GPU_VASPACE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_UNREGISTER_GPU_VASPACE_PARAMS struct { GPUUUID NvUUID @@ -128,6 +148,11 @@ func (p *UVM_UNREGISTER_GPU_VASPACE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_UNREGISTER_GPU_VASPACE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_REGISTER_CHANNEL_PARAMS struct { GPUUUID NvUUID @@ -156,6 +181,11 @@ func (p *UVM_REGISTER_CHANNEL_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_REGISTER_CHANNEL_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_UNREGISTER_CHANNEL_PARAMS struct { GPUUUID NvUUID @@ -169,6 +199,11 @@ func (p *UVM_UNREGISTER_CHANNEL_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_UNREGISTER_CHANNEL_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_ENABLE_PEER_ACCESS_PARAMS struct { GPUUUIDA NvUUID @@ -181,6 +216,11 @@ func (p *UVM_ENABLE_PEER_ACCESS_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_ENABLE_PEER_ACCESS_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_DISABLE_PEER_ACCESS_PARAMS struct { GPUUUIDA NvUUID @@ -193,6 +233,11 @@ func (p *UVM_DISABLE_PEER_ACCESS_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_DISABLE_PEER_ACCESS_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_SET_RANGE_GROUP_PARAMS struct { RangeGroupID uint64 @@ -207,6 +252,11 @@ func (p *UVM_SET_RANGE_GROUP_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_SET_RANGE_GROUP_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_MAP_EXTERNAL_ALLOCATION_PARAMS struct { Base uint64 @@ -235,6 +285,11 @@ func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_MAP_EXTERNAL_ALLOCATION_PARAMS_V550 struct { Base uint64 @@ -263,6 +318,11 @@ func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS_V550) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS_V550) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_FREE_PARAMS struct { Base uint64 @@ -276,6 +336,11 @@ func (p *UVM_FREE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_FREE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_REGISTER_GPU_PARAMS struct { GPUUUID NvUUID @@ -303,6 +368,11 @@ func (p *UVM_REGISTER_GPU_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_REGISTER_GPU_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_UNREGISTER_GPU_PARAMS struct { GPUUUID NvUUID @@ -314,6 +384,11 @@ func (p *UVM_UNREGISTER_GPU_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_UNREGISTER_GPU_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_PAGEABLE_MEM_ACCESS_PARAMS struct { PageableMemAccess uint8 @@ -326,6 +401,11 @@ func (p *UVM_PAGEABLE_MEM_ACCESS_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_PAGEABLE_MEM_ACCESS_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_SET_PREFERRED_LOCATION_PARAMS struct { RequestedBase uint64 @@ -340,6 +420,11 @@ func (p *UVM_SET_PREFERRED_LOCATION_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_SET_PREFERRED_LOCATION_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_SET_PREFERRED_LOCATION_PARAMS_V550 struct { RequestedBase uint64 @@ -354,6 +439,11 @@ func (p *UVM_SET_PREFERRED_LOCATION_PARAMS_V550) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_SET_PREFERRED_LOCATION_PARAMS_V550) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_UNSET_PREFERRED_LOCATION_PARAMS struct { RequestedBase uint64 @@ -367,6 +457,11 @@ func (p *UVM_UNSET_PREFERRED_LOCATION_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_UNSET_PREFERRED_LOCATION_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_DISABLE_READ_DUPLICATION_PARAMS struct { RequestedBase uint64 @@ -380,6 +475,11 @@ func (p *UVM_DISABLE_READ_DUPLICATION_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_DISABLE_READ_DUPLICATION_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_UNSET_ACCESSED_BY_PARAMS struct { RequestedBase uint64 @@ -394,6 +494,11 @@ func (p *UVM_UNSET_ACCESSED_BY_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_UNSET_ACCESSED_BY_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_MIGRATE_PARAMS struct { Base uint64 @@ -415,6 +520,11 @@ func (p *UVM_MIGRATE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MIGRATE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // UVM_MIGRATE_PARAMS_V550 is the updated version of // UVM_MIGRATE_PARAMS since 550.40.07. // @@ -439,6 +549,11 @@ func (p *UVM_MIGRATE_PARAMS_V550) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MIGRATE_PARAMS_V550) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_MIGRATE_RANGE_GROUP_PARAMS struct { RangeGroupID uint64 @@ -452,6 +567,11 @@ func (p *UVM_MIGRATE_RANGE_GROUP_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MIGRATE_RANGE_GROUP_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_TOOLS_READ_PROCESS_MEMORY_PARAMS struct { Buffer uint64 @@ -462,6 +582,16 @@ type UVM_TOOLS_READ_PROCESS_MEMORY_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_TOOLS_READ_PROCESS_MEMORY_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_TOOLS_READ_PROCESS_MEMORY_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_TOOLS_WRITE_PROCESS_MEMORY_PARAMS struct { Buffer uint64 @@ -472,6 +602,16 @@ type UVM_TOOLS_WRITE_PROCESS_MEMORY_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_TOOLS_WRITE_PROCESS_MEMORY_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_TOOLS_WRITE_PROCESS_MEMORY_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_MAP_DYNAMIC_PARALLELISM_REGION_PARAMS struct { Base uint64 @@ -486,6 +626,11 @@ func (p *UVM_MAP_DYNAMIC_PARALLELISM_REGION_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MAP_DYNAMIC_PARALLELISM_REGION_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_UNMAP_EXTERNAL_PARAMS struct { Base uint64 @@ -500,6 +645,11 @@ func (p *UVM_UNMAP_EXTERNAL_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_UNMAP_EXTERNAL_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_ALLOC_SEMAPHORE_POOL_PARAMS struct { Base uint64 @@ -515,6 +665,11 @@ func (p *UVM_ALLOC_SEMAPHORE_POOL_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_ALLOC_SEMAPHORE_POOL_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_ALLOC_SEMAPHORE_POOL_PARAMS_V550 struct { Base uint64 @@ -530,6 +685,11 @@ func (p *UVM_ALLOC_SEMAPHORE_POOL_PARAMS_V550) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_ALLOC_SEMAPHORE_POOL_PARAMS_V550) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_PAGEABLE_MEM_ACCESS_ON_GPU_PARAMS struct { GPUUUID NvUUID @@ -543,6 +703,11 @@ func (p *UVM_PAGEABLE_MEM_ACCESS_ON_GPU_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_PAGEABLE_MEM_ACCESS_ON_GPU_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_VALIDATE_VA_RANGE_PARAMS struct { Base uint64 @@ -556,6 +721,11 @@ func (p *UVM_VALIDATE_VA_RANGE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_VALIDATE_VA_RANGE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_CREATE_EXTERNAL_RANGE_PARAMS struct { Base uint64 @@ -569,6 +739,11 @@ func (p *UVM_CREATE_EXTERNAL_RANGE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_CREATE_EXTERNAL_RANGE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // +marshal type UVM_MM_INITIALIZE_PARAMS struct { UvmFD int32 @@ -580,6 +755,11 @@ func (p *UVM_MM_INITIALIZE_PARAMS) GetStatus() uint32 { return p.RMStatus } +// SetStatus implements HasStatus.SetStatus. +func (p *UVM_MM_INITIALIZE_PARAMS) SetStatus(status uint32) { + p.RMStatus = status +} + // From kernel-open/nvidia-uvm/uvm_types.h: const ( diff --git a/pkg/sentry/devices/nvproxy/frontend.go b/pkg/sentry/devices/nvproxy/frontend.go index 929228706..0f56e6073 100644 --- a/pkg/sentry/devices/nvproxy/frontend.go +++ b/pkg/sentry/devices/nvproxy/frontend.go @@ -307,7 +307,7 @@ type frontendIoctlState struct { // frontendIoctlSimple implements a frontend ioctl whose parameters don't // contain any pointers requiring translation, file descriptors, or special -// cases or effects, and consequently don't need to be typed by the sentry. +// cases or effects. func frontendIoctlSimple[Params any, PtrParams hasStatusPtr[Params]](fi *frontendIoctlState) (uintptr, error) { var ioctlParamsValue Params ioctlParams := PtrParams(&ioctlParamsValue) @@ -328,19 +328,41 @@ func frontendIoctlSimple[Params any, PtrParams hasStatusPtr[Params]](fi *fronten return n, nil } -// frontendIoctlBytes is like frontendIoctlSimple, but for ioctls whose -// parameters don't contain any NvStatus field either. So these can be directly -// copied into byte buffers and proxied to the host. +// frontendIoctlSimpleNoStatus is the same as frontendIoctlSimple, but for +// ioctls whose parameters don't contain a NvStatus field. +func frontendIoctlSimpleNoStatus[Params any, PtrParams marshalPtr[Params]](fi *frontendIoctlState) (uintptr, error) { + var ioctlParamsValue Params + ioctlParams := PtrParams(&ioctlParamsValue) + if int(fi.ioctlParamsSize) != ioctlParams.SizeBytes() { + return 0, linuxerr.EINVAL + } + if _, err := ioctlParams.CopyIn(fi.t, fi.ioctlParamsAddr); err != nil { + return 0, err + } + + n, err := frontendIoctlInvokeNoStatus(fi, ioctlParams) + if err != nil { + return n, err + } + if _, err := ioctlParams.CopyOut(fi.t, fi.ioctlParamsAddr); err != nil { + return n, err + } + return n, nil +} + +// frontendIoctlBytes is like frontendIoctlSimple, but for "simple" ioctls that +// don't have a fixed-size parameter type (like NvU32 array). So these can be +// directly copied into byte buffers and proxied to the host. func frontendIoctlBytes(fi *frontendIoctlState) (uintptr, error) { if fi.ioctlParamsSize == 0 { - return frontendIoctlBytesInvoke(fi, nil) + return frontendIoctlInvokeNoStatus[byte](fi, nil) } ioctlParams := make([]byte, fi.ioctlParamsSize) if _, err := fi.t.CopyInBytes(fi.ioctlParamsAddr, ioctlParams); err != nil { return 0, err } - n, err := frontendIoctlBytesInvoke(fi, &ioctlParams[0]) + n, err := frontendIoctlInvokeNoStatus(fi, &ioctlParams[0]) if err != nil { return n, err } @@ -377,7 +399,7 @@ func frontendRegisterFD(fi *frontendIoctlState) (uintptr, error) { } ioctlParams.CtlFD = ctlFile.hostFD // The returned ctl_fd can't change, so skip copying out. - return frontendIoctlInvoke(fi, &ioctlParams) + return frontendIoctlInvokeNoStatus(fi, &ioctlParams) } func frontendIoctlHasFD[Params any, PtrParams hasFrontendFDAndStatusPtr[Params]](fi *frontendIoctlState) (uintptr, error) { @@ -547,30 +569,22 @@ func rmAllocMemorySimple(fi *frontendIoctlState, ioctlParams *nvgpu.IoctlNVOS02P func rmAllocOSDescriptor(fi *frontendIoctlState, ioctlParams *nvgpu.IoctlNVOS02ParametersWithFD) (uintptr, error) { // Compare src/nvidia/arch/nvalloc/unix/src/escape.c:RmAllocOsDescriptor() // => RmCreateOsDescriptor(). - failWithStatus := func(status uint32) error { - if log.IsLogging(log.Debug) { - fi.ctx.Debugf("nvproxy: NV_ESC_RM_ALLOC_MEMORY with class=NV01_MEMORY_SYSTEM_OS_DESCRIPTOR internally failed: status=%#x", status) - } - ioctlParams.Params.Status = status - _, err := ioctlParams.CopyOut(fi.t, fi.ioctlParamsAddr) - return err - } appAddr := addrFromP64(ioctlParams.Params.PMemory) if !appAddr.IsPageAligned() { - return 0, failWithStatus(nvgpu.NV_ERR_NOT_SUPPORTED) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_NOT_SUPPORTED) } arLen := ioctlParams.Params.Limit + 1 if arLen == 0 { // integer overflow - return 0, failWithStatus(nvgpu.NV_ERR_INVALID_LIMIT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_LIMIT) } var ok bool arLen, ok = hostarch.PageRoundUp(arLen) if !ok { - return 0, failWithStatus(nvgpu.NV_ERR_INVALID_ADDRESS) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ADDRESS) } appAR, ok := appAddr.ToRange(arLen) if !ok { - return 0, failWithStatus(nvgpu.NV_ERR_INVALID_ADDRESS) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ADDRESS) } // The host driver will collect pages from our address space starting at @@ -800,12 +814,6 @@ func rmControlSimple(fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54_PARAMETER return n, nil } -func ctrlCmdFailWithStatus(fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54_PARAMETERS, status uint32) error { - ioctlParams.Status = status - _, err := ioctlParams.CopyOut(fi.t, fi.ioctlParamsAddr) - return err -} - func ctrlHasFrontendFD[Params any, PtrParams hasFrontendFDPtr[Params]](fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54_PARAMETERS) (uintptr, error) { var ctrlParamsValue Params ctrlParams := PtrParams(&ctrlParamsValue) @@ -940,7 +948,7 @@ func ctrlGetNvU32List(fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54_PARAMETE return rmControlSimple(fi, ioctlParams) } if !rmapiParamsSizeCheck(ctrlParams.NumElems, 4 /* sizeof(NvU32) */) { - return 0, ctrlCmdFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } list := make([]uint32, ctrlParams.NumElems) if _, err := primitive.CopyUint32SliceIn(fi.t, addrFromP64(ctrlParams.List), list); err != nil { @@ -958,7 +966,7 @@ func ctrlDevGetCaps(fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54_PARAMETERS return 0, err } if !rmapiParamsSizeCheck(ctrlParams.CapsTblSize, 1) { - return 0, ctrlCmdFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } capsTbl := make([]byte, ctrlParams.CapsTblSize) // No need to copy into capsTbl from ctrlParams.CapsTbl. All callers specify @@ -1407,3 +1415,16 @@ func rmMapMemory(fi *frontendIoctlState) (uintptr, error) { return n, nil } + +func frontendFailWithStatus[Params any, PtrParams hasStatusPtr[Params]](fi *frontendIoctlState, ioctlParams PtrParams, status uint32) error { + return failWithStatus(fi.ctx, fi.t, fi.ioctlParamsAddr, ioctlParams, status) +} + +func failWithStatus[Params any, PtrParams hasStatusPtr[Params]](ctx context.Context, t *kernel.Task, ioctlParamsAddr hostarch.Addr, ioctlParams PtrParams, status uint32) error { + if log.IsLogging(log.Debug) { + ctx.Debugf("nvproxy: ioctl internally failed: status=%#x", status) + } + ioctlParams.SetStatus(status) + _, err := ioctlParams.CopyOut(t, ioctlParamsAddr) + return err +} diff --git a/pkg/sentry/devices/nvproxy/frontend_unsafe.go b/pkg/sentry/devices/nvproxy/frontend_unsafe.go index 57ad474d5..512e27457 100644 --- a/pkg/sentry/devices/nvproxy/frontend_unsafe.go +++ b/pkg/sentry/devices/nvproxy/frontend_unsafe.go @@ -26,20 +26,17 @@ import ( ) func frontendIoctlInvoke[Params any, PtrParams hasStatusPtr[Params]](fi *frontendIoctlState, ioctlParams PtrParams) (uintptr, error) { - n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(fi.fd.hostFD), frontendIoctlCmd(fi.nr, fi.ioctlParamsSize), uintptr(unsafe.Pointer(ioctlParams))) - if errno != 0 { - return n, errno - } - if log.IsLogging(log.Debug) { + n, err := frontendIoctlInvokeNoStatus(fi, ioctlParams) + if err == nil && log.IsLogging(log.Debug) { if status := ioctlParams.GetStatus(); status != nvgpu.NV_OK { fi.ctx.Debugf("nvproxy: frontend ioctl failed: status=%#x", status) } } - return n, nil + return n, err } -func frontendIoctlBytesInvoke(fi *frontendIoctlState, sentryParams *byte) (uintptr, error) { - n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(fi.fd.hostFD), frontendIoctlCmd(fi.nr, fi.ioctlParamsSize), uintptr(unsafe.Pointer(sentryParams))) +func frontendIoctlInvokeNoStatus[Params any](fi *frontendIoctlState, ioctlParams *Params) (uintptr, error) { + n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(fi.fd.hostFD), frontendIoctlCmd(fi.nr, fi.ioctlParamsSize), uintptr(unsafe.Pointer(ioctlParams))) if errno != 0 { return n, errno } @@ -97,7 +94,7 @@ func ctrlIoctlHasInfoList[Params any, PtrParams hasCtrlInfoListPtr[Params]](fi * var infoList []byte if listSize := ctrlParams.ListSize(); listSize > 0 { if !rmapiParamsSizeCheck(listSize, nvgpu.CtrlXxxInfoSize) { - return 0, ctrlCmdFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } infoList = make([]byte, listSize*nvgpu.CtrlXxxInfoSize) if _, err := fi.t.CopyInBytes(addrFromP64(ctrlParams.CtrlInfoList()), infoList); err != nil { @@ -239,7 +236,7 @@ func ctrlClientSystemGetP2PCaps(fi *frontendIoctlState, ioctlParams *nvgpu.NVOS5 origBusPeerIDs := ctrlParams.BusPeerIDs busPeerIDs, busPeerIDsBuf, ok := ctrlClientSystemGetP2PCapsInitializeArray(origBusPeerIDs, ctrlParams.GpuCount) if !ok { - return 0, ctrlCmdFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } ctrlParams.BusPeerIDs = busPeerIDs @@ -269,14 +266,14 @@ func ctrlClientSystemGetP2PCapsV550(fi *frontendIoctlState, ioctlParams *nvgpu.N origBusPeerIDs := ctrlParams.BusPeerIDs busPeerIDs, busPeerIDsBuf, ok := ctrlClientSystemGetP2PCapsInitializeArray(origBusPeerIDs, ctrlParams.GpuCount) if !ok { - return 0, ctrlCmdFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } ctrlParams.BusPeerIDs = busPeerIDs origBusEgmPeerIDs := ctrlParams.BusEgmPeerIDs busEgmPeerIDs, busEgmPeerIDsBuf, ok := ctrlClientSystemGetP2PCapsInitializeArray(origBusEgmPeerIDs, ctrlParams.GpuCount) if !ok { - return 0, ctrlCmdFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, frontendFailWithStatus(fi, ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } ctrlParams.BusEgmPeerIDs = busEgmPeerIDs diff --git a/pkg/sentry/devices/nvproxy/uvm.go b/pkg/sentry/devices/nvproxy/uvm.go index 8606430d9..e80419079 100644 --- a/pkg/sentry/devices/nvproxy/uvm.go +++ b/pkg/sentry/devices/nvproxy/uvm.go @@ -226,24 +226,14 @@ func uvmMMInitialize(ui *uvmIoctlState) (uintptr, error) { return 0, err } - failWithStatus := func(status uint32) error { - if log.IsLogging(log.Debug) { - ui.ctx.Debugf("nvproxy: UVM_MM_INITIALIZE internally failed: status=%#x", status) - } - outIoctlParams := ioctlParams - outIoctlParams.RMStatus = status - _, err := outIoctlParams.CopyOut(ui.t, ui.ioctlParamsAddr) - return err - } - uvmFileGeneric, _ := ui.t.FDTable().Get(ioctlParams.UvmFD) if uvmFileGeneric == nil { - return 0, failWithStatus(nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, uvmFailWithStatus(ui, &ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } defer uvmFileGeneric.DecRef(ui.ctx) uvmFile, ok := uvmFileGeneric.Impl().(*uvmFD) if !ok { - return 0, failWithStatus(nvgpu.NV_ERR_INVALID_ARGUMENT) + return 0, uvmFailWithStatus(ui, &ioctlParams, nvgpu.NV_ERR_INVALID_ARGUMENT) } origFD := ioctlParams.UvmFD @@ -299,3 +289,7 @@ func uvmIoctlHasFrontendFD[Params any, PtrParams hasFrontendFDAndStatusPtr[Param } return n, nil } + +func uvmFailWithStatus[Params any, PtrParams hasStatusPtr[Params]](ui *uvmIoctlState, ioctlParams PtrParams, status uint32) error { + return failWithStatus(ui.ctx, ui.t, ui.ioctlParamsAddr, ioctlParams, status) +} diff --git a/pkg/sentry/devices/nvproxy/uvm_unsafe.go b/pkg/sentry/devices/nvproxy/uvm_unsafe.go index dea8b17d6..26cb759a5 100644 --- a/pkg/sentry/devices/nvproxy/uvm_unsafe.go +++ b/pkg/sentry/devices/nvproxy/uvm_unsafe.go @@ -54,8 +54,8 @@ func (mf *uvmFDMemmapFile) BufferReadAt(off uint64, dst []byte) (uint64, error) if errno != 0 { return 0, errno } - if params.RMStatus != nvgpu.NV_OK { - log.Warningf("nvproxy: UVM_TOOLS_READ_PROCESS_MEMORY(targetVa=%#x, len=%d) returned status %d", off, len(dst), params.RMStatus) + if status := params.GetStatus(); status != nvgpu.NV_OK { + log.Warningf("nvproxy: UVM_TOOLS_READ_PROCESS_MEMORY(targetVa=%#x, len=%d) returned status %d", off, len(dst), status) return params.BytesRead, linuxerr.EINVAL } if params.BytesRead != uint64(len(dst)) { @@ -82,8 +82,8 @@ func (mf *uvmFDMemmapFile) BufferWriteAt(off uint64, src []byte) (uint64, error) if errno != 0 { return 0, errno } - if params.RMStatus != nvgpu.NV_OK { - log.Warningf("nvproxy: UVM_TOOLS_WRITE_PROCESS_MEMORY(targetVa=%#x, len=%d) returned status %d", off, len(src), params.RMStatus) + if status := params.GetStatus(); status != nvgpu.NV_OK { + log.Warningf("nvproxy: UVM_TOOLS_WRITE_PROCESS_MEMORY(targetVa=%#x, len=%d) returned status %d", off, len(src), status) return params.BytesWritten, linuxerr.EINVAL } if params.BytesWritten != uint64(len(src)) { diff --git a/pkg/sentry/devices/nvproxy/version.go b/pkg/sentry/devices/nvproxy/version.go index 4146ac5d1..a2cadea1e 100644 --- a/pkg/sentry/devices/nvproxy/version.go +++ b/pkg/sentry/devices/nvproxy/version.go @@ -107,9 +107,9 @@ func Init() { return &driverABI{ frontendIoctl: map[uint32]frontendIoctlHandler{ nvgpu.NV_ESC_CARD_INFO: feHandler(frontendIoctlBytes, compUtil), // nv_ioctl_card_info_t array - nvgpu.NV_ESC_CHECK_VERSION_STR: feHandler(frontendIoctlSimple[nvgpu.RMAPIVersion], compUtil), + nvgpu.NV_ESC_CHECK_VERSION_STR: feHandler(frontendIoctlSimpleNoStatus[nvgpu.RMAPIVersion], compUtil), nvgpu.NV_ESC_ATTACH_GPUS_TO_FD: feHandler(frontendIoctlBytes, compUtil), // NvU32 array containing GPU IDs - nvgpu.NV_ESC_SYS_PARAMS: feHandler(frontendIoctlSimple[nvgpu.IoctlSysParams], compUtil), + nvgpu.NV_ESC_SYS_PARAMS: feHandler(frontendIoctlSimpleNoStatus[nvgpu.IoctlSysParams], compUtil), nvgpu.NV_ESC_RM_DUP_OBJECT: feHandler(rmDupObject, compUtil), nvgpu.NV_ESC_RM_SHARE: feHandler(frontendIoctlSimple[nvgpu.NVOS57_PARAMETERS], compUtil), nvgpu.NV_ESC_RM_UNMAP_MEMORY: feHandler(frontendIoctlSimple[nvgpu.NVOS34_PARAMETERS], compUtil),