Re: [PATCH] drivers/tpm-xen: Change vTPM shared page ABI

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 03/22/2013 04:26 AM, Jan Beulich wrote:
>>>> On 21.03.13 at 21:12, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>> --- a/include/xen/interface/io/tpmif.h
>> +++ b/include/xen/interface/io/tpmif.h
>> @@ -1,7 +1,7 @@
>>
>> /******************************************************************************
>>    * tpmif.h
>>    *
>> - * TPM I/O interface for Xen guest OSes.
>> + * TPM I/O interface for Xen guest OSes, v2
>>    *
>>    * Permission is hereby granted, free of charge, to any person obtaining a copy
>>    * of this software and associated documentation files (the "Software"), to
>> @@ -21,45 +21,30 @@
>>    * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>    * DEALINGS IN THE SOFTWARE.
>>    *
>> - * Copyright (c) 2005, IBM Corporation
>> - *
>> - * Author: Stefan Berger, stefanb@us.ibm.com
>> - * Grant table support: Mahadevan Gomathisankaran
>> - *
>> - * This code has been derived from tools/libxc/xen/io/netif.h
>> - *
>> - * Copyright (c) 2003-2004, Keir Fraser
>>    */
>>
>>   #ifndef __XEN_PUBLIC_IO_TPMIF_H__
>>   #define __XEN_PUBLIC_IO_TPMIF_H__
>>
>> -#include "../grant_table.h"
>> -
>> -struct tpmif_tx_request {
>> -	unsigned long addr;   /* Machine address of packet.   */
>> -	grant_ref_t ref;      /* grant table access reference */
>> -	uint16_t unused;
>> -	uint16_t size;        /* Packet size in bytes.        */
>> +enum vtpm_shared_page_state {
>> +	VTPM_STATE_IDLE,         /* no contents / vTPM idle / cancel complete */
>> +	VTPM_STATE_SUBMIT,       /* request ready / vTPM working */
>> +	VTPM_STATE_FINISH,       /* response ready / vTPM idle */
>> +	VTPM_STATE_CANCEL,       /* cancel requested / vTPM working */
>>   };
>> -struct tpmif_tx_request;
>> +/* The backend should only change state to IDLE or FINISH, while the
>> + * frontend should only change to SUBMIT or CANCEL. */
>>
>> -/*
>> - * The TPMIF_TX_RING_SIZE defines the number of pages the
>> - * front-end and backend can exchange (= size of array).
>> - */
>> -#define TPMIF_TX_RING_SIZE 1
>>
>> -/* This structure must fit in a memory page. */
>> +struct vtpm_shared_page {
>> +	uint32_t length;         /* request/response length in bytes */
>>
>> -struct tpmif_ring {
>> -	struct tpmif_tx_request req;
>> -};
>> -struct tpmif_ring;
>> +	uint8_t state;           /* enum vtpm_shared_page_state */
>> +	uint8_t locality;        /* for the current request */
>> +	uint8_t pad;
>>
>> -struct tpmif_tx_interface {
>> -	struct tpmif_ring ring[TPMIF_TX_RING_SIZE];
>> +	uint8_t nr_extra_pages;  /* extra pages for long packets; may be zero */
>> +	uint32_t extra_pages[0]; /* grant IDs; length is actually nr_extra_pages */
>>   };
>> -struct tpmif_tx_interface;
>>
>>   #endif
>
> I'm relatively certain I said this before: For one, a patch to the
> master copy of this header is going to be needed. And you can't
> just rip out the old interface - existing consumers must continue
> to build (and work, with an old interface counterpart).
>
> Jan
>

The master copy of the header was changed in the patch series that this
message was a reply to (in patch 01/12). The changes are identical
except for whitespace conversion.

Since the structure names between the v1 and v2 interfaces do not
collide, the new shared page definition can just be added to the header
instead of replacing the old definitions; I will do that in the next
version. The reason I did not do that in the current version is that the
old interface could not have any users as it is not yet in an upstream
kernel.

-- 
Daniel De Graaf
National Security Agency