[Qemu-devel] [PATCH 0/2] tpm: acpi generation tweaks

[Qemu-devel] [PATCH 0/2] tpm: acpi generation tweaks

[Michael S. Tsirkin]

This tweaks tpm code slightly for readability
and robustness. On top of Stefen's patches.

Michael S. Tsirkin (2):
  tpm: remove code duplication
  acpi: cleanups tcpa table generation

 hw/tpm/tpm_tis.h     |  3 ---
 include/sysemu/tpm.h |  4 +++-
 hw/i386/acpi-build.c | 12 ++++--------
 3 files changed, 7 insertions(+), 12 deletions(-)

-- 
MST

[Qemu-devel] [PATCH 1/2] tpm: remove code duplication

[Michael S. Tsirkin]

Move TYPE_TPM_TIS to tpm.h so it can be used
by an inline function there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/tpm/tpm_tis.h     | 3 ---
 include/sysemu/tpm.h | 4 +++-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index 6cdac24..1a0db23 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -29,9 +29,6 @@
 
 #define TPM_TIS_BUFFER_MAX          4096
 
-#define TYPE_TPM_TIS                "tpm-tis"
-
-
 typedef enum {
     TPM_TIS_STATE_IDLE = 0,
     TPM_TIS_STATE_READY,
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 7030109..9b81ce9 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -20,9 +20,11 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
 int tpm_init(void);
 void tpm_cleanup(void);
 
+#define TYPE_TPM_TIS                "tpm-tis"
+
 static inline bool tpm_find(void)
 {
-    return object_resolve_path_type("", "tpm-tis", NULL) != NULL;
+    return object_resolve_path_type("", TYPE_TPM_TIS, NULL);
 }
 
 #endif /* QEMU_TPM_H */
-- 
MST

[Qemu-devel] [PATCH 2/2] acpi: cleanups tcpa table generation

[Michael S. Tsirkin]

Improve readability, no functional changes.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 66cde2b..6ae3a9e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1209,17 +1209,13 @@ build_hpet(GArray *table_data, GArray *linker)
 static void
 build_tpm_tcpa(GArray *table_data, GArray *linker)
 {
-    Acpi20Tcpa *tcpa;
-    uint32_t log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
-    uint64_t log_area_start_address;
-    size_t len = log_area_minimum_length + sizeof(*tcpa);
+    Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
+    uint64_t log_area_start_address = acpi_data_len(table_data);
 
-    log_area_start_address = table_data->len + sizeof(*tcpa);
-
-    tcpa = acpi_data_push(table_data, len);
+    acpi_data_push(table_data, TPM_LOG_AREA_MINIMUM_SIZE);
 
     tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
-    tcpa->log_area_minimum_length = cpu_to_le32(log_area_minimum_length);
+    tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
     tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
 
     /* LASA address to be filled by Guest linker */
-- 
MST

Re: [Qemu-devel] [PATCH 1/2] tpm: remove code duplication

[Stefan Berger]

On 07/30/2014 07:34 AM, Michael S. Tsirkin wrote:
> Move TYPE_TPM_TIS to tpm.h so it can be used
> by an inline function there.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   hw/tpm/tpm_tis.h     | 3 ---
>   include/sysemu/tpm.h | 4 +++-
>   2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
> index 6cdac24..1a0db23 100644
> --- a/hw/tpm/tpm_tis.h
> +++ b/hw/tpm/tpm_tis.h
> @@ -29,9 +29,6 @@
>
>   #define TPM_TIS_BUFFER_MAX          4096
>
> -#define TYPE_TPM_TIS                "tpm-tis"
> -
> -
>   typedef enum {
>       TPM_TIS_STATE_IDLE = 0,
>       TPM_TIS_STATE_READY,
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 7030109..9b81ce9 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -20,9 +20,11 @@ int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
>   int tpm_init(void);
>   void tpm_cleanup(void);
>
> +#define TYPE_TPM_TIS                "tpm-tis"
> +
>   static inline bool tpm_find(void)
>   {
> -    return object_resolve_path_type("", "tpm-tis", NULL) != NULL;
> +    return object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>   }
>
>   #endif /* QEMU_TPM_H */

ACK

Re: [Qemu-devel] [PATCH 2/2] acpi: cleanups tcpa table generation

[Stefan Berger]

On 07/30/2014 07:34 AM, Michael S. Tsirkin wrote:
> Improve readability, no functional changes.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   hw/i386/acpi-build.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 66cde2b..6ae3a9e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1209,17 +1209,13 @@ build_hpet(GArray *table_data, GArray *linker)
>   static void
>   build_tpm_tcpa(GArray *table_data, GArray *linker)
>   {
> -    Acpi20Tcpa *tcpa;
> -    uint32_t log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
> -    uint64_t log_area_start_address;
> -    size_t len = log_area_minimum_length + sizeof(*tcpa);
> +    Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
> +    uint64_t log_area_start_address = acpi_data_len(table_data);
>
> -    log_area_start_address = table_data->len + sizeof(*tcpa);
> -
> -    tcpa = acpi_data_push(table_data, len);
> +    acpi_data_push(table_data, TPM_LOG_AREA_MINIMUM_SIZE);
>
>       tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
> -    tcpa->log_area_minimum_length = cpu_to_le32(log_area_minimum_length);
> +    tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
>       tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
>
>       /* LASA address to be filled by Guest linker */
ACK

Re: [Qemu-devel] [PATCH 0/2] tpm: acpi generation tweaks

[Stefan Berger]

On 07/30/2014 07:34 AM, Michael S. Tsirkin wrote:
> This tweaks tpm code slightly for readability
> and robustness. On top of Stefen's patches.

Are you going to upstream my patches and yours ?

    Stefan

Re: [Qemu-devel] [PATCH 0/2] tpm: acpi generation tweaks

[Michael S. Tsirkin]

On Fri, Aug 08, 2014 at 03:25:46PM -0400, Stefan Berger wrote:
> On 07/30/2014 07:34 AM, Michael S. Tsirkin wrote:
> >This tweaks tpm code slightly for readability
> >and robustness. On top of Stefen's patches.
> 
> Are you going to upstream my patches and yours ?
> 
>    Stefan

Weren't there any comments to address?  I'll have to re-check. If there
were, when you repost to address them, you can include my patches.

Re: [Qemu-devel] [PATCH 0/2] tpm: acpi generation tweaks

[Stefan Berger]

On 08/10/2014 04:04 PM, Michael S. Tsirkin wrote:
> On Fri, Aug 08, 2014 at 03:25:46PM -0400, Stefan Berger wrote:
>> On 07/30/2014 07:34 AM, Michael S. Tsirkin wrote:
>>> This tweaks tpm code slightly for readability
>>> and robustness. On top of Stefen's patches.
>> Are you going to upstream my patches and yours ?
>>
>>     Stefan
> Weren't there any comments to address?  I'll have to re-check. If there
> were, when you repost to address them, you can include my patches.
>
Afaik there were no comments on v2. I can repost as v3 with your patches 
merged in. Should they include your signed-off-by then ?

   Stefan

Re: [Qemu-devel] [PATCH 2/2] acpi: cleanups tcpa table generation

[Stefan Berger]

On 07/30/2014 07:34 AM, Michael S. Tsirkin wrote:
> Improve readability, no functional changes.

There's a subtle problem -- I am fixing this for v3.

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   hw/i386/acpi-build.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 66cde2b..6ae3a9e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1209,17 +1209,13 @@ build_hpet(GArray *table_data, GArray *linker)
>   static void
>   build_tpm_tcpa(GArray *table_data, GArray *linker)
>   {
> -    Acpi20Tcpa *tcpa;
> -    uint32_t log_area_minimum_length = TPM_LOG_AREA_MINIMUM_SIZE;
> -    uint64_t log_area_start_address;
> -    size_t len = log_area_minimum_length + sizeof(*tcpa);
> +    Acpi20Tcpa *tcpa = acpi_data_push(table_data, sizeof *tcpa);
> +    uint64_t log_area_start_address = acpi_data_len(table_data);
>
> -    log_area_start_address = table_data->len + sizeof(*tcpa);
> -
> -    tcpa = acpi_data_push(table_data, len);
> +    acpi_data_push(table_data, TPM_LOG_AREA_MINIMUM_SIZE);

This 2nd acpi_data_push manipulates table_data and the BIOS linker 
loader function further below now operates with this new table_data 
offsets etc. We have to move this acpi_data_push all the way to the end 
and then it works.

>
>       tcpa->platform_class = cpu_to_le16(TPM_TCPA_ACPI_CLASS_CLIENT);
> -    tcpa->log_area_minimum_length = cpu_to_le32(log_area_minimum_length);
> +    tcpa->log_area_minimum_length = cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE);
>       tcpa->log_area_start_address = cpu_to_le64(log_area_start_address);
>
>       /* LASA address to be filled by Guest linker */