amd64: allow base memory segment to start at address different than 0

[royger]

Summary

Current code requires that the first physical memory segment starts at 0,
but this is not really needed. We only need to make sure the bootstrap code
and page tables for APs are allocated below 4GB.

This patch removes this requirement and allows booting a Dell R710 from
UEFI, where the first physical memory segment starts at 0x10000.

Sponsored by: Citrix Systems R&D

[kib]

This looks fine in general.

I am curious about Dell BIOS. AFAIK, no Intel chipset allow not mapping low memory at zero. Did they made a hack to try to prevent some OS from corrupting low memory, or it is just a BIOS bug ?

sys/amd64/amd64/machdep.c

1365 ↗

(On Diff #2969)

How such situation can occur ? Shouldn't the zero-length segment filtered ?

[emaste]

In D1417#3, @kostikbel wrote:

I am curious about Dell BIOS. AFAIK, no Intel chipset allow not mapping low memory at zero. Did they made a hack to try to prevent some OS from corrupting low memory, or it is just a BIOS bug ?

I'm not certain about this Dell, but in general for this case there is memory at zero, it's just not available for the OS. For example @dumbbell has a machine that has RuntimeServicesData from 0x0 to 0xFFFF

[royger]

In D1417#3, @kostikbel wrote:

I am curious about Dell BIOS. AFAIK, no Intel chipset allow not mapping low memory at zero. Did they made a hack to try to prevent some OS from corrupting low memory, or it is just a BIOS bug ?

Not sure about what they are doing TBH... This is the memory map reported when booting from UEFI:

               Type     Physical      Virtual   #Pages Attr
   BootServicesData 000000010000            0 00000090 UC WC WT WB
   BootServicesData 000000100000            0 00003f00 UC WC WT WB
 ConventionalMemory 000004000000            0 0003bffc UC WC WT WB
         LoaderData 00003fffc000            0 00000004 UC WC WT WB
 ConventionalMemory 000040000000            0 0007aba0 UC WC WT WB
         LoaderData 0000baba0000            0 00002200 UC WC WT WB
         LoaderCode 0000bcda0000            0 0000004a UC WC WT WB
         LoaderData 0000bcdea000            0 0000004c UC WC WT WB
         LoaderCode 0000bce36000            0 0000000f UC WC WT WB
 ConventionalMemory 0000bce45000            0 0000000f UC WC WT WB
         LoaderData 0000bce54000            0 00000001 UC WC WT WB
   BootServicesData 0000bce55000            0 00001a43 UC WC WT WB
 ConventionalMemory 0000be898000            0 0000002e UC WC WT WB
   BootServicesCode 0000be8c6000            0 000001d2 UC WC WT WB
RuntimeServicesCode 0000bea98000            0 000000d1 UC WC WT WB RUNTIME
RuntimeServicesCode 0000beb69000            0 0000002f UC WC WT WB RUNTIME
RuntimeServicesData 0000beb98000            0 000000b4 UC WC WT WB RUNTIME
RuntimeServicesData 0000bec4c000            0 0000004c UC WC WT WB RUNTIME
 ConventionalMemory 0000bec98000            0 000000ff UC WC WT WB
      ACPIMemoryNVS 0000bed97000            0 00000005 UC WC WT WB
  ACPIReclaimMemory 0000bed9c000            0 00000080 UC WC WT WB
   BootServicesData 0000bee1c000            0 00000426 UC WC WT WB
           Reserved 0000bf699000            0 00000016 UC
  ACPIReclaimMemory 0000bf6af000            0 0000001f WB
           Reserved 0000bf6ce000            0 00000932 UC
           Reserved 0000e0000000            0 00010000 UC
           Reserved 0000fe000000            0 00002000 UC
 ConventionalMemory 000100000000            0 00740000 UC WC WT WB

sys/amd64/amd64/machdep.c

1365 ↗

(On Diff #2969)

This happens on the first iteration if the added segment doesn't start at 0. If we don't do this we end up with a physmap array that looks like:

physmap[0] = 0
physmap[1] = 0
physmap[2] = 0x10000
physmap[3] = ....

[kib]

In D1417#6, @royger wrote:

In D1417#3, @kostikbel wrote:

I am curious about Dell BIOS. AFAIK, no Intel chipset allow not mapping low memory at zero. Did they made a hack to try to prevent some OS from corrupting low memory, or it is just a BIOS bug ?

Not sure about what they are doing TBH... This is the memory map reported when booting from UEFI:

Is it known whether DOS can be booted on this machine ?

sys/amd64/amd64/machdep.c

1365 ↗

(On Diff #2969)

I think this should be handled differently. E.g. by initizializing physmap_idx in getmemsize() to -2 instead of 0. This probably requires some other changes to make it work, but it sounds like mishandled loop initialization.

[jhb]

sys/amd64/amd64/machdep.c

1365 ↗

(On Diff #2969)

I think I agree that an initial index of -2 would be better (or physmap_idx should point to the first empty slot, not the last used slot. Typically for arrays like this the associated variable would be a 'length' which would start out at 0 and effectively always points to the first empty slot, not the last used slot).

1590 ↗

(On Diff #2969)

basemem effectively means "memory in the first 640k", so this shouldn't assume the first entry.
Instead, the check in the loop should check for a start address <= 640k I think (rather than == 0).

Ironically, i386 handles this better than amd64. If it doesn't find a suitable segment in the SMAP it just assumes 640k instead of panic'ing. (Ideally i386 would have the same check against == 0 updated since it will need EFI support someday.)

[royger]

sys/amd64/amd64/machdep.c

1590 ↗

(On Diff #2969)

Is basemem still meaningful when booted from UEFI? AFAICT basemem is only used in some cases to find the mptable, but are UEFI systems supposed to still support/provide an mptable?

[emaste]

In D1417#9, @royger wrote:

Is basemem still meaningful when booted from UEFI? AFAICT basemem is only used in some cases to find the mptable, but are UEFI systems supposed to still support/provide an mptable?

EFI does provide a mechanism to obtain the mptable, but I'm not sure it's that useful (or widely implemented).

I think @jhb suggested we change the mptable search to iterate over physmap entries instead of using basemem; that seems reasonable to me and will work either way.

Either way though I'd be happy to see a variant of this change go in soon, and we can change the mptable search later on.

[royger]

Changes since v1:

• Updated physmap_idx to point to the next free entry.
• Improved check for basemem segment.

[jhb]

To be clear, I would ultimately like for basemem to die in a fire and change any consumers to just use dump_avail[] directly instead.

[royger]

Closed by commit rS277735 (authored by @royger).