Re: [Patch v4 7/7] crypto/ccp: Implement SNP Download Firmware EX

[Pratik R. Sampat]

On 10/15/26 9:22 PM, Shantanu Sinha wrote:
> On Thu, Oct 15, 2026 at 05:09:38PM -0400, Pratik R. Sampat wrote:
>> On 10/15/26 3:38 PM, Tycho Andersen wrote:
>>>> +	/*
>>>> +	 * Firmware either just requested a rollback or a rollback attempt
>>>> +	 * failed. Keep the PSP dead for all other commands, only another
>>>> +	 * DLFW_EX attempt is allowed through.
>>>> +	 */
>>>> +	if (sev->fwl_rollback_required) {
>>>> +		psp_dead = true;
>>>> +		if (fwl_ret != FW_UPLOAD_ERR_HW_ERROR)
>>>> +			fwl_ret = FW_UPLOAD_ERR_RW_ERROR;
>>>
>>> I think this error switcharoo could use a comment (or maybe just be
>>> dropped?). e.g. if the user does something to set
>>> ->fwl_rollback_required and then tries another firmware upload that is
>>> the wrong size, ERR_INVALID_SIZE -> ERR_RW_ERROR, which seems
>>> misleading to me.
>>>
>>> I saw some discussion about all this:
>>> https://lore.kernel.org/lkml/20260924213737.3833625-1-shansinha@xxxxxxxxxx/
>>> but it's not clear to me why you need to reset the error code here.
>>>
>>
>> My understanding from that discussion was that userspace errors should
>> fall into clean buckets. read-write-error would mean "roll back",
>> whether it came from the first failure or a later rollback attempt,
>> since a failed rollback still leaves the PSP dead and needing a
>> rollback. dmesg would still log the exact reason for the failure.
>>
>> That said, I see how it can be misleading, and I'm fine with dropping
>> the override or putting a comment and re-spinning this.
> 
> I do see how it can be misleading.
> 
> I think dropping it makes sense. When I suggested the changes for v2,
> psp_ret was only logged via dev_dbg(), so the sysfs error was the only
> clean signal userspace had. Since the error code is reported in dmesg
> now, I think it's fine to keep the original error and have the sysfs
> error reflect what happened on the upload rather than the action
> tooling should take.
> 

Sounds good to me. I'll drop it in the next spin.

Thanks!
--Pratik