drivers/acpi/prmt.c:156:29: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch]
The following build regressions are noticed on x86 due to following Warnings and errors with clang-19 and clang-nightly, The builds with gcc-13 do PASS.
Started happening on next-20241023. Good: next-20241022 Bad: next-20241023
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Build log: —------- drivers/acpi/prmt.c:156:29: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch] 156 | (void *)efi_pa_va_lookup(&th->guid, handler_info->handler_address); | ^ drivers/acpi/prmt.c:159:21: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch] 159 | efi_pa_va_lookup(&th->guid, handler_info->static_data_buffer_address); | ^ drivers/acpi/prmt.c:162:21: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch] 162 | efi_pa_va_lookup(&th->guid, handler_info->acpi_param_buffer_address); | ^ 3 errors generated.
Suspecting commit: ACPI: PRM: Find EFI_MEMORY_RUNTIME block for PRM handler and context Commit id: 088984c8d54c0053fc4ae606981291d741c5924b
Build Links: —--- build error link: https://storage.tuxsuite.com/public/linaro/lkft/builds/2npIm4ZOkWenPJ71UOZG5...
Metadata: —---- Git_describe: next-20241023 Git_repo: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git Git_sha: ceab669fdf7b7510b4e4997b33d6f66e433a96db Build_name: clang-nightly-lkftconfig Compiler: clang-nightly Config: https://storage.tuxsuite.com/public/linaro/lkft/builds/2npIm4ZOkWenPJ71UOZG5... Download_url: https://storage.tuxsuite.com/public/linaro/lkft/builds/2npIm4ZOkWenPJ71UOZG5...
-- Linaro LKFT https://lkft.linaro.org
Config: https://storage.tuxsuite.com/public/linaro/lkft/builds/2npIm4ZOkWenPJ71UOZG5...
drivers/acpi/prmt.c:156:29: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch] 156 | (void *)efi_pa_va_lookup(&th->guid, handler_info->handler_address); | ^
The problem is that efi_guid_t is alighned but guid_t is not. I would have thought that Clang would say that even though the alignment in for &th->guid isn't spelled out explicitly, it would still end up being aligned at 8 bytes.
typedef guid_t efi_guid_t __aligned(__alignof__(u32));
The relevant code looks like this:
include/linux/uuid.h 13 #define UUID_SIZE 16 14 15 typedef struct { 16 __u8 b[UUID_SIZE]; 17 } guid_t;
drivers/acpi/prmt.c 54 struct prm_handler_info { 55 guid_t guid; 56 efi_status_t (__efiapi *handler_addr)(u64, void *); 57 u64 static_data_buffer_addr; 58 u64 acpi_param_buffer_addr; 59 60 struct list_head handler_list; 61 }; 62 63 struct prm_module_info { 64 guid_t guid; 65 u16 major_rev; 66 u16 minor_rev; 67 u16 handler_count; 68 struct prm_mmio_info *mmio_info; 69 bool updatable; 70 71 struct list_head module_list; 72 struct prm_handler_info handlers[] __counted_by(handler_count); ^^^^^^^^^^ This is a 64bit config so it's 8 byte aligned, right?
73 };
[ snip ]
drivers/acpi/prmt.c 96 static int __init 97 acpi_parse_prmt(union acpi_subtable_headers *header, const unsigned long end) 98 { 99 struct acpi_prmt_module_info *module_info; 100 struct acpi_prmt_handler_info *handler_info; 101 struct prm_handler_info *th; 102 struct prm_module_info *tm; 103 u64 *mmio_count; 104 u64 cur_handler = 0; 105 u32 module_info_size = 0; 106 u64 mmio_range_size = 0; 107 void *temp_mmio; 108 109 module_info = (struct acpi_prmt_module_info *) header; 110 module_info_size = struct_size(tm, handlers, module_info->handler_info_count); 111 tm = kmalloc(module_info_size, GFP_KERNEL); 112 if (!tm) 113 goto parse_prmt_out1; 114 115 guid_copy(&tm->guid, (guid_t *) module_info->module_guid); 116 tm->major_rev = module_info->major_rev; 117 tm->minor_rev = module_info->minor_rev; 118 tm->handler_count = module_info->handler_info_count; 119 tm->updatable = true; 120 121 if (module_info->mmio_list_pointer) { 122 /* 123 * Each module is associated with a list of addr 124 * ranges that it can use during the service 125 */ 126 mmio_count = (u64 *) memremap(module_info->mmio_list_pointer, 8, MEMREMAP_WB); 127 if (!mmio_count) 128 goto parse_prmt_out2; 129 130 mmio_range_size = struct_size(tm->mmio_info, addr_ranges, *mmio_count); 131 tm->mmio_info = kmalloc(mmio_range_size, GFP_KERNEL); 132 if (!tm->mmio_info) 133 goto parse_prmt_out3; 134 135 temp_mmio = memremap(module_info->mmio_list_pointer, mmio_range_size, MEMREMAP_WB); 136 if (!temp_mmio) 137 goto parse_prmt_out4; 138 memmove(tm->mmio_info, temp_mmio, mmio_range_size); 139 } else { 140 tm->mmio_info = kmalloc(sizeof(*tm->mmio_info), GFP_KERNEL); 141 if (!tm->mmio_info) 142 goto parse_prmt_out2; 143 144 tm->mmio_info->mmio_count = 0; 145 } 146 147 INIT_LIST_HEAD(&tm->module_list); 148 list_add(&tm->module_list, &prm_module_list); 149 150 handler_info = get_first_handler(module_info); 151 do { 152 th = &tm->handlers[cur_handler]; 153 154 guid_copy(&th->guid, (guid_t *)handler_info->handler_guid); 155 th->handler_addr = 156 (void *)efi_pa_va_lookup(&th->guid, handler_info->handler_address); ^^^^^^^^^ Here is the warning/build error.
157 158 th->static_data_buffer_addr = 159 efi_pa_va_lookup(&th->guid, handler_info->static_data_buffer_address); 160
regards, dan carpenter
On Wed, 23 Oct 2024 at 21:06, Dan Carpenter dan.carpenter@linaro.org wrote:
Config: https://storage.tuxsuite.com/public/linaro/lkft/builds/2npIm4ZOkWenPJ71UOZG5...
drivers/acpi/prmt.c:156:29: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch] 156 | (void *)efi_pa_va_lookup(&th->guid, handler_info->handler_address); | ^
The problem is that efi_guid_t is alighned but guid_t is not. I would have thought that Clang would say that even though the alignment in for &th->guid isn't spelled out explicitly, it would still end up being aligned at 8 bytes.
Yeah this is bizarre tbh. The alignment of the type should be irrelevant here.
typedef guid_t efi_guid_t __aligned(__alignof__(u32));
The relevant code looks like this:
include/linux/uuid.h 13 #define UUID_SIZE 16 14 15 typedef struct { 16 __u8 b[UUID_SIZE]; 17 } guid_t;
drivers/acpi/prmt.c 54 struct prm_handler_info { 55 guid_t guid;
So this should be changed to efi_guid_t.
Doing so makes the following pass:
static_assert(__alignof__(((struct prm_handler_info*)0)->guid) > 1);
(it fails with the original guid_t type)
-
Ard Biesheuvel -
Dan Carpenter -
Naresh Kamboju