Re: [PATCH 3/3] x86/sev: add a SVSM vTPM platform device

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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.

Regards,

James