From 61057e63d1711bd2f598048960f643163ca1bd33 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 16 Sep 2024 15:43:23 +0200 Subject: [PATCH] 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 --- cmd/nvidia-container-runtime-hook/main.go | 4 ++ cmd/nvidia-ctk/cdi/generate/generate.go | 7 +++ internal/config/features.go | 5 ++ internal/modifier/cdi.go | 1 + pkg/nvcdi/driver-nvml.go | 65 +---------------------- pkg/nvcdi/firmware.go | 2 +- pkg/nvcdi/lib.go | 4 +- 7 files changed, 22 insertions(+), 66 deletions(-) diff --git a/cmd/nvidia-container-runtime-hook/main.go b/cmd/nvidia-container-runtime-hook/main.go index 90e488d1..6b14496f 100644 --- 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 index 9c2da04f..52623a9f 100644 --- 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 index bde12191..e97a94d7 100644 --- 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 index d3e4e935..64d8ddf8 100644 --- 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 index 59ff91c2..29cc0d27 100644 --- 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 index 7d96422d..8c3b4ee0 100644 --- 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 index e535763b..2ca6e6b1 100644 --- 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) }