Em Wed, 25 Sep 2024 15:23:33 +0100 Jonathan Cameron <jonathan.came...@huawei.com> escreveu:
> On Wed, 25 Sep 2024 06:04:13 +0200 > Mauro Carvalho Chehab <mchehab+hua...@kernel.org> wrote: > > > The current code is actually dependent on having just one > > error structure with a single source. > > > > As the number of sources should be arch-dependent, as it > > will depend on what kind of synchronous/assynchronous > > notifications will exist, change the logic to dynamically > > build the table. > Not really arch dependent. Depends on both arch and some > firmware implementation choices, but I guess that detail > doesn't matter here. > > > > > Yet, for a proper support, we need to get the number of > > sources by reading the number from the HEST table. However, > > bios currently doesn't store a pointer to it. > > > > For now just change the logic at table build time, while > > enforcing that it will behave like before with a single > > source ID. > > > > A future patch will add a HEST table bios pointer and > > change the logic at acpi_ghes_record_errors() to > > dynamically use the new size. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org> > Trivial comment inline > Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com> > > > @@ -335,9 +346,10 @@ static void build_ghes_v2(GArray *table_data, > > build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0, > > 4 /* QWord access */, 0); > > bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > > - address_offset + GAS_ADDR_OFFSET, > > - sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE, > > - (ACPI_GHES_ERROR_SOURCE_COUNT + source_id) * sizeof(uint64_t)); > > + address_offset + GAS_ADDR_OFFSET, > > I'd prefer if we avoided realigning unless absolutely necessary or > that it is split into a separate patch. > Makes things a tiny bit harder to review. Heh, Igor nacked a patch doing the alignment change on a separate patch, so let's do it at the patches that are actually changing the code. At least for me, it is a low easier to review patches that are properly aligned with parenthesis. So, yeah it may be a little more painful to review a patch changing alignments, but IMO it pays off on future revisions, specially if we place one argument per line, like in this function. > > > + sizeof(uint64_t), > > + ACPI_GHES_ERRORS_FW_CFG_FILE, > > + (num_sources + index) * > > sizeof(uint64_t)); > > > Thanks, Mauro
