From 1ca2bd4a69870c0f4683ca6d39676c24c6bb9416 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Tue, 17 Dec 2024 17:22:53 -0800 Subject: [PATCH] nvproxy: Improve debuggability by logging failures coming from the host driver. Most ioctl contain a `Status` field (which in driver code is of type NvStatus) which indicates if the ioctl command succeeded or not. Non NV_OK=0 values indicate some kind of failure. In case of failure, the ioctl(2) syscall still succeeds. So the failure is currently not visible in gVisor strace/debug logs. This helps flag instances where the host invocation of ioctl(2) resulted in a failure in the driver. This also helps avoid the usage of frontendIoctlSimple() in some cases. frontendIoctlSimple() heap allocates a byte buffer. It is better to copy into the ioctl params on the stack when possible. PiperOrigin-RevId: 707312482 --- pkg/abi/nvgpu/frontend.go | 96 +++++++++++ pkg/abi/nvgpu/nvgpu.go | 5 + pkg/abi/nvgpu/uvm.go | 159 +++++++++++++++++- pkg/sentry/devices/nvproxy/frontend.go | 34 +++- pkg/sentry/devices/nvproxy/frontend_unsafe.go | 27 ++- pkg/sentry/devices/nvproxy/nvproxy.go | 11 ++ pkg/sentry/devices/nvproxy/uvm.go | 15 +- pkg/sentry/devices/nvproxy/uvm_unsafe.go | 7 +- pkg/sentry/devices/nvproxy/version.go | 24 +-- 9 files changed, 350 insertions(+), 28 deletions(-) diff --git a/pkg/abi/nvgpu/frontend.go b/pkg/abi/nvgpu/frontend.go index 4f0164ffa..677e79a5f 100644 --- a/pkg/abi/nvgpu/frontend.go +++ b/pkg/abi/nvgpu/frontend.go @@ -62,6 +62,15 @@ type IoctlRegisterFD struct { CtlFD int32 `nvproxy:"nv_ioctl_register_fd_t"` } +// 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 @@ -82,6 +91,11 @@ func (p *IoctlAllocOSEvent) SetFrontendFD(fd int32) { p.FD = uint32(fd) } +// GetStatus implements HasStatus.GetStatus. +func (p *IoctlAllocOSEvent) GetStatus() uint32 { + return p.Status +} + // IoctlFreeOSEvent is the parameter type for NV_ESC_FREE_OS_EVENT. // // +marshal @@ -102,6 +116,11 @@ func (p *IoctlFreeOSEvent) SetFrontendFD(fd int32) { p.FD = uint32(fd) } +// GetStatus implements HasStatus.GetStatus. +func (p *IoctlFreeOSEvent) GetStatus() uint32 { + return p.Status +} + // RMAPIVersion is the parameter type for NV_ESC_CHECK_VERSION_STR. // // +marshal @@ -111,6 +130,14 @@ 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 @@ -118,6 +145,14 @@ type IoctlSysParams struct { MemblockSize uint64 `nvproxy:"nv_ioctl_sys_params_t"` } +// 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 @@ -126,6 +161,11 @@ type IoctlWaitOpenComplete struct { AdapterStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *IoctlWaitOpenComplete) GetStatus() uint32 { + return p.AdapterStatus +} + // IoctlNVOS02ParametersWithFD is the parameter type for NV_ESC_RM_ALLOC_MEMORY. // // +marshal @@ -135,6 +175,11 @@ type IoctlNVOS02ParametersWithFD struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *IoctlNVOS02ParametersWithFD) GetStatus() uint32 { + return p.Params.Status +} + // +marshal type NVOS02Parameters struct { HRoot Handle `nvproxy:"NVOS02_PARAMETERS"` @@ -159,6 +204,11 @@ type NVOS00Parameters struct { Status uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *NVOS00Parameters) GetStatus() uint32 { + return p.Status +} + // RmAllocParamType should be implemented by all possible parameter types for // NV_ESC_RM_ALLOC. type RmAllocParamType interface { @@ -170,6 +220,7 @@ type RmAllocParamType interface { FromOS64(other NVOS64Parameters) ToOS64() NVOS64Parameters GetPointer() uintptr + HasStatus marshal.Marshallable } @@ -242,6 +293,11 @@ func (n *NVOS21Parameters) ToOS64() NVOS64Parameters { } } +// GetStatus implements RmAllocParamType.GetStatus. +func (n *NVOS21Parameters) GetStatus() uint32 { + return n.Status +} + // NVOS55Parameters is the parameter type for NV_ESC_RM_DUP_OBJECT. // // +marshal @@ -255,6 +311,11 @@ type NVOS55Parameters struct { Status uint32 } +// GetStatus implements HasStatus.GetStatus. +func (n *NVOS55Parameters) GetStatus() uint32 { + return n.Status +} + // NVOS57Parameters is the parameter type for NV_ESC_RM_SHARE. // // +marshal @@ -265,6 +326,11 @@ type NVOS57Parameters struct { Status uint32 } +// GetStatus implements HasStatus.GetStatus. +func (n *NVOS57Parameters) GetStatus() uint32 { + return n.Status +} + // NVOS32Parameters is the parameter type for NV_ESC_RM_VID_HEAP_CONTROL. // // +marshal @@ -281,6 +347,11 @@ type NVOS32Parameters struct { Data [144]byte // union } +// GetStatus implements HasStatus.GetStatus. +func (n *NVOS32Parameters) GetStatus() uint32 { + return n.Status +} + // Possible values for NVOS32Parameters.Function: const ( NVOS32_FUNCTION_ALLOC_SIZE = 2 @@ -342,6 +413,11 @@ type IoctlNVOS33ParametersWithFD struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *IoctlNVOS33ParametersWithFD) GetStatus() uint32 { + return p.Params.Status +} + // +marshal type NVOS33Parameters struct { HClient Handle `nvproxy:"NVOS33_PARAMETERS"` @@ -368,6 +444,11 @@ type NVOS34Parameters struct { Flags uint32 } +// GetStatus implements HasStatus.GetStatus. +func (n *NVOS34Parameters) GetStatus() uint32 { + return n.Status +} + // NVOS54Parameters is the parameter type for NV_ESC_RM_CONTROL. // // +marshal @@ -381,6 +462,11 @@ type NVOS54Parameters struct { Status uint32 } +// GetStatus implements HasStatus.GetStatus. +func (n *NVOS54Parameters) GetStatus() uint32 { + return n.Status +} + // NVOS56Parameters is the parameter type for NV_ESC_RM_UPDATE_DEVICE_MAPPING_INFO. // // +marshal @@ -395,6 +481,11 @@ type NVOS56Parameters struct { Pad1 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (n *NVOS56Parameters) GetStatus() uint32 { + return n.Status +} + // NVOS64Parameters is one possible parameter type for NV_ESC_RM_ALLOC. // // +marshal @@ -439,6 +530,11 @@ func (n *NVOS64Parameters) FromOS64(other NVOS64Parameters) { *n = other } // ToOS64 implements RmAllocParamType.ToOS64. func (n *NVOS64Parameters) ToOS64() NVOS64Parameters { return *n } +// GetStatus implements RmAllocParamType.GetStatus. +func (n *NVOS64Parameters) GetStatus() uint32 { + return n.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 c1c1f87b7..d7310b098 100644 --- a/pkg/abi/nvgpu/nvgpu.go +++ b/pkg/abi/nvgpu/nvgpu.go @@ -87,3 +87,8 @@ type RS_SHARE_POLICY struct { // // +marshal type NvUUID [16]uint8 + +// HasStatus is an interface for parameter structs that have a Status field. +type HasStatus interface { + GetStatus() uint32 +} diff --git a/pkg/abi/nvgpu/uvm.go b/pkg/abi/nvgpu/uvm.go index a2fe7d78a..37d60d0aa 100644 --- a/pkg/abi/nvgpu/uvm.go +++ b/pkg/abi/nvgpu/uvm.go @@ -58,6 +58,11 @@ type UVM_INITIALIZE_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_INITIALIZE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // UVM_INITIALIZE_PARAMS flags, from kernel-open/nvidia-uvm/uvm_types.h. const ( UVM_INIT_FLAGS_MULTI_PROCESS_SHARING_MODE = 0x2 @@ -70,6 +75,11 @@ type UVM_CREATE_RANGE_GROUP_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_CREATE_RANGE_GROUP_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_DESTROY_RANGE_GROUP_PARAMS struct { RangeGroupID uint64 `nvproxy:"same"` @@ -77,6 +87,11 @@ type UVM_DESTROY_RANGE_GROUP_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_DESTROY_RANGE_GROUP_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_REGISTER_GPU_VASPACE_PARAMS struct { GPUUUID NvUUID `nvproxy:"same"` @@ -96,12 +111,22 @@ func (p *UVM_REGISTER_GPU_VASPACE_PARAMS) SetFrontendFD(fd int32) { p.RMCtrlFD = fd } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_REGISTER_GPU_VASPACE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_UNREGISTER_GPU_VASPACE_PARAMS struct { GPUUUID NvUUID `nvproxy:"same"` RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_UNREGISTER_GPU_VASPACE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_REGISTER_CHANNEL_PARAMS struct { GPUUUID NvUUID `nvproxy:"same"` @@ -125,6 +150,11 @@ func (p *UVM_REGISTER_CHANNEL_PARAMS) SetFrontendFD(fd int32) { p.RMCtrlFD = fd } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_REGISTER_CHANNEL_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_UNREGISTER_CHANNEL_PARAMS struct { GPUUUID NvUUID `nvproxy:"same"` @@ -133,6 +163,11 @@ type UVM_UNREGISTER_CHANNEL_PARAMS struct { RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_UNREGISTER_CHANNEL_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_ENABLE_PEER_ACCESS_PARAMS struct { GPUUUIDA NvUUID `nvproxy:"same"` @@ -140,6 +175,11 @@ type UVM_ENABLE_PEER_ACCESS_PARAMS struct { RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_ENABLE_PEER_ACCESS_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_DISABLE_PEER_ACCESS_PARAMS struct { GPUUUIDA NvUUID `nvproxy:"same"` @@ -147,6 +187,11 @@ type UVM_DISABLE_PEER_ACCESS_PARAMS struct { RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_DISABLE_PEER_ACCESS_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_SET_RANGE_GROUP_PARAMS struct { RangeGroupID uint64 `nvproxy:"same"` @@ -156,6 +201,11 @@ type UVM_SET_RANGE_GROUP_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_SET_RANGE_GROUP_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_MAP_EXTERNAL_ALLOCATION_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -179,6 +229,11 @@ func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS) SetFrontendFD(fd int32) { p.RMCtrlFD = fd } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_MAP_EXTERNAL_ALLOCATION_PARAMS_V550 struct { Base uint64 `nvproxy:"UVM_MAP_EXTERNAL_ALLOCATION_PARAMS"` @@ -202,6 +257,11 @@ func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS_V550) SetFrontendFD(fd int32) { p.RMCtrlFD = fd } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MAP_EXTERNAL_ALLOCATION_PARAMS_V550) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_FREE_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -210,6 +270,11 @@ type UVM_FREE_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_FREE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_REGISTER_GPU_PARAMS struct { GPUUUID NvUUID `nvproxy:"same"` @@ -232,12 +297,22 @@ func (p *UVM_REGISTER_GPU_PARAMS) SetFrontendFD(fd int32) { p.RMCtrlFD = fd } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_REGISTER_GPU_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_UNREGISTER_GPU_PARAMS struct { GPUUUID NvUUID `nvproxy:"same"` RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_UNREGISTER_GPU_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_PAGEABLE_MEM_ACCESS_PARAMS struct { PageableMemAccess uint8 `nvproxy:"same"` @@ -245,6 +320,11 @@ type UVM_PAGEABLE_MEM_ACCESS_PARAMS struct { RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_PAGEABLE_MEM_ACCESS_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_SET_PREFERRED_LOCATION_PARAMS struct { RequestedBase uint64 `nvproxy:"same"` @@ -254,6 +334,11 @@ type UVM_SET_PREFERRED_LOCATION_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_SET_PREFERRED_LOCATION_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_SET_PREFERRED_LOCATION_PARAMS_V550 struct { RequestedBase uint64 `nvproxy:"UVM_SET_PREFERRED_LOCATION_PARAMS"` @@ -263,6 +348,11 @@ type UVM_SET_PREFERRED_LOCATION_PARAMS_V550 struct { RMStatus uint32 } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_SET_PREFERRED_LOCATION_PARAMS_V550) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_UNSET_PREFERRED_LOCATION_PARAMS struct { RequestedBase uint64 `nvproxy:"same"` @@ -271,6 +361,11 @@ type UVM_UNSET_PREFERRED_LOCATION_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_UNSET_PREFERRED_LOCATION_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_DISABLE_READ_DUPLICATION_PARAMS struct { RequestedBase uint64 `nvproxy:"same"` @@ -279,6 +374,11 @@ type UVM_DISABLE_READ_DUPLICATION_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_DISABLE_READ_DUPLICATION_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_UNSET_ACCESSED_BY_PARAMS struct { RequestedBase uint64 `nvproxy:"same"` @@ -288,6 +388,11 @@ type UVM_UNSET_ACCESSED_BY_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_UNSET_ACCESSED_BY_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_MIGRATE_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -304,6 +409,11 @@ type UVM_MIGRATE_PARAMS struct { _ [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MIGRATE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // UVM_MIGRATE_PARAMS_V550 is the updated version of // UVM_MIGRATE_PARAMS since 550.40.07. // @@ -323,6 +433,11 @@ type UVM_MIGRATE_PARAMS_V550 struct { _ [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MIGRATE_PARAMS_V550) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_MIGRATE_RANGE_GROUP_PARAMS struct { RangeGroupID uint64 `nvproxy:"same"` @@ -331,6 +446,11 @@ type UVM_MIGRATE_RANGE_GROUP_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MIGRATE_RANGE_GROUP_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_TOOLS_READ_PROCESS_MEMORY_PARAMS struct { Buffer uint64 `nvproxy:"same"` @@ -360,6 +480,11 @@ type UVM_MAP_DYNAMIC_PARALLELISM_REGION_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MAP_DYNAMIC_PARALLELISM_REGION_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_UNMAP_EXTERNAL_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -369,6 +494,11 @@ type UVM_UNMAP_EXTERNAL_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_UNMAP_EXTERNAL_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_ALLOC_SEMAPHORE_POOL_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -379,6 +509,11 @@ type UVM_ALLOC_SEMAPHORE_POOL_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_ALLOC_SEMAPHORE_POOL_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_ALLOC_SEMAPHORE_POOL_PARAMS_V550 struct { Base uint64 `nvproxy:"UVM_ALLOC_SEMAPHORE_POOL_PARAMS"` @@ -389,6 +524,11 @@ type UVM_ALLOC_SEMAPHORE_POOL_PARAMS_V550 struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_ALLOC_SEMAPHORE_POOL_PARAMS_V550) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_VALIDATE_VA_RANGE_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -397,6 +537,11 @@ type UVM_VALIDATE_VA_RANGE_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_VALIDATE_VA_RANGE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_CREATE_EXTERNAL_RANGE_PARAMS struct { Base uint64 `nvproxy:"same"` @@ -405,10 +550,20 @@ type UVM_CREATE_EXTERNAL_RANGE_PARAMS struct { Pad0 [4]byte } +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_CREATE_EXTERNAL_RANGE_PARAMS) GetStatus() uint32 { + return p.RMStatus +} + // +marshal type UVM_MM_INITIALIZE_PARAMS struct { - UvmFD int32 `nvproxy:"same"` - Status uint32 + UvmFD int32 `nvproxy:"same"` + RMStatus uint32 +} + +// GetStatus implements HasStatus.GetStatus. +func (p *UVM_MM_INITIALIZE_PARAMS) GetStatus() uint32 { + return p.RMStatus } // From kernel-open/nvidia-uvm/uvm_types.h: diff --git a/pkg/sentry/devices/nvproxy/frontend.go b/pkg/sentry/devices/nvproxy/frontend.go index 82926674a..1f8e53c21 100644 --- a/pkg/sentry/devices/nvproxy/frontend.go +++ b/pkg/sentry/devices/nvproxy/frontend.go @@ -290,16 +290,39 @@ 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. -func frontendIoctlSimple(fi *frontendIoctlState) (uintptr, error) { +func frontendIoctlSimple[Params any, PtrParams hasStatusPtr[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 := frontendIoctlInvoke(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 ioctls whose +// parameters don't contain any NvStatus field either. 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 frontendIoctlInvoke[byte](fi, nil) + return frontendIoctlBytesInvoke(fi, nil) } ioctlParams := make([]byte, fi.ioctlParamsSize) if _, err := fi.t.CopyInBytes(fi.ioctlParamsAddr, ioctlParams); err != nil { return 0, err } - n, err := frontendIoctlInvoke(fi, &ioctlParams[0]) + n, err := frontendIoctlBytesInvoke(fi, &ioctlParams[0]) if err != nil { return n, err } @@ -339,7 +362,7 @@ func frontendRegisterFD(fi *frontendIoctlState) (uintptr, error) { return frontendIoctlInvoke(fi, &ioctlParams) } -func frontendIoctHasFD[Params any, PtrParams hasFrontendFDPtr[Params]](fi *frontendIoctlState) (uintptr, error) { +func frontendIoctlHasFD[Params any, PtrParams hasFrontendFDAndStatusPtr[Params]](fi *frontendIoctlState) (uintptr, error) { var ioctlParamsValue Params ioctlParams := PtrParams(&ioctlParamsValue) if int(fi.ioctlParamsSize) != ioctlParams.SizeBytes() { @@ -400,6 +423,9 @@ func rmAllocOSDescriptor(fi *frontendIoctlState, ioctlParams *nvgpu.IoctlNVOS02P // 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 diff --git a/pkg/sentry/devices/nvproxy/frontend_unsafe.go b/pkg/sentry/devices/nvproxy/frontend_unsafe.go index f7957b0a4..5ff4ec006 100644 --- a/pkg/sentry/devices/nvproxy/frontend_unsafe.go +++ b/pkg/sentry/devices/nvproxy/frontend_unsafe.go @@ -21,10 +21,24 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/nvgpu" "gvisor.dev/gvisor/pkg/errors/linuxerr" + "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/marshal/primitive" ) -func frontendIoctlInvoke[Params any](fi *frontendIoctlState, sentryParams *Params) (uintptr, error) { +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) { + if status := ioctlParams.GetStatus(); status != nvgpu.NV_OK { + fi.ctx.Debugf("nvproxy: frontend ioctl failed: status=%#x", status) + } + } + return n, nil +} + +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))) if errno != 0 { return n, errno @@ -285,16 +299,19 @@ func rmAllocInvoke[Params any](fi *frontendIoctlState, ioctlParams *nvgpu.NVOS64 // identically to the equivalent NVOS21Parameters; compare // src/nvidia/src/kernel/rmapi/entry_points.c:_nv04AllocWithSecInfo() and // _nv04AllocWithAccessSecInfo(). + origParamsSize := fi.ioctlParamsSize + fi.ioctlParamsSize = nvgpu.SizeofNVOS64Parameters fi.fd.dev.nvp.objsLock() - n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(fi.fd.hostFD), frontendIoctlCmd(nvgpu.NV_ESC_RM_ALLOC, nvgpu.SizeofNVOS64Parameters), uintptr(unsafe.Pointer(ioctlParams))) - if errno == 0 && ioctlParams.Status == nvgpu.NV_OK { + n, err := frontendIoctlInvoke(fi, ioctlParams) + fi.ioctlParamsSize = origParamsSize + if err == nil && ioctlParams.Status == nvgpu.NV_OK { addObjLocked(fi, ioctlParams, rightsRequested, allocParams) } fi.fd.dev.nvp.objsUnlock() ioctlParams.PAllocParms = origPAllocParms ioctlParams.PRightsRequested = origPRightsRequested - if errno != 0 { - return n, errno + if err != nil { + return n, err } // Copy updated params out to the application. diff --git a/pkg/sentry/devices/nvproxy/nvproxy.go b/pkg/sentry/devices/nvproxy/nvproxy.go index 3cf12fd6e..d2e00f653 100644 --- a/pkg/sentry/devices/nvproxy/nvproxy.go +++ b/pkg/sentry/devices/nvproxy/nvproxy.go @@ -107,6 +107,17 @@ type hasFrontendFDPtr[T any] interface { nvgpu.HasFrontendFD } +type hasStatusPtr[T any] interface { + marshalPtr[T] + nvgpu.HasStatus +} + +type hasFrontendFDAndStatusPtr[T any] interface { + marshalPtr[T] + nvgpu.HasFrontendFD + nvgpu.HasStatus +} + type hasCtrlInfoListPtr[T any] interface { marshalPtr[T] nvgpu.HasCtrlInfoList diff --git a/pkg/sentry/devices/nvproxy/uvm.go b/pkg/sentry/devices/nvproxy/uvm.go index 8f5beabc3..2871672a6 100644 --- a/pkg/sentry/devices/nvproxy/uvm.go +++ b/pkg/sentry/devices/nvproxy/uvm.go @@ -166,10 +166,14 @@ type uvmIoctlState struct { } func uvmIoctlNoParams(ui *uvmIoctlState) (uintptr, error) { - return uvmIoctlInvoke[byte](ui, nil) + n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(ui.fd.hostFD), uintptr(ui.cmd), 0 /* params */) + if errno != 0 { + return n, errno + } + return n, nil } -func uvmIoctlSimple[Params any, PtrParams marshalPtr[Params]](ui *uvmIoctlState) (uintptr, error) { +func uvmIoctlSimple[Params any, PtrParams hasStatusPtr[Params]](ui *uvmIoctlState) (uintptr, error) { var ioctlParamsValue Params ioctlParams := PtrParams(&ioctlParamsValue) if _, err := ioctlParams.CopyIn(ui.t, ui.ioctlParamsAddr); err != nil { @@ -213,8 +217,11 @@ func uvmMMInitialize(ui *uvmIoctlState) (uintptr, error) { } 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.Status = status + outIoctlParams.RMStatus = status _, err := outIoctlParams.CopyOut(ui.t, ui.ioctlParamsAddr) return err } @@ -242,7 +249,7 @@ func uvmMMInitialize(ui *uvmIoctlState) (uintptr, error) { return n, nil } -func uvmIoctlHasFrontendFD[Params any, PtrParams hasFrontendFDPtr[Params]](ui *uvmIoctlState) (uintptr, error) { +func uvmIoctlHasFrontendFD[Params any, PtrParams hasFrontendFDAndStatusPtr[Params]](ui *uvmIoctlState) (uintptr, error) { var ioctlParamsValue Params ioctlParams := PtrParams(&ioctlParamsValue) if _, err := ioctlParams.CopyIn(ui.t, ui.ioctlParamsAddr); err != nil { diff --git a/pkg/sentry/devices/nvproxy/uvm_unsafe.go b/pkg/sentry/devices/nvproxy/uvm_unsafe.go index 9cc0e4494..dea8b17d6 100644 --- a/pkg/sentry/devices/nvproxy/uvm_unsafe.go +++ b/pkg/sentry/devices/nvproxy/uvm_unsafe.go @@ -24,11 +24,16 @@ import ( "gvisor.dev/gvisor/pkg/log" ) -func uvmIoctlInvoke[Params any](ui *uvmIoctlState, ioctlParams *Params) (uintptr, error) { +func uvmIoctlInvoke[Params any, PtrParams hasStatusPtr[Params]](ui *uvmIoctlState, ioctlParams PtrParams) (uintptr, error) { n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(ui.fd.hostFD), uintptr(ui.cmd), uintptr(unsafe.Pointer(ioctlParams))) if errno != 0 { return n, errno } + if log.IsLogging(log.Debug) { + if status := ioctlParams.GetStatus(); status != nvgpu.NV_OK { + ui.ctx.Debugf("nvproxy: uvm ioctl failed: status=%#x", status) + } + } return n, nil } diff --git a/pkg/sentry/devices/nvproxy/version.go b/pkg/sentry/devices/nvproxy/version.go index 85bbed2cf..3dbca6a43 100644 --- a/pkg/sentry/devices/nvproxy/version.go +++ b/pkg/sentry/devices/nvproxy/version.go @@ -176,17 +176,17 @@ func Init() { // constructed with the entirety of the nvproxy functionality. return &driverABI{ frontendIoctl: map[uint32]frontendIoctlHandler{ - nvgpu.NV_ESC_CARD_INFO: feHandler(frontendIoctlSimple, compUtil), // nv_ioctl_card_info_t array - nvgpu.NV_ESC_CHECK_VERSION_STR: feHandler(frontendIoctlSimple, compUtil), // nv_rm_api_version_t - nvgpu.NV_ESC_ATTACH_GPUS_TO_FD: feHandler(frontendIoctlSimple, compUtil), // NvU32 array containing GPU IDs - nvgpu.NV_ESC_SYS_PARAMS: feHandler(frontendIoctlSimple, compUtil), // nv_ioctl_sys_params_t - nvgpu.NV_ESC_RM_DUP_OBJECT: feHandler(rmDupObject, compUtil), // NVOS55_PARAMETERS - nvgpu.NV_ESC_RM_SHARE: feHandler(frontendIoctlSimple, compUtil), // NVOS57_PARAMETERS - nvgpu.NV_ESC_RM_UNMAP_MEMORY: feHandler(frontendIoctlSimple, compUtil), // NVOS34_PARAMETERS - nvgpu.NV_ESC_RM_UPDATE_DEVICE_MAPPING_INFO: feHandler(frontendIoctlSimple, compUtil), // NVOS56_PARAMETERS + 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_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_RM_DUP_OBJECT: feHandler(rmDupObject, compUtil), + nvgpu.NV_ESC_RM_SHARE: feHandler(frontendIoctlSimple[nvgpu.NVOS57Parameters], compUtil), + nvgpu.NV_ESC_RM_UNMAP_MEMORY: feHandler(frontendIoctlSimple[nvgpu.NVOS34Parameters], compUtil), + nvgpu.NV_ESC_RM_UPDATE_DEVICE_MAPPING_INFO: feHandler(frontendIoctlSimple[nvgpu.NVOS56Parameters], compUtil), nvgpu.NV_ESC_REGISTER_FD: feHandler(frontendRegisterFD, compUtil), - nvgpu.NV_ESC_ALLOC_OS_EVENT: feHandler(frontendIoctHasFD[nvgpu.IoctlAllocOSEvent], compUtil), - nvgpu.NV_ESC_FREE_OS_EVENT: feHandler(frontendIoctHasFD[nvgpu.IoctlFreeOSEvent], compUtil), + nvgpu.NV_ESC_ALLOC_OS_EVENT: feHandler(frontendIoctlHasFD[nvgpu.IoctlAllocOSEvent], compUtil), + nvgpu.NV_ESC_FREE_OS_EVENT: feHandler(frontendIoctlHasFD[nvgpu.IoctlFreeOSEvent], compUtil), nvgpu.NV_ESC_NUMA_INFO: feHandler(rmNumaInfo, compUtil), nvgpu.NV_ESC_RM_ALLOC_MEMORY: feHandler(rmAllocMemory, compUtil), nvgpu.NV_ESC_RM_FREE: feHandler(rmFree, compUtil), @@ -615,7 +615,7 @@ func Init() { // 550.40.07 is an intermediate unqualified version from the main branch. v550_40_07 := func() *driverABI { abi := v545_23_06() - abi.frontendIoctl[nvgpu.NV_ESC_WAIT_OPEN_COMPLETE] = feHandler(frontendIoctlSimple, compUtil) // nv_ioctl_wait_open_complete_t + abi.frontendIoctl[nvgpu.NV_ESC_WAIT_OPEN_COMPLETE] = feHandler(frontendIoctlSimple[nvgpu.IoctlWaitOpenComplete], compUtil) abi.controlCmd[nvgpu.NV0000_CTRL_CMD_GPU_ASYNC_ATTACH_ID] = ctrlHandler(rmControlSimple, compUtil) abi.controlCmd[nvgpu.NV0000_CTRL_CMD_GPU_WAIT_ATTACH_ID] = ctrlHandler(rmControlSimple, compUtil) abi.controlCmd[nvgpu.NV0080_CTRL_CMD_PERF_CUDA_LIMIT_SET_CONTROL] = ctrlHandler(rmControlSimple, compUtil) // NV0080_CTRL_PERF_CUDA_LIMIT_CONTROL_PARAMS @@ -630,7 +630,7 @@ func Init() { prevNames := abi.getStructNames abi.getStructNames = func() *driverStructNames { names := prevNames() - names.frontendNames[nvgpu.NV_ESC_WAIT_OPEN_COMPLETE] = simpleIoctl("nv_ioctl_wait_open_complete_t") + names.frontendNames[nvgpu.NV_ESC_WAIT_OPEN_COMPLETE] = getStructName(nvgpu.IoctlWaitOpenComplete{}) names.controlNames[nvgpu.NV0000_CTRL_CMD_GPU_ASYNC_ATTACH_ID] = simpleIoctl("NV0000_CTRL_GPU_ASYNC_ATTACH_ID_PARAMS") names.controlNames[nvgpu.NV0000_CTRL_CMD_GPU_WAIT_ATTACH_ID] = simpleIoctl("NV0000_CTRL_GPU_WAIT_ATTACH_ID_PARAMS") names.controlNames[nvgpu.NV0080_CTRL_CMD_PERF_CUDA_LIMIT_SET_CONTROL] = simpleIoctl("NV0080_CTRL_PERF_CUDA_LIMIT_CONTROL_PARAMS")