[AMD Official Use Only - AMD Internal Distribution Only]
>-----Original Message-----
>From: Pan, Ellen <[email protected]>
>Sent: Saturday, October 11, 2025 12:19 AM
>To: [email protected]
>Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
><[email protected]>; Lazar, Lijo <[email protected]>; Chan, Hing
>Pong <[email protected]>; Pan, Ellen <[email protected]>
>Subject: [PATCH v3 5/6] drm/amdgpu: Add logic for VF ipd and VF bios to init
>from dynamic crit_region offsets
>
>1. Added VF logic in amdgpu_virt to init IP discovery using the offsets from
>dynamic(v2) critical regions; 2. Added VF logic in amdgpu_virt to init bios
>image
>using the offsets from dynamic(v2) critical regions;
>
>Signed-off-by: Ellen Pan <[email protected]>
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 17 +++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 11 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 53
>+++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +
> 4 files changed, 80 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>index 00e96419fcda..41f8fe04126f 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>@@ -96,7 +96,8 @@ void amdgpu_bios_release(struct amdgpu_device *adev)
> * part of the system bios. On boot, the system bios puts a
> * copy of the igp rom at the start of vram if a discrete card is
> * present.
>- * For SR-IOV, the vbios image is also put in VRAM in the VF.
>+ * For SR-IOV, if dynamic critical region is not enabled,
>+ * the vbios image is also put at the start of VRAM in the VF.
> */
> static bool amdgpu_read_bios_from_vram(struct amdgpu_device *adev) {
>@@ -114,7 +115,19 @@ static bool amdgpu_read_bios_from_vram(struct
>amdgpu_device *adev)
>
> adev->bios = NULL;
> vram_base = pci_resource_start(adev->pdev, 0);
>- bios = ioremap_wc(vram_base, size);
>+
>+ /* For SR-IOV, if dynamic critical region is enabled,
>+ * the vbios image is put at a dynamic offset of VRAM in the VF.
>+ * If dynamic critical region is disabled, exit early to proceed
>+ * the same seq as on baremetal.
>+ */
>+ if (amdgpu_sriov_vf(adev) && adev-
>>virt.is_dynamic_crit_regn_enabled) {
>+ if (amdgpu_virt_get_dynamic_data_info(adev,
>AMD_SRIOV_MSG_VBIOS_IMG_TABLE_ID,
>+ (uint8_t *)&bios, &size))
[lijo]
This doesn't look correct. Please be consistent with the meaning of the
parameter of this function. If it's just a uint8_t *, pass the buffer which is
allocated outside and the function fills it. If that's the route taken, then
you could allocate adev->bios with arbitrary size, pass it and do a realloc if
required. The size param will then be in/out (as out param, it will return the
actual size). If it's a uint8_t **, then the allocation can be done inside the
function.
>+ return false;
>+ } else
>+ bios = ioremap_wc(vram_base, size);
>+
> if (!bios)
> return false;
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>index 73401f0aeb34..23aec57295c0 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>@@ -283,7 +283,6 @@ static int
>amdgpu_discovery_read_binary_from_mem(struct amdgpu_device *adev,
> * wait for this to complete. Once the C2PMSG is updated, we
>can
> * continue.
> */
>-
> for (i = 0; i < 2000; i++) {
> msg = RREG32(mmMP0_SMN_C2PMSG_33);
> if (msg & 0x80000000)
>@@ -292,6 +291,16 @@ static int
>amdgpu_discovery_read_binary_from_mem(struct amdgpu_device *adev,
> }
> }
>
>+ /* For SRIOV VFs, if dynamic critical region is enabled,
>+ * IPD binary is retrieved via this call.
>+ * If dynamic critical is disabled, fallthrough to normal seq below.
>+ */
>+ if (amdgpu_sriov_vf(adev) && adev-
>>virt.is_dynamic_crit_regn_enabled) {
>+ ret = amdgpu_virt_get_dynamic_data_info(adev,
>+ AMD_SRIOV_MSG_IPD_TABLE_ID, binary,
>NULL);
>+ return ret;
>+ }
>+
> vram_size = RREG32(mmRCC_CONFIG_MEMSIZE);
> if (!vram_size || vram_size == U32_MAX)
> sz_valid = false;
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>index 461e83728594..4a7125122ae7 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>@@ -965,6 +965,59 @@ int amdgpu_virt_init_critical_region(struct
>amdgpu_device *adev)
> return r;
> }
>
>+int amdgpu_virt_get_dynamic_data_info(struct amdgpu_device *adev,
>+ int data_id, uint8_t *binary, uint64_t *size) {
>+ uint32_t data_offset = 0;
>+ uint32_t data_size = 0;
>+ enum amd_sriov_msg_table_id_enum data_table_id = data_id;
>+ char *data_name;
>+ uint8_t __iomem *buf;
>+
>+ if (data_table_id >= AMD_SRIOV_MSG_MAX_TABLE_ID)
>+ return -EINVAL;
>+
>+ data_offset = adev->virt.crit_regn_tbl[data_table_id].offset;
>+ data_size = adev->virt.crit_regn_tbl[data_table_id].size_kb << 10;
>+
>+ switch (data_id) {
>+ case AMD_SRIOV_MSG_IPD_TABLE_ID:
>+ data_name = "IPD";
>+ if (!IS_ALIGNED(data_offset, 4) || !IS_ALIGNED(data_size, 4)) {
>+ dev_err(adev->dev, "IP discovery data not aligned to 4
>bytes\n");
>+ return -EINVAL;
>+ }
>+
>+ if (data_size > DISCOVERY_TMR_SIZE) {
>+ dev_err(adev->dev, "Invalid IP discovery size: 0x%x\n",
>data_size);
[lijo]
This looks like validation of table entry. I think that's better done at the
place where you fill in table entries - amdgpu_virt_init_critical_region.
>+ return -EINVAL;
>+ }
>+
>+ amdgpu_device_vram_access(adev,
>+ (uint64_t)data_offset, (uint32_t *)binary,
>data_size, false);
[lijo]
There is no NULL check for the binary or validation of size. Better the size be
taken always as in/out parameter.
>+ break;
>+
>+ case AMD_SRIOV_MSG_VBIOS_IMG_TABLE_ID:
>+ data_name = "BIOS";
>+ if (data_size > *size) {
>+ dev_err(adev->dev, "Invalid vbios size: 0x%x\n",
>data_size);
>+ return -EINVAL;
>+ }
>+
>+ buf = ioremap_wc(pci_resource_start(adev->pdev, 0) +
>data_offset,
>+data_size);
[lijo]
This doesn't look correct. You just need to fill in the binary with
amdgpu_device_vram_access().
Thanks,
Lijo
>+
>+ *(uint8_t __iomem **)binary = buf;
>+ *size = (uint64_t)data_size;
>+ break;
>+ }
>+
>+ dev_info(adev->dev,
>+ "Got %s info from dynamic crit_region_table at offset 0x%x
>with size of 0x%x bytes.\n",
>+ data_name, data_offset, data_size);
>+
>+ return 0;
>+}
>+
> void amdgpu_virt_init(struct amdgpu_device *adev) {
> bool is_sriov = false;
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>index f46edc03f57f..5d8e3260f677 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>@@ -438,6 +438,8 @@ void amdgpu_virt_fini_data_exchange(struct
>amdgpu_device *adev); void amdgpu_virt_init(struct amdgpu_device *adev);
>
> int amdgpu_virt_init_critical_region(struct amdgpu_device *adev);
>+int amdgpu_virt_get_dynamic_data_info(struct amdgpu_device *adev,
>+ int data_id, uint8_t *binary, uint64_t *size);
>
> bool amdgpu_virt_can_access_debugfs(struct amdgpu_device *adev); int
>amdgpu_virt_enable_access_debugfs(struct amdgpu_device *adev);
>--
>2.34.1
