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.
