qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Berger
Subject: Re: [Qemu-devel] [PATCH V19 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu
Date: Mon, 12 Nov 2012 08:16:28 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0

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;
+                }
[...]


+            case TPM_TIS_STATUS_EXECUTION:
+            case TPM_TIS_STATUS_RECEPTION:
+                /* abort currently running command */
+                dprintf("tpm_tis: %s: Initiating abort.\n",
+                        __func__);
+                tpm_tis_prep_abort(s, locty, locty);
+            break;
+
+            case TPM_TIS_STATUS_COMPLETION:

Does this path need to abort if TPM_STS_x.dataAvail is on? This
comment is based on "Table 19: State Transition Table." from the TPM
TIS document.


If TPM_TIS_STATUS_COMPLETION is the current state, then independent of
the TPM_TIS_STS_DATA_AVAILABLE flag the state transition is to idle
(states 30 and 37 in the spec). Following state 0.B in the spec, we
implement a TPM without idle state and so we transition to READY state
immediately. The data available flag should be reset, though.


Ok. But row 30 in the table also says it aborts the command in the "Action Taken" column.


Row 30 describes that abort for while the TPM is in 'Completion' state, meaning the TPM has delivered the response from the TPM to the TIS and now an application can pick up the response. The abort in this case is achieved through changing the state and resetting the (response) buffer pointers so that the application will not receive more response bytes.

Regards,
    Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]