Re: [Qemu-devel] [PATCH V14 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

[Michael S. Tsirkin]

On Mon, Feb 20, 2012 at 10:48:09AM -0500, Stefan Berger wrote:
> On 02/20/2012 03:51 AM, Michael S. Tsirkin wrote:
> >On Wed, Dec 14, 2011 at 08:43:17AM -0500, Stefan Berger wrote:
> >>This patch adds the main code of the TPM frontend driver, the TPM TIS
> >>interface, to Qemu. The code is largely based on the previous implementation
> >>for Xen but has been significantly extended to meet the standard's
> >>requirements, such as the support for changing of localities and all the
> >>functionality of the available flags.
> >>
> >looks good overall
> >some questions on erro handling and behaviour with
> >a malicious guest, below.
> >
> 
> [...]
> >>+#ifndef RAISE_STS_IRQ
> >>+
> >>+# define TPM_TIS_INTERRUPTS_SUPPORTED (TPM_TIS_INT_LOCALITY_CHANGED | \
> >>+                                       TPM_TIS_INT_DATA_AVAILABLE   | \
> >>+                                       TPM_TIS_INT_COMMAND_READY)
> >>+
> >>+#else
> >>+
> >>+# define TPM_TIS_INTERRUPTS_SUPPORTED (TPM_TIS_INT_LOCALITY_CHANGED | \
> >>+                                       TPM_TIS_INT_DATA_AVAILABLE   | \
> >>+                                       TPM_TIS_INT_STS_VALID | \
> >>+                                       TPM_TIS_INT_COMMAND_READY)
> >>+
> >let's keep these as #define
> >
> 
> Will fix it.
> >>+/* utility functions */
> >>+
> >>+static uint8_t tpm_tis_locality_from_addr(target_phys_addr_t addr)
> >>+{
> >>+    return (uint8_t)((addr>>  12)&  0x7);
> >>+}
> >>+
> >note this can give a number 0..6
> 
> Yes, but that's ok. We are only registering the MMIO region [fed4
> 0000, fed4 5000[, so anything larger than 4 cannot actually come
> from this function.

I see 12 matches 0x1000 where region is created.
I think there should be a macro like TPM_TIS_LOCALITY_SHIFT
and then you can register region of size
 NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT.