[Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Michael S. Tsirkin]

I wanted to help Laszlo completely move ACPI tables out of seabios in
time for 1.5, but had to put out some fires so didn't manage to do this
in time, and going offline for now.

Meanwhile, here is some refactoring that I did complete.  This does not
do much by itself, but works fine for me and will be helpful down the
road.

This includes a couple of patches I posted previously
and another one on top.

Pls consider for 1.5.


Michael S. Tsirkin (3):
  acpi: move declarations from pc.h to acpi.h
  acpi: add acpi_table_add_hdr
  acpi.h: make it self contained

 arch_init.c            |  1 +
 hw/acpi/core.c         | 97 +++++++++++++++++++++++++++++++++++---------------
 hw/i386/pc.c           |  1 +
 hw/i386/pc_piix.c      |  1 +
 include/hw/acpi/acpi.h | 20 +++++++++++
 include/hw/i386/pc.h   |  8 -----
 6 files changed, 91 insertions(+), 37 deletions(-)

-- 
MST

[Qemu-devel] [PATCH 1/3] acpi: move declarations from pc.h to acpi.h

[Michael S. Tsirkin]

Functions defined in acpi/ should be declared in
acpi.h

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch_init.c            |  1 +
 hw/i386/pc.c           |  1 +
 hw/i386/pc_piix.c      |  1 +
 include/hw/acpi/acpi.h | 10 ++++++++++
 include/hw/i386/pc.h   |  8 --------
 5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 769ce77..fba0889 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -49,6 +49,7 @@
 #include "qmp-commands.h"
 #include "trace.h"
 #include "exec/cpu-all.h"
+#include "hw/acpi/acpi.h"
 
 #ifdef DEBUG_ARCH_INIT
 #define DPRINTF(fmt, ...) \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8d75b34..0d6e72b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -52,6 +52,7 @@
 #include "sysemu/arch_init.h"
 #include "qemu/bitmap.h"
 #include "qemu/config-file.h"
+#include "hw/acpi/acpi.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cff8013..943758a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -43,6 +43,7 @@
 #include "hw/xen/xen.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
+#include "hw/acpi/acpi.h"
 #include "cpu.h"
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index e18ef28..80e955d 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -154,4 +154,14 @@ void acpi_gpe_reset(ACPIREGS *ar);
 void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val);
 uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr);
 
+/* acpi.c */
+extern int acpi_enabled;
+extern char unsigned *acpi_tables;
+extern size_t acpi_tables_len;
+
+void acpi_table_install(const char unsigned *blob, size_t bloblen,
+                        bool has_header, const struct AcpiTableOptions *hdrs,
+                        Error **errp);
+void acpi_table_add(const QemuOpts *opts, Error **errp);
+
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 5d40914..9bcc819 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -107,14 +107,6 @@ void cpu_smm_register(cpu_set_smm_t callback, void *arg);
 
 void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
-/* acpi.c */
-extern int acpi_enabled;
-extern char unsigned *acpi_tables;
-extern size_t acpi_tables_len;
-
-void acpi_bios_init(void);
-void acpi_table_add(const QemuOpts *opts, Error **errp);
-
 /* acpi_piix.c */
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-- 
MST

[Qemu-devel] [PATCH 2/3] acpi: add acpi_table_add_hdr

[Michael S. Tsirkin]

Export a function for magling headers, without updating
the global acpitables blob.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/core.c         | 97 +++++++++++++++++++++++++++++++++++---------------
 include/hw/acpi/acpi.h |  4 +++
 2 files changed, 72 insertions(+), 29 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 64b8718..b76d58c 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -78,27 +78,27 @@ static int acpi_checksum(const uint8_t *data, int len)
     return (-sum) & 0xff;
 }
 
-
-/* Install a copy of the ACPI table specified in @blob.
+/*
+ * Fill an ACPI header in a copy of the ACPI table specified in @blob,
+ * of length @bloblen.
+ * An updated blob is allocated and returned in @out_blob,
+ * updated length in @out_bloblen.
  *
  * If @has_header is set, @blob starts with the System Description Table Header
  * structure. Otherwise, "dfl_hdr" is prepended. In any case, each header field
  * is optionally overwritten from @hdrs.
  *
  * It is valid to call this function with
- * (@blob == NULL && bloblen == 0 && !has_header).
+ * (@blob == NULL && bloblen == 0 &&!@has_header).
  *
  * @hdrs->file and @hdrs->data are ignored.
  *
  * SIZE_MAX is considered "infinity" in this function.
- *
- * The number of tables that can be installed is not limited, but the 16-bit
- * counter at the beginning of "acpi_tables" wraps around after UINT16_MAX.
  */
-static void acpi_table_install(const char unsigned *blob, size_t bloblen,
-                               bool has_header,
-                               const struct AcpiTableOptions *hdrs,
-                               Error **errp)
+int acpi_table_add_hdr(const char unsigned *blob, size_t bloblen,
+                       bool has_header, const struct AcpiTableOptions *hdrs,
+                       char unsigned **out_blob, size_t *out_bloblen,
+                       Error **errp)
 {
     size_t body_start;
     const char unsigned *hdr_src;
@@ -124,7 +124,7 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
             error_setg(errp, "ACPI table claiming to have header is too "
                        "short, available: %zu, expected: %zu", bloblen,
                        body_start);
-            return;
+            return -1;
         }
         hdr_src = blob;
     } else {
@@ -145,33 +145,29 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     if (acpi_payload_size > UINT16_MAX) {
         error_setg(errp, "ACPI table too big, requested: %zu, max: %u",
                    acpi_payload_size, (unsigned)UINT16_MAX);
-        return;
-    }
-
-    /* We won't fail from here on. Initialize / extend the globals. */
-    if (acpi_tables == NULL) {
-        acpi_tables_len = sizeof(uint16_t);
-        acpi_tables = g_malloc0(acpi_tables_len);
+        return -1;
     }
 
-    acpi_tables = g_realloc(acpi_tables, acpi_tables_len +
-                                         ACPI_TABLE_PFX_SIZE +
-                                         sizeof dfl_hdr + body_size);
+    /* We won't fail from here on. Allocate memory. */
+    *out_bloblen = sizeof(uint16_t);
+    *out_blob = g_malloc0(*out_bloblen +
+                          ACPI_TABLE_PFX_SIZE +
+                          sizeof dfl_hdr + body_size);
 
-    ext_hdr = (struct acpi_table_header *)(acpi_tables + acpi_tables_len);
-    acpi_tables_len += ACPI_TABLE_PFX_SIZE;
+    ext_hdr = (struct acpi_table_header *)(*out_blob + *out_bloblen);
+    *out_bloblen += ACPI_TABLE_PFX_SIZE;
 
-    memcpy(acpi_tables + acpi_tables_len, hdr_src, sizeof dfl_hdr);
-    acpi_tables_len += sizeof dfl_hdr;
+    memcpy(*out_blob + *out_bloblen, hdr_src, sizeof dfl_hdr);
+    *out_bloblen += sizeof dfl_hdr;
 
     if (blob != NULL) {
-        memcpy(acpi_tables + acpi_tables_len, blob + body_start, body_size);
-        acpi_tables_len += body_size;
+        memcpy(*out_blob + *out_bloblen, blob + body_start, body_size);
+        *out_bloblen += body_size;
     }
 
     /* increase number of tables */
-    cpu_to_le16wu((uint16_t *)acpi_tables,
-                  le16_to_cpupu((uint16_t *)acpi_tables) + 1u);
+    cpu_to_le16wu((uint16_t *)*out_blob,
+                  le16_to_cpupu((uint16_t *)*out_blob) + 1u);
 
     /* Update the header fields. The strings need not be NUL-terminated. */
     changed_fields = 0;
@@ -227,6 +223,49 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     /* recalculate checksum */
     ext_hdr->checksum = acpi_checksum((const char unsigned *)ext_hdr +
                                       ACPI_TABLE_PFX_SIZE, acpi_payload_size);
+    return 0;
+}
+
+/* Install a copy of the ACPI table specified in @blob.
+ *
+ * If @has_header is set, @blob starts with the System Description Table Header
+ * structure. Otherwise, "dfl_hdr" is prepended. In any case, each header field
+ * is optionally overwritten from @hdrs.
+ *
+ * It is valid to call this function with
+ * (@blob == NULL && bloblen == 0 && !has_header).
+ *
+ * @hdrs->file and @hdrs->data are ignored.
+ *
+ * SIZE_MAX is considered "infinity" in this function.
+ *
+ * The number of tables that can be installed is not limited, but the 16-bit
+ * counter at the beginning of "acpi_tables" wraps around after UINT16_MAX.
+ */
+void acpi_table_install(const char unsigned *blob, size_t bloblen,
+                        bool has_header, const struct AcpiTableOptions *hdrs,
+                        Error **errp)
+{
+    char unsigned *out_blob;
+    size_t out_bloblen;
+    int r;
+
+    r = acpi_table_add_hdr(blob, bloblen, has_header, hdrs,
+                           &out_blob, &out_bloblen, errp);
+    if (r < 0) {
+        return;
+    }
+
+    /* We won't fail from here on. Initialize / extend the globals. */
+    if (acpi_tables == NULL) {
+        acpi_tables_len = sizeof(uint16_t);
+        acpi_tables = g_malloc0(acpi_tables_len);
+    }
+
+    acpi_tables = g_realloc(acpi_tables, acpi_tables_len + out_bloblen);
+    memcpy(acpi_tables + acpi_tables_len, out_blob, out_bloblen);
+    acpi_tables_len += out_bloblen;
+    g_free(out_blob);
 }
 
 void acpi_table_add(const QemuOpts *opts, Error **errp)
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 80e955d..35f7e09 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -159,6 +159,10 @@ extern int acpi_enabled;
 extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 
+int acpi_table_add_hdr(const char unsigned *blob, size_t bloblen,
+                       bool has_header, const struct AcpiTableOptions *hdrs,
+                       char unsigned **out_blob, size_t *out_bloblen,
+                       Error **errp);
 void acpi_table_install(const char unsigned *blob, size_t bloblen,
                         bool has_header, const struct AcpiTableOptions *hdrs,
                         Error **errp);
-- 
MST

[Qemu-devel] [PATCH 3/3] acpi.h: make it self contained

[Michael S. Tsirkin]

Headers shouldn't assume another header is included,
pull in everything necessary.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/acpi.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 35f7e09..88f7378 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -19,6 +19,12 @@
  * <http://www.gnu.org/licenses/>.
  */
 
+#include "qapi/error.h"
+#include "qemu/typedefs.h"
+#include "qemu/notify.h"
+#include "qemu/option.h"
+#include "exec/memory.h"
+
 /* from linux include/acpi/actype.h */
 /* Default ACPI register widths */
 
-- 
MST

Re: [Qemu-devel] [PATCH 1/3] acpi: move declarations from pc.h to acpi.h

[Paolo Bonzini]

Il 15/04/2013 11:37, Michael S. Tsirkin ha scritto:
> +/* acpi.c */
> +extern int acpi_enabled;
> +extern char unsigned *acpi_tables;
> +extern size_t acpi_tables_len;
> +
> +void acpi_table_install(const char unsigned *blob, size_t bloblen,
> +                        bool has_header, const struct AcpiTableOptions *hdrs,
> +                        Error **errp);

This function is static, I've fixed it up and will include it in the hw/
fixups pull request.

I picked up patch 3 as well, I'll leave 2 to Laszlo.

Paolo

> +void acpi_table_add(const QemuOpts *opts, Error **errp);

Re: [Qemu-devel] [PATCH 1/3] acpi: move declarations from pc.h to acpi.h

[Laszlo Ersek]

On 04/15/13 12:42, Paolo Bonzini wrote:
> Il 15/04/2013 11:37, Michael S. Tsirkin ha scritto:
>> +/* acpi.c */
>> +extern int acpi_enabled;
>> +extern char unsigned *acpi_tables;
>> +extern size_t acpi_tables_len;
>> +
>> +void acpi_table_install(const char unsigned *blob, size_t bloblen,
>> +                        bool has_header, const struct AcpiTableOptions *hdrs,
>> +                        Error **errp);
> 
> This function is static, I've fixed it up and will include it in the hw/
> fixups pull request.
> 
> I picked up patch 3 as well, I'll leave 2 to Laszlo.
> 
> Paolo
> 
>> +void acpi_table_add(const QemuOpts *opts, Error **errp);

Doesn't this series conflict with my pending
<http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg01427.html>?

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 1/3] acpi: move declarations from pc.h to acpi.h

[Paolo Bonzini]

Il 15/04/2013 13:00, Laszlo Ersek ha scritto:
>> > 
>>> >> +void acpi_table_add(const QemuOpts *opts, Error **errp);
> Doesn't this series conflict with my pending
> <http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg01427.html>?

Yeah, but the conflicts are trivial.

Paolo

Re: [Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Michael S. Tsirkin]

On Mon, Apr 15, 2013 at 12:37:37PM +0300, Michael S. Tsirkin wrote:
> I wanted to help Laszlo completely move ACPI tables out of seabios in
> time for 1.5, but had to put out some fires so didn't manage to do this
> in time, and going offline for now.
> 
> Meanwhile, here is some refactoring that I did complete.  This does not
> do much by itself, but works fine for me and will be helpful down the
> road.
> 
> This includes a couple of patches I posted previously
> and another one on top.
> 
> Pls consider for 1.5.

Just two clarifications
1. This conflicts with Laszlo's MADT change. As such if Laszlo's patch is applied,
   this will need to be rebased on top.
2. This was lightly tested but there's a holiday here so I run out of
   time for testing. Will report in a couple of days.

> 
> Michael S. Tsirkin (3):
>   acpi: move declarations from pc.h to acpi.h
>   acpi: add acpi_table_add_hdr
>   acpi.h: make it self contained
> 
>  arch_init.c            |  1 +
>  hw/acpi/core.c         | 97 +++++++++++++++++++++++++++++++++++---------------
>  hw/i386/pc.c           |  1 +
>  hw/i386/pc_piix.c      |  1 +
>  include/hw/acpi/acpi.h | 20 +++++++++++
>  include/hw/i386/pc.h   |  8 -----
>  6 files changed, 91 insertions(+), 37 deletions(-)
> 
> -- 
> MST

Re: [Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Laszlo Ersek]

On 04/15/13 13:25, Michael S. Tsirkin wrote:
> On Mon, Apr 15, 2013 at 12:37:37PM +0300, Michael S. Tsirkin wrote:
>> I wanted to help Laszlo completely move ACPI tables out of seabios in
>> time for 1.5, but had to put out some fires so didn't manage to do this
>> in time, and going offline for now.
>>
>> Meanwhile, here is some refactoring that I did complete.  This does not
>> do much by itself, but works fine for me and will be helpful down the
>> road.
>>
>> This includes a couple of patches I posted previously
>> and another one on top.
>>
>> Pls consider for 1.5.
> 
> Just two clarifications
> 1. This conflicts with Laszlo's MADT change. As such if Laszlo's patch is applied,
>    this will need to be rebased on top.
> 2. This was lightly tested but there's a holiday here so I run out of
>    time for testing. Will report in a couple of days.

First, thank you very much for the help!

Second, 2/3 breaks the binary format exported under the
FW_CFG_ACPI_TABLES key. The format is as follows:

  * number of tables in entire fw_cfg blob : uint16_t
  for each table:
    * ACPI payload size belonging to this table : uint16_t
    * standard ACPI header
    * ACPI table contents

whereas 2/3 changes the format to:

  * zero-filled uint16_t (i)
  for each table:
    * uint16_t storing value 1 in LE order (ii)
    * number of bytes in blob segment belonging to this table : uint16_t
    * standard ACPI header
    * ACPI table contents

(ii) shouldn't exist in per-table storage, the increments should target
(i) actually.


Another note: 2/3 introduces two ways to spell "has_header" in the
leading comments. The first use is "&&!@has_header" (correct with @, but
whitespace missing). The second use is "&& !has_header" (from my last
series, also see "bloblen"), which has the whitespace OK, but misses the
at-sign @ customary in comments when referring to function paramteres.


Finally, regarding the purpose of this patch. It seems to be to prepend
a standard ACPI header to a "headerles" table.

I think we can do without the reallocation. When preparing any ACPI
table, we know in advance the size of the ACPI header we're going to
need. So we can take it into account when allocating the buffer: fill in
the table body, and then call a function that puts the header at front,
into existing storage. See pc_acpi_install() in "[Qemu-devel] [qemu
PATCH 5/5] i386/pc: build ACPI MADT (APIC) for fw_cfg clients".

I propose the following:

(1) I'll resend my "[qemu PATCH 0/5] publish etc/acpi/APIC in fw_cfg"
series from last Monday, rebased to current (or then) master, with the
following changes suggested by Michael in private:
(1a) separating some stuff out into different files,
(1b) adding licensing bits related to SeaBIOS,
(1c) I'll try to consider 1/3 and 3/3 in this series that Paolo has
already picked up (may not match or be useful any longer),

(2) Then, please review / apply that updated series of mine,

(3) then please rebase 2/3 from here on top of (2). Again, I suggest
that acpi_table_add_hdr(), if any, fill in preallocated space; this is
consistent with updating the checksum too.

(4) I *do* anticipate that my code in (2) is not / will not be flexible
enough for all tables. However, please split up functions / extract bits
/ reorganize them only in a series that puts those new extracted
functions to use *at once* (probably near the end of said series). I
can't reason about "librarization" without actual use.

I'll wait for Michael's answer before doing anything.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Laszlo Ersek]

On 04/15/13 15:24, Laszlo Ersek wrote:

> Second, 2/3 breaks the binary format exported under the
> FW_CFG_ACPI_TABLES key. The format is as follows:
> 
>   * number of tables in entire fw_cfg blob : uint16_t
>   for each table:
>     * ACPI payload size belonging to this table : uint16_t
>     * standard ACPI header
>     * ACPI table contents
> 
> whereas 2/3 changes the format to:
> 
>   * zero-filled uint16_t (i)
>   for each table:
>     * uint16_t storing value 1 in LE order (ii)
>     * number of bytes in blob segment belonging to this table : uint16_t

Sorry, this was mis-editing the email on my part: the _length field is
computed alright, it's only the (i)<->(ii) thing that goes wrong.

>     * standard ACPI header
>     * ACPI table contents
> 
> (ii) shouldn't exist in per-table storage, the increments should target
> (i) actually.

Laszlo

Re: [Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Michael S. Tsirkin]

On Mon, Apr 15, 2013 at 03:24:00PM +0200, Laszlo Ersek wrote:
> On 04/15/13 13:25, Michael S. Tsirkin wrote:
> > On Mon, Apr 15, 2013 at 12:37:37PM +0300, Michael S. Tsirkin wrote:
> >> I wanted to help Laszlo completely move ACPI tables out of seabios in
> >> time for 1.5, but had to put out some fires so didn't manage to do this
> >> in time, and going offline for now.
> >>
> >> Meanwhile, here is some refactoring that I did complete.  This does not
> >> do much by itself, but works fine for me and will be helpful down the
> >> road.
> >>
> >> This includes a couple of patches I posted previously
> >> and another one on top.
> >>
> >> Pls consider for 1.5.
> > 
> > Just two clarifications
> > 1. This conflicts with Laszlo's MADT change. As such if Laszlo's patch is applied,
> >    this will need to be rebased on top.
> > 2. This was lightly tested but there's a holiday here so I run out of
> >    time for testing. Will report in a couple of days.
> 
> First, thank you very much for the help!
> 
> Second, 2/3 breaks the binary format exported under the
> FW_CFG_ACPI_TABLES key. The format is as follows:
> 
>   * number of tables in entire fw_cfg blob : uint16_t
>   for each table:
>     * ACPI payload size belonging to this table : uint16_t
>     * standard ACPI header
>     * ACPI table contents
> 
> whereas 2/3 changes the format to:
> 
>   * zero-filled uint16_t (i)
>   for each table:
>     * uint16_t storing value 1 in LE order (ii)
>     * number of bytes in blob segment belonging to this table : uint16_t
>     * standard ACPI header
>     * ACPI table contents
> 
> (ii) shouldn't exist in per-table storage, the increments should target
> (i) actually.
> 
> 
> Another note: 2/3 introduces two ways to spell "has_header" in the
> leading comments. The first use is "&&!@has_header" (correct with @, but
> whitespace missing). The second use is "&& !has_header" (from my last
> series, also see "bloblen"), which has the whitespace OK, but misses the
> at-sign @ customary in comments when referring to function paramteres.
> 
> 
> Finally, regarding the purpose of this patch. It seems to be to prepend
> a standard ACPI header to a "headerles" table.
> 
> I think we can do without the reallocation. When preparing any ACPI
> table, we know in advance the size of the ACPI header we're going to
> need. So we can take it into account when allocating the buffer: fill in
> the table body, and then call a function that puts the header at front,
> into existing storage. See pc_acpi_install() in "[Qemu-devel] [qemu
> PATCH 5/5] i386/pc: build ACPI MADT (APIC) for fw_cfg clients".
> 
> I propose the following:
> 
> (1) I'll resend my "[qemu PATCH 0/5] publish etc/acpi/APIC in fw_cfg"
> series from last Monday, rebased to current (or then) master, with the
> following changes suggested by Michael in private:
> (1a) separating some stuff out into different files,
> (1b) adding licensing bits related to SeaBIOS,
> (1c) I'll try to consider 1/3 and 3/3 in this series that Paolo has
> already picked up (may not match or be useful any longer),
> 
> (2) Then, please review / apply that updated series of mine,
> 
> (3) then please rebase 2/3 from here on top of (2). Again, I suggest
> that acpi_table_add_hdr(), if any, fill in preallocated space; this is
> consistent with updating the checksum too.
> 
> (4) I *do* anticipate that my code in (2) is not / will not be flexible
> enough for all tables. However, please split up functions / extract bits
> / reorganize them only in a series that puts those new extracted
> functions to use *at once* (probably near the end of said series). I
> can't reason about "librarization" without actual use.
> 
> I'll wait for Michael's answer before doing anything.
> 
> Thanks
> Laszlo

I'm fine with this. FYI, I'm offline for the rest of today and
tomorrow.

-- 
MST

Re: [Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/3] acpi: more infrastructure cleanups

[Laszlo Ersek]

On 04/15/13 23:10, Anthony Liguori wrote:
> Applied.  Thanks.
> 
> Regards,
> 
> Anthony Liguori
> 

Please revert. As I wrote in my review, 2/3 breaks stuff. 1/3 and 3/3
have been picked up by Paolo.

Thanks,
Laszlo