Skip injection of GSP firmware by default

This change skips the injection of GSP firmware by default. An include-gsp-firmware feature flag is added to allow the firmware paths to be injected if required. Signed-off-by: Evan Lezar <[email protected]>
NVIDIA · Sep 18, 2024 · 61057e6 · 61057e6
1 parent 30921c2
commit 61057e6
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 66 deletions.
diff --git a/cmd/nvidia-container-runtime-hook/main.go b/cmd/nvidia-container-runtime-hook/main.go
@@ -90,6 +90,10 @@ func doPrestart() {
 
 	args := []string{getCLIPath(cli)}
 
+	// Only include GSP firmware if explicitly renabled.
+	if !hook.Features.IncludeGSPFirmware.IsEnabled() {
+		args = append(args, "--no-gsp-firmware")
+	}
 	// Only include the nvidia-persistenced socket if it is explicitly enabled.
 	if !hook.Features.IncludePersistencedSocket.IsEnabled() {
 		args = append(args, "--no-persistenced")

diff --git a/cmd/nvidia-ctk/cdi/generate/generate.go b/cmd/nvidia-ctk/cdi/generate/generate.go
@@ -61,6 +61,7 @@ type options struct {
 		ignorePatterns cli.StringSlice
 	}
 
+	includeGSPFirmware        bool
 	includePersistencedSocket bool
 }
 
@@ -171,6 +172,11 @@ func (m command) build() *cli.Command {
 			Usage:       "Specify a pattern the CSV mount specifications.",
 			Destination: &opts.csv.ignorePatterns,
 		},
+		&cli.BoolFlag{
+			Name:        "include-gsp-firmware",
+			Usage:       "Include the GSP firmware in the generated CDI specification.",
+			Destination: &opts.includeGSPFirmware,
+		},
 		&cli.BoolFlag{
 			Name:        "include-persistenced-socket",
 			Usage:       "Include the nvidia-persistenced socket in the generated CDI specification.",
@@ -280,6 +286,7 @@ func (m command) generateSpec(opts *options) (spec.Interface, error) {
 		nvcdi.WithLibrarySearchPaths(opts.librarySearchPaths.Value()),
 		nvcdi.WithCSVFiles(opts.csv.files.Value()),
 		nvcdi.WithCSVIgnorePatterns(opts.csv.ignorePatterns.Value()),
+		nvcdi.WithOptInFeature("include-gsp-firmware", opts.includeGSPFirmware),
 		nvcdi.WithOptInFeature("include-persistenced-socket", opts.includePersistencedSocket),
 	)
 	if err != nil {

diff --git a/internal/config/features.go b/internal/config/features.go
@@ -24,6 +24,7 @@ const (
 	FeatureNVSWITCH                   = featureName("nvswitch")
 	FeatureGDRCopy                    = featureName("gdrcopy")
 	FeatureAllowLDConfigFromContainer = featureName("allow-ldconfig-from-container")
+	FeatureIncludeGSPFirmware         = featureName("include-gsp-firmware")
 	FeatureIncludePersistencedSocket  = featureName("include-persistenced-socket")
 )
 
@@ -37,6 +38,8 @@ type features struct {
 	// If this feature flag is not set to 'true' only host-rooted config paths
 	// (i.e. paths starting with an '@' are considered valid)
 	AllowLDConfigFromContainer *feature `toml:"allow-ldconfig-from-container,omitempty"`
+	// IncludeGSPFirmware enables the injection of GSP firmware into containers.
+	IncludeGSPFirmware *feature `toml:"include-gsp-firmware,omitempty"`
 	// IncludePersistencedSocket enables the injection of the nvidia-persistenced
 	// socket into containers.
 	IncludePersistencedSocket *feature `toml:"include-persistenced-socket,omitempty"`
@@ -61,6 +64,8 @@ func (fs features) IsEnabledInEnvironment(n featureName, in ...getenver) bool {
 	// Features without envvar overrides
 	case FeatureAllowLDConfigFromContainer:
 		return fs.AllowLDConfigFromContainer.IsEnabled()
+	case FeatureIncludeGSPFirmware:
+		return fs.IncludeGSPFirmware.IsEnabled()
 	case FeatureIncludePersistencedSocket:
 		return fs.IncludePersistencedSocket.IsEnabled()
 	default:

diff --git a/internal/modifier/cdi.go b/internal/modifier/cdi.go
@@ -189,6 +189,7 @@ func generateAutomaticCDISpec(logger logger.Interface, cfg *config.Config, devic
 		nvcdi.WithDriverRoot(cfg.NVIDIAContainerCLIConfig.Root),
 		nvcdi.WithVendor("runtime.nvidia.com"),
 		nvcdi.WithClass("gpu"),
+		nvcdi.WithOptInFeature("include-gsp-firmware", cfg.Features.IncludeGSPFirmware.IsEnabled()),
 		nvcdi.WithOptInFeature("include-persistenced-socket", cfg.Features.IncludePersistencedSocket.IsEnabled()),
 	)
 	if err != nil {

diff --git a/pkg/nvcdi/driver-nvml.go b/pkg/nvcdi/driver-nvml.go
@@ -18,12 +18,10 @@ package nvcdi
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/NVIDIA/go-nvml/pkg/nvml"
-	"golang.org/x/sys/unix"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
@@ -63,7 +61,7 @@ func (l *nvcdilib) newDriverVersionDiscoverer(version string) (discover.Discover
 		return nil, fmt.Errorf("failed to create discoverer for IPC sockets: %v", err)
 	}
 
-	firmwares, err := NewDriverFirmwareDiscoverer(l.logger, l.driver.Root, version)
+	firmwares, err := l.newDriverFirmwareDiscoverer(l.logger, l.driver.Root, version)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create discoverer for GSP firmware: %v", err)
 	}
@@ -107,67 +105,6 @@ func NewDriverLibraryDiscoverer(logger logger.Interface, driver *root.Driver, nv
 	return d, nil
 }
 
-func getUTSRelease() (string, error) {
-	utsname := &unix.Utsname{}
-	if err := unix.Uname(utsname); err != nil {
-		return "", err
-	}
-	return unix.ByteSliceToString(utsname.Release[:]), nil
-}
-
-func getFirmwareSearchPaths(logger logger.Interface) ([]string, error) {
-
-	var firmwarePaths []string
-	if p := getCustomFirmwareClassPath(logger); p != "" {
-		logger.Debugf("using custom firmware class path: %s", p)
-		firmwarePaths = append(firmwarePaths, p)
-	}
-
-	utsRelease, err := getUTSRelease()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get UTS_RELEASE: %v", err)
-	}
-
-	standardPaths := []string{
-		filepath.Join("/lib/firmware/updates/", utsRelease),
-		"/lib/firmware/updates/",
-		filepath.Join("/lib/firmware/", utsRelease),
-		"/lib/firmware/",
-	}
-
-	return append(firmwarePaths, standardPaths...), nil
-}
-
-// getCustomFirmwareClassPath returns the custom firmware class path if it exists.
-func getCustomFirmwareClassPath(logger logger.Interface) string {
-	customFirmwareClassPath, err := os.ReadFile("/sys/module/firmware_class/parameters/path")
-	if err != nil {
-		logger.Warningf("failed to get custom firmware class path: %v", err)
-		return ""
-	}
-
-	return strings.TrimSpace(string(customFirmwareClassPath))
-}
-
-// NewDriverFirmwareDiscoverer creates a discoverer for GSP firmware associated with the specified driver version.
-func NewDriverFirmwareDiscoverer(logger logger.Interface, driverRoot string, version string) (discover.Discover, error) {
-	gspFirmwareSearchPaths, err := getFirmwareSearchPaths(logger)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get firmware search paths: %v", err)
-	}
-	gspFirmwarePaths := filepath.Join("nvidia", version, "gsp*.bin")
-	return discover.NewMounts(
-		logger,
-		lookup.NewFileLocator(
-			lookup.WithLogger(logger),
-			lookup.WithRoot(driverRoot),
-			lookup.WithSearchPaths(gspFirmwareSearchPaths...),
-		),
-		driverRoot,
-		[]string{gspFirmwarePaths},
-	), nil
-}
-
 // NewDriverBinariesDiscoverer creates a discoverer for GSP firmware associated with the GPU driver.
 func NewDriverBinariesDiscoverer(logger logger.Interface, driverRoot string) discover.Discover {
 	return discover.NewMounts(

diff --git a/pkg/nvcdi/firmware.go b/pkg/nvcdi/firmware.go
@@ -31,7 +31,7 @@ import (
 
 // newDriverFirmwareDiscoverer creates a discoverer for GSP firmware associated with the specified driver version.
 func (l *nvcdilib) newDriverFirmwareDiscoverer(logger logger.Interface, driverRoot string, version string) (discover.Discover, error) {
-	if !l.optInFeatures["allow-gsp-firmware"] {
+	if !l.optInFeatures["include-gsp-firmware"] {
 		return discover.None{}, nil
 	}
 

diff --git a/pkg/nvcdi/lib.go b/pkg/nvcdi/lib.go
@@ -69,7 +69,9 @@ type nvcdilib struct {
 
 // New creates a new nvcdi library
 func New(opts ...Option) (Interface, error) {
-	l := &nvcdilib{}
+	l := &nvcdilib{
+		optInFeatures: make(map[string]bool),
+	}
 	for _, opt := range opts {
 		opt(l)
 	}