diff --git a/pkg/sentry/devices/nvproxy/BUILD b/pkg/sentry/devices/nvproxy/BUILD index 9ef77baf2..c1841e256 100644 --- a/pkg/sentry/devices/nvproxy/BUILD +++ b/pkg/sentry/devices/nvproxy/BUILD @@ -96,6 +96,7 @@ go_test( data = ["//tools/nvidia_driver_differ:driver_ast_parser"], deps = [ ":nvproxy", + "//pkg/abi/nvgpu", "//pkg/test/testutil", "//tools/nvidia_driver_differ/parser", ], diff --git a/pkg/sentry/devices/nvproxy/nvproxy_driver_parity_test.go b/pkg/sentry/devices/nvproxy/nvproxy_driver_parity_test.go index d3d8ffd1e..9203349d1 100644 --- a/pkg/sentry/devices/nvproxy/nvproxy_driver_parity_test.go +++ b/pkg/sentry/devices/nvproxy/nvproxy_driver_parity_test.go @@ -20,18 +20,24 @@ package nvproxy_driver_parity_test import ( + "fmt" "os" + "reflect" + "regexp" + "strconv" + "strings" "testing" + "gvisor.dev/gvisor/pkg/abi/nvgpu" "gvisor.dev/gvisor/pkg/test/testutil" "gvisor.dev/gvisor/pkg/sentry/devices/nvproxy" "gvisor.dev/gvisor/tools/nvidia_driver_differ/parser" ) -// TestSupportedStructNames tests that all the structs listed in nvproxy are found in the driver -// source code. -func TestSupportedStructNames(t *testing.T) { +func createParserRunner(t *testing.T) (*os.File, *parser.Runner) { + t.Helper() + // Find the parser binary parserPath, err := testutil.FindFile("tools/nvidia_driver_differ/driver_ast_parser") if err != nil { @@ -41,36 +47,48 @@ func TestSupportedStructNames(t *testing.T) { if err != nil { t.Fatalf("Failed to open driver_ast_parser: %v", err) } - defer func() { - if err := parserFile.Close(); err != nil { - t.Fatalf("Failed to close driver_ast_parser: %v", err) - } - }() runner, err := parser.NewRunner((*parser.ParserFile)(parserFile)) if err != nil { t.Fatalf("Failed to create parser runner: %v", err) } + return parserFile, runner +} + +func getDriverDefs(t *testing.T, runner *parser.Runner, version nvproxy.DriverVersion) ([]nvproxy.DriverStructName, *parser.OutputJSON) { + t.Helper() + + structNames, ok := nvproxy.SupportedStructNames(version) + if !ok { + t.Fatalf("failed to get struct names for driver %q", version.String()) + } + + // Create structs file for parser + if err := runner.CreateStructsFile(structNames); err != nil { + t.Fatalf("failed to create temporary structs list: %v", err) + } + + // Run parser + defs, err := runner.ParseDriver(version) + if err != nil { + t.Fatalf("failed to run driver_ast_parser: %v", err) + } + + return structNames, defs +} + +// TestSupportedStructNames tests that all the structs listed in nvproxy are found in the driver +// source code. +func TestSupportedStructNames(t *testing.T) { + f, runner := createParserRunner(t) + defer f.Close() nvproxy.Init() + // Run the parser on all supported driver versions nvproxy.ForEachSupportDriver(func(version nvproxy.DriverVersion, checksum string) { t.Run(version.String(), func(t *testing.T) { - structNames, ok := nvproxy.SupportedStructNames(version) - if !ok { - t.Fatalf("failed to get struct names for driver %q", version.String()) - } - - // Create structs file for parser - if err := runner.CreateStructsFile(structNames); err != nil { - t.Fatalf("failed to create temporary structs list: %v", err) - } - - // Run parser - defs, err := runner.ParseDriver(version) - if err != nil { - t.Fatalf("failed to run driver_ast_parser: %v", err) - } + structNames, defs := getDriverDefs(t, runner, version) // Check that every struct is found in the parser output. for _, name := range structNames { @@ -83,3 +101,332 @@ func TestSupportedStructNames(t *testing.T) { }) }) } + +func TestStructDefinitionParity(t *testing.T) { + f, runner := createParserRunner(t) + defer f.Close() + nvproxy.Init() + + nvproxy.ForEachSupportDriver(func(version nvproxy.DriverVersion, checksum string) { + t.Run(version.String(), func(t *testing.T) { + _, defs := getDriverDefs(t, runner, version) + + nvproxyDefs, ok := nvproxy.SupportedStructTypes(version) + if !ok { + t.Fatalf("failed to get struct instances for driver %q", version.String()) + } + + for _, nvproxyDef := range nvproxyDefs { + // Check if the nvproxy definition has disallowed types. + if nvproxyDef.Type != nil { + fields := flattenNvproxyStruct(t, nvproxyDef.Type) + for _, field := range fields { + if _, ok := typeAllowlist[field.Type.Kind()]; !ok { + t.Errorf("struct %q has disallowed type %q in nvproxy", nvproxyDef.Name, field.Type.Name()) + } + } + } + + // Compare the nvproxy definition to the parser output. + name := nvproxyDef.Name + _, isRecord := defs.Records[name] + aliasDef, isAlias := defs.Aliases[name] + if !isRecord && !isAlias { + t.Errorf("struct %q not found in parser output for version %q", name, version.String()) + continue + } + + switch { + case isRecord && nvproxyDef.Type == nil: + checkSimpleRecord(t, name, defs) + case isRecord && nvproxyDef.Type != nil: + if err := compareStructs(t, nvproxyDef.Type, name, defs); err != nil { + t.Errorf("struct %q has different definitions between nvproxy and driver: %v", name, err) + } + case isAlias && nvproxyDef.Type == nil: + // For now, there is no good way to check if an alias is still simple. + // Regardless, none of the current ioctls fall into this category. + t.Errorf("struct %q is a simple alias, which is not supported yet", name) + case isAlias && nvproxyDef.Type != nil: + checkComplexAlias(t, nvproxyDef, aliasDef) + } + } + }) + }) +} + +// checkSimpleRecord checks that a record is still a simple ioctl. +func checkSimpleRecord(t *testing.T, name string, output *parser.OutputJSON) { + t.Helper() + + // This is a simple ioctl, so we want to see if it's still simple + // This means seeing if a field is NvP64, or if a field name ends in "fd" + driverFields := flattenDriverStruct(t, name, output) + for _, field := range driverFields { + if field.Type == "NvP64" { + t.Errorf("struct %q is a simple ioctl in nvproxy, but field %q has type NvP64 in the driver", name, field.Name) + } + if strings.HasSuffix(strings.ToLower(field.Name), "fd") { + t.Errorf("struct %q is a simple ioctl in nvproxy, but field %q ends in \"fd\" in the driver", name, field.Name) + return + } + } +} + +// checkComplexAlias checks that the nvproxy struct definition is still compatible +// with the driver alias definition. This only applies to a very small set of cases where +// the type is a struct in nvproxy but an alias in the driver (e.g. NvHandle). +func checkComplexAlias(t *testing.T, nvproxyDef nvproxy.DriverStruct, aliasDef parser.TypeDef) { + t.Helper() + + // To compare a struct against an alias, we compare the sizes. + nvproxySize := uint64(nvproxyDef.Type.Size()) + driverSize := aliasDef.Size + if nvproxySize != driverSize { + t.Errorf("struct %q has different sizes between nvproxy (%d) and driver (%d) (bytes)", + nvproxyDef.Name, nvproxySize, driverSize) + } +} + +// typeMap maps the base types defined in the driver to their corresponding reflect.Type. +var typeMap = map[string]reflect.Type{ + "NvP64": reflect.TypeFor[nvgpu.P64](), + "NvHandle": reflect.TypeFor[nvgpu.Handle](), + "NvProcessorUuid": reflect.TypeFor[nvgpu.NvUUID](), + "char": reflect.TypeFor[byte](), + "unsigned char": reflect.TypeFor[uint8](), + "short": reflect.TypeFor[int16](), + "unsigned short": reflect.TypeFor[uint16](), + "int": reflect.TypeFor[int32](), + "unsigned int": reflect.TypeFor[uint32](), + "long long": reflect.TypeFor[int64](), + "unsigned long long": reflect.TypeFor[uint64](), +} + +// typeAllowlist is a set of types that are allowed on the nvproxy side. +var typeAllowlist = map[reflect.Kind]struct{}{ + reflect.Int8: struct{}{}, + reflect.Uint8: struct{}{}, + reflect.Int16: struct{}{}, + reflect.Uint16: struct{}{}, + reflect.Int32: struct{}{}, + reflect.Uint32: struct{}{}, + reflect.Int64: struct{}{}, + reflect.Uint64: struct{}{}, + reflect.Array: struct{}{}, + reflect.Struct: struct{}{}, +} + +func isDriverBaseType(t string) bool { + _, ok := typeMap[t] + return ok +} + +func isNvproxyBaseType(t reflect.Type) bool { + for _, baseType := range typeMap { + if t == baseType { + return true + } + } + return false +} + +// flattenNvproxyStruct flattens a nvproxy struct by recursively flattening any nested structs. +func flattenNvproxyStruct(t *testing.T, structType reflect.Type) []reflect.StructField { + t.Helper() + + if structType.Kind() != reflect.Struct { + t.Fatalf("nvproxy struct %q is not a struct", structType.Name()) + } + + var fields []reflect.StructField + for i := 0; i < structType.NumField(); i++ { + field := structType.Field(i) + // Check if the field is any base type defined in typeMap. + // This avoids flattening the fields of a struct that is a base type. + if field.Type.Kind() != reflect.Struct || isNvproxyBaseType(field.Type) { + fields = append(fields, field) + continue + } + + // This is a nested struct, so we flatten the field. + nestedFields := flattenNvproxyStruct(t, field.Type) + + // Update offset of each nested field to be relative to the parent struct. + for i := range nestedFields { + nestedFields[i].Offset += field.Offset + } + fields = append(fields, nestedFields...) + } + return fields +} + +// flattenDriverStruct flattens a driver struct by recursively flattening any nested structs. +func flattenDriverStruct(t *testing.T, structName string, output *parser.OutputJSON) []parser.RecordField { + t.Helper() + + structDef, ok := output.Records[structName] + if !ok { + t.Fatalf("driver struct %q not found in parser output", structName) + } + + var fields []parser.RecordField + for _, field := range structDef.Fields { + // Check if the field is any base type defined in typeMap. + // This avoids flattening the fields of a struct that is a base type. + if isDriverBaseType(field.Type) { + fields = append(fields, field) + continue + } + + if fieldDef, isRecord := output.Records[field.Type]; isRecord && !fieldDef.IsUnion { + nestedFields := flattenDriverStruct(t, field.Type, output) + + // Update offset of each nested field to be relative to the parent struct. + for i := range nestedFields { + nestedFields[i].Offset += field.Offset + } + fields = append(fields, nestedFields...) + } else { + fields = append(fields, field) + } + } + return fields +} + +// compareStructs compares the definition of a struct in nvproxy to its definition in the driver. +// It checks that the size and field types are the same, and returns an error if they are not. +func compareStructs(t *testing.T, nvproxyStruct reflect.Type, driverStructName string, output *parser.OutputJSON) error { + t.Helper() + + driverStructDef, ok := output.Records[driverStructName] + if !ok { + t.Fatalf("driver struct %q not found in parser output", driverStructName) + } + + if uint64(nvproxyStruct.Size()) != driverStructDef.Size { + return fmt.Errorf("mismatched sizes for struct %q between nvproxy (%d) and driver (%d) (bytes)", + driverStructName, nvproxyStruct.Size(), driverStructDef.Size) + } + + // Flatten structs so we don't have to worry about nested structs. + nvproxyFields := flattenNvproxyStruct(t, nvproxyStruct) + driverFields := flattenDriverStruct(t, driverStructName, output) + + // We loop through both definitions with two pointers. We only increment the driver pointer + // when we find a field in the nvproxy struct with the same offset, at which point we compare + // the types. + var driverFieldNum = 0 + for _, nvproxyField := range nvproxyFields { + // We get this case if there are padding fields at the very end of the nvproxy struct. + // Since we check the size above, we can just ignore these fields. + if driverFieldNum == len(driverFields) { + break + } + + driverField := driverFields[driverFieldNum] + if uint64(nvproxyField.Offset) == driverField.Offset { + if err := compareTypes(t, nvproxyField.Type, driverField.Type, output); err != nil { + return fmt.Errorf("mismatched field types for struct %q between nvproxy field %q and driver field %q: %w"+ + "\n nvproxy fields: %v\n driver fields: %v", + driverStructName, nvproxyField.Name, driverField.Name, err, + nvproxyFields, driverFields) + } + driverFieldNum++ + } + } + + // If we have not reached the end of the driver fields, then we have not found a match for every + // field. + if driverFieldNum != len(driverFields) { + return fmt.Errorf("unable to find a match for driver field %q for struct %q in nvproxy"+ + "\n nvproxy fields: %v\n driver fields: %v", + driverFields[driverFieldNum].Name, driverStructName, + nvproxyFields, driverFields) + } + + return nil +} + +// arrayTypeRegex matches array types in the form of "type[size]". +// To allow for nested arrays, we accept brackets in the type name. +// Types may also have colons (e.g. "struct::field_t"). +var arrayTypeRegex = regexp.MustCompile(`([\w:\[\]]+)\[(\d+)\]`) + +// compareTypes compares the type of a field in nvproxy to its type in the driver, returning an +// error if they are not. +func compareTypes(t *testing.T, nvproxyType reflect.Type, driverTypeName string, output *parser.OutputJSON) error { + t.Helper() + + // Check if we have array type + if matches := arrayTypeRegex.FindStringSubmatch(driverTypeName); matches != nil { + // Get the base type name and array size + if len(matches) != 3 { + t.Fatalf("failed to parse array type %q", driverTypeName) + } + baseTypeName := matches[1] + arraySize, err := strconv.Atoi(matches[2]) + if err != nil { + t.Fatalf("failed to parse array size %q", matches[2]) + } + + // Compare size and base type of the arrays + if nvproxyType.Kind() != reflect.Array || nvproxyType.Len() != arraySize { + return fmt.Errorf("mismatched array size between nvproxy (%d) and driver (%d)", + nvproxyType.Len(), arraySize) + } + return compareTypes(t, nvproxyType.Elem(), baseTypeName, output) + } + + compareReflectTypes := func(nvproxyType, driverType reflect.Type) error { + // We need a special case for ClassID, which nvproxy uses for allocation classes but the driver + // uses as a uint32. + if nvproxyType == reflect.TypeFor[nvgpu.ClassID]() { + nvproxyType = reflect.TypeFor[uint32]() + } + + if nvproxyType != driverType { + return fmt.Errorf("mismatched type between nvproxy (%q) and driver (%q)", + nvproxyType.Name(), driverType.Name()) + } + return nil + } + + // First check if the field is any base type defined in typeMap. + // This avoids simplifying types like NvP64 which we have special nvproxy types for. + if isDriverBaseType(driverTypeName) { + return compareReflectTypes(nvproxyType, typeMap[driverTypeName]) + } + + // If the field is not a base type, try and work with its alias to + // simplify the comparison. + if typeAlias, ok := output.Aliases[driverTypeName]; ok { + driverTypeName = typeAlias.Type + } + + // We have the following cases to compare: + // - Base type is given by typeMap + // - Enum type should always map to uint32 + // - Struct type we compare using compareStructs + // - Union type we compare sizes + if isDriverBaseType(driverTypeName) { + return compareReflectTypes(nvproxyType, typeMap[driverTypeName]) + } + if strings.HasPrefix(driverTypeName, "enum") { + return compareReflectTypes(nvproxyType, reflect.TypeFor[uint32]()) + } + if recordDef, isRecord := output.Records[driverTypeName]; isRecord { + if !recordDef.IsUnion { + return compareStructs(t, nvproxyType, driverTypeName, output) + } + if uint64(nvproxyType.Size()) != recordDef.Size { + return fmt.Errorf("mismatched union sizes between nvproxy (%d) and driver (%d)", + nvproxyType.Size(), recordDef.Size) + } + + return nil + } + + t.Fatalf("unknown driver type %q", driverTypeName) + return nil +} diff --git a/pkg/sentry/devices/nvproxy/version.go b/pkg/sentry/devices/nvproxy/version.go index 30ec51e0d..afcf0805e 100644 --- a/pkg/sentry/devices/nvproxy/version.go +++ b/pkg/sentry/devices/nvproxy/version.go @@ -138,17 +138,23 @@ type driverABI struct { // To help with verifying and supporting new driver versions, we want to keep // track of all the driver structs that we currently support. We do so by mapping ioctl -// numbers to a list of struct names used by that ioctl. +// numbers to a list of DriverStructs used by that ioctl. type driverStructNames struct { - frontendNames map[uint32][]DriverStructName - uvmNames map[uint32][]DriverStructName - controlNames map[uint32][]DriverStructName - allocationNames map[nvgpu.ClassID][]DriverStructName + frontendNames map[uint32][]DriverStruct + uvmNames map[uint32][]DriverStruct + controlNames map[uint32][]DriverStruct + allocationNames map[nvgpu.ClassID][]DriverStruct } // DriverStructName is the name of a struct used by the Nvidia driver. type DriverStructName = string +// DriverStruct ties an nvproxy struct type to its corresponding driver struct name. +type DriverStruct struct { + Name DriverStructName + Type reflect.Type +} + // abis is a global map containing all supported Nvidia driver ABIs. This is // initialized on Init() and is immutable henceforth. var abis map[DriverVersion]abiConAndChecksum @@ -369,7 +375,7 @@ func Init() { getStructNames: func() *driverStructNames { return &driverStructNames{ - frontendNames: map[uint32][]DriverStructName{ + frontendNames: map[uint32][]DriverStruct{ nvgpu.NV_ESC_CARD_INFO: simpleIoctl("nv_ioctl_card_info_t"), nvgpu.NV_ESC_CHECK_VERSION_STR: getStructName(nvgpu.RMAPIVersion{}), nvgpu.NV_ESC_ATTACH_GPUS_TO_FD: nil, // NvU32 array containing GPU IDs @@ -389,7 +395,7 @@ func Init() { nvgpu.NV_ESC_RM_VID_HEAP_CONTROL: getStructName(nvgpu.NVOS32Parameters{}), nvgpu.NV_ESC_RM_MAP_MEMORY: getStructName(nvgpu.IoctlNVOS33ParametersWithFD{}), }, - uvmNames: map[uint32][]DriverStructName{ + uvmNames: map[uint32][]DriverStruct{ nvgpu.UVM_INITIALIZE: getStructName(nvgpu.UVM_INITIALIZE_PARAMS{}), nvgpu.UVM_DEINITIALIZE: nil, // Doesn't have any params nvgpu.UVM_CREATE_RANGE_GROUP: getStructName(nvgpu.UVM_CREATE_RANGE_GROUP_PARAMS{}), @@ -416,7 +422,7 @@ func Init() { nvgpu.UVM_CREATE_EXTERNAL_RANGE: getStructName(nvgpu.UVM_CREATE_EXTERNAL_RANGE_PARAMS{}), nvgpu.UVM_MM_INITIALIZE: getStructName(nvgpu.UVM_MM_INITIALIZE_PARAMS{}), }, - controlNames: map[uint32][]DriverStructName{ + controlNames: map[uint32][]DriverStruct{ nvgpu.NV0000_CTRL_CMD_CLIENT_GET_ADDR_SPACE_TYPE: simpleIoctl("NV0000_CTRL_CLIENT_GET_ADDR_SPACE_TYPE_PARAMS"), nvgpu.NV0000_CTRL_CMD_CLIENT_SET_INHERITED_SHARE_POLICY: simpleIoctl("NV0000_CTRL_CLIENT_SET_INHERITED_SHARE_POLICY_PARAMS"), nvgpu.NV0000_CTRL_CMD_GPU_GET_ATTACHED_IDS: simpleIoctl("NV0000_CTRL_GPU_GET_ATTACHED_IDS_PARAMS"), @@ -523,7 +529,7 @@ func Init() { nvgpu.NV2080_CTRL_CMD_GR_GET_INFO: getStructName(nvgpu.NV2080_CTRL_GR_GET_INFO_PARAMS{}), nvgpu.NV503C_CTRL_CMD_REGISTER_VA_SPACE: getStructName(nvgpu.NV503C_CTRL_REGISTER_VA_SPACE_PARAMS{}), }, - allocationNames: map[nvgpu.ClassID][]DriverStructName{ + allocationNames: map[nvgpu.ClassID][]DriverStruct{ nvgpu.NV01_ROOT: getStructName(nvgpu.Handle{}), nvgpu.NV01_ROOT_NON_PRIV: getStructName(nvgpu.Handle{}), nvgpu.NV01_MEMORY_SYSTEM: getStructName(nvgpu.NV_MEMORY_ALLOCATION_PARAMS{}), @@ -665,13 +671,18 @@ func Init() { // simpleIoctl simply returns a slice containing structName. This is used for ioctls that don't // have a struct defined in nvproxy, but we know the driver struct name. -func simpleIoctl(structName string) []DriverStructName { - return []DriverStructName{structName} +func simpleIoctl(structName string) []DriverStruct { + return []DriverStruct{ + DriverStruct{ + Name: structName, + Type: nil, + }, + } } // getStructName takes an instance of an nvproxy struct and reads the `nvproxy` tag to determine the // struct name. If the tag is empty, then it returns nil. -func getStructName(params any) []DriverStructName { +func getStructName(params any) []DriverStruct { paramType := reflect.TypeOf(params) // Right now, we only expect parameter structs @@ -680,14 +691,14 @@ func getStructName(params any) []DriverStructName { } // Look through each field for the tag, panicking if there are not exactly one. - driverName, found := "", false + tagName, found := "", false for i := 0; i < paramType.NumField(); i++ { field := paramType.Field(i) if name, ok := field.Tag.Lookup("nvproxy"); ok { if found { panic(fmt.Sprintf("multiple nvproxy tags for %v", paramType.Name())) } - driverName = name + tagName = name found = true } } @@ -695,13 +706,21 @@ func getStructName(params any) []DriverStructName { if !found { panic(fmt.Sprintf("missing nvproxy tag for %v", paramType.Name())) } - switch driverName { + var driverName string + switch tagName { case "": return nil case "same": - return []DriverStructName{paramType.Name()} + driverName = paramType.Name() default: - return []DriverStructName{driverName} + driverName = tagName + } + + return []DriverStruct{ + DriverStruct{ + Name: driverName, + Type: paramType, + }, } } @@ -786,25 +805,50 @@ func SupportedStructNames(version DriverVersion) ([]DriverStructName, bool) { names := abi.getStructNames() var allNames []DriverStructName - for _, names := range names.frontendNames { - if names != nil { - allNames = append(allNames, names...) + addNames := func(names []DriverStruct) { + for _, name := range names { + allNames = append(allNames, name.Name) } } + + for _, names := range names.frontendNames { + addNames(names) + } for _, names := range names.uvmNames { - if names != nil { - allNames = append(allNames, names...) - } + addNames(names) } for _, names := range names.controlNames { - if names != nil { - allNames = append(allNames, names...) - } + addNames(names) } for _, names := range names.allocationNames { - if names != nil { - allNames = append(allNames, names...) - } + addNames(names) + } + + return allNames, true +} + +// SupportedStructTypes returns the list of struct types supported by the given driver version. +// It merges the frontend, uvm, control, and allocation names into one slice. +func SupportedStructTypes(version DriverVersion) ([]DriverStruct, bool) { + namesCons, ok := abis[version] + if !ok { + return nil, false + } + abi := namesCons.cons() + names := abi.getStructNames() + + var allNames []DriverStruct + for _, names := range names.frontendNames { + allNames = append(allNames, names...) + } + for _, names := range names.uvmNames { + allNames = append(allNames, names...) + } + for _, names := range names.controlNames { + allNames = append(allNames, names...) + } + for _, names := range names.allocationNames { + allNames = append(allNames, names...) } return allNames, true