On 24. 2. 1., Dmitry Salychev wrote:
Hi,
Jung-uk Kim <j...@freebsd.org> writes:
On 24. 1. 31., Baptiste Daroussin wrote:
Hello,
Either this one or the previous import is breaking arm64 build
--- acpi_iort.o ---
/home/bapt/worktrees/main/sys/arm64/acpica/acpi_iort.c:103:4: error: field
'data' with variable sized type 'union (unnamed union at
/home/bapt/worktrees/main/sys/arm64/acpica/acpi_iort.c:98:2
)' not at the end of a struct or class is a GNU extension
[-Werror,-Wgnu-variable-sized-type-not-at-end]
103 | } data;
| ^
Sorry for the breakage. I will fix it soon.
BTW, this code was added by this:
https://reviews.freebsd.org/D31267
It seems struct iort_named_component was a hack, which duplicated
ACPI_IORT_NAMED_COMPONENT but with a fixed length field
DeviceName[32]. Is it really necessary?
Jung-uk Kim
I'm struggling to understand (a) how the entire anonymous "data" union
was added by https://reviews.freebsd.org/D31267 as was "named_comp"
only and (b) what the problem with the "struct iort_named_component" really
is as ACPI_IORT_ROOT_COMPLEX and ACPI_IORT_SMMU were re-defined as
incomplete types in the new version of ACPICA. Everything is compilable
as it used to be with the attached patch.
FYI, ACPICA 20230331 dropped support for ANSI C (C89) and minimum
requirement is C99 now. One of the first feature they used was the
variable-length array, which replaced "foo[1]" hack. Previously,
"sizeof(struct bar)" was not real size because of the (fake
variable-length) array at the end of it. Now they removed the hack and
started using real variable-length arrays. I believe the hack in
acpi_iort.c was to work around the fake 1-byte array but it does not
work any more because all ACPI_IORT_* are now using correct VLA. So,
the cleanest way to fix this is using ACPI_IORT_NAMED_COMPONENT instead
of "struct iort_named_component" (see attached PoC patch), which also
eliminates the need for DeviceName[32].
Jung-uk Kim
diff --git a/sys/arm64/acpica/acpi_iort.c b/sys/arm64/acpica/acpi_iort.c
index a0e24788b775..298679d05c7d 100644
--- a/sys/arm64/acpica/acpi_iort.c
+++ b/sys/arm64/acpica/acpi_iort.c
@@ -74,14 +74,6 @@ struct iort_its_entry {
int pxm;
};
-struct iort_named_component
-{
- UINT32 NodeFlags;
- UINT64 MemoryProperties;
- UINT8 MemoryAddressLimit;
- char DeviceName[32]; /* Path of namespace object */
-};
-
/*
* IORT node. Each node has some device specific data depending on the
* type of the node. The node can also have a set of mappings, OR in
@@ -103,7 +95,7 @@ struct iort_node {
ACPI_IORT_ROOT_COMPLEX pci_rc; /* PCI root complex */
ACPI_IORT_SMMU smmu;
ACPI_IORT_SMMU_V3 smmu_v3;
- struct iort_named_component named_comp;
+ ACPI_IORT_NAMED_COMPONENT named_comp;
} data;
};
@@ -325,6 +317,7 @@ iort_add_nodes(ACPI_IORT_NODE *node_entry, u_int node_offset)
ACPI_IORT_SMMU_V3 *smmu_v3;
ACPI_IORT_NAMED_COMPONENT *named_comp;
struct iort_node *node;
+ size_t name_len;
node = malloc(sizeof(*node), M_DEVBUF, M_WAITOK | M_ZERO);
node->type = node_entry->Type;
@@ -360,10 +353,14 @@ iort_add_nodes(ACPI_IORT_NODE *node_entry, u_int node_offset)
memcpy(&node->data.named_comp, named_comp, sizeof(*named_comp));
/* Copy name of the node separately. */
+ name_len = node_entry->Length;
+ name_len -= ACPI_OFFSET(ACPI_IORT_NAMED_COMPONENT, DeviceName);
+ name_len = strnlen(node->data.named_comp.DeviceName, name_len);
+ named_comp = realloc(named_comp, sizeof(*node) + name_len + 1,
+ M_DEVBUF, M_WAITOK | M_ZERO);
strncpy(node->data.named_comp.DeviceName,
- named_comp->DeviceName,
- sizeof(node->data.named_comp.DeviceName));
- node->data.named_comp.DeviceName[31] = 0;
+ named_comp->DeviceName, name_len + 1);
+ node->data.named_comp.DeviceName[name_len] = 0;
iort_copy_data(node, node_entry);
TAILQ_INSERT_TAIL(&named_nodes, node, next);
