Skip to content

Commit db1ac77

Browse files
Joan Olguy Canéusmeta-codesync[bot]
authored andcommitted
Make silicon pcie_address optional
Summary: `icecube800bc`'s spec carried a placeholder NPU PCIe address (`00:01.0`) that its own `_review_notes` flagged as not-yet-known. A placeholder is worse than nothing here: it passes pre-flight and then gets aggregated into the platform's PCIe presence-check list as if it were real. Drop it from the spec, and make `platform.silicon[].pcie_address` optional so omission is the supported way to say "not known yet": - `schemas/platform-spec.v1.json`: remove `pcie_address` from the `silicon` item `required` list. The `BB:DD.F` pattern is unchanged, so a present value is still validated exactly as before. - `validator/spec_validator.py`: `_Silicon.pcie_address` becomes `str | None = None`, and the format check is skipped when absent. - `configs/icecube800bc/spec.json`: `{ "name": "TH6" }`, with the review note reworded from "replace this placeholder" to "add it once confirmed". - `references/vendor-spec-schema.md`: field marked optional, with a worked example and an explicit note that an empty string is not a valid stand-in. An empty string is deliberately still rejected — the `BB:DD.F` pattern does not match `""`, so there is exactly one way to express "unknown" and consumers do not have to special-case a sentinel. Per-component `pcie_address` (in `$defs/component`) was already optional and is untouched. ___ Differential Revision: D117156310 fbshipit-source-id: 6d61075f2c2dff2b4b40666a4616efea6d0939a9
1 parent 63b56c3 commit db1ac77

5 files changed

Lines changed: 38 additions & 8 deletions

File tree

fboss/platform/firmware_onboarding/configs/icecube800bc/spec.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"platform": {
55
"name": "icecube800bc",
66
"silicon": [
7-
{ "name": "TH6", "pcie_address": "00:01.0" }
7+
{ "name": "TH6" }
88
]
99
},
1010

@@ -104,7 +104,7 @@
104104
],
105105

106106
"_review_notes": [
107-
"platform.silicon[0].pcie_address — '00:01.0' is a placeholder; replace with the actual NPU PCIe BB:DD.F address from the platform reference design",
107+
"platform.silicon[0].pcie_address — omitted because the NPU PCIe address is not yet known; add it (BB:DD.F, from the platform reference design) once confirmed",
108108
"components — mirrors the current fbcode/fboss/platform/configs/icecube/fw_util.json. If icecube800bc actually has additional firmware components (e.g. fan_cpld, smb_fpga, psu) that aren't yet in fw_util.json, add them to BOTH this spec AND the platform's fw_util.json. (Note: pre-flight does NOT cross-check fw_util.json — that gate was deferred; the spec is the source of truth for the orchestrator.)",
109109
"components[*].location and display_location — best-effort placeholders inferred from component name conventions; confirm against the actual icecube800bc reference design",
110110
"components[*].field_replaceable — confirm each setting matches the actual chassis design",

fboss/platform/firmware_onboarding/references/vendor-spec-schema.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ A worked example is in `configs/icecube800bc/spec.json`.
3838

3939
## `platform`
4040

41-
Platform-wide identity. Both fields are required.
41+
Platform-wide identity. `name` and `silicon` are required.
4242

4343
```jsonc
4444
"platform": {
@@ -49,12 +49,24 @@ Platform-wide identity. Both fields are required.
4949
}
5050
```
5151

52+
If the NPU's PCIe address is not yet known, omit `pcie_address` entirely rather
53+
than supplying a placeholder — `{ "name": "TH6" }` is valid:
54+
55+
```jsonc
56+
"platform": {
57+
"name": "icecube800bc",
58+
"silicon": [
59+
{ "name": "TH6" }
60+
]
61+
}
62+
```
63+
5264
| Field | Type | Notes |
5365
|---|---|---|
5466
| `name` | string | Lowercase canonical name of the platform (e.g. `"icecube800bc"`). This is the name Meta uses to refer to your platform across all systems — it's the same name that is programmed in the eeprom. The name that was communicated to you by Meta. No spaces, no uppercase. |
5567
| `silicon` | list of objects | NPU silicon families on the platform. Always a list (even for a single NPU) so dual-NPU platforms can be expressed uniformly. |
5668
| `silicon[].name` | string | Canonical short token for the silicon family (e.g. `"TH6"`, `"TH5"`, `"J3"`, `"R3"`). |
57-
| `silicon[].pcie_address` | string | PCIe bus address of the NPU on this platform, format `BB:DD.F` (lowercase hex, e.g. `"00:01.0"`). Aggregated by Meta into the platform's PCIe presence-check list alongside per-component `pcie_address` entries. |
69+
| `silicon[].pcie_address` | string, optional | PCIe bus address of the NPU on this platform, format `BB:DD.F` (lowercase hex, e.g. `"00:01.0"`). Aggregated by Meta into the platform's PCIe presence-check list alongside per-component `pcie_address` entries. Omit the field if the address is not yet known — do not supply a placeholder or an empty string, both of which will fail pre-flight. |
5870

5971
> **Component PCIe addresses** are declared per-component (see `pcie_address` in the components section), not here. Meta aggregates the silicon addresses with per-component addresses into the platform's PCIe presence-check list at onboarding time.
6072

fboss/platform/firmware_onboarding/schemas/platform-spec.v1.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"minItems": 1,
2525
"items": {
2626
"type": "object",
27-
"required": ["name", "pcie_address"],
27+
"required": ["name"],
2828
"additionalProperties": false,
2929
"properties": {
3030
"name": {"type": "string", "minLength": 1},

fboss/platform/firmware_onboarding/validator/spec_validator.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,15 @@ class _Silicon(BaseModel):
161161
model_config = ConfigDict(extra="forbid")
162162

163163
name: str = Field(min_length=1)
164-
pcie_address: str
164+
# Optional: omit when the NPU's PCIe address is not yet known. Meta skips
165+
# the entry when aggregating the platform's PCIe presence-check list.
166+
pcie_address: str | None = None
165167

166168
@model_validator(mode="after")
167169
def _check(self) -> _Silicon: # noqa: B902 — pydantic mode="after" instance method
168-
if not _PCIE_ADDRESS_RE.match(self.pcie_address):
170+
if self.pcie_address is not None and not _PCIE_ADDRESS_RE.match(
171+
self.pcie_address
172+
):
169173
raise ValueError(
170174
f"silicon '{self.name}': pcie_address '{self.pcie_address}' "
171175
"is not in BB:DD.F format (lowercase hex)"

fboss/platform/firmware_onboarding/validator/tests/test_spec_validator.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,23 @@ def test_silicon_empty_list_fails(self) -> None:
136136
errors = validate_spec_dict(spec)
137137
self.assertTrue(any("silicon" in e for e in errors))
138138

139-
def test_silicon_missing_pcie_address_fails(self) -> None:
139+
def test_silicon_omitted_pcie_address_validates(self) -> None:
140+
# pcie_address is optional: omit it when the NPU address isn't known yet.
140141
spec = _spec_with()
141142
spec["platform"]["silicon"] = [{"name": "TH6"}]
143+
self.assertEqual([], validate_spec_dict(spec))
144+
145+
def test_silicon_missing_name_fails(self) -> None:
146+
spec = _spec_with()
147+
spec["platform"]["silicon"] = [{"pcie_address": "00:01.0"}]
148+
errors = validate_spec_dict(spec)
149+
self.assertTrue(any("name" in e for e in errors))
150+
151+
def test_silicon_empty_pcie_address_fails(self) -> None:
152+
# An empty string is not an accepted stand-in for "unknown" — omit the
153+
# field instead.
154+
spec = _spec_with()
155+
spec["platform"]["silicon"] = [{"name": "TH6", "pcie_address": ""}]
142156
errors = validate_spec_dict(spec)
143157
self.assertTrue(any("pcie_address" in e for e in errors))
144158

0 commit comments

Comments
 (0)