From 6767c0a805a206fc81f4b9469bb856469289cd0c Mon Sep 17 00:00:00 2001 From: Tuomas Katila Date: Fri, 11 Sep 2026 13:26:06 +0300 Subject: [PATCH 1/3] recovery: crd: allow SBR type for default reset type SBR is functional on Pro B50/60 etc. cards. Signed-off-by: Tuomas Katila --- api/v1alpha1/gpurecoveryplan_types.go | 6 ++-- api/v1alpha1/gpurecoveryplan_webhook.go | 12 +++---- api/v1alpha1/gpurecoveryplan_webhook_test.go | 34 +++++++++++-------- .../crds/gpurecoveryplans.yaml | 5 +-- .../crd/bases/intel.com_gpurecoveryplans.yaml | 5 +-- .../samples/recoveryplan/gpurecoveryplan.yaml | 12 +++---- .../controller/gpurecoveryplan_helpers.go | 6 ++-- .../gpurecoveryplan_recoverytype_test.go | 9 ++--- 8 files changed, 49 insertions(+), 40 deletions(-) diff --git a/api/v1alpha1/gpurecoveryplan_types.go b/api/v1alpha1/gpurecoveryplan_types.go index 2943d7b..7199273 100644 --- a/api/v1alpha1/gpurecoveryplan_types.go +++ b/api/v1alpha1/gpurecoveryplan_types.go @@ -94,9 +94,9 @@ type GPURecoveryPlanSpec struct { XpuSmi XpuSmiSpec `json:"xpuSmi,omitempty"` // DefaultResetType is the reset the operator runs for every reset-type recovery event it - // creates on this plan. Either "slot" (PCIe slot power cycle, also called the PM reset) or - // "amc" (out-of-band reset through the card's AMC). - // +kubebuilder:validation:Enum=slot;amc + // creates on this plan. One of "slot" (PCIe slot power cycle, also called the PM reset), + // "amc" (out-of-band reset through the card's AMC) or "sbr" (Secondary Bus Reset). + // +kubebuilder:validation:Enum=sbr;slot;amc // +kubebuilder:validation:Required DefaultResetType RecoveryType `json:"defaultResetType"` diff --git a/api/v1alpha1/gpurecoveryplan_webhook.go b/api/v1alpha1/gpurecoveryplan_webhook.go index cfe9cc6..bff4d61 100644 --- a/api/v1alpha1/gpurecoveryplan_webhook.go +++ b/api/v1alpha1/gpurecoveryplan_webhook.go @@ -186,15 +186,15 @@ func validateRecoveryPlanSpec(spec *GPURecoveryPlanSpec) error { return fmt.Errorf("spec.deviceId %q must match pattern 0x[0-9a-fA-F]{4}", spec.DeviceID) } - // Mandatory, and restricted to the two platform-selected resets. + // Mandatory, and restricted to the resets: reflash is not one, and no value is safe to guess. if spec.DefaultResetType == "" { - return fmt.Errorf("spec.defaultResetType is required: %q where the PCIe slots support hot-plug, %q otherwise", - RecoveryTypeSlot, RecoveryTypeAMC) + return fmt.Errorf("spec.defaultResetType is required: %q, %q or %q", + RecoveryTypeSlot, RecoveryTypeSBR, RecoveryTypeAMC) } - if spec.DefaultResetType != RecoveryTypeSlot && spec.DefaultResetType != RecoveryTypeAMC { - return fmt.Errorf("spec.defaultResetType %q is not a platform reset; use %q or %q, and "+ - "spec.approvals[].override to run %q on a single event", + if spec.DefaultResetType != RecoveryTypeSlot && spec.DefaultResetType != RecoveryTypeAMC && + spec.DefaultResetType != RecoveryTypeSBR { + return fmt.Errorf("spec.defaultResetType %q is not a reset; use %q, %q or %q", spec.DefaultResetType, RecoveryTypeSlot, RecoveryTypeAMC, RecoveryTypeSBR) } diff --git a/api/v1alpha1/gpurecoveryplan_webhook_test.go b/api/v1alpha1/gpurecoveryplan_webhook_test.go index cd03f63..2074311 100644 --- a/api/v1alpha1/gpurecoveryplan_webhook_test.go +++ b/api/v1alpha1/gpurecoveryplan_webhook_test.go @@ -81,9 +81,9 @@ var _ = Describe("GPURecoveryPlan Webhook", func() { Expect(obj.Spec.Approvals).To(BeEmpty()) }) - // defaultResetType is deliberately not defaulted: neither accepted value is safe to - // assume, and a wrong guess is silent — the Job runs a reset the platform cannot perform - // and exits 0. The validator rejects the omission instead. + // defaultResetType is deliberately not defaulted: no accepted value is safe to assume, + // and a wrong guess is silent — the Job runs a reset the platform cannot perform and + // exits 0. The validator rejects the omission instead. It("should not invent a defaultResetType", func() { obj.Spec.DefaultResetType = "" @@ -162,30 +162,34 @@ var _ = Describe("GPURecoveryPlan Webhook", func() { obj.Spec.DefaultResetType = "" _, err := validator.ValidateCreate(ctx, obj) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("defaultResetType")) - Expect(err.Error()).To(ContainSubstring("hot-plug")) + Expect(err.Error()).To(ContainSubstring("defaultResetType is required")) }) - // sbr and reflash are valid RecoveryTypes but not platform defaults: sbr is the per-card - // backup and reflash is not a reset. As a cluster-wide default either would apply to every - // wedged GPU the DRA driver reports. - DescribeTable("should reject a defaultResetType that is not a platform reset", + // reflash is a valid RecoveryType but not a reset, so it cannot stand in as the default + // for every wedged GPU the DRA driver reports. + DescribeTable("should reject a defaultResetType that is not a reset", func(rt RecoveryType) { obj.Spec.DefaultResetType = rt _, err := validator.ValidateCreate(ctx, obj) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("defaultResetType")) }, - Entry("sbr, the per-card backup", RecoveryTypeSBR), Entry("reflash, not a reset at all", RecoveryTypeReflash), Entry("a value outside the enum", RecoveryType("flr")), ) - It("should accept amc as a defaultResetType", func() { - obj.Spec.DefaultResetType = RecoveryTypeAMC - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).NotTo(HaveOccurred()) - }) + DescribeTable("should accept any of the resets as a defaultResetType", + func(rt RecoveryType) { + obj.Spec.DefaultResetType = rt + _, err := validator.ValidateCreate(ctx, obj) + Expect(err).NotTo(HaveOccurred()) + }, + Entry("slot", RecoveryTypeSlot), + Entry("amc", RecoveryTypeAMC), + // SBR works cluster-wide on BMG Pro cards (B50, B60, …), so it is + // a legitimate platform default there, not only a per-event override. + Entry("sbr", RecoveryTypeSBR), + ) It("should reject an invalid subDeviceId format", func() { obj.Spec.SubDeviceID = "0xGGGG" diff --git a/charts/gpu-base-operator/crds/gpurecoveryplans.yaml b/charts/gpu-base-operator/crds/gpurecoveryplans.yaml index 0aa67e1..08b6bdc 100644 --- a/charts/gpu-base-operator/crds/gpurecoveryplans.yaml +++ b/charts/gpu-base-operator/crds/gpurecoveryplans.yaml @@ -148,12 +148,13 @@ spec: - amc - reflash - enum: + - sbr - slot - amc description: |- DefaultResetType is the reset the operator runs for every reset-type recovery event it - creates on this plan. Either "slot" (PCIe slot power cycle, also called the PM reset) or - "amc" (out-of-band reset through the card's AMC). + creates on this plan. One of "slot" (PCIe slot power cycle, also called the PM reset), + "amc" (out-of-band reset through the card's AMC) or "sbr" (Secondary Bus Reset). type: string deviceId: description: 'DeviceID is the mandatory PCI device ID of the target diff --git a/config/crd/bases/intel.com_gpurecoveryplans.yaml b/config/crd/bases/intel.com_gpurecoveryplans.yaml index 0aa67e1..08b6bdc 100644 --- a/config/crd/bases/intel.com_gpurecoveryplans.yaml +++ b/config/crd/bases/intel.com_gpurecoveryplans.yaml @@ -148,12 +148,13 @@ spec: - amc - reflash - enum: + - sbr - slot - amc description: |- DefaultResetType is the reset the operator runs for every reset-type recovery event it - creates on this plan. Either "slot" (PCIe slot power cycle, also called the PM reset) or - "amc" (out-of-band reset through the card's AMC). + creates on this plan. One of "slot" (PCIe slot power cycle, also called the PM reset), + "amc" (out-of-band reset through the card's AMC) or "sbr" (Secondary Bus Reset). type: string deviceId: description: 'DeviceID is the mandatory PCI device ID of the target diff --git a/config/samples/recoveryplan/gpurecoveryplan.yaml b/config/samples/recoveryplan/gpurecoveryplan.yaml index db3ad48..1831e84 100644 --- a/config/samples/recoveryplan/gpurecoveryplan.yaml +++ b/config/samples/recoveryplan/gpurecoveryplan.yaml @@ -9,12 +9,12 @@ spec: deviceId: "0xe20b" # Mandatory. Which reset works is a property of the platform, not of the fault: use slot — the - # PCIe slot power cycle, also called the PM reset — where the slots support hot-plug, and amc - # where they do not. The DRA driver only reports "needs a reset", so this is the operator's only - # way to know which one to run, and getting it wrong is silent: the Job runs a reset the platform - # cannot perform, exits 0, and the event succeeds with the GPU still broken. - # sbr is not accepted here — it is the per-card backup, reached through an approval's override - # when the platform's normal reset does not revive one particular GPU. + # PCIe slot power cycle, also called the PM reset — where the slots support hot-plug, sbr on a + # BMG Pro card (B50, B60 etc.), and amc otherwise. The DRA driver only + # reports "needs a reset", so this is the operator's only way to know which one to run, and + # getting it wrong is silent: the Job runs a reset the platform cannot perform, exits 0, and the + # event succeeds with the GPU still broken. An approval's override switches a single event to a + # different reset when the plan's default does not revive one particular GPU. defaultResetType: "slot" # The node drain that runs before a reset. None of it applies to a reflash: that writes firmware to diff --git a/internal/controller/gpurecoveryplan_helpers.go b/internal/controller/gpurecoveryplan_helpers.go index 84b1f46..17d59f9 100644 --- a/internal/controller/gpurecoveryplan_helpers.go +++ b/internal/controller/gpurecoveryplan_helpers.go @@ -80,8 +80,10 @@ func taintToDeviceNeed(taintKey string, defaultReset intelv1a1.RecoveryType) (de // resetTypeOrDefault resolves spec.defaultResetType, falling back to SBR when it is unset. // // The field is required by the CRD, so an empty value means an object that never went through -// the API server. Falling back matters because an empty type produces a malformed event ID: SBR -// rather than slot or amc, because guessing between those two is guessing at the platform. +// the API server. Falling back matters because an empty type produces a malformed event ID. Which +// reset actually works is a platform property the operator cannot discover, so the value picked +// here is arbitrary — SBR only because it is the least invasive of the three, and the event still +// needs an admin approval before anything runs. func resetTypeOrDefault(rt intelv1a1.RecoveryType) intelv1a1.RecoveryType { if rt == "" { klog.Warningf("GPURecoveryPlan has no spec.defaultResetType; falling back to %s", diff --git a/internal/controller/gpurecoveryplan_recoverytype_test.go b/internal/controller/gpurecoveryplan_recoverytype_test.go index ed655a7..62f5c6e 100644 --- a/internal/controller/gpurecoveryplan_recoverytype_test.go +++ b/internal/controller/gpurecoveryplan_recoverytype_test.go @@ -49,8 +49,8 @@ var _ = Describe("GPURecoveryPlan Controller: recovery type selection", func() { }) // The field is mandatory, so an empty value means an object that never reached the API - // server. Falling back matters because an empty type produces a malformed event ID. SBR - // rather than slot or amc: guessing between those two is guessing at the platform. + // server. Falling back matters because an empty type produces a malformed event ID; which + // reset it picks hardly matters, since nothing runs without an approval anyway. It("should fall back to SBR when the plan carries no default reset type", func() { need, ok := taintToDeviceNeed(deviceTaintKeyReset, "") Expect(ok).To(BeTrue()) @@ -101,8 +101,8 @@ var _ = Describe("GPURecoveryPlan Controller: recovery type selection", func() { }) // Which reset works is a property of the platform (hot-plug capable slots → the slot power - // cycle, otherwise AMC), and the DRA driver only reports "needs a reset". - // spec.defaultResetType is the only thing that can tell the two apart. + // cycle, a BMG Pro card → SBR, otherwise AMC), and the DRA driver only + // reports "needs a reset". spec.defaultResetType is the only thing that can tell them apart. Context("spec.defaultResetType", func() { const ( drtSlice = "drt-slice" @@ -141,6 +141,7 @@ var _ = Describe("GPURecoveryPlan Controller: recovery type selection", func() { }, Entry("slot, where the PCIe slots support hot-plug", intelv1a1.RecoveryTypeSlot), Entry("amc, where they do not", intelv1a1.RecoveryTypeAMC), + Entry("sbr, on a BMG Pro cards", intelv1a1.RecoveryTypeSBR), ) // The field is a default for events created afterwards, not a retroactive rewrite. An From cc42ca783698bf394960a39784e299d85ec21412 Mon Sep 17 00:00:00 2001 From: Tuomas Katila Date: Fri, 11 Sep 2026 13:26:55 +0300 Subject: [PATCH 2/3] recovery: refresh docs and set samples container images as "invalid" Signed-off-by: Tuomas Katila --- RECOVERY.md | 48 +++++++++---------- .../samples/recoveryplan/gpurecoveryplan.yaml | 4 +- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/RECOVERY.md b/RECOVERY.md index 13bbf4e..f6de8eb 100644 --- a/RECOVERY.md +++ b/RECOVERY.md @@ -13,6 +13,20 @@ detects the need, reports it, and waits for a cluster admin to approve the opera Recovery is driven by the cluster-scoped `GPURecoveryPlan` CRD. +> **NOTE:** Recovery functionality is dependent on multiple components external to the operator: xpumd, DRA and xpu-smi. +> Current versions of xpumd and xpu-smi (v2.1.0) are not yet compatible with the recovery functionality. + +## Hardware dependency + +Recovery functionality is dependent on both the underlying system as well as the installed Intel GPU. + +| Recovery | Requirements | +| --- | --- | +| SBR | A BMG Pro card (B50, B60 etc.). | +| Slot | `HotPlug` and `PwrCtrl` support in the PCIe bus. Intel GPU supporting HotPlug. | +| AMC | Intel GPU equipped with AMC. | +| Reflash | Intel GPU supporting FDO mode. | + ## How it works ``` @@ -53,14 +67,16 @@ path: where the binary lives is the image's business. |---|---|---| | `slot` | `xpu-smi config -d --coldreset` | PCIe slot power cycle; requires PCIe hot-plug support | | `amc` | `xpu-smi amc --gpuReset -d ` | Out-of-band reset through the card's AMC | -| `sbr` | `xpu-smi config -d --reset` | Secondary Bus Reset; per-card backup, via an approval override only | +| `sbr` | `xpu-smi config -d --reset` | Secondary Bus Reset | | `reflash` | `xpu-smi updatefw -d -t FDO -f ` | Flash the known good firmware onto a card in FDO mode | -These are **not** a severity ladder. Exactly one of `slot` and `amc` works on a given platform — -slot where the PCIe slots do hot-plug, AMC where they do not — and the DRA driver can only say "this -device needs a reset", not which mechanism applies. That is why `spec.defaultResetType` is mandatory -with no default: a reset the platform cannot perform **exits 0**, so a wrong value produces a clean -run to `succeeded` over a GPU that was never touched. +The three resets are **not** a severity ladder. Which one works is a property of the platform, not of +the fault — `slot` where the PCIe slots do hot-plug, `sbr` on a BMG Pro card with a new enough kernel, +`amc` on cards carrying an AMC — and the DRA driver can only say "this device needs a reset", not +which mechanism applies. That is why `spec.defaultResetType` is mandatory with no default: a reset the +platform cannot perform **exits 0**, so a wrong value produces a clean run to `succeeded` over a GPU +that was never touched. Any of the three can be the plan-wide default, and +`spec.approvals[].override` still switches a single event to a different one. ### Event states @@ -124,7 +140,7 @@ metadata: name: recoveryplan-bmg spec: deviceId: "0xe20b" # mandatory; one plan per GPU model - defaultResetType: "slot" # mandatory: "slot" or "amc" + defaultResetType: "slot" # mandatory: "slot", "sbr" or "amc" drain: enable: true @@ -222,20 +238,6 @@ kubectl patch gpurecoveryplan --type=json \ -p='[{"op":"add","path":"/spec/approvals/-","value":{"eventId":""}}]' ``` -## Metrics - -Exposed on the operator's own `/metrics` endpoint (there is deliberately no `status.stats` field — -the CR holds current state, the counters hold history): - -| Metric | Labels | -|---|---| -| `gpu_recovery_events_total` | `plan`, `type`, `reason` | -| `gpu_recovery_attempts_total` | `plan`, `type` | -| `gpu_recovery_outcomes_total` | `plan`, `type`, `result` | -| `gpu_recovery_overrides_total` | `plan`, `suggested_type`, `chosen_type` | -| `gpu_recovery_events` (gauge) | `plan`, `node`, `type`, `state` | -| `gpu_recovery_plan_state` (gauge) | `plan`, `state` | - ## Current limitations * **Sibling devices are not protected.** Nothing stops a slot reset or SBR on one card from @@ -247,8 +249,4 @@ the CR holds current state, the counters hold history): driver does not publish those attributes yet. * **`firmware.source.volumeSource` is not implemented.** A reflash event on a volume-only plan stays in `missing-firmware`; use `containerSource`. -* **Reset efficacy on BMG.** On some B580 cards a reset leaves the GPU non-working; end-to-end - validation depends on driver/firmware fixes. -* **The `health-xpumd-gpu.wedged` taint key is not yet confirmed against a shipping DRA driver.** - The survivability key and the `pciId` / `pciAddress` device attributes are. diff --git a/config/samples/recoveryplan/gpurecoveryplan.yaml b/config/samples/recoveryplan/gpurecoveryplan.yaml index 1831e84..3ce2f48 100644 --- a/config/samples/recoveryplan/gpurecoveryplan.yaml +++ b/config/samples/recoveryplan/gpurecoveryplan.yaml @@ -55,7 +55,7 @@ spec: # The xpu-smi image every recovery Job runs, for both the resets and the reflash. xpuSmi: - image: "docker.io/intel/gpu-fwupdater-mock:devel" + image: "registry.local/xpu-smi:devel" pullPolicy: "Always" # Skip registry certificate validation for the operator's pre-flight check on the image @@ -69,7 +69,7 @@ spec: firmware: source: containerSource: - name: "docker.io/intel/intel-gpu-fw-binaries:devel" + name: registry.local/intel-gpu-fw-binaries:devel # Same as spec.xpuSmi.insecureSkipTLSVerify, and separate from it because the known good # firmware for one card model often lives on an internal registry while xpu-smi comes From b710d6b0a68146128d98756c9cd57e28bab9fc2c Mon Sep 17 00:00:00 2001 From: Tuomas Katila Date: Fri, 11 Sep 2026 14:21:31 +0300 Subject: [PATCH 3/3] recovery: add missing force flag for slot reset Signed-off-by: Tuomas Katila --- RECOVERY.md | 2 +- config/deployments/xpum/xpum-reset-job.yaml | 2 +- internal/controller/gpurecoveryplan_helpers.go | 2 +- internal/controller/gpurecoveryplan_jobs_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/RECOVERY.md b/RECOVERY.md index f6de8eb..1cfa089 100644 --- a/RECOVERY.md +++ b/RECOVERY.md @@ -65,7 +65,7 @@ path: where the binary lives is the image's business. | Type | Command | Notes | |---|---|---| -| `slot` | `xpu-smi config -d --coldreset` | PCIe slot power cycle; requires PCIe hot-plug support | +| `slot` | `xpu-smi config -d --coldreset --force-reset-gpus` | PCIe slot power cycle; requires PCIe hot-plug support | | `amc` | `xpu-smi amc --gpuReset -d ` | Out-of-band reset through the card's AMC | | `sbr` | `xpu-smi config -d --reset` | Secondary Bus Reset | | `reflash` | `xpu-smi updatefw -d -t FDO -f ` | Flash the known good firmware onto a card in FDO mode | diff --git a/config/deployments/xpum/xpum-reset-job.yaml b/config/deployments/xpum/xpum-reset-job.yaml index d817b7e..d4fd004 100644 --- a/config/deployments/xpum/xpum-reset-job.yaml +++ b/config/deployments/xpum/xpum-reset-job.yaml @@ -12,7 +12,7 @@ # # Supported commands (set by the operator from the event's recoveryType): # sbr: xpu-smi config -d --reset -# slot: xpu-smi config -d --coldreset +# slot: xpu-smi config -d --coldreset --force-reset-gpus # amc: xpu-smi amc --gpureset -d -y --- apiVersion: batch/v1 diff --git a/internal/controller/gpurecoveryplan_helpers.go b/internal/controller/gpurecoveryplan_helpers.go index 17d59f9..d8054e7 100644 --- a/internal/controller/gpurecoveryplan_helpers.go +++ b/internal/controller/gpurecoveryplan_helpers.go @@ -250,7 +250,7 @@ func buildResetCommand(bdf string, rt intelv1a1.RecoveryType) string { case intelv1a1.RecoveryTypeSBR: return fmt.Sprintf("xpu-smi config -d %s --reset", bdf) case intelv1a1.RecoveryTypeSlot: - return fmt.Sprintf("xpu-smi config -d %s --coldreset", bdf) + return fmt.Sprintf("xpu-smi config -d %s --coldreset --force-reset-gpus", bdf) case intelv1a1.RecoveryTypeAMC: return fmt.Sprintf("xpu-smi amc --gpureset -d %s -y", bdf) default: diff --git a/internal/controller/gpurecoveryplan_jobs_test.go b/internal/controller/gpurecoveryplan_jobs_test.go index 71bb26b..177812a 100644 --- a/internal/controller/gpurecoveryplan_jobs_test.go +++ b/internal/controller/gpurecoveryplan_jobs_test.go @@ -342,7 +342,7 @@ var _ = Describe("GPURecoveryPlan Controller: recovery Job construction", func() // The BDF has to reach the command line, not just the event. One argument, because the // template runs /bin/sh -c: see the reflash Job's specs for what splitting it costs. Expect(resetter.Command).To(Equal([]string{"/bin/sh", "-c"})) - Expect(resetter.Args).To(Equal([]string{"xpu-smi config -d 0000:02:00.0 --coldreset"})) + Expect(resetter.Args).To(Equal([]string{"xpu-smi config -d 0000:02:00.0 --coldreset --force-reset-gpus"})) }) It("should give the Job the operator's own pull secret", func() {