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

[Stefano Garzarella]

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