From: Stefan Berger <[email protected]>
Support TPM 2 in the IBM vTPM driver. The hypervisor tells us what
version of TPM is connected through the vio_device_id.
In case a TPM 2 is found, we set the TPM_OPS_AUTO_STARTUP flag to
have properly initialize the TPM and driver.
Signed-off-by: Stefan Berger <[email protected]>
---
drivers/char/tpm/tpm_ibmvtpm.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index eee566eddb35..d479d64a65aa 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -29,6 +29,7 @@ static const char tpm_ibmvtpm_driver_name[] = "tpm_ibmvtpm";
static const struct vio_device_id tpm_ibmvtpm_device_table[] = {
{ "IBM,vtpm", "IBM,vtpm"},
+ { "IBM,vtpm", "IBM,vtpm20"},
{ "", "" }
};
MODULE_DEVICE_TABLE(vio, tpm_ibmvtpm_device_table);
@@ -443,7 +444,7 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
return (status == 0);
}
-static const struct tpm_class_ops tpm_ibmvtpm = {
+static struct tpm_class_ops tpm_ibmvtpm = {
.recv = tpm_ibmvtpm_recv,
.send = tpm_ibmvtpm_send,
.cancel = tpm_ibmvtpm_cancel,
@@ -672,6 +673,11 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (rc)
goto init_irq_cleanup;
+ if (!strcmp(id->compat, "IBM,vtpm20")) {
+ chip->flags |= TPM_CHIP_FLAG_TPM2;
+ tpm_ibmvtpm.flags = TPM_OPS_AUTO_STARTUP;
+ }
+
if (!wait_event_timeout(ibmvtpm->crq_queue.wq,
ibmvtpm->rtce_buf != NULL,
HZ)) {
--
2.23.0
On 2/4/20 8:27 AM, Stefan Berger wrote:
> From: Stefan Berger <[email protected]>
>
> Support TPM 2 in the IBM vTPM driver. The hypervisor tells us what
> version of TPM is connected through the vio_device_id.
>
> In case a TPM 2 is found, we set the TPM_OPS_AUTO_STARTUP flag to
> have properly initialize the TPM and driver.
>
> Signed-off-by: Stefan Berger <[email protected]>
> ---
> drivers/char/tpm/tpm_ibmvtpm.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index eee566eddb35..d479d64a65aa 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -29,6 +29,7 @@ static const char tpm_ibmvtpm_driver_name[] = "tpm_ibmvtpm";
>
> static const struct vio_device_id tpm_ibmvtpm_device_table[] = {
> { "IBM,vtpm", "IBM,vtpm"},
> + { "IBM,vtpm", "IBM,vtpm20"},
> { "", "" }
> };
> MODULE_DEVICE_TABLE(vio, tpm_ibmvtpm_device_table);
> @@ -443,7 +444,7 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
> return (status == 0);
> }
>
> -static const struct tpm_class_ops tpm_ibmvtpm = {
> +static struct tpm_class_ops tpm_ibmvtpm = {
> .recv = tpm_ibmvtpm_recv,
> .send = tpm_ibmvtpm_send,
> .cancel = tpm_ibmvtpm_cancel,
> @@ -672,6 +673,11 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> if (rc)
> goto init_irq_cleanup;
>
> + if (!strcmp(id->compat, "IBM,vtpm20")) {
> + chip->flags |= TPM_CHIP_FLAG_TPM2;
> + tpm_ibmvtpm.flags = TPM_OPS_AUTO_STARTUP;
TPM_OPS_AUTO_STARTUP flag isn't set for vTPM 1.2. What is different in
case of vTPM 2.0 ?
Thanks & Regards,
- Nayna
On 2/13/20 12:53 PM, Nayna wrote:
>
> On 2/4/20 8:27 AM, Stefan Berger wrote:
>> From: Stefan Berger <[email protected]>
>>
>> Support TPM 2 in the IBM vTPM driver. The hypervisor tells us what
>> version of TPM is connected through the vio_device_id.
>>
>> In case a TPM 2 is found, we set the TPM_OPS_AUTO_STARTUP flag to
>> have properly initialize the TPM and driver.
>>
>> Signed-off-by: Stefan Berger <[email protected]>
>> ---
>> drivers/char/tpm/tpm_ibmvtpm.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c
>> b/drivers/char/tpm/tpm_ibmvtpm.c
>> index eee566eddb35..d479d64a65aa 100644
>> --- a/drivers/char/tpm/tpm_ibmvtpm.c
>> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
>> @@ -29,6 +29,7 @@ static const char tpm_ibmvtpm_driver_name[] =
>> "tpm_ibmvtpm";
>>
>> static const struct vio_device_id tpm_ibmvtpm_device_table[] = {
>> { "IBM,vtpm", "IBM,vtpm"},
>> + { "IBM,vtpm", "IBM,vtpm20"},
>> { "", "" }
>> };
>> MODULE_DEVICE_TABLE(vio, tpm_ibmvtpm_device_table);
>> @@ -443,7 +444,7 @@ static bool tpm_ibmvtpm_req_canceled(struct
>> tpm_chip *chip, u8 status)
>> return (status == 0);
>> }
>>
>> -static const struct tpm_class_ops tpm_ibmvtpm = {
>> +static struct tpm_class_ops tpm_ibmvtpm = {
>> .recv = tpm_ibmvtpm_recv,
>> .send = tpm_ibmvtpm_send,
>> .cancel = tpm_ibmvtpm_cancel,
>> @@ -672,6 +673,11 @@ static int tpm_ibmvtpm_probe(struct vio_dev
>> *vio_dev,
>> if (rc)
>> goto init_irq_cleanup;
>>
>> + if (!strcmp(id->compat, "IBM,vtpm20")) {
>> + chip->flags |= TPM_CHIP_FLAG_TPM2;
>> + tpm_ibmvtpm.flags = TPM_OPS_AUTO_STARTUP;
>
> TPM_OPS_AUTO_STARTUP flag isn't set for vTPM 1.2. What is different in
> case of vTPM 2.0 ?
I don't want side effects for the TPM 1.2 case here, so I am only
modifying the flag for the case where the new TPM 2 is being used.
Here's the code where it shows the effect.
int tpm_auto_startup(struct tpm_chip *chip)
{
int rc;
if (!(chip->ops->flags & TPM_OPS_AUTO_STARTUP))
return 0;
if (chip->flags & TPM_CHIP_FLAG_TPM2)
rc = tpm2_auto_startup(chip);
else
rc = tpm1_auto_startup(chip);
return rc;
}
In the TPM 2 case we then get timeouts, do the TPM self test, send
TPM2_STARTUP if necessary and get attributes of the TPM 2 command from
the device. All necessary to start it up.
https://elixir.bootlin.com/linux/latest/source/drivers/char/tpm/tpm2-cmd.c#L719
Does this answer your question ?
Stefan
>
> Thanks & Regards,
>
> - Nayna
>
On Thu, Feb 13, 2020 at 01:20:12PM -0500, Stefan Berger wrote:
> I don't want side effects for the TPM 1.2 case here, so I am only modifying
> the flag for the case where the new TPM 2 is being used. Here's the code
> where it shows the effect.
I'm surprised this driver is using AUTO_STARTUP, it was intended for
embedded cases where their is no firmware to boot the TPM.
Chips using AUTO_STARTUP are basically useless for PCRs/etc.
I'd expect somthing called vtpm to have been started and PCRs working
before Linux is started??
Jason
On 2/13/20 1:35 PM, Jason Gunthorpe wrote:
> On Thu, Feb 13, 2020 at 01:20:12PM -0500, Stefan Berger wrote:
>
>> I don't want side effects for the TPM 1.2 case here, so I am only modifying
>> the flag for the case where the new TPM 2 is being used. Here's the code
>> where it shows the effect.
> I'm surprised this driver is using AUTO_STARTUP, it was intended for
> embedded cases where their is no firmware to boot the TPM.
The TIS is also using it on any device.
static const struct tpm_class_ops tpm_tis = {
.flags = TPM_OPS_AUTO_STARTUP,
.status = tpm_tis_status,
https://elixir.bootlin.com/linux/latest/source/drivers/char/tpm/tpm_tis_core.c#L917
>
> Chips using AUTO_STARTUP are basically useless for PCRs/etc.
>
> I'd expect somthing called vtpm to have been started and PCRs working
> before Linux is started??
Yes, there's supposed to be firmware.
I only see one caller to tpm2_get_cc_attrs_tbl(chip), which is necessary
to call. This caller happens to be in tpm2_auto_startup.
Stefan
>
> Jason
On Thu, Feb 13, 2020 at 02:04:12PM -0500, Stefan Berger wrote:
> On 2/13/20 1:35 PM, Jason Gunthorpe wrote:
> > On Thu, Feb 13, 2020 at 01:20:12PM -0500, Stefan Berger wrote:
> >
> > > I don't want side effects for the TPM 1.2 case here, so I am only modifying
> > > the flag for the case where the new TPM 2 is being used. Here's the code
> > > where it shows the effect.
> > I'm surprised this driver is using AUTO_STARTUP, it was intended for
> > embedded cases where their is no firmware to boot the TPM.
>
> The TIS is also using it on any device.
TIS is a generic driver, and can run on TPMs without firmware
support. It doesn't know either way
> > Chips using AUTO_STARTUP are basically useless for PCRs/etc.
> >
> > I'd expect somthing called vtpm to have been started and PCRs working
> > before Linux is started??
>
> Yes, there's supposed to be firmware.
>
> I only see one caller to tpm2_get_cc_attrs_tbl(chip), which is necessary to
> call. This caller happens to be in tpm2_auto_startup.
That seems to be a mistake, proper startup of the driver should never
require auto_startup.
Jason
On 2/13/20 2:11 PM, Jason Gunthorpe wrote:
> On Thu, Feb 13, 2020 at 02:04:12PM -0500, Stefan Berger wrote:
>> On 2/13/20 1:35 PM, Jason Gunthorpe wrote:
>>> On Thu, Feb 13, 2020 at 01:20:12PM -0500, Stefan Berger wrote:
>>>
>>>> I don't want side effects for the TPM 1.2 case here, so I am only modifying
>>>> the flag for the case where the new TPM 2 is being used. Here's the code
>>>> where it shows the effect.
>>> I'm surprised this driver is using AUTO_STARTUP, it was intended for
>>> embedded cases where their is no firmware to boot the TPM.
>> The TIS is also using it on any device.
> TIS is a generic driver, and can run on TPMs without firmware
> support. It doesn't know either way
The following drivers are all using it:
drivers/char/tpm/st33zp24/st33zp24.c, line 493
drivers/char/tpm/tpm-interface.c, line 374
drivers/char/tpm/tpm_crb.c, line 421
drivers/char/tpm/tpm_ftpm_tee.c, line 184
drivers/char/tpm/tpm_i2c_atmel.c, line 139
drivers/char/tpm/tpm_i2c_infineon.c, line 602
drivers/char/tpm/tpm_i2c_nuvoton.c, line 465
drivers/char/tpm/tpm_tis_core.c, line 917
drivers/char/tpm/tpm_vtpm_proxy.c, line 435
https://elixir.bootlin.com/linux/latest/ident/TPM_OPS_AUTO_STARTUP
>
>>> Chips using AUTO_STARTUP are basically useless for PCRs/etc.
>>>
>>> I'd expect somthing called vtpm to have been started and PCRs working
>>> before Linux is started??
>> Yes, there's supposed to be firmware.
>>
>> I only see one caller to tpm2_get_cc_attrs_tbl(chip), which is necessary to
>> call. This caller happens to be in tpm2_auto_startup.
> That seems to be a mistake, proper startup of the driver should never
> require auto_startup.
Is this IBM vTPM driver special that it should do things differently
than all those drivers listed above? From looking at the code is seems
it is to be set for the TPM 2.0 case.
Stefan
On Thu, Feb 13, 2020 at 02:15:03PM -0500, Stefan Berger wrote:
> On 2/13/20 2:11 PM, Jason Gunthorpe wrote:
> > On Thu, Feb 13, 2020 at 02:04:12PM -0500, Stefan Berger wrote:
> > > On 2/13/20 1:35 PM, Jason Gunthorpe wrote:
> > > > On Thu, Feb 13, 2020 at 01:20:12PM -0500, Stefan Berger wrote:
> > > >
> > > > > I don't want side effects for the TPM 1.2 case here, so I am only modifying
> > > > > the flag for the case where the new TPM 2 is being used. Here's the code
> > > > > where it shows the effect.
> > > > I'm surprised this driver is using AUTO_STARTUP, it was intended for
> > > > embedded cases where their is no firmware to boot the TPM.
> > > The TIS is also using it on any device.
> > TIS is a generic driver, and can run on TPMs without firmware
> > support. It doesn't know either way
>
> The following drivers are all using it:
>
>
> drivers/char/tpm/st33zp24/st33zp24.c, line 493
> drivers/char/tpm/tpm-interface.c, line 374
> drivers/char/tpm/tpm_crb.c, line 421
> drivers/char/tpm/tpm_ftpm_tee.c, line 184
> drivers/char/tpm/tpm_i2c_atmel.c, line 139
> drivers/char/tpm/tpm_i2c_infineon.c, line 602
> drivers/char/tpm/tpm_i2c_nuvoton.c, line 465
> drivers/char/tpm/tpm_tis_core.c, line 917
> drivers/char/tpm/tpm_vtpm_proxy.c, line 435
>
> https://elixir.bootlin.com/linux/latest/ident/TPM_OPS_AUTO_STARTUP
These are all general purpose drivers.
Though perhaps vtpm_proxy shouldn't include it, not sure.
> > > > Chips using AUTO_STARTUP are basically useless for PCRs/etc.
> > > >
> > > > I'd expect somthing called vtpm to have been started and PCRs working
> > > > before Linux is started??
> > > Yes, there's supposed to be firmware.
> > >
> > > I only see one caller to tpm2_get_cc_attrs_tbl(chip), which is necessary to
> > > call. This caller happens to be in tpm2_auto_startup.
> > That seems to be a mistake, proper startup of the driver should never
> > require auto_startup.
>
> Is this IBM vTPM driver special that it should do things differently than
> all those drivers listed above? From looking at the code is seems it is to
> be set for the TPM 2.0 case.
Any driver that knows the TPM must be started prior to Linux
booting should not use the flag. vtpm drivers in general would seem
to be the case where we can make this statement.
If it was mandatory then it would not be a flag the driver has to specify.
Jason
On 2/13/20 2:39 PM, Jason Gunthorpe wrote:
> On Thu, Feb 13, 2020 at 02:15:03PM -0500, Stefan Berger wrote:
>> On 2/13/20 2:11 PM, Jason Gunthorpe wrote:
>>> On Thu, Feb 13, 2020 at 02:04:12PM -0500, Stefan Berger wrote:
>>>> On 2/13/20 1:35 PM, Jason Gunthorpe wrote:
>>>>> On Thu, Feb 13, 2020 at 01:20:12PM -0500, Stefan Berger wrote:
>>>>>
>>>>>> I don't want side effects for the TPM 1.2 case here, so I am only modifying
>>>>>> the flag for the case where the new TPM 2 is being used. Here's the code
>>>>>> where it shows the effect.
>>>>> I'm surprised this driver is using AUTO_STARTUP, it was intended for
>>>>> embedded cases where their is no firmware to boot the TPM.
>>>> The TIS is also using it on any device.
>>> TIS is a generic driver, and can run on TPMs without firmware
>>> support. It doesn't know either way
>> The following drivers are all using it:
>>
>>
>> drivers/char/tpm/st33zp24/st33zp24.c, line 493
>> drivers/char/tpm/tpm-interface.c, line 374
>> drivers/char/tpm/tpm_crb.c, line 421
>> drivers/char/tpm/tpm_ftpm_tee.c, line 184
>> drivers/char/tpm/tpm_i2c_atmel.c, line 139
>> drivers/char/tpm/tpm_i2c_infineon.c, line 602
>> drivers/char/tpm/tpm_i2c_nuvoton.c, line 465
>> drivers/char/tpm/tpm_tis_core.c, line 917
>> drivers/char/tpm/tpm_vtpm_proxy.c, line 435
>>
>> https://elixir.bootlin.com/linux/latest/ident/TPM_OPS_AUTO_STARTUP
> These are all general purpose drivers.
>
> Though perhaps vtpm_proxy shouldn't include it, not sure.
>
>>>>> Chips using AUTO_STARTUP are basically useless for PCRs/etc.
>>>>>
>>>>> I'd expect somthing called vtpm to have been started and PCRs working
>>>>> before Linux is started??
>>>> Yes, there's supposed to be firmware.
>>>>
>>>> I only see one caller to tpm2_get_cc_attrs_tbl(chip), which is necessary to
>>>> call. This caller happens to be in tpm2_auto_startup.
>>> That seems to be a mistake, proper startup of the driver should never
>>> require auto_startup.
>> Is this IBM vTPM driver special that it should do things differently than
>> all those drivers listed above? From looking at the code is seems it is to
>> be set for the TPM 2.0 case.
> Any driver that knows the TPM must be started prior to Linux
> booting should not use the flag. vtpm drivers in general would seem
> to be the case where we can make this statement.
Wouldn't this statement apply to all systems, including embedded ones?
Basically all firmwares should implement the CRTM and do the TPM
initialization.
>
> If it was mandatory then it would not be a flag the driver has to specify.
I'll add a case where we call into tpm2_get_cc_attrs_tbl(chip) in case
the auto startup flag is not set. I believe this is the only part that's
missing for TPM 2 to work if not autostarted.
Stefan
>
> Jason
On Thu, Feb 13, 2020 at 02:45:49PM -0500, Stefan Berger wrote:
> > Any driver that knows the TPM must be started prior to Linux
> > booting should not use the flag. vtpm drivers in general would seem
> > to be the case where we can make this statement.
>
> Wouldn't this statement apply to all systems, including embedded ones?
> Basically all firmwares should implement the CRTM and do the TPM
> initialization.
It is not mandatory that systems with TPMs start it in the
firmware. That is only required if the TPM PCR feature is going to be
used. A TPM can quite happily be used for key storage without FW
support.
Arguably this is sort of done wrong. eg if the platform has provided
TPM information through uEFI or something then we shouldn't try to
auto start the system TPM. At least for TPM1 detecting a non-started
TPM was harmless, so nobody really cared. I wonder if TPM2 is much
different..
But certainly auto startup should *not* be required to have a working
TPM driver, that is just fundamentally wrong.
Jason