Rather than returning ~0, return 0 for every register in case of
failure mode. The '0' is better to indicate that there's no device
there.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.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 fec2fc6..42d647d 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -545,7 +545,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
     uint8_t v;
 
     if (tpm_backend_had_startup_error(s->be_driver)) {
-        return val;
+        return 0;
     }
 
     switch (offset) {
-- 
2.5.5

Hi

On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Rather than returning ~0, return 0 for every register in case of
> failure mode. The '0' is better to indicate that there's no device
> there.

For most registers, 0 makes more sense. However, I wonder if we
shouldn't just fail to start qemu in this case...

Not convincing me this is 2.11 material either. Does this fix a specific bug?

> Signed-off-by: Stefan Berger <stefanb@linux.vnet.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 fec2fc6..42d647d 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -545,7 +545,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
>      uint8_t v;
>
>      if (tpm_backend_had_startup_error(s->be_driver)) {
> -        return val;
> +        return 0;
>      }
>
>      switch (offset) {
> --
> 2.5.5
>



-- 
Marc-André Lureau

On 11/14/2017 06:47 PM, Marc-André Lureau wrote:
> Hi
>
> On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
> <stefanb@linux.vnet.ibm.com> wrote:
>> Rather than returning ~0, return 0 for every register in case of
>> failure mode. The '0' is better to indicate that there's no device
>> there.
> For most registers, 0 makes more sense. However, I wonder if we
> shouldn't just fail to start qemu in this case...
>
> Not convincing me this is 2.11 material either. Does this fix a specific bug?

Yes, SeaBIOS detects the ~0 when it probes and thinks there's a device 
there. It then hangs trying to set flags and read registers to be able 
to use the device.

    Stefan

>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.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 fec2fc6..42d647d 100644
>> --- a/hw/tpm/tpm_tis.c
>> +++ b/hw/tpm/tpm_tis.c
>> @@ -545,7 +545,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
>>       uint8_t v;
>>
>>       if (tpm_backend_had_startup_error(s->be_driver)) {
>> -        return val;
>> +        return 0;
>>       }
>>
>>       switch (offset) {
>> --
>> 2.5.5
>>
>
>

Hi

On Wed, Nov 15, 2017 at 2:18 AM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> On 11/14/2017 06:47 PM, Marc-André Lureau wrote:
>>
>> Hi
>>
>> On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
>> <stefanb@linux.vnet.ibm.com> wrote:
>>>
>>> Rather than returning ~0, return 0 for every register in case of
>>> failure mode. The '0' is better to indicate that there's no device
>>> there.
>>
>> For most registers, 0 makes more sense. However, I wonder if we
>> shouldn't just fail to start qemu in this case...
>>
>> Not convincing me this is 2.11 material either. Does this fix a specific
>> bug?
>
>
> Yes, SeaBIOS detects the ~0 when it probes and thinks there's a device
> there. It then hangs trying to set flags and read registers to be able to
> use the device.
>

Please update the commit message with that added,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>    Stefan
>
>
>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.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 fec2fc6..42d647d 100644
>>> --- a/hw/tpm/tpm_tis.c
>>> +++ b/hw/tpm/tpm_tis.c
>>> @@ -545,7 +545,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque,
>>> hwaddr addr,
>>>       uint8_t v;
>>>
>>>       if (tpm_backend_had_startup_error(s->be_driver)) {
>>> -        return val;
>>> +        return 0;
>>>       }
>>>
>>>       switch (offset) {
>>> --
>>> 2.5.5
>>>
>>
>>
>



-- 
Marc-André Lureau