| Message ID | 20210609184955.1193081-3-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: Return QMP error when TPM is disabled in build | expand |
On Wed, Jun 09, 2021 at 08:49:55PM +0200, Philippe Mathieu-Daudé wrote: > When the management layer queries a binary built using --disable-tpm > for TPM devices, it gets confused by getting empty responses: > ... > > To make it clearer by returning an error: > - Make the TPM QAPI schema conditional > - Adapt the HMP command > - Remove stubs which became unnecessary > > The management layer now gets a 'CommandNotFound' error: > > { "execute": "query-tpm" } > { > "error": { > "class": "CommandNotFound", > "desc": "The command query-tpm has not been found" > } > } > > Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > qapi/tpm.json | 9 ++++++--- > monitor/hmp-cmds.c | 4 ++++ > stubs/tpm.c | 16 ---------------- > 3 files changed, 10 insertions(+), 19 deletions(-) Yes, looks nicer. > > diff --git a/qapi/tpm.json b/qapi/tpm.json > index 6a10c9ed8d2..09332e6f996 100644 > --- a/qapi/tpm.json > +++ b/qapi/tpm.json > @@ -33,7 +33,8 @@ > # <- { "return": [ "tpm-tis", "tpm-crb", "tpm-spapr" ] } > # > ## > -{ 'command': 'query-tpm-models', 'returns': ['TpmModel'] } > +{ 'command': 'query-tpm-models', 'returns': ['TpmModel'], > + 'if': 'defined(CONFIG_TPM)' } May need a rebase if the series to make 'if' language-agnostic lands first (in fact, that would probably result in a build-time semantic conflict rather than a patch-application-time merge conflict), but that's not enough to prevent me from giving: Reviewed-by: Eric Blake <eblake@redhat.com
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > When the management layer queries a binary built using --disable-tpm > for TPM devices, it gets confused by getting empty responses: What software exactly gets confused, and how? > > { "execute": "query-tpm" } > { > "return": [ > ] > } > { "execute": "query-tpm-types" } > { > "return": [ > ] > } > { "execute": "query-tpm-models" } > { > "return": [ > ] > } > > To make it clearer by returning an error: > - Make the TPM QAPI schema conditional > - Adapt the HMP command > - Remove stubs which became unnecessary > > The management layer now gets a 'CommandNotFound' error: > > { "execute": "query-tpm" } > { > "error": { > "class": "CommandNotFound", > "desc": "The command query-tpm has not been found" > } > } > > Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Yes, please. But see my review of PATCH 1.
diff --git a/qapi/tpm.json b/qapi/tpm.json index 6a10c9ed8d2..09332e6f996 100644 --- a/qapi/tpm.json +++ b/qapi/tpm.json @@ -33,7 +33,8 @@ # <- { "return": [ "tpm-tis", "tpm-crb", "tpm-spapr" ] } # ## -{ 'command': 'query-tpm-models', 'returns': ['TpmModel'] } +{ 'command': 'query-tpm-models', 'returns': ['TpmModel'], + 'if': 'defined(CONFIG_TPM)' } ## # @TpmType: @@ -63,7 +64,8 @@ # <- { "return": [ "passthrough", "emulator" ] } # ## -{ 'command': 'query-tpm-types', 'returns': ['TpmType'] } +{ 'command': 'query-tpm-types', 'returns': ['TpmType'], + 'if': 'defined(CONFIG_TPM)' } ## # @TPMPassthroughOptions: @@ -152,4 +154,5 @@ # } # ## -{ 'command': 'query-tpm', 'returns': ['TPMInfo'] } +{ 'command': 'query-tpm', 'returns': ['TPMInfo'], + 'if': 'defined(CONFIG_TPM)' } diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index d10ee141109..f6cadede40f 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -901,6 +901,9 @@ void hmp_info_pci(Monitor *mon, const QDict *qdict) void hmp_info_tpm(Monitor *mon, const QDict *qdict) { +#ifndef CONFIG_TPM + monitor_printf(mon, "TPM device not supported\n"); +#else TPMInfoList *info_list, *info; Error *err = NULL; unsigned int c = 0; @@ -946,6 +949,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) c++; } qapi_free_TPMInfoList(info_list); +#endif /* CONFIG_TPM */ } void hmp_quit(Monitor *mon, const QDict *qdict) diff --git a/stubs/tpm.c b/stubs/tpm.c index 9bded191d9d..b1dc6370a5e 100644 --- a/stubs/tpm.c +++ b/stubs/tpm.c @@ -6,7 +6,6 @@ */ #include "qemu/osdep.h" -#include "qapi/qapi-commands-tpm.h" #include "sysemu/tpm.h" #include "hw/acpi/tpm.h" @@ -19,21 +18,6 @@ void tpm_cleanup(void) { } -TPMInfoList *qmp_query_tpm(Error **errp) -{ - return NULL; -} - -TpmTypeList *qmp_query_tpm_types(Error **errp) -{ - return NULL; -} - -TpmModelList *qmp_query_tpm_models(Error **errp) -{ - return NULL; -} - void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev) { }
When the management layer queries a binary built using --disable-tpm for TPM devices, it gets confused by getting empty responses: { "execute": "query-tpm" } { "return": [ ] } { "execute": "query-tpm-types" } { "return": [ ] } { "execute": "query-tpm-models" } { "return": [ ] } To make it clearer by returning an error: - Make the TPM QAPI schema conditional - Adapt the HMP command - Remove stubs which became unnecessary The management layer now gets a 'CommandNotFound' error: { "execute": "query-tpm" } { "error": { "class": "CommandNotFound", "desc": "The command query-tpm has not been found" } } Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- qapi/tpm.json | 9 ++++++--- monitor/hmp-cmds.c | 4 ++++ stubs/tpm.c | 16 ---------------- 3 files changed, 10 insertions(+), 19 deletions(-)