[SLOF] [PATCH v6 4/7] tpm: Add TPM CRQ driver implementation
Stefan Berger
stefanb at linux.ibm.com
Tue Jan 21 02:45:55 AEDT 2020
On 1/20/20 3:09 AM, Alexey Kardashevskiy wrote:
>
> All this can be reduces to "QEMU has vTPM support since the v5.0.0
> release", or similar.
I will do that then.
>
>> +
>> +To start a QEMU VM with an attached vTPM (swtpm), run the following commands
>> +as 'root'.
> Why as 'root'? Will swtpm forward these requests to an actual tpm
> device? How is it expected to work on the real system?
I will modify this to use sudo for qemu.
The hardware TPM cannot be used for virtualization so we won't forward
anything to it.
> +#undef PAPR_VTPM_DEBUG
> +//#define PAPR_VTPM_DEBUG
> +#ifdef PAPR_VTPM_DEBUG
> +#define dprintf(_x ...) do { printf("VTPM CRQ: " _x); } while(0)
> +#else
> +#define dprintf(_x ...)
> +#endif
> +
> +#define MIN(a, b) ((a) > (b) ? (b) : (a))
> +
> +/* layout of the command request queue for vTPM */
>
> Is every single packed struct added by this patch defined as a big endian?
Every structure interacting with the hypervisor/QEMU is defined as big
endian and this is the only packed data structure in this patch, so yes,
it is big endian. The rest can be native.
[TPM 2 commands are all in big endian.]
> Or everything with the "tpm2_" prefix is little endian, and the rest is
> big endian?
>
>
>> +struct crq {
>> + uint8_t valid;
>> + uint8_t msg;
>> + uint16_t len;
>> + uint32_t data;
>> + uint64_t reserved;
>> +} __attribute__((packed));
>> +
>> +#define PAPR_VTPM_INIT_CRQ_COMMAND 0xC0
>> +#define PAPR_VTPM_VALID_COMMAND 0x80
>> +#define PAPR_VTPM_MSG_RESULT 0x80
>> +
>> +/* crq.msg request types when crq.valid = PAPR_VTPM_INIT_CRQ_COMMAND */
>> +#define PAPR_VTPM_INIT_CRQ_RESULT 0x1
>> +
>> +/* crq.msg request types when crq.valid = PAPR_VTPM_VALID_COMMAND */
>> +#define PAPR_VTPM_GET_VERSION 0x1
>> +#define PAPR_VTPM_TPM_COMMAND 0x2
>> +#define PAPR_VTPM_GET_RTCE_BUFFER_SIZE 0x3
>> +
>> +#define TPM2_DEFAULT_DURATION_SHORT 750000 /* us */
>> +#define TPM2_DEFAULT_DURATION_MEDIUM 2000000 /* us */
>> +#define TPM2_DEFAULT_DURATION_LONG 2000000 /* us */
>> +
>> +static const uint32_t tpm2_durations[3] = {
>> + TPM2_DEFAULT_DURATION_SHORT,
>> + TPM2_DEFAULT_DURATION_MEDIUM,
>> + TPM2_DEFAULT_DURATION_LONG,
>> +};
>> +
>> +#define QUEUE_SIZE 4096
>> +
>> +/* state of the PAPR CRQ VTPM driver */
>> +static struct spapr_vtpm_driver_state {
> s/spapr_vtpm_driver_state// as this name is not used anyway anywhere.
I removed the name from it.
>
>
>> + /* whether it driver been initialized */
>> + bool initialized;
>> +
>> + /* unit number */
>> + unsigned long unit;
>> +
>> + /* CRQ queue address and size */
>> + unsigned char *qaddr;
>> + unsigned long qsize;
>
> The whole struct is static so you could simply do qaddr[QUEUE_SIZE] and
> drop @qsize and allocation.
I have tried to do this with a 32-byte, 4096-byte and 65536-byte aligned
queue embedded in this struct, but it's rejected by the HV when trying
to register the CRQ. The addresses were as follows:
32 byte: 0x7dc61e40
4096 byte: 0x7dc64000
65536 byte: 0x7dc80000
The allocated queue at 0x7e469000, which seems to be 4096 byte aligned,
is the only one accepted.
>
>> +
>> + /* current q_entry */
>> + unsigned int curr_q_entry;
>> +
>> + /* current response CRQ */
>> + struct crq *response;
>> +
>> + /* power firmware defined state and error code */
>> + vtpm_drv_state driver_state;
>> + vtpm_drv_error driver_error;
>> +
>> + /* size of buffer supported by hypervisor */
>> + unsigned int buffer_size;
>> +
>> + /* buffer for commands and responses */
>> + char *buffer;
>
> Indentation broke.
Fixed.
>
>> +
>> + /* version of the TPM we talk to -- from CRQ message */
>> + uint32_t tpm_version;
> It is always v2.0 for now.
Dropped the field but still querying the version from the HV.
>
>
>> +} spapr_vtpm = {
>> + .qsize = QUEUE_SIZE,
>> + .driver_state = VTPM_DRV_STATE_INVALID,
>> + .driver_error = VTPM_DRV_ERROR_NO_FAILURE,
>> +};
>> +
>> +static void vtpm_drv_state_set(vtpm_drv_state s, vtpm_drv_error e)
>
>
> Some functions have "vtpm_" prefix, some "spapr_", some none (such as
> "get_crq").
I prefixed them with spapr_ now mostly except those two for driver state.
>
>
>> +{
>> + spapr_vtpm.driver_state = s;
>> + spapr_vtpm.driver_error = e;
>> +}
>> +
>> +static vtpm_drv_error vtpm_drv_error_get(void)
>> +{
>> + return spapr_vtpm.driver_error;
>> +}
>> +
>> +static struct crq* get_crq(void *qaddr, unsigned long q_entry)
> Please s/crq* get_crq/crq *get_crq/g as everywhere else.
Fixed it. This should have been the only occurrence.
>> +void spapr_vtpm_finalize(void)
>> +{
>> + if (spapr_vtpm.unit)
>> + hv_free_crq(spapr_vtpm.unit);
>
> spapr_vtpm.unit = 0; as above?
Fixed.
>
>> +}
>> +
>> +/*
>> + * Check whether we have a CRQ underneath us; if we do, the CRQ will
>> + * be left open.
>> + */
>> +static bool spapr_vtpm_probe(void)
>> +{
>> + if (!spapr_vtpm.qaddr) {
>> + spapr_vtpm.qaddr = SLOF_alloc_mem(spapr_vtpm.qsize);
>> + if (!spapr_vtpm.qaddr) {
>> + printf("%s: Unable to allocate memory\n", __func__);
>> + return false;
>> + }
>> + memset(spapr_vtpm.qaddr, 0, spapr_vtpm.qsize);
>> +
>> + dprintf("getting FORTH vtpm-unit\n");
>> + spapr_vtpm.unit = SLOF_get_vtpm_unit();
>> + if (!spapr_vtpm.unit) {
>> + printf("%s: Could not get valid vtpm-unit\n", __func__);
>> + return false;
>> + }
>> + }
>> +
>> + dprintf("vtpm_unit = %lx, buffer = %p\n",
>> + spapr_vtpm.unit, spapr_vtpm.qaddr);
>> +
>> + if (!spapr_vtpm_activate())
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static bool spapr_vtpm_senddata(const uint8_t *const data, uint32_t len)
>> +{
>> + struct crq crq;
>> + long rc;
>> +
>> + if (vtpm_drv_error_get() != VTPM_DRV_ERROR_NO_FAILURE) {
>> + printf("%s: VTPM CRQ: In failure mode\n", __func__);
>> + return false;
>> + }
>> +
>> + if (len > spapr_vtpm.buffer_size) {
>> + printf("%s: VTPM CRQ: Send buffer too large: %u > %u\n",
>> + __func__, len, spapr_vtpm.buffer_size);
>> + return false;
>> + }
>> +
>> + spapr_vtpm.response = get_response_crq();
>> + spapr_vtpm.response->data = (uint64_t)spapr_vtpm.buffer;
>> + /* response CRQ has been set and valid field cleared */
>
> Where does "response CRQ has been set" happen? Is it hv_reg_crq() from
> spapr_vtpm_activate()? If that so, then I guess we can just drop all
> three "response CRQ has been set".
It happens in get_response_crq(). I dropped the comments.
> +
> + vtpm_drv_state_set(VTPM_DRV_STATE_SEND_TPM_CMD,
> + VTPM_DRV_ERROR_NO_FAILURE);
> +
> + rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
> +
> + if (rc == H_SUCCESS) {
> + vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_TPM_RSP,
> + VTPM_DRV_ERROR_NO_FAILURE);
> + } else {
> + vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_INIT,
> + VTPM_DRV_ERROR_UNEXPECTED_SEND_ERROR);
> + }
> No need in curly braces here.
Fixed.
Thanks.
Stefan
More information about the SLOF
mailing list