From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45218) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXz4I-0004tU-UH for qemu-devel@nongnu.org; Mon, 12 Nov 2012 13:49:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TXz4F-0000v2-RX for qemu-devel@nongnu.org; Mon, 12 Nov 2012 13:49:10 -0500 Received: from e39.co.us.ibm.com ([32.97.110.160]:53358) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXz4F-0000rD-Ji for qemu-devel@nongnu.org; Mon, 12 Nov 2012 13:49:07 -0500 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 12 Nov 2012 11:49:05 -0700 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 3BC75C40008 for ; Mon, 12 Nov 2012 11:48:45 -0700 (MST) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qACImdnq098152 for ; Mon, 12 Nov 2012 11:48:39 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qACImcG6024449 for ; Mon, 12 Nov 2012 11:48:38 -0700 Message-ID: <50A14484.1000109@linux.vnet.ibm.com> Date: Mon, 12 Nov 2012 13:48:36 -0500 From: Corey Bryant MIME-Version: 1.0 References: <1338838668-7544-1-git-send-email-stefanb@linux.vnet.ibm.com> <1338838668-7544-3-git-send-email-stefanb@linux.vnet.ibm.com> <5064612E.5080700@linux.vnet.ibm.com> <5088376B.8020504@linux.vnet.ibm.com> <509BD240.50502@linux.vnet.ibm.com> <50A0F6AC.4010407@linux.vnet.ibm.com> In-Reply-To: <50A0F6AC.4010407@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V19 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Berger Cc: andreas.niederl@iaik.tugraz.at, qemu-devel@nongnu.org, anthony@codemonkey.ws, mst@redhat.com On 11/12/2012 08:16 AM, Stefan Berger wrote: > On 11/08/2012 10:39 AM, Corey Bryant wrote: >> Thanks for your responses. I have a few comments below. >> >> On 10/24/2012 02:46 PM, Stefan Berger wrote: >>> On 09/27/2012 10:22 AM, Corey Bryant wrote: >>>> >>>> >>>> On 06/04/2012 03:37 PM, Stefan Berger wrote: >>> >>>>> + /* check for ongoing seize by a higher locality */ >>>>> + for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES; >>>>> l++) { >>>>> + if ((tis->loc[l].access & >>>>> TPM_TIS_ACCESS_SEIZE)) { >>>>> + break; >>>> >>>> Were you intending to break from the for loop or the while? >>>> >>> >>> Right. I am setting a flag here now to then leave the while loop. >>> >> >> Are you setting the flag or testing it? I'm not sure this code is >> serving any purpose the way it is, since it is testing the flag and >> then breaking from the for loop if it's on. That's why I was >> wondering if you meant to break from the while loop instead. >> > > Here's how the patch looks now: > > > + if ((val & TPM_TIS_ACCESS_SEIZE)) { > + /* > + * allow seize if a locality is active and the requesting > + * locality is higher than the one that's active > + * OR > + * allow seize for requesting locality if no locality is > + * active > + */ > + while ((TPM_TIS_IS_VALID_LOCTY(tis->active_locty) && > + locty > tis->active_locty) || > + !TPM_TIS_IS_VALID_LOCTY(tis->active_locty)) { > + bool higher_seize = FALSE; > + > + /* already a pending SEIZE ? */ > + if ((tis->loc[locty].access & TPM_TIS_ACCESS_SEIZE)) { > + break; > + } > + > + /* check for ongoing seize by a higher locality */ > + for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES; l++) { > + if ((tis->loc[l].access & TPM_TIS_ACCESS_SEIZE)) { > + higher_seize = TRUE; > + break; > + } > + } > + > + if (higher_seize) { > + break; > + } > + > + /* cancel any seize by a lower locality */ > + for (l = 0; l < locty - 1; l++) { > + tis->loc[l].access &= ~TPM_TIS_ACCESS_SEIZE; > + } > [...] > > Ok that looks good. -- Regards, Corey Bryant