[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/2] tpm: tpm_spapr: Exit on TPM backend failures
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 1/2] tpm: tpm_spapr: Exit on TPM backend failures |
Date: |
Tue, 7 Jul 2020 15:24:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 7/7/20 2:52 PM, Stefan Berger wrote:
> On 7/7/20 12:20 AM, Philippe Mathieu-Daudé wrote:
>> Hi Stefan,
>>
>> On 7/7/20 6:05 AM, Stefan Berger wrote:
>>> Exit on TPM backend failures in the same way as the TPM CRB and TIS
>>> device
>>> models do.
>> Maybe the other models are not the best examples ;)
>
> At least they are known to report the error...
>
>
>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>> ---
>>> hw/tpm/tpm_spapr.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
>>> index cb4dfd1e6a..8288ab0a15 100644
>>> --- a/hw/tpm/tpm_spapr.c
>>> +++ b/hw/tpm/tpm_spapr.c
>>> @@ -306,7 +306,10 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
>>> TPM_SPAPR_BUFFER_MAX);
>>> tpm_backend_reset(s->be_driver);
>>> - tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
>>> +
>>> + if (tpm_spapr_do_startup_tpm(s, s->be_buffer_size) < 0) {
>> I don't see error reported, how users can know the cause of the exit?
>
>
> virt-manager does report the error then. It seems to be taking it from
> the last error message reported in the emulator backend when TPM_INIT
> fails with error code 0x101:
>
> error: internal error: qemu unexpectedly closed the monitor:
> 2020-07-07T12:49:28.333928Z qemu-system-ppc64: tpm-emulator: TPM result
> for CMD_INIT: 0x101 operation failed
Ah, good.
>
>>
>>> + exit(1);
>> What about using this instead?
>>
>> qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_ERROR);
>
> It doesn't have any effect, the VM just keeps on running. So the exit(1)
> is better and does report an error.
>
Hmm maybe something is missing or it was never totally implemented?
Anyway since virt-manager is notified, I'm not objecting to this patch
:)