[Qemu-devel] [PATCH v2 1/3] tpm_tis: fix loop that cancels any seizure by a lower locality

[Qemu-devel] [PATCH v2 1/3] tpm_tis: fix loop that cancels any seizure by a lower locality

[Liam Merwick]

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>
---
 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 fd6bb9b59a96..61a130beef35 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;
                 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/3] tpm_tis: assert valid addr passed to tpm_tis_locality_from_addr()

[Liam Merwick]

Defensive check to prevent future caller passing incorrect address
or catch if the MMIO address parameters were not all changed together.

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
---

I've been running static analysis tools on QEMU and one reports this check.
While it's just theoretically correct (impossible to hit with current code),
fixing this helps minimise noise and find other issues using those static
analyzers as well as defending against the addition of future bugs.

 hw/tpm/tpm_tis.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 61a130beef35..860c2ace7d99 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -100,6 +100,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
 
 static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
 {
+    assert(addr < TPM_TIS_ADDR_SIZE);
     return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Liam Merwick]

cppcheck reports:

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

Fix this by using %u instead of %d

Signed-off-by: Liam Merwick <liam.merwick@oracle.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 860c2ace7d99..395500e8a61d 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -110,7 +110,7 @@ static void tpm_tis_show_buffer(const unsigned char *buffer,
     uint32_t len, i;
 
     len = MIN(tpm_cmd_get_size(buffer), buffer_size);
-    printf("tpm_tis: %s length = %d\n", string, len);
+    printf("tpm_tis: %s length = %u\n", string, len);
     for (i = 0; i < len; i++) {
         if (i && !(i % 16)) {
             printf("\n");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Philippe Mathieu-Daudé]

Hi Liam,

On 2/11/19 4:03 PM, Liam Merwick wrote:
> cppcheck reports:
> 
> [hw/tpm/tpm_tis.c:113]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'
> 
> Fix this by using %u instead of %d
> 
> Signed-off-by: Liam Merwick <liam.merwick@oracle.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 860c2ace7d99..395500e8a61d 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -110,7 +110,7 @@ static void tpm_tis_show_buffer(const unsigned char *buffer,
>      uint32_t len, i;

If you want to clean this, use a size_t here instead of u32.

>  
>      len = MIN(tpm_cmd_get_size(buffer), buffer_size);
> -    printf("tpm_tis: %s length = %d\n", string, len);
> +    printf("tpm_tis: %s length = %u\n", string, len);

So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and replace
them with trace points.

>      for (i = 0; i < len; i++) {
>          if (i && !(i % 16)) {
>              printf("\n");
> 

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Stefan Berger]

On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
> Hi Liam,
>
> On 2/11/19 4:03 PM, Liam Merwick wrote:
>> cppcheck reports:
>>
>> [hw/tpm/tpm_tis.c:113]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'
>>
>> Fix this by using %u instead of %d


Liam, Neither gcc nor cppcheck 1.86 complains about this on my system. 
What version of cppcheck are you using?


>>
>> Signed-off-by: Liam Merwick <liam.merwick@oracle.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 860c2ace7d99..395500e8a61d 100644
>> --- a/hw/tpm/tpm_tis.c
>> +++ b/hw/tpm/tpm_tis.c
>> @@ -110,7 +110,7 @@ static void tpm_tis_show_buffer(const unsigned char *buffer,
>>       uint32_t len, i;
> If you want to clean this, use a size_t here instead of u32.
>
>>   
>>       len = MIN(tpm_cmd_get_size(buffer), buffer_size);
>> -    printf("tpm_tis: %s length = %d\n", string, len);
>> +    printf("tpm_tis: %s length = %u\n", string, len);
> So here the format is '%zu'.
> However in code cleanup we try go get ride of printf() calls and replace
> them with trace points.


This code is only used for debugging if DEBUG_TIS has been #defined. No 
need to add tracing here.


>
>>       for (i = 0; i < len; i++) {
>>           if (i && !(i % 16)) {
>>               printf("\n");
>>

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Liam Merwick]

[-- Attachment #1: Type: text/plain, Size: 2021 bytes --]

On 11/02/2019 19:56, Stefan Berger wrote:
> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
>> Hi Liam,
>>
>> On 2/11/19 4:03 PM, Liam Merwick wrote:
>>> cppcheck reports:
>>>
>>> [hw/tpm/tpm_tis.c:113]: (warning) %d in format string (no. 2) 
>>> requires 'int' but the argument type is 'unsigned int'
>>>
>>> Fix this by using %u instead of %d
> 
> 
> Liam, Neither gcc nor cppcheck 1.86 complains about this on my system. 
> What version of cppcheck are you using?

I was using 1.86 too but I had warnings enabled.

% cppcheck -j 8 --platform=unix64 --force --inline-suppr 
--enable=warning --error-exitcode=1 hw/tpm/tpm_tis.c
Checking hw/tpm/tpm_tis.c ...
[hw/tpm/tpm_tis.c:112]: (warning) %d in format string (no. 2) requires 
'int' but the argument type is 'unsigned int'.


> 
> 
>>>
>>> Signed-off-by: Liam Merwick <liam.merwick@oracle.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 860c2ace7d99..395500e8a61d 100644
>>> --- a/hw/tpm/tpm_tis.c
>>> +++ b/hw/tpm/tpm_tis.c
>>> @@ -110,7 +110,7 @@ static void tpm_tis_show_buffer(const unsigned 
>>> char *buffer,
>>>       uint32_t len, i;
>> If you want to clean this, use a size_t here instead of u32.
>>
>>>       len = MIN(tpm_cmd_get_size(buffer), buffer_size);
>>> -    printf("tpm_tis: %s length = %d\n", string, len);
>>> +    printf("tpm_tis: %s length = %u\n", string, len);
>> So here the format is '%zu'.
>> However in code cleanup we try go get ride of printf() calls and replace
>> them with trace points.
> 
> 
> This code is only used for debugging if DEBUG_TIS has been #defined. No 
> need to add tracing here.

I'd come up the attached change (but that seems like overkill).

Regards,
Liam


> 
> 
>>
>>>       for (i = 0; i < len; i++) {
>>>           if (i && !(i % 16)) {
>>>               printf("\n");
>>>
> 


[-- Attachment #2: v2-0003-tpm_tis-fix-format-string-specifier-in-tpm_tis_sh.patch --]
[-- Type: text/x-patch, Size: 2196 bytes --]

>From 5dc2333c009583f5ce7338d5d67f857a4e8f4c74 Mon Sep 17 00:00:00 2001
From: Liam Merwick <liam.merwick@oracle.com>
Date: Mon, 11 Feb 2019 13:15:17 +0000
Subject: [PATCH v2 3/3] tpm_tis: fix format string specifier in
 tpm_tis_show_buffer()

cppcheck reports:

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

Fix this by converting it, and the other calls to printf in
tpm_tis_show_buffer(), to use trace points.

Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
---
 hw/tpm/tpm_tis.c    | 11 ++++++-----
 hw/tpm/trace-events |  3 +++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 860c2ace7d99..5d85b1cf61b1 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -107,17 +107,18 @@ 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;
+    uint32_t i;
 
     len = MIN(tpm_cmd_get_size(buffer), buffer_size);
-    printf("tpm_tis: %s length = %d\n", string, len);
+    trace_tpm_tis_show_buffer_hdr(string, len);
     for (i = 0; i < len; i++) {
         if (i && !(i % 16)) {
-            printf("\n");
+            trace_tpm_tis_show_buffer_newline();
         }
-        printf("%.2X ", buffer[i]);
+        trace_tpm_tis_show_buffer_entry(buffer[i]);
     }
-    printf("\n");
+    trace_tpm_tis_show_buffer_newline();
 }
 
 /*
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 920d32ad5514..41d99d489097 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -36,6 +36,9 @@ tpm_emulator_pre_save(void) ""
 tpm_emulator_inst_init(void) ""
 
 # hw/tpm/tpm_tis.c
+tpm_tis_show_buffer_hdr(const char *string, size_t buffer_size) ": %s length = %zu"
+tpm_tis_show_buffer_entry(const unsigned char entry) "%.2X "
+tpm_tis_show_buffer_newline(void) "\n"
 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"
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 1/3] tpm_tis: fix loop that cancels any seizure by a lower locality

[Stefan Berger]

On 2/11/19 10:03 AM, Liam Merwick wrote:
> 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>
> ---
>   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 fd6bb9b59a96..61a130beef35 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;
>                   }
>   


Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Stefan Berger]

On 2/11/19 3:09 PM, Liam Merwick wrote:
>
>
> I'd come up the attached change (but that seems like overkill).


I don't think we need tracing for this.

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Stefan Berger]

On 2/11/19 10:03 AM, Liam Merwick wrote:
> cppcheck reports:
>
> [hw/tpm/tpm_tis.c:113]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'
>
> Fix this by using %u instead of %d
>
> Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
Reviewed-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 860c2ace7d99..395500e8a61d 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -110,7 +110,7 @@ static void tpm_tis_show_buffer(const unsigned char *buffer,
>       uint32_t len, i;
>   
>       len = MIN(tpm_cmd_get_size(buffer), buffer_size);
> -    printf("tpm_tis: %s length = %d\n", string, len);
> +    printf("tpm_tis: %s length = %u\n", string, len);
>       for (i = 0; i < len; i++) {
>           if (i && !(i % 16)) {
>               printf("\n");

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Philippe Mathieu-Daudé]

On 2/11/19 10:13 PM, Stefan Berger wrote:
> On 2/11/19 3:09 PM, Liam Merwick wrote:
>> On 11/02/2019 19:56, Stefan Berger wrote:
>>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
>>>> On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
>>>>> -    printf("tpm_tis: %s length = %d\n", string, len);
>>>>> +    printf("tpm_tis: %s length = %u\n", string, len);
>>>> So here the format is '%zu'.
>>>> However in code cleanup we try go get ride of printf() calls and
>>>> replace them with trace points.
>>>
>>>
>>> This code is only used for debugging if DEBUG_TIS has been #defined.
>>> No need to add tracing here.
>>
>> I'd come up the attached change (but that seems like overkill).
> 
> 
> I don't think we need tracing for this.

So if you think the code is mature enough, let's remove the DEBUG calls!

Else we prefer to convert DEBUG printf to trace events because (at least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.

Thanks,

Phil.

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Stefan Berger]

On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
> On 2/11/19 10:13 PM, Stefan Berger wrote:
>> On 2/11/19 3:09 PM, Liam Merwick wrote:
>>> On 11/02/2019 19:56, Stefan Berger wrote:
>>>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
>>>>> On 2/11/19 4:03 PM, Liam Merwick wrote:
> [...]
>>>>>> -    printf("tpm_tis: %s length = %d\n", string, len);
>>>>>> +    printf("tpm_tis: %s length = %u\n", string, len);
>>>>> So here the format is '%zu'.
>>>>> However in code cleanup we try go get ride of printf() calls and
>>>>> replace them with trace points.
>>>>
>>>> This code is only used for debugging if DEBUG_TIS has been #defined.
>>>> No need to add tracing here.
>>> I'd come up the attached change (but that seems like overkill).
>>
>> I don't think we need tracing for this.
> So if you think the code is mature enough, let's remove the DEBUG calls!
>
> Else we prefer to convert DEBUG printf to trace events because (at least):
> - no need to recompile to enable debugging
> - when compiled with debugging, you don't mess with STDIO which can be
> used as a chardev backend.
Fine. Then I withdraw my reviewed-by.
> Thanks,
>
> Phil.
>

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Liam Merwick]

On 12/02/2019 13:27, Stefan Berger wrote:
> On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
>> On 2/11/19 10:13 PM, Stefan Berger wrote:
>>> On 2/11/19 3:09 PM, Liam Merwick wrote:
>>>> On 11/02/2019 19:56, Stefan Berger wrote:
>>>>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
>>>>>> On 2/11/19 4:03 PM, Liam Merwick wrote:
>> [...]
>>>>>>> -    printf("tpm_tis: %s length = %d\n", string, len);
>>>>>>> +    printf("tpm_tis: %s length = %u\n", string, len);
>>>>>> So here the format is '%zu'.
>>>>>> However in code cleanup we try go get ride of printf() calls and
>>>>>> replace them with trace points.
>>>>>
>>>>> This code is only used for debugging if DEBUG_TIS has been #defined.
>>>>> No need to add tracing here.
>>>> I'd come up the attached change (but that seems like overkill).
>>>
>>> I don't think we need tracing for this.
>> So if you think the code is mature enough, let's remove the DEBUG calls!
>>
>> Else we prefer to convert DEBUG printf to trace events because (at 
>> least):
>> - no need to recompile to enable debugging
>> - when compiled with debugging, you don't mess with STDIO which can be
>> used as a chardev backend.
> Fine. Then I withdraw my reviewed-by.


I don't see a way of removing the DEBUG calls without adding the 
overhead of a call to tpm_tis_show_buffer() each time even if tracing is 
not enabled (the 3 trace calls are interdependent and need a for loop).
Is there any example of a trace point that calls a function that then 
does non-trivial printing?

I could send a v3 with the patch I attached previously with the 3 printf 
calls changed to trace points but still wrapped in 'if (DEBUG_TIS)' and 
optimised out in non-debug.

Regards,
Liam

Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Philippe Mathieu-Daudé]

On 2/12/19 2:43 PM, Liam Merwick wrote:
> On 12/02/2019 13:27, Stefan Berger wrote:
>> On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
>>> On 2/11/19 10:13 PM, Stefan Berger wrote:
>>>> On 2/11/19 3:09 PM, Liam Merwick wrote:
>>>>> On 11/02/2019 19:56, Stefan Berger wrote:
>>>>>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
>>>>>>> On 2/11/19 4:03 PM, Liam Merwick wrote:
>>> [...]
>>>>>>>> -    printf("tpm_tis: %s length = %d\n", string, len);
>>>>>>>> +    printf("tpm_tis: %s length = %u\n", string, len);
>>>>>>> So here the format is '%zu'.
>>>>>>> However in code cleanup we try go get ride of printf() calls and
>>>>>>> replace them with trace points.
>>>>>>
>>>>>> This code is only used for debugging if DEBUG_TIS has been #defined.
>>>>>> No need to add tracing here.
>>>>> I'd come up the attached change (but that seems like overkill).
>>>>
>>>> I don't think we need tracing for this.
>>> So if you think the code is mature enough, let's remove the DEBUG calls!
>>>
>>> Else we prefer to convert DEBUG printf to trace events because (at
>>> least):
>>> - no need to recompile to enable debugging
>>> - when compiled with debugging, you don't mess with STDIO which can be
>>> used as a chardev backend.
>> Fine. Then I withdraw my reviewed-by.
> 
> 
> I don't see a way of removing the DEBUG calls without adding the
> overhead of a call to tpm_tis_show_buffer() each time even if tracing is
> not enabled (the 3 trace calls are interdependent and need a for loop).
> Is there any example of a trace point that calls a function that then
> does non-trivial printing?
> 
> I could send a v3 with the patch I attached previously with the 3 printf
> calls changed to trace points but still wrapped in 'if (DEBUG_TIS)' and
> optimised out in non-debug.

You can look at commit 56853498648.

Thanks!

Phil.

Re: [Qemu-devel] [PATCH v2 2/3] tpm_tis: assert valid addr passed to tpm_tis_locality_from_addr()

[Stefan Berger]

On 2/11/19 10:03 AM, Liam Merwick wrote:
> Defensive check to prevent future caller passing incorrect address
> or catch if the MMIO address parameters were not all changed together.
>
> Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
> ---
>
> I've been running static analysis tools on QEMU and one reports this check.
> While it's just theoretically correct (impossible to hit with current code),
> fixing this helps minimise noise and find other issues using those static
> analyzers as well as defending against the addition of future bugs.
>
>   hw/tpm/tpm_tis.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 61a130beef35..860c2ace7d99 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -100,6 +100,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
>   
>   static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
>   {
> +    assert(addr < TPM_TIS_ADDR_SIZE);
>       return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
>   }
>   

If we want to add a check then we should check the value of the locality 
that's being derived from the address here since that one serves as an 
index into the array. However, we shouldn't need it with the way we 
register the amount of MMIO memory derived from the number of localities 
and the way we then derive the locality from the address.

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 61a130beef..797f107246 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -100,7 +100,10 @@ static uint64_t tpm_tis_mmio_read(void *opaque, 
hwaddr addr,

  static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
  {
-    return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
+    uint8_t locty = (uint8_t)(addr >> TPM_TIS_LOCALITY_SHIFT);
+    fprintf(stderr, "addr=%08lx locty=%u\n", addr, locty);
+    assert(TPM_TIS_IS_VALID_LOCTY(locty));
+    return locty;
  }

  static void tpm_tis_show_buffer(const unsigned char *buffer,