[Qemu-devel] [PULL v1 0/2] Merge tpm 2019/02/25 v1

[Qemu-devel] [PULL v1 0/2] Merge tpm 2019/02/25 v1

[Stefan Berger]

The 1st patch in this series fixes a bug in the TPM TIS emulator that
skipped a locality when resetting the seize flag when a higher locality
seized access from a lower locality. The 2nd patch converts debugging
code to tracing.

   Stefan


The following changes since commit 8eb29f1bf5a974dc4c11d2d1f5e7c7f7a62be116:

  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190221.0' into staging (2019-02-22 15:48:04 +0000)

are available in the Git repository at:

  git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2019-02-25-1

for you to fetch changes up to cd38cc519b60bbe5dd8d85f6aaa72b2d7902930b:

  tpm_tis: convert tpm_tis_show_buffer() to use trace event (2019-02-24 14:46:14 -0500)

----------------------------------------------------------------
Liam Merwick (2):
      tpm_tis: fix loop that cancels any seizure by a lower locality
      tpm_tis: convert tpm_tis_show_buffer() to use trace event

 hw/tpm/tpm_tis.c    | 33 ++++++++++++++++++++-------------
 hw/tpm/trace-events |  1 +
 2 files changed, 21 insertions(+), 13 deletions(-)

-- 
2.17.2

[Qemu-devel] [PULL v1 1/2] tpm_tis: fix loop that cancels any seizure by a lower locality

[Stefan Berger]

From: Liam Merwick <liam.merwick@oracle.com>

In tpm_tis_mmio_write() if the requesting locality is seizing
access, any seizure by a lower locality is cancelled.  However the
loop doing the seizure had an off-by-one error and the locality
immediately preceding the requesting locality was not being cleared.
This is fixed by adjusting the test in the for loop to check the
localities up to the requesting locality.

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index fd6bb9b59a..61a130beef 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -624,7 +624,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
                 }
 
                 /* cancel any seize by a lower locality */
-                for (l = 0; l < locty - 1; l++) {
+                for (l = 0; l < locty; l++) {
                     s->loc[l].access &= ~TPM_TIS_ACCESS_SEIZE;
                 }
 
-- 
2.17.2

[Qemu-devel] [PULL v1 2/2] tpm_tis: convert tpm_tis_show_buffer() to use trace event

[Stefan Berger]

From: Liam Merwick <liam.merwick@oracle.com>

cppcheck reports:

[hw/tpm/tpm_tis.c:113]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'

Rather than just converting the format specifier to use '%u", the
tpm_tis_show_buffer() function is converted to use trace points and
the two debug callers use the trace event infrastructure so that it's
available in production cases also and not just when DEBUG_TIS is enabled.

Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis.c    | 31 +++++++++++++++++++------------
 hw/tpm/trace-events |  1 +
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 61a130beef..fd183e8deb 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -106,17 +106,26 @@ static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
 static void tpm_tis_show_buffer(const unsigned char *buffer,
                                 size_t buffer_size, const char *string)
 {
-    uint32_t len, i;
+    size_t len, i;
+    char *line_buffer, *p;
 
     len = MIN(tpm_cmd_get_size(buffer), buffer_size);
-    printf("tpm_tis: %s length = %d\n", string, len);
-    for (i = 0; i < len; i++) {
+
+    /*
+     * allocate enough room for 3 chars per buffer entry plus a
+     * newline after every 16 chars and a final null terminator.
+     */
+    line_buffer = g_malloc(len * 3 + (len / 16) + 1);
+
+    for (i = 0, p = line_buffer; i < len; i++) {
         if (i && !(i % 16)) {
-            printf("\n");
+            p += sprintf(p, "\n");
         }
-        printf("%.2X ", buffer[i]);
+        p += sprintf(p, "%.2X ", buffer[i]);
     }
-    printf("\n");
+    trace_tpm_tis_show_buffer(string, len, line_buffer);
+
+    g_free(line_buffer);
 }
 
 /*
@@ -143,9 +152,8 @@ static void tpm_tis_sts_set(TPMLocality *l, uint32_t flags)
  */
 static void tpm_tis_tpm_send(TPMState *s, uint8_t locty)
 {
-    if (DEBUG_TIS) {
-        tpm_tis_show_buffer(s->buffer, s->be_buffer_size,
-                            "tpm_tis: To TPM");
+    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
+        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "To TPM");
     }
 
     /*
@@ -313,9 +321,8 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
     s->loc[locty].state = TPM_TIS_STATE_COMPLETION;
     s->rw_offset = 0;
 
-    if (DEBUG_TIS) {
-        tpm_tis_show_buffer(s->buffer, s->be_buffer_size,
-                            "tpm_tis: From TPM");
+    if (trace_event_get_state_backends(TRACE_TPM_TIS_SHOW_BUFFER)) {
+        tpm_tis_show_buffer(s->buffer, s->be_buffer_size, "From TPM");
     }
 
     if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 920d32ad55..f45dcd2209 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -36,6 +36,7 @@ tpm_emulator_pre_save(void) ""
 tpm_emulator_inst_init(void) ""
 
 # hw/tpm/tpm_tis.c
+tpm_tis_show_buffer(const char *direction, size_t len, const char *buf) "direction: %s len: %zu\nbuf: %s"
 tpm_tis_raise_irq(uint32_t irqmask) "Raising IRQ for flag 0x%08x"
 tpm_tis_new_active_locality(uint8_t locty) "Active locality is now %d"
 tpm_tis_abort(uint8_t locty) "New active locality is %d"
-- 
2.17.2

Re: [Qemu-devel] [PULL v1 0/2] Merge tpm 2019/02/25 v1

[Peter Maydell]

On Mon, 4 Mar 2019 at 15:47, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
> The 1st patch in this series fixes a bug in the TPM TIS emulator that
> skipped a locality when resetting the seize flag when a higher locality
> seized access from a lower locality. The 2nd patch converts debugging
> code to tracing.
>
>    Stefan
>
>
> The following changes since commit 8eb29f1bf5a974dc4c11d2d1f5e7c7f7a62be116:
>
>   Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190221.0' into staging (2019-02-22 15:48:04 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2019-02-25-1
>
> for you to fetch changes up to cd38cc519b60bbe5dd8d85f6aaa72b2d7902930b:
>
>   tpm_tis: convert tpm_tis_show_buffer() to use trace event (2019-02-24 14:46:14 -0500)
>
> ----------------------------------------------------------------
> Liam Merwick (2):
>       tpm_tis: fix loop that cancels any seizure by a lower locality
>       tpm_tis: convert tpm_tis_show_buffer() to use trace event

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PULL v1 0/2] Merge tpm 2019/02/25 v1

[Stefan Berger]

On 3/4/19 11:43 AM, Peter Maydell wrote:
> On Mon, 4 Mar 2019 at 15:47, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>> The 1st patch in this series fixes a bug in the TPM TIS emulator that
>> skipped a locality when resetting the seize flag when a higher locality
>> seized access from a lower locality. The 2nd patch converts debugging
>> code to tracing.
>>
>>     Stefan
>>
>>
>> The following changes since commit 8eb29f1bf5a974dc4c11d2d1f5e7c7f7a62be116:
>>
>>    Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190221.0' into staging (2019-02-22 15:48:04 +0000)
>>
>> are available in the Git repository at:
>>
>>    git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2019-02-25-1
>>
>> for you to fetch changes up to cd38cc519b60bbe5dd8d85f6aaa72b2d7902930b:
>>
>>    tpm_tis: convert tpm_tis_show_buffer() to use trace event (2019-02-24 14:46:14 -0500)
>>
>> ----------------------------------------------------------------
>> Liam Merwick (2):
>>        tpm_tis: fix loop that cancels any seizure by a lower locality
>>        tpm_tis: convert tpm_tis_show_buffer() to use trace event
> Applied, thanks.
>
> Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
> for any user-visible changes.


Done.


>
> -- PMM
>