| Message ID | 20241103124824.943659-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | modpost: fix acpi MODULE_DEVICE_TABLE built with mismatched endianness | expand |
On Sun, Nov 3, 2024 at 9:48 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > When CONFIG_SATA_AHCI_PLATFORM=m, modpost outputs incorect MODULE_ALIAS() > if the endianness of the target and the build machine do not match. > > When the endianness of the target kernel and the build machine match, > the output is correct: > > $ grep 'MODULE_ALIAS("acpi' drivers/ata/ahci_platform.mod.c > MODULE_ALIAS("acpi*:APMC0D33:*"); > MODULE_ALIAS("acpi*:010601:*"); > > However, when building a little-endian kernel on a big-endian machine > (or vice versa), the output is incorrect: > > $ grep 'MODULE_ALIAS("acpi' drivers/ata/ahci_platform.mod.c > MODULE_ALIAS("acpi*:APMC0D33:*"); > MODULE_ALIAS("acpi*:0601??:*"); > > The 'cls' and 'cls_msk' fields are 32-bit. > > DEF_FIELD() must be used instead of DEF_FIELD_ALIAS() to correctly handle This is a typo: DEF_FIELD_ALIAS() -> DEF_FIELD_ADDR() > endianness of these 32-bit fields. > > The check 'if (cls)' was unnecessary; it never became NULL, as it was the > pointer to 'symval' plus the offset to the 'cls' field. > > Fixes: 26095a01d359 ("ACPI / scan: Add support for ACPI _CLS device matching") > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > scripts/mod/file2alias.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c > index 99dce93a4188..16154449dde1 100644 > --- a/scripts/mod/file2alias.c > +++ b/scripts/mod/file2alias.c > @@ -567,12 +567,12 @@ static int do_acpi_entry(const char *filename, > void *symval, char *alias) > { > DEF_FIELD_ADDR(symval, acpi_device_id, id); > - DEF_FIELD_ADDR(symval, acpi_device_id, cls); > - DEF_FIELD_ADDR(symval, acpi_device_id, cls_msk); > + DEF_FIELD(symval, acpi_device_id, cls); > + DEF_FIELD(symval, acpi_device_id, cls_msk); > > if (id && strlen((const char *)*id)) > sprintf(alias, "acpi*:%s:*", *id); > - else if (cls) { > + else { > int i, byte_shift, cnt = 0; > unsigned int msk; > > @@ -580,10 +580,10 @@ static int do_acpi_entry(const char *filename, > cnt = 6; > for (i = 1; i <= 3; i++) { > byte_shift = 8 * (3-i); > - msk = (*cls_msk >> byte_shift) & 0xFF; > + msk = (cls_msk >> byte_shift) & 0xFF; > if (msk) > sprintf(&alias[cnt], "%02x", > - (*cls >> byte_shift) & 0xFF); > + (cls >> byte_shift) & 0xFF); > else > sprintf(&alias[cnt], "??"); > cnt += 2; > -- > 2.43.0 >
On Sun, Nov 3, 2024 at 9:56 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sun, Nov 3, 2024 at 9:48 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > When CONFIG_SATA_AHCI_PLATFORM=m, modpost outputs incorect MODULE_ALIAS() > > if the endianness of the target and the build machine do not match. > > > > When the endianness of the target kernel and the build machine match, > > the output is correct: > > > > $ grep 'MODULE_ALIAS("acpi' drivers/ata/ahci_platform.mod.c > > MODULE_ALIAS("acpi*:APMC0D33:*"); > > MODULE_ALIAS("acpi*:010601:*"); > > > > However, when building a little-endian kernel on a big-endian machine > > (or vice versa), the output is incorrect: > > > > $ grep 'MODULE_ALIAS("acpi' drivers/ata/ahci_platform.mod.c > > MODULE_ALIAS("acpi*:APMC0D33:*"); > > MODULE_ALIAS("acpi*:0601??:*"); > > > > The 'cls' and 'cls_msk' fields are 32-bit. > > > > DEF_FIELD() must be used instead of DEF_FIELD_ALIAS() to correctly handle > > This is a typo: > > DEF_FIELD_ALIAS() -> DEF_FIELD_ADDR() > Applied to linux-kbuild/fixes.
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 99dce93a4188..16154449dde1 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -567,12 +567,12 @@ static int do_acpi_entry(const char *filename, void *symval, char *alias) { DEF_FIELD_ADDR(symval, acpi_device_id, id); - DEF_FIELD_ADDR(symval, acpi_device_id, cls); - DEF_FIELD_ADDR(symval, acpi_device_id, cls_msk); + DEF_FIELD(symval, acpi_device_id, cls); + DEF_FIELD(symval, acpi_device_id, cls_msk); if (id && strlen((const char *)*id)) sprintf(alias, "acpi*:%s:*", *id); - else if (cls) { + else { int i, byte_shift, cnt = 0; unsigned int msk; @@ -580,10 +580,10 @@ static int do_acpi_entry(const char *filename, cnt = 6; for (i = 1; i <= 3; i++) { byte_shift = 8 * (3-i); - msk = (*cls_msk >> byte_shift) & 0xFF; + msk = (cls_msk >> byte_shift) & 0xFF; if (msk) sprintf(&alias[cnt], "%02x", - (*cls >> byte_shift) & 0xFF); + (cls >> byte_shift) & 0xFF); else sprintf(&alias[cnt], "??"); cnt += 2;
When CONFIG_SATA_AHCI_PLATFORM=m, modpost outputs incorect MODULE_ALIAS() if the endianness of the target and the build machine do not match. When the endianness of the target kernel and the build machine match, the output is correct: $ grep 'MODULE_ALIAS("acpi' drivers/ata/ahci_platform.mod.c MODULE_ALIAS("acpi*:APMC0D33:*"); MODULE_ALIAS("acpi*:010601:*"); However, when building a little-endian kernel on a big-endian machine (or vice versa), the output is incorrect: $ grep 'MODULE_ALIAS("acpi' drivers/ata/ahci_platform.mod.c MODULE_ALIAS("acpi*:APMC0D33:*"); MODULE_ALIAS("acpi*:0601??:*"); The 'cls' and 'cls_msk' fields are 32-bit. DEF_FIELD() must be used instead of DEF_FIELD_ALIAS() to correctly handle endianness of these 32-bit fields. The check 'if (cls)' was unnecessary; it never became NULL, as it was the pointer to 'symval' plus the offset to the 'cls' field. Fixes: 26095a01d359 ("ACPI / scan: Add support for ACPI _CLS device matching") Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/file2alias.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)