Hi Stefan,
On 6/2/20 3:30 PM, Stefan Berger wrote:
> On 6/1/20 5:57 AM, Eric Auger wrote:
>> In preparation of its move to the generic acpi code,
>> let's convert build_tpm2() to use build_append API. This
>> latter now is prefered in place of direct ACPI struct field
>> settings with manual endianness conversion.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> ---
>> hw/i386/acpi-build.c | 28 +++++++++++++++++++---------
>> 1 file changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index b5669d6c65..f0d35d7b17 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -2298,30 +2298,40 @@ build_tpm_tcpa(GArray *table_data, BIOSLinker
>> *linker, GArray *tcpalog)
>> static void
>> build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
>> {
>> - Acpi20TPM2 *tpm2_ptr = acpi_data_push(table_data, sizeof *tpm2_ptr);
>> + Acpi20TPM2 *tpm2_ptr = acpi_data_push(table_data,
>> sizeof(AcpiTableHeader));
>
> And now you want to build the data structure by pushing fields? I would
> definitely NOT do this.
If I didn't misinterpret things, this was recommended by Drew and Igor
as buid_append* API avoids to take care of endianness and this is the
API now used in the generic ACPI code. Besides I also think that in that
case it does not simplify things but maybe I did that the wrong way? Or
maybe I didn't understand your remark?
Thanks
Eric
>
>
>> unsigned log_addr_size = sizeof(tpm2_ptr->log_area_start_address);
>> unsigned log_addr_offset =
>> (char *)&tpm2_ptr->log_area_start_address - table_data->data;
>> + uint8_t start_method_params[12] = {};
>> - tpm2_ptr->platform_class = cpu_to_le16(TPM2_ACPI_CLASS_CLIENT);
>> + /* platform class */
>> + build_append_int_noprefix(table_data, TPM2_ACPI_CLASS_CLIENT, 2);
>> + /* reserved */
>> + build_append_int_noprefix(table_data, 0, 2);
>> if (TPM_IS_TIS_ISA(tpm_find())) {
>> - tpm2_ptr->control_area_address = cpu_to_le64(0);
>> - tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO);
>> + /* address of control area */
>> + build_append_int_noprefix(table_data, 0, 8);
>> + /* start method */
>> + build_append_int_noprefix(table_data, TPM2_START_METHOD_MMIO,
>> 4);
>> } else if (TPM_IS_CRB(tpm_find())) {
>> - tpm2_ptr->control_area_address = cpu_to_le64(TPM_CRB_ADDR_CTRL);
>> - tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_CRB);
>> + build_append_int_noprefix(table_data, TPM_CRB_ADDR_CTRL, 8);
>> + build_append_int_noprefix(table_data, TPM2_START_METHOD_CRB, 4);
>> } else {
>> g_warn_if_reached();
>> }
>> - tpm2_ptr->log_area_minimum_length =
>> - cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
>> + /* platform specific parameters */
>> + g_array_append_vals(table_data, &start_method_params, 12);
>> - acpi_data_push(tcpalog,
>> le32_to_cpu(tpm2_ptr->log_area_minimum_length));
>> + /* log area minimum length */
>> + build_append_int_noprefix(table_data, TPM_LOG_AREA_MINIMUM_SIZE, 4);
>> +
>> + acpi_data_push(tcpalog, TPM_LOG_AREA_MINIMUM_SIZE);
>> bios_linker_loader_alloc(linker, ACPI_BUILD_TPMLOG_FILE,
>> tcpalog, 1,
>> false);
>> /* log area start address to be filled by Guest linker */
>> + build_append_int_noprefix(table_data, 0, 8);
>> bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
>> log_addr_offset, log_addr_size,
>> ACPI_BUILD_TPMLOG_FILE, 0);
>
>
