Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <151bb1ab-8b2b-afaf-2976-5f60b756c4ca@amd.com>
Date:   Wed, 4 Jan 2023 10:23:10 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH v4 05/27] drm/amd: Add a new helper for loading/validating
 microcode
Content-Language: en-US
To:     Mario Limonciello <mario.limonciello@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        linux-kernel@vger.kernel.org
Cc:     Javier Martinez Canillas <javierm@redhat.com>,
        Carlos Soriano Sanchez <csoriano@redhat.com>,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, christian.koenig@amd.com,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>
References: <20230103221852.22813-1-mario.limonciello@amd.com>
 <20230103221852.22813-6-mario.limonciello@amd.com>
From:   "Lazar, Lijo" <lijo.lazar@amd.com>
In-Reply-To: <20230103221852.22813-6-mario.limonciello@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bFBJQTlYWnZibkUrNFE1MFgzOG1BaStkQjNkbjAyOUZESXZZKzFPdWZQRnVX?=
 =?utf-8?B?YTVLVmdjMzluSzdlcVQwUmdJRUlRcFFqcVhRaHE2WG54cHc1dHFUTkNWQ3Br?=
 =?utf-8?B?QXRkTHBOaFgxMFJwU2NRRDY4RHR1NGdNWFduQStMMUk4b1pnSmxkcTlmY2JS?=
 =?utf-8?B?QmZPRThLYWJMekZmUFZtVHVRTXVqZGNVT1FkY2c5ZTBzM1dscWpTbWpWTSty?=
 =?utf-8?B?VnBuWTg5a1ovOGg4bEhKSGVxSjJTYWxZV0I3Z0d4OXdHNHcya1ZzSFhwdE9W?=
 =?utf-8?B?R2R6dGd5amljSW5yUjZyVnBQdGpac3FDNlRaL2NzTk1DZExxR21NZ2FSVUpl?=
 =?utf-8?B?WlRNczQyYWxHSFJYTG04dEJFTGR6Vi9CTzlBd2wrVGlmbjRmU2hpTW9yWVZo?=
 =?utf-8?B?V1dEclhMS2N4SHAzS2piYW1uUEE0T3lxTDNWSlh4S3VkQnpYVWh2Nk1BTlI0?=
 =?utf-8?B?RUhTUkZqMzkvZDB5ZFo0Q3ZUN3RYR3E5cWdGMTlvU1FId2FObXVTK2g5bDJY?=
 =?utf-8?B?YkhWZTNIWjRScllzSXQ1YkJNaFNEUm5paXJTanNvd0h5OXZRclVHeVZHOHB3?=
 =?utf-8?B?K0cyT3YxUFpVNENJZzlFNEU4cEROUzZIbnArL2p0RnpyWVFuT1ZyeVlnOXRX?=
 =?utf-8?B?WXRMOHJoZkNIRDN6dnJzdnlQTlN5bUxTK01QUkhUeWhNKzlGdWlkbjIwSXBX?=
 =?utf-8?B?aTlzMzNJdXVCcURodytpWURyNEhIOVU4bE1rVTNKY1pxNkljWHFmU2hDRXlo?=
 =?utf-8?B?aStCNVgzcytPMk12ZTErMHVGTGZSdXlIRHhqU2hxWW1PMy9JNWw2cS82L3Zu?=
 =?utf-8?B?OENBczNpVFB5OVVycFc5V0FDTjZ1WDA3Y2VhNEsyZnIyNnBzaU5UZko0OHF5?=
 =?utf-8?B?dElqL0dXS09zcjBkamJtbS9kRjVZSDY3TXRLSnRMZWEzZkRNbS9nNHNyZTht?=
 =?utf-8?B?Uy9CM09lYWpOb0JqTWM1RW5xR3g0bTVWS0RJQUY1MS9YOWs0bFBsSGJ6eGtt?=
 =?utf-8?B?KzI0cE1yRmk1Yzg0ZER2YklYU2ZzMm5oSFUyZDZrTmx5VjAyc3FTbVRvclVt?=
 =?utf-8?B?aTZYOHVKSUhvb2ptYXFmc3dNZ2hJM0ROYmdvNEQ1ck9JbENCOXV5cXFLT2FW?=
 =?utf-8?B?N2dhbTNRdXFuK3BZR0tlSUtwUStIQ05aRHdlT1dSR0RYdFVCaFlmSkY5dENi?=
 =?utf-8?B?MElGYThtVGxHWE56cDJwNFRHZUhGbXMvOGlyY0VUSk9oRHhmcGptRzBSNVRq?=
 =?utf-8?B?LzBVQ2QwQ2JXRWIzZHYxdTl5alFRM0l5YXlsaHo0VmFDb1N0RTV2V0dGb2xB?=
 =?utf-8?B?TkswUjhTenBUVFBCSHBzVVdzZzNnSmpzM2Ftci93K3hSRFRsMXk4QUlhSk9r?=
 =?utf-8?B?SURBU2VGZlhzTVNUaTRBMmhmMXJMdDhyTHlPMkFkQTBadG9UbW5HR2E4K3lH?=
 =?utf-8?B?NitQRGtwa2xDdURkL0I4UTJsU3l2MUNZSkwxRDhBT2luZGZLYVZXVnpOUjd6?=
 =?utf-8?B?WFE0TjRYZzJYQ0RIRjBUSTR0bjJuNzNWekdTdWlSQm5xOVBDWUpXMzZEaGxo?=
 =?utf-8?B?NlhQTXJZNlp5dzlDNlZVUVRtS01RWXExK2s5TkRCWlJVa3JrMURTdDViNlNk?=
 =?utf-8?B?UnlyQWNwME1mbVVZZ0pmcEhXT2ZqeTVtVCt6K0tDcmlyeFNNaXhPbkd4WmZD?=
 =?utf-8?B?Mk5nUmc5aEltdHF6TDRNekoxVzgzRHNLWjFUajVVTWIrZmhxZlRRdEpqN3dB?=
 =?utf-8?B?bi96RGxNOGVodVlOYjZkdFFTemZKY1NzZ2xjSnhWd0NxL090aSttZ2RBOXdk?=
 =?utf-8?B?b0lpN3FQVStTb25sVElYdTJBN3NEMFVWWVg0U0dSeit4NjFtY1Y0bmp2M2Fa?=
 =?utf-8?B?SkZXekdrMnYwa1VqaTNIZXVha1dYNFRZRU9pOFNyRWJjVFE5WDU1R2VTaWJp?=
 =?utf-8?B?Q1JuZzY5SzY2Wnk0YnJSTDhzaWdEb1grTG9hNmpCS0RnRW11ZG51VFpUR0pI?=
 =?utf-8?B?VmZURzhGckY0Nm9qSTQ4RGxGTUdkNkppeG9QRFB3K29teEY0cXkycjZMZTlF?=
 =?utf-8?B?cVJnQUN5UWI3UWp2bVdzVzRjakdmSUg1djVkS3dYWkZUODlJcENqTk55MXQ5?=
 =?utf-8?Q?ALhvJOL8zMtqGPPThfIE381gx?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bedb6ea3-cf88-4216-fead-08daee0f9bb0
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB4614.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2023 04:53:23.4240
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: vpNx/EgK6ffaVZoLsnII5vy1D3LLqjh+8krtobOnP0M2SPqlE3uZUVuaUudTgTin
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5858
X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 1/4/2023 3:48 AM, Mario Limonciello wrote:
> All microcode runs a basic validation after it's been loaded. Each
> IP block as part of init will run both.
> 
> Introduce a wrapper for request_firmware and amdgpu_ucode_validate.
> This wrapper will also remap any error codes from request_firmware
> to -ENODEV.  This is so that early_init will fail if firmware couldn't
> be loaded instead of the IP block being disabled.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v3-v4:
>   * New patch
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 24 +++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  1 +
>   2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index eafcddce58d3..8c4a7b09e344 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -1312,3 +1312,27 @@ void amdgpu_ucode_ip_version_decode(struct amdgpu_device *adev, int block_type,
>   
>   	snprintf(ucode_prefix, len, "%s_%d_%d_%d", ip_name, maj, min, rev);
>   }
> +
> +/*
> + * amdgpu_ucode_load - Load and validate amdgpu microcode
> + *
> + * @adev: amdgpu device
> + * @fw: pointer to load firmware to
> + * @fw_name: firmware to load
> + *
> + * This is a helper that will use request_firmware and amdgpu_ucode_validate
> + * to load and run basic validation on firmware. If the load fails, remap
> + * the error code to -ENODEV, so that early_init functions will fail to load.
> + */
> +int amdgpu_ucode_load(struct amdgpu_device *adev, const struct firmware **fw, char *fw_name)

'load' also takes a different meaning of loading firmware to ASIC. Maybe 
keep it as 'get' and keep another corresponding common 'put' for 
release_firmware?

Thanks,
Lijo

> +{
> +	int err = request_firmware(fw, fw_name, adev->dev);
> +
> +	if (err)
> +		return -ENODEV;
> +	err = amdgpu_ucode_validate(*fw);
> +	if (err)
> +		dev_dbg(adev->dev, "\"%s\" failed to validate\n", fw_name);
> +
> +	return err;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index 552e06929229..b9139fb44506 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -544,6 +544,7 @@ void amdgpu_ucode_print_sdma_hdr(const struct common_firmware_header *hdr);
>   void amdgpu_ucode_print_psp_hdr(const struct common_firmware_header *hdr);
>   void amdgpu_ucode_print_gpu_info_hdr(const struct common_firmware_header *hdr);
>   int amdgpu_ucode_validate(const struct firmware *fw);
> +int amdgpu_ucode_load(struct amdgpu_device *adev, const struct firmware **fw, char *fw_name);
>   bool amdgpu_ucode_hdr_version(union amdgpu_firmware_header *hdr,
>   				uint16_t hdr_major, uint16_t hdr_minor);
>