Page MenuHomeFreeBSD

Fix fdt_get_mem_regions() to work with 64-bit addresses
ClosedPublic

Authored by mst_semihalf.com on Feb 22 2016, 3:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 1 2024, 1:55 AM
Unknown Object (File)
Dec 20 2023, 12:17 AM
Unknown Object (File)
Dec 11 2023, 4:16 AM
Unknown Object (File)
Dec 10 2023, 9:08 PM
Unknown Object (File)
Nov 30 2023, 5:21 PM
Unknown Object (File)
Nov 10 2023, 9:37 AM
Unknown Object (File)
Nov 8 2023, 9:22 PM
Unknown Object (File)
Nov 7 2023, 8:55 PM
Subscribers

Details

Summary

Use u_long instead of uint32_t variables to avoid overflow on 64-bit architectures.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mst_semihalf.com retitled this revision from to Fix fdt_get_mem_regions() to work with 64-bit addresses.
mst_semihalf.com updated this object.
mst_semihalf.com edited the test plan for this revision. (Show Details)
mst_semihalf.com added reviewers: andrew, imp, wma.
mst_semihalf.com set the repository for this revision to rS FreeBSD src repository - subversion.
mst_semihalf.com added a subscriber: arm64.

My only concern is if we can really trust u_long to be 64-bit on every platform. I can see that MIPS64 defines uint64_t as "long long" in one case. I'm not knowledgeable enough to tell if the MIPS part is even compiled now or if "unsigned long" won't work there, but it might be safer to use a type that is guaranteed to be 32/64b, like uintptr_t or vm_offset_t.

In D5393#115079, @wma wrote:

My only concern is if we can really trust u_long to be 64-bit on every platform. I can see that MIPS64 defines uint64_t as "long long" in one case. I'm not knowledgeable enough to tell if the MIPS part is even compiled now or if "unsigned long" won't work there, but it might be safer to use a type that is guaranteed to be 32/64b, like uintptr_t or vm_offset_t.

We could use uintptr_t too, but perhaps it would be better to stay with the convention used in other fdt_common functions - they all use u_long for memory addresses. All these functions would fail if u_long was 32-bit on some 64-bit arch. All rman functions also (currently) use u_long type arguments which contain memory addresses.

According to mips/include/_types.h, MIPS32 defines uint64_t as unsigned long long, MIPS64 defines it as unsigned long, and I think other archs are consistent.

wma edited edge metadata.

Ok, as we discussed locally, let's leave u_long for now. All its instances in fdt stuff are going to be replaced anyway, so it's better to be consistent with the rest of the file now.

This revision is now accepted and ready to land.Feb 26 2016, 8:46 AM
andrew requested changes to this revision.Feb 26 2016, 1:57 PM
andrew added a reviewer: br.
andrew edited edge metadata.

riscv also needs to be updated

This revision now requires changes to proceed.Feb 26 2016, 1:57 PM
br edited edge metadata.

If @br is happy with this feel free to ignore me & let him fix riscv.

mst_semihalf.com edited edge metadata.
mst_semihalf.com removed rS FreeBSD src repository - subversion as the repository for this revision.

Also fixed riscv.

wma edited edge metadata.

Seems that everything is fixed now. I'll commit this patch then, thanks.

For the record: tested on armv8, armv7 (Alpine, Sheevaplug), MIPS (Beri).

This revision was automatically updated to reflect the committed changes.