acpi/acpigen: rework acpigen_pop_len for different size PkgLength
Previously acpigen_pop_len always wrote a 3 byte PkgLength to the 3
bytes reserved by acpigen_write_len_f. After this patch acpigen_pop_len
encodes PkgLength in 1-3 bytes depending on the PkgLength. When less
than the 3 bytes that were previously reserved in the corresponding
acpigen_write_len_f call are needed for PkgLength, the payload data will
be moved back by the number of reserved bytes that aren't needed for the
PkgLength.
This fixes the problem that the Windows AML parser doesn't like a 3 byte
PkgLength being used for the size of the buffer containing UTF-16
strings when the length could be encoded in a single PkgLength byte. In
that case, Windows previously ignored the whole SSDT containing this
larger than necessary PkgLength encoding. It should however be noted
that the ACPI 6.4 spec doesn't specify if it's required to always use
the most compact possible encoding of the PkgLength or not. Since iasl
generates the shortest possible PkgLength encoding, it's also a good
idea to make coreboot's acpigen do the same although it's not required
by the specification.
With this patch applied, Windows still boots on Mandolin and the time it
takes to write the tables doesn't change. To measure the times, the log
level in bs_sample_time was increased to BIOS_CRIT and the console log
level was increased to BIOS_CRIT too to only get those times as output.
BS: BS_WRITE_TABLES run times (exec / console): 8 / 0 ms
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ib897b08a05a7cdc52902d51364246c260ea1f206
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79002
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Jérémy Compostella <jeremy.compostella@intel.com>
diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c
index 57f0d10..e5d76ba 100644
--- a/src/acpi/acpigen.c
+++ b/src/acpi/acpigen.c
@@ -38,16 +38,57 @@
void acpigen_pop_len(void)
{
- int len;
+ size_t len;
ASSERT(ltop > 0)
char *p = len_stack[--ltop];
len = gencurrent - p;
ASSERT(len <= ACPIGEN_MAXLEN)
- /* generate store length for 0xfffff max */
- p[0] = (0x80 | (len & 0xf));
- p[1] = (len >> 4 & 0xff);
- p[2] = (len >> 12 & 0xff);
+ const size_t payload_len = len - 3;
+ if (len <= 0x3f + 2) {
+ /* PkgLength of up to 0x3f can be encoded in one PkgLength byte instead of the
+ reserved 3 bytes. Since only 1 PkgLength byte will be written, the payload
+ data needs to be moved by 2 bytes */
+ memmove(&p[1], &p[3], payload_len);
+ /* Adjust the PkgLength to take into account that we only use 1 of the 3
+ reserved bytes */
+ len -= 2;
+ /* The two most significant bits of PkgLength get the value of 0 to indicate
+ there are no additional PkgLength bytes. In this case the single PkgLength
+ byte encodes the length in its lower 6 bits */
+ p[0] = len;
+ /* Adjust pointer for next ACPI bytecode byte */
+ acpigen_set_current(p + len);
+ } else if (len <= 0xfff + 1) {
+ /* PkgLength of up to 0xfff can be encoded in 2 PkgLength bytes instead of the
+ reserved 3 bytes. Since only 2 PkgLength bytes will be written, the payload
+ data needs to be moved by 1 byte */
+ memmove(&p[2], &p[3], payload_len);
+ /* Adjust the PkgLength to take into account that we only use 2 of the 3
+ reserved bytes */
+ len -= 1;
+ /* The two most significant bits of PkgLength get the value of 1 to indicate
+ there's a second PkgLength byte. The lower 4 bits of the first PkgLength
+ byte and the second PkgLength byte encode the length */
+ p[0] = (0x1 << 6 | (len & 0xf));
+ p[1] = (len >> 4 & 0xff);
+ /* Adjust pointer for next ACPI bytecode byte */
+ acpigen_set_current(p + len);
+ } else if (len <= 0xfffff) {
+ /* PkgLength of up to 0xfffff can be encoded in 3 PkgLength bytes. Since this
+ is the amount of reserved bytes, no need to move the payload in this case */
+ /* The two most significant bits of PkgLength get the value of 2 to indicate
+ there are two more PkgLength bytes following the first one. The lower 4 bits
+ of the first PkgLength byte and the two following PkgLength bytes encode the
+ length */
+ p[0] = (0x2 << 6 | (len & 0xf));
+ p[1] = (len >> 4 & 0xff);
+ p[2] = (len >> 12 & 0xff);
+ /* No need to adjust pointer for next ACPI bytecode byte */
+ } else {
+ /* The case of PkgLength up to 0xfffffff isn't supported at the moment */
+ printk(BIOS_ERR, "%s: package length exceeds maximum of 0xfffff.\n", __func__);
+ }
}
void acpigen_set_current(char *curr)