Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The BAR index calculation for NVKM_BAR3_IO lacks bounds checking, leading to PCI ROM resource aliasing if all preceding BARs are 64-bit. --
--- Patch [5]: [PATCH 5/6] nouveau/gsp: add some more fields to r570 system info --- commit 18c8db5e682be4972a386c192c00409e01054607 Author: Dave Airlie <[email protected]> nouveau/gsp: add some more fields to r570 system info This commit adds the pci io bar, link cap and os page size to the r570 system info. > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c > b/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c > --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/pci.c [ ... ] > @@ -1576,6 +1576,10 @@ nvkm_device_pci_resource_idx(struct nvkm_device_pci > *pdev, enum nvkm_bar_id bar) > if (bar == NVKM_BAR2_INST) > return idx; > > + idx += (pci_resource_flags(pdev->pdev, idx) & IORESOURCE_MEM_64) ? 2 : > 1; > + if (bar == NVKM_BAR3_IO) > + return idx; [Severity: Medium] Does this calculation lack a bounds check against PCI_STD_NUM_BARS? If a GPU is configured with three 64-bit BARs (BAR0, BAR1, and BAR2), the calculation idx += 2 will occur three times. This results in idx becoming 6. Since index 6 corresponds to PCI_ROM_RESOURCE, could this cause the IO BAR lookup to accidentally alias with the PCI Expansion ROM resource? > + > WARN_ON(1); > return -1; > } [ ... ] > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c [ ... ] > @@ -165,6 +165,7 @@ r570_gsp_set_system_info(struct nvkm_gsp *gsp) > info->gpuPhysAddr = device->func->resource_addr(device, NVKM_BAR0_PRI); > info->gpuPhysFbAddr = device->func->resource_addr(device, NVKM_BAR1_FB); > info->gpuPhysInstAddr = device->func->resource_addr(device, > NVKM_BAR2_INST); > + info->gpuPhysIoAddr = device->func->resource_addr(device, NVKM_BAR3_IO); [Severity: Medium] If the above index calculation results in 6 due to having three 64-bit BARs, this assignment will populate info->gpuPhysIoAddr with the physical address of the PCI Expansion ROM instead of a non-existent IO BAR. Could exposing this incorrect hardware layout cause undefined firmware behavior or hardware faults if the GSP firmware attempts to access the ROM address assuming it is an IO space? > info->nvDomainBusDeviceFunc = pci_dev_id(pdev); > info->maxUserVa = TASK_SIZE; > info->pciConfigMirrorBase = device->pci->func->cfg.addr; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
