Recovery: allow sbr as default, update docs and add missing flag to slot reset

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
 
 ```
@@ -51,16 +65,18 @@ path: where the binary lives is the image's business.
 
 | Type | Command | Notes |
 |---|---|---|
-| `slot` | `xpu-smi config -d <bdf> --coldreset` | PCIe slot power cycle; requires PCIe hot-plug support |
+| `slot` | `xpu-smi config -d <bdf> --coldreset --force-reset-gpus` | PCIe slot power cycle; requires PCIe hot-plug support |
 | `amc` | `xpu-smi amc --gpuReset -d <bdf>` | Out-of-band reset through the card's AMC |
-| `sbr` | `xpu-smi config -d <bdf> --reset` | Secondary Bus Reset; per-card backup, via an approval override only |
+| `sbr` | `xpu-smi config -d <bdf> --reset` | Secondary Bus Reset |
 | `reflash` | `xpu-smi updatefw -d <bdf> -t FDO -f <file>` | 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 <plan> --type=json \
   -p='[{"op":"add","path":"/spec/approvals/-","value":{"eventId":"<event-id>"}}]'
 ```
 
-## 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.
 

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"`
 

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)
 	}
 

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"

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

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

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 <BDF> --reset
-#   slot:  xpu-smi config -d <BDF> --coldreset
+#   slot:  xpu-smi config -d <BDF> --coldreset --force-reset-gpus
 #   amc:   xpu-smi amc --gpureset -d <BDF> -y
 ---
 apiVersion: batch/v1

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
@@ -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

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",
@@ -248,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:

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() {

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