From 15cc3fcbbd77101638854d5b575ea234804d7f7c Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 5 Oct 2023 20:24:30 -0700 Subject: [PATCH] Add RmAllocParamType interface for parameter types of NV_ESC_RM_ALLOC. Refactor codebase to use this interface, instead of manually copying back and forth into various structs. PiperOrigin-RevId: 571208903 --- pkg/abi/nvgpu/BUILD | 2 + pkg/abi/nvgpu/frontend.go | 92 +++++++++++++++++++ pkg/abi/nvgpu/frontend_unsafe.go | 27 ++++++ pkg/sentry/devices/nvproxy/frontend.go | 28 ++---- pkg/sentry/devices/nvproxy/frontend_unsafe.go | 70 ++++++-------- 5 files changed, 156 insertions(+), 63 deletions(-) create mode 100644 pkg/abi/nvgpu/frontend_unsafe.go diff --git a/pkg/abi/nvgpu/BUILD b/pkg/abi/nvgpu/BUILD index c45c20f1b..5b5c0e275 100644 --- a/pkg/abi/nvgpu/BUILD +++ b/pkg/abi/nvgpu/BUILD @@ -10,10 +10,12 @@ go_library( "classes.go", "ctrl.go", "frontend.go", + "frontend_unsafe.go", "nvgpu.go", "status.go", "uvm.go", ], marshal = True, visibility = ["//pkg/sentry:internal"], + deps = ["//pkg/marshal"], ) diff --git a/pkg/abi/nvgpu/frontend.go b/pkg/abi/nvgpu/frontend.go index 1e7e1a763..f0ca1d34b 100644 --- a/pkg/abi/nvgpu/frontend.go +++ b/pkg/abi/nvgpu/frontend.go @@ -14,6 +14,10 @@ package nvgpu +import ( + "gvisor.dev/gvisor/pkg/marshal" +) + // NV_IOCTL_MAGIC is the "canonical" IOC_TYPE for frontend ioctls. // The driver ignores IOC_TYPE, allowing any value to be passed. const NV_IOCTL_MAGIC = uint32('F') @@ -132,6 +136,28 @@ type NVOS00Parameters struct { Status uint32 } +// RmAllocParamType should be implemented by all possible parameter types for +// NV_ESC_RM_ALLOC. +type RmAllocParamType interface { + GetPAllocParms() P64 + GetPRightsRequested() P64 + SetPAllocParms(p P64) + SetPRightsRequested(p P64) + FromOS64(other NVOS64Parameters) + ToOS64() NVOS64Parameters + GetPointer() uintptr + marshal.Marshallable +} + +// GetRmAllocParamObj returns the appropriate implementation of +// RmAllocParamType based on passed parameters. +func GetRmAllocParamObj(isNVOS64 bool) RmAllocParamType { + if isNVOS64 { + return &NVOS64Parameters{} + } + return &NVOS21Parameters{} +} + // NVOS21Parameters is NVOS21_PARAMETERS, one possible parameter type for // NV_ESC_RM_ALLOC. // @@ -146,6 +172,46 @@ type NVOS21Parameters struct { Pad0 [4]byte } +// GetPAllocParms implements RmAllocParamType.GetPAllocParms. +func (n *NVOS21Parameters) GetPAllocParms() P64 { + return n.PAllocParms +} + +// GetPRightsRequested implements RmAllocParamType.GetPRightsRequested. +func (n *NVOS21Parameters) GetPRightsRequested() P64 { + return 0 +} + +// SetPAllocParms implements RmAllocParamType.SetPAllocParms. +func (n *NVOS21Parameters) SetPAllocParms(p P64) { n.PAllocParms = p } + +// SetPRightsRequested implements RmAllocParamType.SetPRightsRequested. +func (n *NVOS21Parameters) SetPRightsRequested(p P64) { + panic("impossible") +} + +// FromOS64 implements RmAllocParamType.FromOS64. +func (n *NVOS21Parameters) FromOS64(other NVOS64Parameters) { + n.HRoot = other.HRoot + n.HObjectParent = other.HObjectParent + n.HObjectNew = other.HObjectNew + n.HClass = other.HClass + n.PAllocParms = other.PAllocParms + n.Status = other.Status +} + +// ToOS64 implements RmAllocParamType.ToOS64. +func (n *NVOS21Parameters) ToOS64() NVOS64Parameters { + return NVOS64Parameters{ + HRoot: n.HRoot, + HObjectParent: n.HObjectParent, + HObjectNew: n.HObjectNew, + HClass: n.HClass, + PAllocParms: n.PAllocParms, + Status: n.Status, + } +} + // NVOS55Parameters is NVOS55_PARAMETERS, the parameter type for // NV_ESC_RM_DUP_OBJECT. // @@ -301,6 +367,32 @@ type NVOS64Parameters struct { Status uint32 } +// GetPAllocParms implements RmAllocParamType.GetPAllocParms. +func (n *NVOS64Parameters) GetPAllocParms() P64 { + return n.PAllocParms +} + +// GetPRightsRequested implements RmAllocParamType.GetPRightsRequested. +func (n *NVOS64Parameters) GetPRightsRequested() P64 { + return n.PRightsRequested +} + +// SetPAllocParms implements RmAllocParamType.SetPAllocParms. +func (n *NVOS64Parameters) SetPAllocParms(p P64) { n.PAllocParms = p } + +// SetPRightsRequested implements RmAllocParamType.SetPRightsRequested. +func (n *NVOS64Parameters) SetPRightsRequested(p P64) { n.PRightsRequested = p } + +// FromOS64 implements RmAllocParamType.FromOS64. +func (n *NVOS64Parameters) FromOS64(other NVOS64Parameters) { + *n = other +} + +// ToOS64 implements RmAllocParamType.ToOS64. +func (n *NVOS64Parameters) ToOS64() NVOS64Parameters { + return *n +} + // Frontend ioctl parameter struct sizes. var ( SizeofIoctlRegisterFD = uint32((*IoctlRegisterFD)(nil).SizeBytes()) diff --git a/pkg/abi/nvgpu/frontend_unsafe.go b/pkg/abi/nvgpu/frontend_unsafe.go new file mode 100644 index 000000000..8918860df --- /dev/null +++ b/pkg/abi/nvgpu/frontend_unsafe.go @@ -0,0 +1,27 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package nvgpu + +import "unsafe" + +// GetPointer implements RmAllocParamType.GetPointer. +func (n *NVOS21Parameters) GetPointer() uintptr { + return uintptr(unsafe.Pointer(n)) +} + +// GetPointer implements RmAllocParamType.GetPointer. +func (n *NVOS64Parameters) GetPointer() uintptr { + return uintptr(unsafe.Pointer(n)) +} diff --git a/pkg/sentry/devices/nvproxy/frontend.go b/pkg/sentry/devices/nvproxy/frontend.go index 07428ab04..b60299dda 100644 --- a/pkg/sentry/devices/nvproxy/frontend.go +++ b/pkg/sentry/devices/nvproxy/frontend.go @@ -605,33 +605,21 @@ func ctrlSubdevFIFODisableChannels(fi *frontendIoctlState, ioctlParams *nvgpu.NV } func rmAlloc(fi *frontendIoctlState) (uintptr, error) { - // Copy in parameters and convert to NVOS64Parameters. - var ( - ioctlParams nvgpu.NVOS64Parameters - isNVOS64 bool - ) + var isNVOS64 bool switch fi.ioctlParamsSize { case nvgpu.SizeofNVOS21Parameters: - var buf nvgpu.NVOS21Parameters - if _, err := buf.CopyIn(fi.t, fi.ioctlParamsAddr); err != nil { - return 0, err - } - ioctlParams = nvgpu.NVOS64Parameters{ - HRoot: buf.HRoot, - HObjectParent: buf.HObjectParent, - HObjectNew: buf.HObjectNew, - HClass: buf.HClass, - PAllocParms: buf.PAllocParms, - Status: buf.Status, - } case nvgpu.SizeofNVOS64Parameters: - if _, err := ioctlParams.CopyIn(fi.t, fi.ioctlParamsAddr); err != nil { - return 0, err - } isNVOS64 = true default: return 0, linuxerr.EINVAL } + // Copy in parameters and convert to NVOS64ParametersR535, which is a super + // set of all parameter types we support. + buf := nvgpu.GetRmAllocParamObj(isNVOS64) + if _, err := buf.CopyIn(fi.t, fi.ioctlParamsAddr); err != nil { + return 0, err + } + ioctlParams := buf.ToOS64() // hClass determines the type of pAllocParms. if log.IsLogging(log.Debug) { diff --git a/pkg/sentry/devices/nvproxy/frontend_unsafe.go b/pkg/sentry/devices/nvproxy/frontend_unsafe.go index 8f0ef51f1..f38724ff6 100644 --- a/pkg/sentry/devices/nvproxy/frontend_unsafe.go +++ b/pkg/sentry/devices/nvproxy/frontend_unsafe.go @@ -32,6 +32,14 @@ func frontendIoctlInvoke[Params any](fi *frontendIoctlState, sentryParams *Param return n, nil } +func frontendIoctlInvokePtr(fi *frontendIoctlState, sentryParams uintptr) (uintptr, error) { + n, _, errno := unix.RawSyscall(unix.SYS_IOCTL, uintptr(fi.fd.hostFD), frontendIoctlCmd(fi.nr, fi.ioctlParamsSize), sentryParams) + if errno != 0 { + return n, errno + } + return n, nil +} + func rmControlInvoke[Params any](fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54Parameters, ctrlParams *Params) (uintptr, error) { defer runtime.KeepAlive(ctrlParams) // since we convert to non-pointer-typed P64 sentryIoctlParams := *ioctlParams @@ -153,55 +161,31 @@ func ctrlSubdevGRGetInfo(fi *frontendIoctlState, ioctlParams *nvgpu.NVOS54Parame func rmAllocInvoke[Params any](fi *frontendIoctlState, ioctlParams *nvgpu.NVOS64Parameters, allocParams *Params, isNVOS64 bool) (uintptr, error) { defer runtime.KeepAlive(allocParams) // since we convert to non-pointer-typed P64 - if isNVOS64 { - sentryIoctlParams := *ioctlParams - sentryIoctlParams.PAllocParms = p64FromPtr(unsafe.Pointer(allocParams)) - var rightsRequested nvgpu.RS_ACCESS_MASK - if ioctlParams.PRightsRequested != 0 { - if _, err := rightsRequested.CopyIn(fi.t, addrFromP64(ioctlParams.PRightsRequested)); err != nil { - return 0, err - } - sentryIoctlParams.PRightsRequested = p64FromPtr(unsafe.Pointer(&rightsRequested)) + sentryIoctlParams := nvgpu.GetRmAllocParamObj(isNVOS64) + sentryIoctlParams.FromOS64(*ioctlParams) + sentryIoctlParams.SetPAllocParms(p64FromPtr(unsafe.Pointer(allocParams))) + var rightsRequested nvgpu.RS_ACCESS_MASK + if ioctlParams.PRightsRequested != 0 { + if _, err := rightsRequested.CopyIn(fi.t, addrFromP64(ioctlParams.PRightsRequested)); err != nil { + return 0, err } - n, err := frontendIoctlInvoke(fi, &sentryIoctlParams) - if err != nil { - return n, err - } - if ioctlParams.PRightsRequested != 0 { - if _, err := rightsRequested.CopyOut(fi.t, addrFromP64(ioctlParams.PRightsRequested)); err != nil { - return n, err - } - } - outIoctlParams := sentryIoctlParams - outIoctlParams.PAllocParms = ioctlParams.PAllocParms - outIoctlParams.PRightsRequested = ioctlParams.PRightsRequested - if _, err := outIoctlParams.CopyOut(fi.t, fi.ioctlParamsAddr); err != nil { - return n, err - } - return n, nil + sentryIoctlParams.SetPRightsRequested(p64FromPtr(unsafe.Pointer(&rightsRequested))) } - - sentryIoctlParams := nvgpu.NVOS21Parameters{ - HRoot: ioctlParams.HRoot, - HObjectParent: ioctlParams.HObjectParent, - HObjectNew: ioctlParams.HObjectNew, - HClass: ioctlParams.HClass, - PAllocParms: p64FromPtr(unsafe.Pointer(allocParams)), - Status: ioctlParams.Status, - } - n, err := frontendIoctlInvoke(fi, &sentryIoctlParams) + n, err := frontendIoctlInvokePtr(fi, sentryIoctlParams.GetPointer()) if err != nil { return n, err } - outIoctlParams := nvgpu.NVOS21Parameters{ - HRoot: sentryIoctlParams.HRoot, - HObjectParent: sentryIoctlParams.HObjectParent, - HObjectNew: sentryIoctlParams.HObjectNew, - HClass: sentryIoctlParams.HClass, - PAllocParms: ioctlParams.PAllocParms, - Status: sentryIoctlParams.Status, + if ioctlParams.PRightsRequested != 0 { + if _, err := rightsRequested.CopyOut(fi.t, addrFromP64(ioctlParams.PRightsRequested)); err != nil { + return n, err + } } - if _, err := outIoctlParams.CopyOut(fi.t, fi.ioctlParamsAddr); err != nil { + // Reuse sentryIoctlParams to write out params. + sentryIoctlParams.SetPAllocParms(ioctlParams.PAllocParms) + if ioctlParams.PRightsRequested != 0 { + sentryIoctlParams.SetPRightsRequested(ioctlParams.PRightsRequested) + } + if _, err := sentryIoctlParams.CopyOut(fi.t, fi.ioctlParamsAddr); err != nil { return n, err } return n, nil