[PATCH 8/19] bhyve: add basl support for pointers

[corvink]

Summary

Some ACPI tables like XSDT contain pointers to other ACPI tables. When
an ACPI table is loaded by qemu's loader, the address in the guest
memory is unknown. For that reason, the qemu loader supports patching
those pointers. Basl keeps track of all pointers and causes the qemu
loader to patch all pointers.

The qemu ACPI table loader is unsupport yet. However, in a future commit bhyve will use dynamic ACPI table offsets based on the size and alignment requirements of each ACPI table. Therefore, tracking ACPI table pointer is required too.

This is the 8. patch required to merge D36983

[markj]

usr.sbin/bhyve/basl.c

46

We have a few of the same nits as before: this list should be static, there should be no space after STAILQ_FOREACH, empty parameter lists should be void. I'll stop pointing them out.

235

Should we also check for overflow in gva + pointer->off?

250

To be paranoid, should there be a check that pointer->off + pointer->size <= table->len?

251

This trick with memcpy() assumes that the host is little-endian, if pointer->size < 8.

[corvink]

• add some assertions as suggested by reviewers
• fix pointer patching for big endian hosts
• turn basl_pointers into a member variable of basl_table to avoid global variables and backpointers

[corvink]

usr.sbin/bhyve/basl.c

235

I've added an assertion for pointer->off < table->len. So, if gva + pointer->off overflows, vm_map_gpa(ctx, gpa, table->len) should fail. Am I missing something?

[corvink]

• fix some style nits

[jhb]

usr.sbin/bhyve/basl.c

195

"sign" being its own word is slightly less readable (it's not immediately obvious that it is an abbreviation for signature. I would suggest either expanding "sign" to signature in various places here, or perhaps just using "name" instead (the constant is already ACPI_NAMESEG_SIZE)

197–198

This is more the style(9) rule about having all variables declared at the start of a block with a blank line before code.

207

You could use %.4s instead of the four %c and then just use sign as the argument. Just shorter to write.

231

Again, this isn't really about guest BIOS, but instead about guest kernels, etc.

254

I would instead do something like:

switch (pointer->size) {
case 4:
            val = le32dec(gva + pointer->off);
            break;
case 8:
            val = le64dec(gva + pointer->off);
            break;
}

This combines the memcpy + le*toh in one step.

Also, I would just assert() in basl_table_add_pointer that the pointer size is either 4 or 8.

282

This could use the helper routine I suggested previously.

[jhb]

usr.sbin/bhyve/basl.c

223

%,4s here as well

324–325

[corvink]

• make use of basl_le_dec and basl_le_enc helper
• fix some style nits

[corvink]

usr.sbin/bhyve/basl.c

207

Thanks.