| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <66dc51e8-751a-d1e2-60b4-3ab27541eae8@amd.com>
Date: Fri, 6 Jan 2023 08:57:21 +0530
From: "Lazar, Lijo" <lijo.lazar@....com>
To: Mario Limonciello <mario.limonciello@....com>,
Alex Deucher <alexander.deucher@....com>,
linux-kernel@...r.kernel.org
Cc: Javier Martinez Canillas <javierm@...hat.com>,
Carlos Soriano Sanchez <csoriano@...hat.com>,
amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, christian.koenig@....com,
"Pan, Xinhui" <Xinhui.Pan@....com>, David Airlie <airlied@...ux.ie>
Subject: Re: [PATCH v7 20/45] drm/amd: Parse both v1 and v2 TA microcode
headers using same function
On 1/5/2023 10:31 PM, Mario Limonciello wrote:
> Several IP versions duplicate code and can't use the common helpers.
> Move this code into a single function so that the helpers can be used.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v6->v7:
> * Drop tags
> * Only set adev->psp.securedisplay_context.context on PSPv12 Renoir and
> PSP v10 which matches previous behavior. If it should match for Cezanne
> and PSPv11 too we can undo this part of the check.
> v5->v6:
> * Rebase on earlier patches
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 123 ++++++++++++++++++------
> drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 64 +-----------
> drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 80 ++-------------
> drivers/gpu/drm/amd/amdgpu/psp_v12_0.c | 66 ++-----------
> 4 files changed, 115 insertions(+), 218 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 7a2fc920739b..bdc2bf87a286 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -3272,41 +3272,76 @@ static int parse_ta_bin_descriptor(struct psp_context *psp,
> return 0;
> }
>
> -int psp_init_ta_microcode(struct psp_context *psp,
> - const char *chip_name)
> +static int parse_ta_v1_microcode(struct psp_context *psp)
> {
> + const struct ta_firmware_header_v1_0 *ta_hdr;
> struct amdgpu_device *adev = psp->adev;
> - char fw_name[PSP_FW_NAME_LEN];
> - const struct ta_firmware_header_v2_0 *ta_hdr;
> - int err = 0;
> - int ta_index = 0;
>
> - if (!chip_name) {
> - dev_err(adev->dev, "invalid chip name for ta microcode\n");
> + ta_hdr = (const struct ta_firmware_header_v1_0 *) adev->psp.ta_fw->data;
> +
> + if (le16_to_cpu(ta_hdr->header.header_version_major) != 1)
> return -EINVAL;
> - }
>
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> - err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> - if (err)
> - goto out;
> + adev->psp.xgmi_context.context.bin_desc.fw_version =
> + le32_to_cpu(ta_hdr->xgmi.fw_version);
> + adev->psp.xgmi_context.context.bin_desc.size_bytes =
> + le32_to_cpu(ta_hdr->xgmi.size_bytes);
> + adev->psp.xgmi_context.context.bin_desc.start_addr =
> + (uint8_t *)ta_hdr +
> + le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
> +
> + adev->psp.ras_context.context.bin_desc.fw_version =
> + le32_to_cpu(ta_hdr->ras.fw_version);
> + adev->psp.ras_context.context.bin_desc.size_bytes =
> + le32_to_cpu(ta_hdr->ras.size_bytes);
> + adev->psp.ras_context.context.bin_desc.start_addr =
> + (uint8_t *)adev->psp.xgmi_context.context.bin_desc.start_addr +
> + le32_to_cpu(ta_hdr->ras.offset_bytes);
> +
> + adev->psp.hdcp_context.context.bin_desc.fw_version =
> + le32_to_cpu(ta_hdr->hdcp.fw_version);
> + adev->psp.hdcp_context.context.bin_desc.size_bytes =
> + le32_to_cpu(ta_hdr->hdcp.size_bytes);
> + adev->psp.hdcp_context.context.bin_desc.start_addr =
> + (uint8_t *)ta_hdr +
> + le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
> +
> + adev->psp.dtm_context.context.bin_desc.fw_version =
> + le32_to_cpu(ta_hdr->dtm.fw_version);
> + adev->psp.dtm_context.context.bin_desc.size_bytes =
> + le32_to_cpu(ta_hdr->dtm.size_bytes);
> + adev->psp.dtm_context.context.bin_desc.start_addr =
> + (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
> + le32_to_cpu(ta_hdr->dtm.offset_bytes);
> +
> + adev->psp.securedisplay_context.context.bin_desc.fw_version =
> + le32_to_cpu(ta_hdr->securedisplay.fw_version);
> + adev->psp.securedisplay_context.context.bin_desc.size_bytes =
> + le32_to_cpu(ta_hdr->securedisplay.size_bytes);
> + adev->psp.securedisplay_context.context.bin_desc.start_addr =
> + (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
> + le32_to_cpu(ta_hdr->securedisplay.offset_bytes);
> +
> + adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
>
> - err = amdgpu_ucode_validate(adev->psp.ta_fw);
> - if (err)
> - goto out;
> + return 0;
> +}
> +
> +static int parse_ta_v2_microcode(struct psp_context *psp)
> +{
> + const struct ta_firmware_header_v2_0 *ta_hdr;
> + struct amdgpu_device *adev = psp->adev;
> + int err = 0;
> + int ta_index = 0;
>
> ta_hdr = (const struct ta_firmware_header_v2_0 *)adev->psp.ta_fw->data;
>
> - if (le16_to_cpu(ta_hdr->header.header_version_major) != 2) {
> - dev_err(adev->dev, "unsupported TA header version\n");
> - err = -EINVAL;
> - goto out;
> - }
> + if (le16_to_cpu(ta_hdr->header.header_version_major) != 2)
> + return -EINVAL;
>
> if (le32_to_cpu(ta_hdr->ta_fw_bin_count) >= UCODE_MAX_PSP_PACKAGING) {
> dev_err(adev->dev, "packed TA count exceeds maximum limit\n");
> - err = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> for (ta_index = 0; ta_index < le32_to_cpu(ta_hdr->ta_fw_bin_count); ta_index++) {
> @@ -3314,14 +3349,46 @@ int psp_init_ta_microcode(struct psp_context *psp,
> &ta_hdr->ta_fw_bin[ta_index],
> ta_hdr);
> if (err)
> - goto out;
> + return err;
> }
>
> return 0;
> -out:
> - dev_err(adev->dev, "fail to initialize ta microcode\n");
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> +}
> +
> +int psp_init_ta_microcode(struct psp_context *psp, const char *chip_name)
> +{
> + const struct common_firmware_header *hdr;
> + struct amdgpu_device *adev = psp->adev;
> + char fw_name[PSP_FW_NAME_LEN];
> + int err;
> +
> + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> + err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> + if (err)
> + return err;
> + err = amdgpu_ucode_validate(adev->psp.ta_fw);
> + if (err)
> + return err;
> +
> + hdr = (const struct common_firmware_header *)adev->psp.ta_fw->data;
> + switch (le16_to_cpu(hdr->header_version_major)) {
> + case 1:
> + err = parse_ta_v1_microcode(psp);
> + break;
> + case 2:
> + err = parse_ta_v2_microcode(psp);
> + break;
> + default:
> + dev_err(adev->dev, "unsupported TA header version\n");
> + err = -EINVAL;
> + }
> +
> + if (err) {
> + dev_err(adev->dev, "fail to initialize ta microcode\n");
> + release_firmware(adev->psp.ta_fw);
> + adev->psp.ta_fw = NULL;
> + }
> +
> return err;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index 9de46fa8f46c..f14fcfb9c425 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -48,9 +48,8 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
> {
> struct amdgpu_device *adev = psp->adev;
> const char *chip_name;
> - char fw_name[30];
> + char ucode_prefix[30];
> int err = 0;
> - const struct ta_firmware_header_v1_0 *ta_hdr;
> DRM_DEBUG("\n");
>
> switch (adev->asic_type) {
> @@ -64,66 +63,13 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
> break;
> default: BUG();
> }
> + amdgpu_ucode_ip_version_decode(adev, MP0_HWIP, ucode_prefix, sizeof(ucode_prefix));
>
> - err = psp_init_asd_microcode(psp, chip_name);
> + err = psp_init_asd_microcode(psp, ucode_prefix);
> if (err)
> - goto out;
> -
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> - err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> - if (err) {
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> - dev_info(adev->dev,
> - "psp v10.0: Failed to load firmware \"%s\"\n",
> - fw_name);
> - } else {
> - err = amdgpu_ucode_validate(adev->psp.ta_fw);
> - if (err)
> - goto out2;
> -
> - ta_hdr = (const struct ta_firmware_header_v1_0 *)
> - adev->psp.ta_fw->data;
> - adev->psp.hdcp_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->hdcp.fw_version);
> - adev->psp.hdcp_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->hdcp.size_bytes);
> - adev->psp.hdcp_context.context.bin_desc.start_addr =
> - (uint8_t *)ta_hdr +
> - le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
> -
> - adev->psp.dtm_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->dtm.fw_version);
> - adev->psp.dtm_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->dtm.size_bytes);
> - adev->psp.dtm_context.context.bin_desc.start_addr =
> - (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
> - le32_to_cpu(ta_hdr->dtm.offset_bytes);
> -
> - adev->psp.securedisplay_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->securedisplay.fw_version);
> - adev->psp.securedisplay_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->securedisplay.size_bytes);
> - adev->psp.securedisplay_context.context.bin_desc.start_addr =
> - (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
> - le32_to_cpu(ta_hdr->securedisplay.offset_bytes);
> -
> - adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
> - }
> -
> - return 0;
> -
> -out2:
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> -out:
> - if (err) {
> - dev_err(adev->dev,
> - "psp v10.0: Failed to load firmware \"%s\"\n",
> - fw_name);
> - }
> + return err;
>
> - return err;
> + return psp_init_ta_microcode(psp, ucode_prefix);
> }
>
> static int psp_v10_0_ring_create(struct psp_context *psp,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index bd3e3e23a939..41e29b777666 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -89,9 +89,8 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
> {
> struct amdgpu_device *adev = psp->adev;
> const char *chip_name;
> - char fw_name[PSP_FW_NAME_LEN];
> + char ucode_prefix[30];
> int err = 0;
> - const struct ta_firmware_header_v1_0 *ta_hdr;
>
> DRM_DEBUG("\n");
>
> @@ -129,7 +128,7 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
> default:
> BUG();
> }
> -
> + amdgpu_ucode_ip_version_decode(adev, MP0_HWIP, ucode_prefix, sizeof(ucode_prefix));
>
> switch (adev->ip_versions[MP0_HWIP][0]) {
> case IP_VERSION(11, 0, 2):
> @@ -140,35 +139,8 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
> err = psp_init_asd_microcode(psp, chip_name);
> if (err)
> return err;
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> - err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> - if (err) {
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> - dev_info(adev->dev,
> - "psp v11.0: Failed to load firmware \"%s\"\n", fw_name);
> - } else {
> - err = amdgpu_ucode_validate(adev->psp.ta_fw);
> - if (err)
> - goto out2;
> -
> - ta_hdr = (const struct ta_firmware_header_v1_0 *)adev->psp.ta_fw->data;
> - adev->psp.xgmi_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->xgmi.fw_version);
> - adev->psp.xgmi_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->xgmi.size_bytes);
> - adev->psp.xgmi_context.context.bin_desc.start_addr =
> - (uint8_t *)ta_hdr +
> - le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
> - adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
> - adev->psp.ras_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->ras.fw_version);
> - adev->psp.ras_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->ras.size_bytes);
> - adev->psp.ras_context.context.bin_desc.start_addr =
> - (uint8_t *)adev->psp.xgmi_context.context.bin_desc.start_addr +
> - le32_to_cpu(ta_hdr->ras.offset_bytes);
> - }
> + err = psp_init_ta_microcode(psp, ucode_prefix);
> + adev->psp.securedisplay_context.context.bin_desc.size_bytes = 0;
> break;
> case IP_VERSION(11, 0, 0):
> case IP_VERSION(11, 0, 5):
> @@ -179,39 +151,8 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
> err = psp_init_asd_microcode(psp, chip_name);
> if (err)
> return err;
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> - err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> - if (err) {
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> - dev_info(adev->dev,
> - "psp v11.0: Failed to load firmware \"%s\"\n", fw_name);
> - } else {
> - err = amdgpu_ucode_validate(adev->psp.ta_fw);
> - if (err)
> - goto out2;
> -
> - ta_hdr = (const struct ta_firmware_header_v1_0 *)adev->psp.ta_fw->data;
> - adev->psp.hdcp_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->hdcp.fw_version);
> - adev->psp.hdcp_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->hdcp.size_bytes);
> - adev->psp.hdcp_context.context.bin_desc.start_addr =
> - (uint8_t *)ta_hdr +
> - le32_to_cpu(
> - ta_hdr->header.ucode_array_offset_bytes);
> -
> - adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
> -
> - adev->psp.dtm_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->dtm.fw_version);
> - adev->psp.dtm_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->dtm.size_bytes);
> - adev->psp.dtm_context.context.bin_desc.start_addr =
> - (uint8_t *)adev->psp.hdcp_context.context
> - .bin_desc.start_addr +
> - le32_to_cpu(ta_hdr->dtm.offset_bytes);
> - }
> + err = psp_init_ta_microcode(psp, ucode_prefix);
> + adev->psp.securedisplay_context.context.bin_desc.size_bytes = 0;
> break;
> case IP_VERSION(11, 0, 7):
> case IP_VERSION(11, 0, 11):
> @@ -221,26 +162,17 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
> if (err)
> return err;
> err = psp_init_ta_microcode(psp, chip_name);
> - if (err)
> - return err;
> break;
> case IP_VERSION(11, 5, 0):
> err = psp_init_asd_microcode(psp, chip_name);
> if (err)
> return err;
> err = psp_init_toc_microcode(psp, chip_name);
> - if (err)
> - return err;
> break;
> default:
> BUG();
> }
>
> - return 0;
> -
> -out2:
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> return err;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> index 8ed2281b6557..67118e699219 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> @@ -49,9 +49,8 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
> {
> struct amdgpu_device *adev = psp->adev;
> const char *chip_name;
> - char fw_name[30];
> + char ucode_prefix[30];
> int err = 0;
> - const struct ta_firmware_header_v1_0 *ta_hdr;
> DRM_DEBUG("\n");
>
> switch (adev->asic_type) {
> @@ -64,67 +63,20 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
> default:
> BUG();
> }
> + amdgpu_ucode_ip_version_decode(adev, MP0_HWIP, ucode_prefix, sizeof(ucode_prefix));
>
> err = psp_init_asd_microcode(psp, chip_name);
> + if (err)
> + return err;
> + err = psp_init_ta_microcode(psp, ucode_prefix);
> if (err)
> return err;
>
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> - err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> - if (err) {
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> - dev_info(adev->dev,
> - "psp v12.0: Failed to load firmware \"%s\"\n",
> - fw_name);
> - } else {
> - err = amdgpu_ucode_validate(adev->psp.ta_fw);
> - if (err)
> - goto out;
> -
> - ta_hdr = (const struct ta_firmware_header_v1_0 *)
> - adev->psp.ta_fw->data;
> - adev->psp.hdcp_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->hdcp.fw_version);
> - adev->psp.hdcp_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->hdcp.size_bytes);
> - adev->psp.hdcp_context.context.bin_desc.start_addr =
> - (uint8_t *)ta_hdr +
> - le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
> -
> - adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
> -
> - adev->psp.dtm_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->dtm.fw_version);
> - adev->psp.dtm_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->dtm.size_bytes);
> - adev->psp.dtm_context.context.bin_desc.start_addr =
> - (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
> - le32_to_cpu(ta_hdr->dtm.offset_bytes);
> -
> - if (adev->apu_flags & AMD_APU_IS_RENOIR) {
> - adev->psp.securedisplay_context.context.bin_desc.fw_version =
> - le32_to_cpu(ta_hdr->securedisplay.fw_version);
> - adev->psp.securedisplay_context.context.bin_desc.size_bytes =
> - le32_to_cpu(ta_hdr->securedisplay.size_bytes);
> - adev->psp.securedisplay_context.context.bin_desc.start_addr =
> - (uint8_t *)adev->psp.hdcp_context.context.bin_desc.start_addr +
> - le32_to_cpu(ta_hdr->securedisplay.offset_bytes);
> - }
> - }
> -
> - return 0;
> -
> -out:
> - release_firmware(adev->psp.ta_fw);
> - adev->psp.ta_fw = NULL;
> - if (err) {
> - dev_err(adev->dev,
> - "psp v12.0: Failed to load firmware \"%s\"\n",
> - fw_name);
> - }
> + /* only supported on renoir */
> + if (!(adev->apu_flags & AMD_APU_IS_RENOIR))
> + adev->psp.securedisplay_context.context.bin_desc.size_bytes = 0;
>
> - return err;
> + return psp_init_ta_microcode(psp, ucode_prefix);
I meant the below lines to come after this call. Even if it gets some
value during parsing for other SOCs, force to 0.
if (!(adev->apu_flags & AMD_APU_IS_RENOIR))
adev->psp.securedisplay_context.context.bin_desc.size_bytes = 0;
Thanks,
Lijo
> }
>
> static int psp_v12_0_bootloader_load_sysdrv(struct psp_context *psp)
Powered by blists - more mailing lists