-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[https://nvbugs/6506990][fix] Don't treat NVLE-only as CC enabled #16850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| # Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. | ||
| # | ||
| # 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. | ||
|
|
||
| import ctypes | ||
| import sys | ||
| import types | ||
|
|
||
| import pytest | ||
|
|
||
| from tensorrt_llm._utils import confidential_compute_enabled, get_cc_and_nvle_status | ||
|
|
||
|
|
||
| class _NvmlError(Exception): | ||
| pass | ||
|
|
||
|
|
||
| class _NvmlErrorNotSupported(_NvmlError): | ||
| pass | ||
|
|
||
|
|
||
| class _ConfComputeSettings(ctypes.Structure): | ||
| _fields_ = [ | ||
| ("ccFeature", ctypes.c_uint), | ||
| ("multiGpuMode", ctypes.c_uint), | ||
| ] | ||
|
|
||
|
|
||
| def _make_pynvml( | ||
| *, | ||
| cc_feature: int, | ||
| multi_gpu_mode: int, | ||
| settings_supported: bool = True, | ||
| legacy_cc_feature: int = 0, | ||
| ) -> types.ModuleType: | ||
| pynvml = types.ModuleType("pynvml") | ||
| pynvml.NVMLError = _NvmlError | ||
| pynvml.NVMLError_NotSupported = _NvmlErrorNotSupported | ||
| pynvml.NVML_CC_SYSTEM_FEATURE_ENABLED = 1 | ||
| pynvml.NVML_CC_SYSTEM_MULTIGPU_PROTECTED_PCIE = 1 | ||
| pynvml.NVML_CC_SYSTEM_MULTIGPU_NVLE = 2 | ||
| pynvml.c_nvmlSystemConfComputeSettings_v1_t = _ConfComputeSettings | ||
| pynvml.nvmlInit = lambda: None | ||
| pynvml.nvmlShutdown = lambda: None | ||
| pynvml._nvmlCheckReturn = lambda _: None | ||
| pynvml.settings_queries = 0 | ||
|
|
||
| def get_settings(settings_ptr): | ||
| pynvml.settings_queries += 1 | ||
| if not settings_supported: | ||
| raise _NvmlErrorNotSupported | ||
| settings_ptr._obj.ccFeature = cc_feature | ||
| settings_ptr._obj.multiGpuMode = multi_gpu_mode | ||
| return 0 | ||
|
|
||
| pynvml.nvmlSystemGetConfComputeSettings = get_settings | ||
| pynvml.nvmlSystemGetConfComputeState = lambda: types.SimpleNamespace( | ||
| ccFeature=legacy_cc_feature | ||
| ) | ||
| return pynvml | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def clear_confidential_compute_status_cache(): | ||
| get_cc_and_nvle_status.cache_clear() | ||
| yield | ||
| get_cc_and_nvle_status.cache_clear() | ||
|
Comment on lines
+73
to
+77
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Cover the missing- The new As per path instructions, test-code changes require an actionable coverage assessment. 🤖 Prompt for AI AgentsSource: Path instructions 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add the required function annotations. The autouse fixture and all test functions omit parameter and return annotations. As per coding guidelines, “Annotate every function.” Also applies to: 89-96, 99-105, 117-124, 127-136 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "cc_feature,multi_gpu_mode,expected", | ||
| [ | ||
| (0, 0, (False, False)), | ||
| (0, 1, (True, False)), | ||
| (0, 2, (False, True)), | ||
| (1, 2, (True, True)), | ||
| ], | ||
| ) | ||
| def test_get_cc_and_nvle_status(monkeypatch, cc_feature, multi_gpu_mode, expected): | ||
| pynvml = _make_pynvml( | ||
| cc_feature=cc_feature, | ||
| multi_gpu_mode=multi_gpu_mode, | ||
| ) | ||
| monkeypatch.setitem(sys.modules, "pynvml", pynvml) | ||
|
|
||
| assert get_cc_and_nvle_status() == expected | ||
|
|
||
|
|
||
| def test_get_cc_and_nvle_status_is_cached(monkeypatch): | ||
| pynvml = _make_pynvml(cc_feature=0, multi_gpu_mode=2) | ||
| monkeypatch.setitem(sys.modules, "pynvml", pynvml) | ||
|
|
||
| assert get_cc_and_nvle_status() == (False, True) | ||
| assert get_cc_and_nvle_status() == (False, True) | ||
| assert pynvml.settings_queries == 1 | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "cc_feature,multi_gpu_mode,expected", | ||
| [ | ||
| (0, 0, False), | ||
| (0, 1, True), | ||
| (0, 2, False), | ||
| (1, 2, True), | ||
| ], | ||
| ) | ||
| def test_confidential_compute_enabled(monkeypatch, cc_feature, multi_gpu_mode, expected): | ||
| pynvml = _make_pynvml( | ||
| cc_feature=cc_feature, | ||
| multi_gpu_mode=multi_gpu_mode, | ||
| ) | ||
| monkeypatch.setitem(sys.modules, "pynvml", pynvml) | ||
|
|
||
| assert confidential_compute_enabled() is expected | ||
|
Comment on lines
+117
to
+124
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Test delegation rather than only equivalent output. This test would still pass if As per path instructions, test-code changes require an actionable coverage assessment. 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
|
|
||
| def test_get_cc_and_nvle_status_uses_legacy_cc_fallback(monkeypatch): | ||
| pynvml = _make_pynvml( | ||
| cc_feature=0, | ||
| multi_gpu_mode=0, | ||
| settings_supported=False, | ||
| legacy_cc_feature=1, | ||
| ) | ||
| monkeypatch.setitem(sys.modules, "pynvml", pynvml) | ||
|
|
||
| assert get_cc_and_nvle_status() == (True, False) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not silently treat unexpected NVML/API failures as CC-disabled.
Catching
Exceptionconverts programming or API-compatibility failures into(False, False), which can bypass CC-specific restrictions. Catchpynvml.NVMLErrorhere and let unexpected failures surface.Proposed fix
As per coding guidelines, “Catch the narrowest possible exceptions.”
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 1212-1212: Do not catch blind exception:
Exception(BLE001)
[warning] 1213-1213: Use explicit conversion flag
Replace with conversion flag
(RUF010)
[warning] 1214-1214: Do not catch blind exception:
Exception(BLE001)
[warning] 1215-1215: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Sources: Coding guidelines, Linters/SAST tools