Date: Thu, 1 Feb 2024 16:42:55 -0500 From: Jung-uk Kim <jkim@FreeBSD.org> To: Dmitry Salychev <dsl@FreeBSD.org> Cc: Baptiste Daroussin <bapt@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 07c64d74917e - main - acpica: Import ACPICA 20230628 Message-ID: <dfaeaf57-21c2-4051-bf94-9f3a8b536384@FreeBSD.org> In-Reply-To: <86o7cz99u6.fsf@peasant.tower.home> References: <202401310406.40V46B9a000876@gitrepo.freebsd.org> <thik6e6mp53h242l6bhgyxkzl45jne5zhxb7retxgyvot3x6jq@v2yuxtpvczg7> <04c4a0e1-aa79-4d25-a1f7-2196cfa65578@FreeBSD.org> <86o7cz99u6.fsf@peasant.tower.home>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On 24. 2. 1., Dmitry Salychev wrote: > > Hi, > > Jung-uk Kim <jkim@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 [-- Attachment #2 --] 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?dfaeaf57-21c2-4051-bf94-9f3a8b536384>