On 09/27/2012 10:12 AM, Corey Bryant wrote:
On 06/04/2012 03:37 PM, Stefan Berger wrote:
+
+#ifdef CONFIG_TPM
+
+static const TPMDriverOps *bes[] = {
I think bes[] would be more descriptive if it were named be_drivers[]
or be_ops[]?
Renamed to be_drivers.
+ if (!QLIST_EMPTY(&tpm_backends)) {
+ error_report("Only one TPM is allowed.\n");
+ return 1;
+ }
A list of tpm_backends is maintained and walked in a few places, but
only one is allowed to be added to the list. Will it ever make sense
to enable multiple backends at one time?
A list is also returned through the monitor. This list can at the moment
only have maximum of one entry. I would keep that list there unless
someone else opposes. It may be possible to create different types of
hardware emulation interfaces or simply replicate the TPM TIS at
different addresses. So I cannot say whether it will 'ever make sense'
to do that but I'd rather keep the opportunity there than close it and
with that also let the monitor return a list of items rather than a
single item.
I removed the processing of the lists in this part of the code at least.
+
+ value = qemu_opt_get(opts, "type");
+ if (!value) {
+ qerror_report(QERR_MISSING_PARAMETER, "type");
+ tpm_display_backend_drivers();
+ return 1;
+ }
+
+ be = tpm_get_backend_driver(value);
The "type" value is being passed but the tpm_get_backend_driver()
defines the parameter as "id". Maybe "id" could be renamed to "type"
for consistency. See similar comment further down in this email.
Done.
+ */
+int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+{
+ QemuOpts *opts;
+
+ if (strcmp("none", optarg) != 0) {
What's the point of supporting "-tpmdev none"?
Removed.
+typedef struct TPMBackend {
+ char *id;
For consistency, this could be named "type" instead of "id" since it
corresponds to -tpmdev's type.
Yes.
+ uint8_t command_locty;
It would be easier to read if locality was spelled out fully, instead
of locty. But if that causes lines to be too long then maybe it's not
worth it.
I rather keep it 'locty'.
+ TPMLocality *cmd_locty;
There's a cmd_locty and a command_locty. command_locty is the locality
number and cmd_locty is the locality data. Could these variable names
be updated to be more unique and descriptive?
Will rename them command_locty -> locty_number and cmd_locty -> locty_data.
Regards,
Stefan
