All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface
@ 2018-03-28 20:59 Stefan Berger
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default Stefan Berger
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Stefan Berger @ 2018-03-28 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, Stefan Berger

This patch improves yet more flags of the TPM CRB interface and adds
more test to the CRB test case. Ideally we could have this for 2.12.
I tested it with UEFI and it works as before.

   Stefan

Stefan Berger (4):
  tpm: CRB: set the Idle flag by default
  tpm: CRB: Reset Granted flag when relinquishing locality
  tpm: CRB: Enforce locality is requested before processing buffer
  tests: Tests more flags of the CRB interface

 hw/tpm/tpm_crb.c     | 17 +++++++++++-
 tests/tpm-crb-test.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 88 insertions(+), 3 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default
  2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
@ 2018-03-28 20:59 ` Stefan Berger
  2018-03-29 10:44   ` Marc-André Lureau
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 2/4] tpm: CRB: Reset Granted flag when relinquishing locality Stefan Berger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2018-03-28 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, Stefan Berger

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_crb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index ef8b80e..e728b55 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -220,6 +220,8 @@ static void tpm_crb_reset(void *dev)
 
     ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
                      tpmRegValidSts, 1);
+    ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS,
+                     tpmIdle, 1);
     ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
                      InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE);
     ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/4] tpm: CRB: Reset Granted flag when relinquishing locality
  2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default Stefan Berger
@ 2018-03-28 20:59 ` Stefan Berger
  2018-03-29 10:43   ` Marc-André Lureau
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 3/4] tpm: CRB: Enforce locality is requested before processing buffer Stefan Berger
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2018-03-28 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, Stefan Berger

Reset the Granted flag when relinquishing a locality.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_crb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index e728b55..ee6c87e 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -145,6 +145,8 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
         case CRB_LOC_CTRL_RELINQUISH:
             ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
                              locAssigned, 0);
+            ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS,
+                             Granted, 0);
             break;
         case CRB_LOC_CTRL_REQUEST_ACCESS:
             ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/4] tpm: CRB: Enforce locality is requested before processing buffer
  2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default Stefan Berger
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 2/4] tpm: CRB: Reset Granted flag when relinquishing locality Stefan Berger
@ 2018-03-28 20:59 ` Stefan Berger
  2018-03-29 10:45   ` Marc-André Lureau
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 4/4] tests: Tests more flags of the CRB interface Stefan Berger
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2018-03-28 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, Stefan Berger

Section 5.5.3.2.2 of the CRB specs states that use of the TPM
through the localty control method must first be requested,
otherwise TPM commands will be dropped. This patch makes sure
that the current locality is the active locality and only then
sends off the command for processing.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 hw/tpm/tpm_crb.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index ee6c87e..520e74e 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -76,6 +76,8 @@ enum crb_cancel {
     CRB_CANCEL_INVOKE = BIT(0),
 };
 
+#define TPM_CRB_NO_LOCALITY 0xff
+
 static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr,
                                   unsigned size)
 {
@@ -95,10 +97,18 @@ static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr,
     return val;
 }
 
+static uint8_t tpm_crb_get_active_locty(CRBState *s)
+{
+    if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned))
+         return TPM_CRB_NO_LOCALITY;
+    return ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, activeLocality);
+}
+
 static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
                                uint64_t val, unsigned size)
 {
     CRBState *s = CRB(opaque);
+    uint8_t locty =  addr >> 12;
 
     trace_tpm_crb_mmio_write(addr, size, val);
 
@@ -123,7 +133,8 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
         break;
     case A_CRB_CTRL_START:
         if (val == CRB_START_INVOKE &&
-            !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE)) {
+            !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) &&
+            tpm_crb_get_active_locty(s) == locty) {
             void *mem = memory_region_get_ram_ptr(&s->cmdmem);
 
             s->regs[R_CRB_CTRL_START] |= CRB_START_INVOKE;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/4] tests: Tests more flags of the CRB interface
  2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
                   ` (2 preceding siblings ...)
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 3/4] tpm: CRB: Enforce locality is requested before processing buffer Stefan Berger
@ 2018-03-28 20:59 ` Stefan Berger
  2018-03-29 12:30   ` Marc-André Lureau
  2018-03-29 10:21 ` [Qemu-devel] [PATCH 0/4] tpm: More improvements on " no-reply
  2018-03-31  8:03 ` no-reply
  5 siblings, 1 reply; 11+ messages in thread
From: Stefan Berger @ 2018-03-28 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, Stefan Berger

Test and modify more flags of the CRB interface.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 tests/tpm-crb-test.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/tests/tpm-crb-test.c b/tests/tpm-crb-test.c
index e1513cb..d8f9569 100644
--- a/tests/tpm-crb-test.c
+++ b/tests/tpm-crb-test.c
@@ -28,6 +28,10 @@ static void tpm_crb_test(const void *data)
     uint64_t caddr = readq(TPM_CRB_ADDR_BASE + A_CRB_CTRL_CMD_LADDR);
     uint32_t rsize = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_RSP_SIZE);
     uint64_t raddr = readq(TPM_CRB_ADDR_BASE + A_CRB_CTRL_RSP_ADDR);
+    uint8_t locstate = readb(TPM_CRB_ADDR_BASE + A_CRB_LOC_STATE);
+    uint32_t locctrl = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL);
+    uint32_t locsts = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_STS);
+    uint32_t sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
 
     g_assert_cmpint(FIELD_EX32(intfid, CRB_INTF_ID, InterfaceType), ==, 1);
     g_assert_cmpint(FIELD_EX32(intfid, CRB_INTF_ID, InterfaceVersion), ==, 1);
@@ -45,9 +49,47 @@ static void tpm_crb_test(const void *data)
     g_assert_cmpint(caddr, >, TPM_CRB_ADDR_BASE);
     g_assert_cmpint(raddr, >, TPM_CRB_ADDR_BASE);
 
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmEstablished), ==, 1);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, locAssigned), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, activeLocality), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, reserved), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmRegValidSts), ==, 1);
+
+    g_assert_cmpint(locctrl, ==, 0);
+
+    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, Granted), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, beenSeized), ==, 0);
+
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 1);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
+
+    /* request access to locality 0 */
+    writeb(TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL, 1);
+
+    /* granted bit must be set now */
+    locsts = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_STS);
+    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, Granted), ==, 1);
+    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, beenSeized), ==, 0);
+
+    /* we must have an assigned locality */
+    locstate = readb(TPM_CRB_ADDR_BASE + A_CRB_LOC_STATE);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmEstablished), ==, 1);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, locAssigned), ==, 1);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, activeLocality), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, reserved), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmRegValidSts), ==, 1);
+
+    /* set into ready state */
+    writel(TPM_CRB_ADDR_BASE + A_CRB_CTRL_REQ, 1);
+
+    /* TPM must not be in the idle state */
+    sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 0);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
+
     memwrite(caddr, TPM_CMD, sizeof(TPM_CMD));
 
-    uint32_t sts, start = 1;
+    uint32_t start = 1;
     uint64_t end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
     writel(TPM_CRB_ADDR_BASE + A_CRB_CTRL_START, start);
     do {
@@ -58,12 +100,40 @@ static void tpm_crb_test(const void *data)
     } while (g_get_monotonic_time() < end_time);
     start = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_START);
     g_assert_cmpint(start & 1, ==, 0);
+
+    /* TPM must still not be in the idle state */
     sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
-    g_assert_cmpint(sts & 1, ==, 0);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 0);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
 
     struct tpm_hdr tpm_msg;
     memread(raddr, &tpm_msg, sizeof(tpm_msg));
     g_assert_cmpmem(&tpm_msg, sizeof(tpm_msg), s->tpm_msg, sizeof(*s->tpm_msg));
+
+    /* set TPM into idle state */
+    writel(TPM_CRB_ADDR_BASE + A_CRB_CTRL_REQ, 2);
+
+    /* idle state must be indicated now */
+    sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 1);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
+
+    /* relinquish locality */
+    writel(TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL, 2);
+
+    /* Granted flag must be cleared */
+    sts = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_STS);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_LOC_STS, Granted), ==, 0);
+    g_assert_cmpint(FIELD_EX32(sts, CRB_LOC_STS, beenSeized), ==, 0);
+
+    /* no locality may be assigned */
+    locstate = readb(TPM_CRB_ADDR_BASE + A_CRB_LOC_STATE);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmEstablished), ==, 1);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, locAssigned), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, activeLocality), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, reserved), ==, 0);
+    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmRegValidSts), ==, 1);
+
 }
 
 int main(int argc, char **argv)
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface
  2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
                   ` (3 preceding siblings ...)
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 4/4] tests: Tests more flags of the CRB interface Stefan Berger
@ 2018-03-29 10:21 ` no-reply
  2018-03-31  8:03 ` no-reply
  5 siblings, 0 replies; 11+ messages in thread
From: no-reply @ 2018-03-29 10:21 UTC (permalink / raw)
  To: stefanb; +Cc: famz, qemu-devel, marcandre.lureau

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1522270761-29646-1-git-send-email-stefanb@linux.vnet.ibm.com
Subject: [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
09491e26e3 tests: Tests more flags of the CRB interface
d55c9446c4 tpm: CRB: Enforce locality is requested before processing buffer
bc0cf354f6 tpm: CRB: Reset Granted flag when relinquishing locality
d5030ad3b6 tpm: CRB: set the Idle flag by default

=== OUTPUT BEGIN ===
Checking PATCH 1/4: tpm: CRB: set the Idle flag by default...
Checking PATCH 2/4: tpm: CRB: Reset Granted flag when relinquishing locality...
Checking PATCH 3/4: tpm: CRB: Enforce locality is requested before processing buffer...
ERROR: suspect code indent for conditional statements (4, 9)
#35: FILE: hw/tpm/tpm_crb.c:102:
+    if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned))
+         return TPM_CRB_NO_LOCALITY;

ERROR: braces {} are necessary for all arms of this statement
#35: FILE: hw/tpm/tpm_crb.c:102:
+    if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned))
[...]

total: 2 errors, 0 warnings, 35 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/4: tests: Tests more flags of the CRB interface...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] tpm: CRB: Reset Granted flag when relinquishing locality
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 2/4] tpm: CRB: Reset Granted flag when relinquishing locality Stefan Berger
@ 2018-03-29 10:43   ` Marc-André Lureau
  0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2018-03-29 10:43 UTC (permalink / raw)
  To: Stefan Berger; +Cc: QEMU

On Wed, Mar 28, 2018 at 10:59 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Reset the Granted flag when relinquishing a locality.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Makes sense. I tried to avoid looking at locality management so far,
but I guess a minimum is necessary. I suppose no locality will be
granted in this case after relinquish, until a new request. But the
spec says: "If two localities have requested use of the TPMwhen
the current locality relinquishes it, the locality with the highest
priority getsaccess to the TPM." Probably that doesn't make sense with
only loc 0, as you can't request, then relinquish, and expect it to be
granted immediately after.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  hw/tpm/tpm_crb.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index e728b55..ee6c87e 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -145,6 +145,8 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
>          case CRB_LOC_CTRL_RELINQUISH:
>              ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
>                               locAssigned, 0);
> +            ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS,
> +                             Granted, 0);
>              break;
>          case CRB_LOC_CTRL_REQUEST_ACCESS:
>              ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS,
> --
> 2.5.5
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default Stefan Berger
@ 2018-03-29 10:44   ` Marc-André Lureau
  0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2018-03-29 10:44 UTC (permalink / raw)
  To: Stefan Berger; +Cc: QEMU

On Wed, Mar 28, 2018 at 10:59 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---

This is also the default state according to the spec.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>  hw/tpm/tpm_crb.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index ef8b80e..e728b55 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -220,6 +220,8 @@ static void tpm_crb_reset(void *dev)
>
>      ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE,
>                       tpmRegValidSts, 1);
> +    ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS,
> +                     tpmIdle, 1);
>      ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
>                       InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE);
>      ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID,
> --
> 2.5.5
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] tpm: CRB: Enforce locality is requested before processing buffer
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 3/4] tpm: CRB: Enforce locality is requested before processing buffer Stefan Berger
@ 2018-03-29 10:45   ` Marc-André Lureau
  0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2018-03-29 10:45 UTC (permalink / raw)
  To: Stefan Berger; +Cc: QEMU

On Wed, Mar 28, 2018 at 10:59 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Section 5.5.3.2.2 of the CRB specs states that use of the TPM
> through the localty control method must first be requested,
> otherwise TPM commands will be dropped. This patch makes sure
> that the current locality is the active locality and only then
> sends off the command for processing.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  hw/tpm/tpm_crb.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index ee6c87e..520e74e 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -76,6 +76,8 @@ enum crb_cancel {
>      CRB_CANCEL_INVOKE = BIT(0),
>  };
>
> +#define TPM_CRB_NO_LOCALITY 0xff
> +
>  static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr,
>                                    unsigned size)
>  {
> @@ -95,10 +97,18 @@ static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr,
>      return val;
>  }
>
> +static uint8_t tpm_crb_get_active_locty(CRBState *s)
> +{
> +    if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned))
> +         return TPM_CRB_NO_LOCALITY;
> +    return ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, activeLocality);
> +}
> +
>  static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
>                                 uint64_t val, unsigned size)
>  {
>      CRBState *s = CRB(opaque);
> +    uint8_t locty =  addr >> 12;
>
>      trace_tpm_crb_mmio_write(addr, size, val);
>
> @@ -123,7 +133,8 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr,
>          break;
>      case A_CRB_CTRL_START:
>          if (val == CRB_START_INVOKE &&
> -            !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE)) {
> +            !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) &&
> +            tpm_crb_get_active_locty(s) == locty) {
>              void *mem = memory_region_get_ram_ptr(&s->cmdmem);
>
>              s->regs[R_CRB_CTRL_START] |= CRB_START_INVOKE;
> --
> 2.5.5
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] tests: Tests more flags of the CRB interface
  2018-03-28 20:59 ` [Qemu-devel] [PATCH 4/4] tests: Tests more flags of the CRB interface Stefan Berger
@ 2018-03-29 12:30   ` Marc-André Lureau
  0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2018-03-29 12:30 UTC (permalink / raw)
  To: Stefan Berger; +Cc: QEMU

On Wed, Mar 28, 2018 at 10:59 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Test and modify more flags of the CRB interface.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>  tests/tpm-crb-test.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/tests/tpm-crb-test.c b/tests/tpm-crb-test.c
> index e1513cb..d8f9569 100644
> --- a/tests/tpm-crb-test.c
> +++ b/tests/tpm-crb-test.c
> @@ -28,6 +28,10 @@ static void tpm_crb_test(const void *data)
>      uint64_t caddr = readq(TPM_CRB_ADDR_BASE + A_CRB_CTRL_CMD_LADDR);
>      uint32_t rsize = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_RSP_SIZE);
>      uint64_t raddr = readq(TPM_CRB_ADDR_BASE + A_CRB_CTRL_RSP_ADDR);
> +    uint8_t locstate = readb(TPM_CRB_ADDR_BASE + A_CRB_LOC_STATE);
> +    uint32_t locctrl = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL);
> +    uint32_t locsts = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_STS);
> +    uint32_t sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
>
>      g_assert_cmpint(FIELD_EX32(intfid, CRB_INTF_ID, InterfaceType), ==, 1);
>      g_assert_cmpint(FIELD_EX32(intfid, CRB_INTF_ID, InterfaceVersion), ==, 1);
> @@ -45,9 +49,47 @@ static void tpm_crb_test(const void *data)
>      g_assert_cmpint(caddr, >, TPM_CRB_ADDR_BASE);
>      g_assert_cmpint(raddr, >, TPM_CRB_ADDR_BASE);
>
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmEstablished), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, locAssigned), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, activeLocality), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, reserved), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmRegValidSts), ==, 1);
> +
> +    g_assert_cmpint(locctrl, ==, 0);
> +
> +    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, Granted), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, beenSeized), ==, 0);
> +
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
> +
> +    /* request access to locality 0 */
> +    writeb(TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL, 1);
> +
> +    /* granted bit must be set now */
> +    locsts = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_STS);
> +    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, Granted), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(locsts, CRB_LOC_STS, beenSeized), ==, 0);
> +
> +    /* we must have an assigned locality */
> +    locstate = readb(TPM_CRB_ADDR_BASE + A_CRB_LOC_STATE);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmEstablished), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, locAssigned), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, activeLocality), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, reserved), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmRegValidSts), ==, 1);
> +
> +    /* set into ready state */
> +    writel(TPM_CRB_ADDR_BASE + A_CRB_CTRL_REQ, 1);
> +
> +    /* TPM must not be in the idle state */
> +    sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
> +
>      memwrite(caddr, TPM_CMD, sizeof(TPM_CMD));
>
> -    uint32_t sts, start = 1;
> +    uint32_t start = 1;
>      uint64_t end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
>      writel(TPM_CRB_ADDR_BASE + A_CRB_CTRL_START, start);
>      do {
> @@ -58,12 +100,40 @@ static void tpm_crb_test(const void *data)
>      } while (g_get_monotonic_time() < end_time);
>      start = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_START);
>      g_assert_cmpint(start & 1, ==, 0);
> +
> +    /* TPM must still not be in the idle state */
>      sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
> -    g_assert_cmpint(sts & 1, ==, 0);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
>
>      struct tpm_hdr tpm_msg;
>      memread(raddr, &tpm_msg, sizeof(tpm_msg));
>      g_assert_cmpmem(&tpm_msg, sizeof(tpm_msg), s->tpm_msg, sizeof(*s->tpm_msg));
> +
> +    /* set TPM into idle state */
> +    writel(TPM_CRB_ADDR_BASE + A_CRB_CTRL_REQ, 2);
> +
> +    /* idle state must be indicated now */
> +    sts = readl(TPM_CRB_ADDR_BASE + A_CRB_CTRL_STS);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmIdle), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_CTRL_STS, tpmSts), ==, 0);
> +
> +    /* relinquish locality */
> +    writel(TPM_CRB_ADDR_BASE + A_CRB_LOC_CTRL, 2);
> +
> +    /* Granted flag must be cleared */
> +    sts = readl(TPM_CRB_ADDR_BASE + A_CRB_LOC_STS);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_LOC_STS, Granted), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(sts, CRB_LOC_STS, beenSeized), ==, 0);
> +
> +    /* no locality may be assigned */
> +    locstate = readb(TPM_CRB_ADDR_BASE + A_CRB_LOC_STATE);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmEstablished), ==, 1);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, locAssigned), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, activeLocality), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, reserved), ==, 0);
> +    g_assert_cmpint(FIELD_EX32(locstate, CRB_LOC_STATE, tpmRegValidSts), ==, 1);
> +
>  }
>
>  int main(int argc, char **argv)
> --
> 2.5.5
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface
  2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
                   ` (4 preceding siblings ...)
  2018-03-29 10:21 ` [Qemu-devel] [PATCH 0/4] tpm: More improvements on " no-reply
@ 2018-03-31  8:03 ` no-reply
  5 siblings, 0 replies; 11+ messages in thread
From: no-reply @ 2018-03-31  8:03 UTC (permalink / raw)
  To: stefanb; +Cc: famz, qemu-devel, marcandre.lureau

Hi,

This series failed docker-build@min-glib build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1522270761-29646-1-git-send-email-stefanb@linux.vnet.ibm.com
Subject: [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
63bd6bb2f9 tests: Tests more flags of the CRB interface
2931ea72ea tpm: CRB: Enforce locality is requested before processing buffer
484cf347e0 tpm: CRB: Reset Granted flag when relinquishing locality
7726cdcf02 tpm: CRB: set the Idle flag by default

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-g2vwh6vb/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   min-glib
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-g2vwh6vb/src'
  GEN     /var/tmp/patchew-tester-tmp-g2vwh6vb/src/docker-src.2018-03-31-04.03.14.12625/qemu.tar
Cloning into '/var/tmp/patchew-tester-tmp-g2vwh6vb/src/docker-src.2018-03-31-04.03.14.12625/qemu.tar.vroot'...
done.
Checking out files:  32% (1958/6066)   
Checking out files:  33% (2002/6066)   
Checking out files:  34% (2063/6066)   
Checking out files:  35% (2124/6066)   
Checking out files:  36% (2184/6066)   
Checking out files:  37% (2245/6066)   
Checking out files:  38% (2306/6066)   
Checking out files:  39% (2366/6066)   
Checking out files:  40% (2427/6066)   
Checking out files:  41% (2488/6066)   
Checking out files:  42% (2548/6066)   
Checking out files:  43% (2609/6066)   
Checking out files:  44% (2670/6066)   
Checking out files:  45% (2730/6066)   
Checking out files:  45% (2739/6066)   
Checking out files:  46% (2791/6066)   
Checking out files:  47% (2852/6066)   
Checking out files:  48% (2912/6066)   
Checking out files:  49% (2973/6066)   
Checking out files:  50% (3033/6066)   
Checking out files:  51% (3094/6066)   
Checking out files:  52% (3155/6066)   
Checking out files:  53% (3215/6066)   
Checking out files:  54% (3276/6066)   
Checking out files:  55% (3337/6066)   
Checking out files:  56% (3397/6066)   
Checking out files:  57% (3458/6066)   
Checking out files:  58% (3519/6066)   
Checking out files:  59% (3579/6066)   
Checking out files:  60% (3640/6066)   
Checking out files:  61% (3701/6066)   
Checking out files:  62% (3761/6066)   
Checking out files:  63% (3822/6066)   
Checking out files:  64% (3883/6066)   
Checking out files:  65% (3943/6066)   
Checking out files:  66% (4004/6066)   
Checking out files:  67% (4065/6066)   
Checking out files:  68% (4125/6066)   
Checking out files:  69% (4186/6066)   
Checking out files:  70% (4247/6066)   
Checking out files:  71% (4307/6066)   
Checking out files:  72% (4368/6066)   
Checking out files:  73% (4429/6066)   
Checking out files:  74% (4489/6066)   
Checking out files:  75% (4550/6066)   
Checking out files:  76% (4611/6066)   
Checking out files:  77% (4671/6066)   
Checking out files:  78% (4732/6066)   
Checking out files:  79% (4793/6066)   
Checking out files:  80% (4853/6066)   
Checking out files:  81% (4914/6066)   
Checking out files:  82% (4975/6066)   
Checking out files:  82% (5005/6066)   
Checking out files:  83% (5035/6066)   
Checking out files:  84% (5096/6066)   
Checking out files:  85% (5157/6066)   
Checking out files:  86% (5217/6066)   
Checking out files:  87% (5278/6066)   
Checking out files:  88% (5339/6066)   
Checking out files:  89% (5399/6066)   
Checking out files:  90% (5460/6066)   
Checking out files:  91% (5521/6066)   
Checking out files:  92% (5581/6066)   
Checking out files:  93% (5642/6066)   
Checking out files:  94% (5703/6066)   
Checking out files:  95% (5763/6066)   
Checking out files:  96% (5824/6066)   
Checking out files:  97% (5885/6066)   
Checking out files:  98% (5945/6066)   
Checking out files:  99% (6006/6066)   
Checking out files: 100% (6066/6066)   
Checking out files: 100% (6066/6066), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-g2vwh6vb/src/docker-src.2018-03-31-04.03.14.12625/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-g2vwh6vb/src/docker-src.2018-03-31-04.03.14.12625/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
tar: /var/tmp/patchew-tester-tmp-g2vwh6vb/src/docker-src.2018-03-31-04.03.14.12625/qemu.tar: Wrote only 2048 of 10240 bytes
tar: Error is not recoverable: exiting now
failed to create tar file
  COPY    RUNNER
    RUN test-build in qemu:min-glib 
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
/var/tmp/qemu/run: line 32: prep_fail: command not found
Environment variables:
HOSTNAME=9f20b3428f8b
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

/var/tmp/qemu/run: line 52: cd: /tmp/qemu-test/src/tests/docker: No such file or directory
/var/tmp/qemu/run: line 57: /test-build: No such file or directory
/var/tmp/qemu/run: line 57: exec: /test-build: cannot execute: No such file or directory
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 261, in run
    return Docker().run(argv, args.keep, quiet=args.quiet)
  File "./tests/docker/docker.py", line 229, in run
    quiet=quiet)
  File "./tests/docker/docker.py", line 147, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'run', '--label', 'com.qemu.instance.uuid=03bd9f6234ba11e8a9ca52540069c830', '-u', '0', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-g2vwh6vb/src/docker-src.2018-03-31-04.03.14.12625:/var/tmp/qemu:z,ro', 'qemu:min-glib', '/var/tmp/qemu/run', 'test-build']' returned non-zero exit status 126
make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-g2vwh6vb/src'
make: *** [tests/docker/Makefile.include:163: docker-run-test-build@min-glib] Error 2

real	0m34.998s
user	0m9.431s
sys	0m6.617s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-03-31  8:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 20:59 [Qemu-devel] [PATCH 0/4] tpm: More improvements on CRB interface Stefan Berger
2018-03-28 20:59 ` [Qemu-devel] [PATCH 1/4] tpm: CRB: set the Idle flag by default Stefan Berger
2018-03-29 10:44   ` Marc-André Lureau
2018-03-28 20:59 ` [Qemu-devel] [PATCH 2/4] tpm: CRB: Reset Granted flag when relinquishing locality Stefan Berger
2018-03-29 10:43   ` Marc-André Lureau
2018-03-28 20:59 ` [Qemu-devel] [PATCH 3/4] tpm: CRB: Enforce locality is requested before processing buffer Stefan Berger
2018-03-29 10:45   ` Marc-André Lureau
2018-03-28 20:59 ` [Qemu-devel] [PATCH 4/4] tests: Tests more flags of the CRB interface Stefan Berger
2018-03-29 12:30   ` Marc-André Lureau
2018-03-29 10:21 ` [Qemu-devel] [PATCH 0/4] tpm: More improvements on " no-reply
2018-03-31  8:03 ` no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.