Re: [PATCH v4] crypto/ccp: Introduce SNP_VERIFY_MITIGATION command

[Pratik R. Sampat]

On 6/9/26 12:06 PM, Tom Lendacky wrote:
> On 6/8/26 15:58, Pratik R. Sampat wrote:
>> The SEV-SNP firmware provides the SNP_VERIFY_MITIGATION command, which
>> can be used to query the status of currently supported vulnerability
>> mitigations and to initiate mitigations within the firmware.
>>
>> This command is an explicit mechanism to ascertain if a firmware
>> mitigation is applied without needing a full RMP re-build, which is most
>> useful in a live firmware update scenario.
>>
>> The firmware supports two subcommands: STATUS and VERIFY. The STATUS
>> subcommand is used to query the supported and verified mitigation bits.
>> The VERIFY subcommand initiates the mitigation process within the FW for
>> the specified vulnerability. Expose a userspace interface under:
>> /sys/firmware/sev/vulnerabilities/
>>   - supported_mitigations (read-only): supported mitigation vector mask
>>   - verified_mitigations (read/write): current verified mask; write a
>>     vector to request VERIFY for that bit
>>
>> The behavior of SNP_VERIFY_MITIGATION and the pre-requisites for using
>> it are bug-specific. Information about supported mitigations and its
>> corresponding vector is to be published as part of the AMD Security
>> Bulletin.
>>
>> See SEV-SNP Firmware ABI specifications 1.58, SNP_VERIFY_MITIGATION for
>> more details.
>>
>> Signed-off-by: Pratik R. Sampat <prsampat@xxxxxxx>
> 
> Just a few minor comments below, but in general...
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> 
>> +static ssize_t verified_mitigations_store(struct kobject *kobj,
>> +					  struct kobj_attribute *attr,
>> +					  const char *buf, size_t count)
>> +{
>> +	struct sev_data_snp_verify_mitigation_dst dst;
>> +	struct sev_device *sev = psp_master->sev_data;
>> +	u64 vector;
>> +	int ret;
>> +
>> +	ret = kstrtoull(buf, 0, &vector);
>> +	if (ret)
>> +		return ret;
> 
> Should we save some time and check for vector having multiple bits set
> before sending it to the firmware?
> 

Sure. This can be a quick sanity check and can save a FW call.


>> +err_verify_mit:
>> +	kobject_put(sev->verify_mit);
>> +	sev->verify_mit = NULL;
>> +err_sev_kobj:
>> +	kobject_put(sev->sev_kobj);
>> +	sev->sev_kobj = NULL;
>> +
> 
> Extra blank line.
> 

Ack.

>> +}
>> +
>> +static void sev_snp_unregister_verify_mitigation(struct sev_device *sev)
>> +{
>> +	if (sev->verify_mit) {
>> +		sysfs_remove_group(sev->verify_mit, &mit_attr_group);
>> +		kobject_put(sev->verify_mit);
>> +		sev->verify_mit = NULL;
>> +	}
>> +
>> +	if (sev->sev_kobj) {
>> +		kobject_put(sev->sev_kobj);
>> +		sev->sev_kobj = NULL;
>> +	}
>> +}
>> +#else
> 
> Just add the CONFIG option to the else and endif, e.g.:
> 
> #else	// CONFIG_SYSFS
> 
> ...
> 
> #endif	// CONFIG_SYSFS
> 

Ack.

>> +static void sev_snp_register_verify_mitigation(struct sev_device *sev) { }
>> +static void sev_snp_unregister_verify_mitigation(struct sev_device *sev) { }
>> +#endif
>> +
>>  static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>  {
>>  	struct sev_data_range_list *snp_range_list __free(kfree) = NULL;
>> @@ -1673,6 +1824,17 @@ int sev_platform_init(struct sev_platform_init_args *args)
>>  	rc = _sev_platform_init_locked(args);
>>  	mutex_unlock(&sev_cmd_mutex);
>>  
>> +	/*
>> +	 * Register the sysfs interface outside the sev_cmd_mutex. The
>> +	 * _show()/_store() handlers issue SEV commands that acquire the
>> +	 * sev_cmd_mutex, so creating (and on the shutdown path, removing) the
>> +	 * sysfs group must stay outside that lock. sysfs provides its own
>> +	 * synchronization between group creation/removal and concurrent
>> +	 * attribute access.
>> +	 */
>> +	if (!rc)
>> +		sev_snp_register_verify_mitigation(psp_master->sev_data);
>> +
>>  	return rc;
>>  }
>>  EXPORT_SYMBOL_GPL(sev_platform_init);
>> @@ -2752,6 +2914,15 @@ static void sev_firmware_shutdown(struct sev_device *sev)
>>  	if (sev->tio_status)
>>  		sev_tsm_uninit(sev);
>>  
>> +	/*
>> +	 * Remove the sysfs interface before taking the sev_cmd_mutex.
>> +	 * sysfs_remove_group() waits for in-flight _show()/_store() handlers
>> +	 * to drain, and those handlers issue SNP_VERIFY_MITIGATION via
>> +	 * sev_do_cmd() which acquires the sev_cmd_mutex. Removing the group
>> +	 * while holding the mutex would therefore deadlock.
> 
> s/would/could/
> 

Will do and re-spin, thanks!
--Pratik