From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: iwx: support pnvm data in firmware image
To: Peter Hessler <phessler@theapt.org>
Cc: tech@openbsd.org
Date: Thu, 05 Mar 2026 20:03:55 +0100

> Date: Thu, 5 Mar 2026 12:50:44 +0100
> From: Peter Hessler <phessler@theapt.org>
> 
> On 2026 Mar 03 (Tue) at 15:25:49 +0100 (+0100), Stefan Sperling wrote:
> :Newer firmware images for BZ devices have a segment which contains
> :data that was previously distributed in separate .pnvm files.
> :
> :If the firmware image contains pnvm data we must use it instead of
> :the external file. The external file might be incompatible.
> :
> :ok?
> 
> OK

Missed this one for some reason.  Works on MA and looks good to me.

ok kettenis@

> :diff /usr/src
> :path + /usr/src
> :commit - c58e64aeb4e092373382ec4900136445f091367c
> :blob - da9e3d2d42cdbb343313ea89ddfa6a89e1d39a31
> :file + sys/dev/pci/if_iwx.c
> :--- sys/dev/pci/if_iwx.c
> :+++ sys/dev/pci/if_iwx.c
> :@@ -1183,6 +1183,9 @@ iwx_fw_info_free(struct iwx_fw_info *fw)
> : 	free(fw->iml, M_DEVBUF, fw->iml_len);
> : 	fw->iml = NULL;
> : 	fw->iml_len = 0;
> :+	free(fw->pnvm, M_DEVBUF, fw->pnvm_len);
> :+	fw->pnvm = NULL;
> :+	fw->pnvm_len = 0;
> : }
> : 
> : #define IWX_FW_ADDR_CACHE_CONTROL 0xC0000000
> :@@ -1550,6 +1553,19 @@ iwx_read_firmware(struct iwx_softc *sc)
> : 		case IWX_UCODE_TLV_FW_RECOVERY_INFO:
> : 			break;
> : 
> :+		case IWX_UCODE_TLV_PNVM_DATA:
> :+			if (fw->pnvm != NULL)
> :+				break;
> :+			fw->pnvm = malloc(tlv_len, M_DEVBUF,
> :+			    M_WAITOK | M_CANFAIL);
> :+			if (fw->pnvm == NULL) {
> :+				err = ENOMEM;
> :+				goto parse_out;
> :+			}
> :+			memcpy(fw->pnvm, tlv_data, tlv_len);
> :+			fw->pnvm_len = tlv_len;
> :+			break;
> :+
> : 		case IWX_UCODE_TLV_FW_FSEQ_VERSION:
> : 		case IWX_UCODE_TLV_PHY_INTEGRATION_VERSION:
> : 		case IWX_UCODE_TLV_FW_NUM_STATIONS:
> :@@ -4294,6 +4310,7 @@ iwx_load_pnvm(struct iwx_softc *sc)
> : 	int s, err = 0;
> : 	u_char *pnvm_data = NULL;
> : 	size_t pnvm_size = 0;
> :+	struct iwx_fw_info *fw = &sc->sc_fw;
> : 
> : 	if (sc->sc_sku_id[0] == 0 &&
> : 	    sc->sc_sku_id[1] == 0 &&
> :@@ -4302,18 +4319,28 @@ iwx_load_pnvm(struct iwx_softc *sc)
> : 
> : 	if (sc->sc_pnvm_name) {
> : 		if (sc->pnvm_dma.vaddr == NULL) {
> :-			err = loadfirmware(sc->sc_pnvm_name,
> :-			    &pnvm_data, &pnvm_size);
> :-			if (err) {
> :-				printf("%s: could not read %s (error %d)\n",
> :-				    DEVNAME(sc), sc->sc_pnvm_name, err);
> :-				return err;
> :-			}
> :+			/* Prefer PNVM data embedded in firmware image. */
> :+			if (fw->pnvm) {
> :+				err = iwx_pnvm_parse(sc, fw->pnvm,
> :+				    fw->pnvm_len);
> :+				if (err && err != ENOENT)
> :+					return err;
> :+			} else {
> :+				err = loadfirmware(sc->sc_pnvm_name,
> :+				    &pnvm_data, &pnvm_size);
> :+				if (err) {
> :+					printf("%s: could not read %s "
> :+					    "(error %d)\n",
> :+					    DEVNAME(sc), sc->sc_pnvm_name,
> :+					    err);
> :+					return err;
> :+				}
> : 
> :-			err = iwx_pnvm_parse(sc, pnvm_data, pnvm_size);
> :-			if (err && err != ENOENT) {
> :-				free(pnvm_data, M_DEVBUF, pnvm_size);
> :-				return err;
> :+				err = iwx_pnvm_parse(sc, pnvm_data, pnvm_size);
> :+				if (err && err != ENOENT) {
> :+					free(pnvm_data, M_DEVBUF, pnvm_size);
> :+					return err;
> :+				}
> : 			}
> : 		} else
> : 			iwx_ctxt_info_gen3_set_pnvm(sc);
> :commit - c58e64aeb4e092373382ec4900136445f091367c
> :blob - 1af2759f445d0b05c35efd4ec8ff9ddab3ca419c
> :file + sys/dev/pci/if_iwxreg.h
> :--- sys/dev/pci/if_iwxreg.h
> :+++ sys/dev/pci/if_iwxreg.h
> :@@ -1584,6 +1584,7 @@ struct iwx_ucode_header {
> : #define IWX_UCODE_TLV_SEC_TABLE_ADDR		66
> : #define IWX_UCODE_TLV_D3_KEK_KCK_ADDR		67
> : #define IWX_UCODE_TLV_CURRENT_PC		68
> :+#define IWX_UCODE_TLV_PNVM_DATA			74
> : 
> : #define IWX_UCODE_TLV_CONST_BASE		0x100
> : #define IWX_UCODE_TLV_FW_NUM_STATIONS		(IWX_UCODE_TLV_CONST_BASE + 0)
> :commit - c58e64aeb4e092373382ec4900136445f091367c
> :blob - 7be82818bc54423e510492e7b7e8974a0f63000d
> :file + sys/dev/pci/if_iwxvar.h
> :--- sys/dev/pci/if_iwxvar.h
> :+++ sys/dev/pci/if_iwxvar.h
> :@@ -173,6 +173,13 @@ struct iwx_fw_info {
> : 	/* Copy of firmware image loader found in file. */
> : 	uint8_t *iml;
> : 	size_t iml_len;
> :+
> :+	/*
> :+	 * Copy of PNVM image found in file.
> :+	 * Used in preference to external .pnvm file if present.
> :+	 */
> :+	uint8_t *pnvm;
> :+	size_t pnvm_len;
> : };
> : 
> : struct iwx_nvm_data {
> :
> 
>