Re: [Xen-devel] [PATCH RFC 07/20] acpi/hvmloader: Set TIS header address in hvmloader

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.04.16 at 03:25, <boris.ostrovsky@xxxxxxxxxx> wrote:
> @@ -371,41 +370,43 @@ static int construct_secondary_tables(unsigned long 
> *table_ptrs,
>          printf("S4 disabled\n");
>      }
>  
> -    /* TPM TCPA and SSDT. */
> -    tis_hdr = (uint16_t *)0xFED40F00;
> -    if ( (tis_hdr[0] == tis_signature[0]) &&
> -         (tis_hdr[1] == tis_signature[1]) &&
> -         (tis_hdr[2] == tis_signature[2]) )
> +    if ( config->table_flags & ACPI_BUILD_TCPA )
>      {
> -        ssdt = mem_alloc(sizeof(ssdt_tpm), 16);
> -        if (!ssdt) return -1;
> -        memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
> -        table_ptrs[nr_tables++] = (unsigned long)ssdt;
> -
> -        tcpa = mem_alloc(sizeof(struct acpi_20_tcpa), 16);
> -        if (!tcpa) return -1;
> -        memset(tcpa, 0, sizeof(*tcpa));
> -        table_ptrs[nr_tables++] = (unsigned long)tcpa;
> -
> -        tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
> -        tcpa->header.length    = sizeof(*tcpa);
> -        tcpa->header.revision  = ACPI_2_0_TCPA_REVISION;
> -        fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
> -        fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
> -        tcpa->header.oem_revision = ACPI_OEM_REVISION;
> -        tcpa->header.creator_id   = ACPI_CREATOR_ID;
> -        tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
> -        if ( (lasa = mem_alloc(ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
> +        /* TPM TCPA and SSDT. */
> +        if ( (config->tis_hdr[0] == tis_signature[0]) &&
> +             (config->tis_hdr[1] == tis_signature[1]) &&
> +             (config->tis_hdr[2] == tis_signature[2]) )
>          {
> -            tcpa->lasa = virt_to_phys(lasa);
> -            tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
> -            memset(lasa, 0, tcpa->laml);
> -            set_checksum(tcpa,
> -                         offsetof(struct acpi_header, checksum),
> -                         tcpa->header.length);
> +            ssdt = mem_alloc(sizeof(ssdt_tpm), 16);
> +            if (!ssdt) return -1;
> +            memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
> +            table_ptrs[nr_tables++] = (unsigned long)ssdt;
> +
> +            tcpa = mem_alloc(sizeof(struct acpi_20_tcpa), 16);
> +            if (!tcpa) return -1;
> +            memset(tcpa, 0, sizeof(*tcpa));
> +            table_ptrs[nr_tables++] = (unsigned long)tcpa;
> +
> +            tcpa->header.signature = ACPI_2_0_TCPA_SIGNATURE;
> +            tcpa->header.length    = sizeof(*tcpa);
> +            tcpa->header.revision  = ACPI_2_0_TCPA_REVISION;
> +            fixed_strcpy(tcpa->header.oem_id, ACPI_OEM_ID);
> +            fixed_strcpy(tcpa->header.oem_table_id, ACPI_OEM_TABLE_ID);
> +            tcpa->header.oem_revision = ACPI_OEM_REVISION;
> +            tcpa->header.creator_id   = ACPI_CREATOR_ID;
> +            tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
> +            if ( (lasa = mem_alloc(ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
> +            {
> +                tcpa->lasa = virt_to_phys(lasa);
> +                tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
> +                memset(lasa, 0, tcpa->laml);
> +                set_checksum(tcpa,
> +                             offsetof(struct acpi_header, checksum),
> +                             tcpa->header.length);
> +            }

As it looks, all of this code churn could be avoided by simply
merging the two if()s at the top.

>          }
>      }
> -
> +    

Stray addition of trailing blanks.

> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -911,6 +911,8 @@ void hvmloader_acpi_build_tables(struct acpi_config 
> *config,
>      if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1)  )
>          config->table_flags |= ACPI_BUILD_SSDT_S4;
>  
> +    config->table_flags |= ACPI_BUILD_TCPA;
> +
>      config->nr_vcpus = hvm_info->nr_vcpus;
>      config->vcpu_online = hvm_info->vcpu_online;
>      config->apic_mode = 1;
> @@ -921,6 +923,8 @@ void hvmloader_acpi_build_tables(struct acpi_config 
> *config,
>      config->numa.vdistance = vdistance;
>      config->numa.vmemrange = vmemrange;
>  
> +    config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;

Please put these two additions next to each other.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel