On Wed, Dec 11, 2024 at 12:02:49PM -0500, James Bottomley wrote:
On Wed, 2024-12-11 at 10:30 -0600, Tom Lendacky wrote:
On 12/10/24 08:34, Stefano Garzarella wrote:
[...]
> +static bool is_svsm_vtpm_send_command_supported(void)
> +{
> + struct svsm_call call = {};
> + u64 send_cmd_mask = 0;
> + u64 platform_cmds;
> + u64 features;
> + int ret;
> +
> + call.caa = svsm_get_caa();
> + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> +
> + ret = svsm_perform_call_protocol(&call);
> +
> + if (ret != SVSM_SUCCESS)
> + return false;
> +
> + features = call.rdx_out;
> + platform_cmds = call.rcx_out;
> +
> + /* No feature supported, it must be zero */
> + if (features)
> + return false;
I think this check should be removed. The SVSM currently returns all
zeroes for the features to allow for future support. If a new feature
is added in the future, this then allows a driver that supports that
feature to operate with a version of an SVSM that doesn't have that
feature implemented. It also allows a version of the driver that
doesn't know about that feature to work with an SVSM that has that
feature.
A feature added to the vTPM shouldn't alter the behavior of something
that isn't using or understands that feature.
I actually don't think this matters, because I can't see any reason to
use the SVSM features flag for the vTPM. The reason is that the TPM
itself contains a versioned feature mechanism that external programs
already use, so there's no real need to duplicate it.
That said, I'm happy with either keeping or removing this.
If we remove the check, should we print some warning if `feature` is not
0 or just ignore it?
Thanks,
Stefano
