Add a new IPMI sensor to disable the TPM

[anoo1]

Hostboot is requesting a new IPMI sensor to be added to the BMC to disable the TPM.

The requirements are:

• This is only requested for P9, therefore all changes would be made in the ibm-openbmc repo so that they're not picked up for P10 onwards when rebasing with upstream.
• The new sensor can be implemented the same way as the existing "tpm_required_sensor" sensor.

These are the changes identified:

1. Add a new dbus interface.
   • The existing interface is called TPMEnable: https://github.com/ibm-openbmc/phosphor-dbus-interfaces/blob/OP940/xyz/openbmc_project/Control/TPM/Policy.interface.yaml
   • Perhaps the new interface could be called DisableTPM - boolean - When true, the TPM is disabled, or some other relevant name.
2. Set the default value of the new dbus interface to false in the Settings yaml file.
   • Example of the existing interface: https://github.com/ibm-openbmc/openbmc/blob/11d2cfce079ac3a3a89f5e892b680acfd534d973/meta-phosphor/recipes-phosphor/settings/phosphor-settings-defaults/defaults.yaml#L101
3. Add the new interface to the mrw yaml file with the sensor name defined by hostboot.
   • Example of the existing interface: https://github.com/ibm-openbmc/openbmc/blob/11d2cfce079ac3a3a89f5e892b680acfd534d973/meta-phosphor/recipes-phosphor/ipmi/phosphor-ipmi-sensor-inventory-mrw-config/config.yaml#L168
   • The "tpm_required_sensor" name comes from hostboot: https://github.com/open-power/witherspoon-xml/commit/9e31dc8f12263dde49613045b2af54128be14fef
   • There will be a new sensor name defined for disabling the TPM.

Mike Baiocchi will be making the hostboot changes. I'll post the name of the sensor once that's defined since it'll be needed for step 3 above. Mike will also do the end-to-end testing once we have the BMC changes ready.

[lxwinspur]

Mike Baiocchi will be making the hostboot changes. I'll post the name of the sensor once that's defined

@anoo1 Is there any progress on this issue?

[mabaiocchi]

@lxwinspur I have a witherspoon-xml pull request that should have the info for what you are looking for. I based the changes on the existing "tpm_required_sensor" and called it "disable_tpm_sensor".

https://github.com/open-power/witherspoon-xml/pull/90

I am still in the process of testing this PR on a local witherspoon, but I think you should be able to pick it up and test with it. I'm hoping to have it merged next week.

[mabaiocchi]

FYI the above witherspoon-xml PR was merged

[lxwinspur]

Thanks @mabaiocchi.

Perhaps the new interface could be called DisableTPM - boolean - When true, the TPM is disabled, or some other relevant name.

@anoo1 Why not continue to use TPMEnable? true->Enable TPM, false->Disable TPM.

[mabaiocchi]

@lxwinspur I am ok if you want to re-use the TPMEnable interface already on the BMC. I'll let @anoo1 answer is that would be a problem.

As a hostboot developer all I primarily care about is that the right value gets put into the newly created "disable_tpm_sensor". Hostboot will be using the IPMI interface to request the value of this sensor. There is not a "TPMEnabled" sensor.

[anoo1]

They're 2 different sensors, the existing TPMEnable corresponds to the hostboot "tpm_required_sensor".

Hostboot will add this logic: On every IPL Hostboot checks if “Disable TPM” to true: o If true Hostboot will disable the TPM and set “TPM Required” to false via OpenBMC sensor and shared FSP attribute o Else if false, boot as normal o Tell customer that “Disable TPM” setting overrides TPM Required setting

So we need to keep both, the existing "Required" sensor (which has the dbus name TPMEnable, that probably was not the best naming), and the new "Disable" sensor.

[lxwinspur]

Thanks @mabaiocchi @anoo1

Although I don't understand IBM's design philosophy, it is strange to use two sensors to control the TPM. But I'm ok, I'll push some new patches this week. BTW, Should I send a PR to the 1060 branch? eg: PDI, and some YAML files.

[lxwinspur]

https://github.com/ibm-openbmc/phosphor-dbus-interfaces/pull/90 (1040 branch) https://github.com/ibm-openbmc/openbmc/pull/301(1050.00 branch)

@anoo1 @mabaiocchi Please review these patches, thanks.

[mzipse]

@lxwinspur , this tpm disable is only needed for our P9. The op940 branch is where the TPM disable needs to go.

[mzipse]

There are no plans to disable the tpm for P10.

[anoo1]

Thanks @lxwinspur

Although I don't understand IBM's design philosophy, it is strange to use two sensors to control the TPM.

Yeah it can be seen as confusing, here is more explanation:

• The existing sensor just tells the system to not use the TPM during the power on process, it doesn't touch the TPM.
• The new requested sensor is going to actually disable the TPM chip, like if you remove it from the system. That's why hostboot will add the additional logic to set the existing sensor if the customer disables the TPM chip via this new sensor because during power on there's no need to check for TPM (it's already turned off / disabled).

So two options available for the customer, one to just ignore TPM during power on, and this new one to disable the chip.

[lxwinspur]

Thanks @mzipse @anoo1

Resented two PRs to the OP940 branch.

https://github.com/ibm-openbmc/openbmc/pull/302 https://github.com/ibm-openbmc/phosphor-dbus-interfaces/pull/91

Please review it.

[anoo1]

Thanks @lxwinspur for addressing the review comments from 301 into 302. Approved both PRs.

@rfrandse We'll need a test driver for @mabaiocchi to verify.

[mabaiocchi]

FYI I'm still trying to get the verification completed this week.

[mzipse]

Closing this issue. BMC work is complete. Mike will cover the overall verification.