From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:53035)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1RzZ3Q-0002yT-5Y
	for qemu-devel@nongnu.org; Mon, 20 Feb 2012 14:37:45 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1RzZ3O-0001XV-Tt
	for qemu-devel@nongnu.org; Mon, 20 Feb 2012 14:37:44 -0500
Received: from mx1.redhat.com ([209.132.183.28]:41702)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1RzZ3O-0001X9-NV
	for qemu-devel@nongnu.org; Mon, 20 Feb 2012 14:37:42 -0500
Date: Mon, 20 Feb 2012 21:37:28 +0200
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20120220193728.GB18308@redhat.com>
References: <1323870202-25742-1-git-send-email-stefanb@linux.vnet.ibm.com>
	<1323870202-25742-3-git-send-email-stefanb@linux.vnet.ibm.com>
	<20120220085150.GB10364@redhat.com>
	<4F426B39.4020703@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4F426B39.4020703@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH V14 2/7] Add TPM (frontend) hardware
 interface (TPM TIS) to Qemu
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: qemu-devel@nongnu.org, andreas.niederl@iaik.tugraz.at

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.