| Message ID | 1510696349-5151-4-git-send-email-stefanb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: a few fixes | expand |
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 >
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 >>> >> >> >
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) {
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(-)