Improve memory detection in biosmem.c

[grembo]

Summary

Original motivation is to make the Acer C720 (and potentially other
chromebooks boot. This patch is an extended version of adrian's patch
that can be found here (the changes in biosdisk.c were found to be
unnecessary by kib and are therefore not part of this anymore):

https://people.freebsd.org/~adrian/c720/20150117-seabios-loader-changes-1.diff

• Add a quirks table based on BIOS vendor / maker / product (NULL matches any), making use of the new smbios_match function
• pass in an smap+acpi entry size for e820; like what biossmap.c does;
• don't blindly believe e820 if it doesn't give us enough memory to successfully load things, but ONLY if the machine is listed as BQ_DISTRUST_E820_EXTMEM in the quirks table (in this patch this is only the Acer C720 aka Peppy, since it was the reason to write this patch). There are probably other models out there with these issues, but without data it's probably best to add them one at a time.
• don't blindly concat cx/dx with e801 - ensure that cx indicates there's no hole and there's a full 15mb of ram before concat'ing;
• truncate how much RAM we probe for via e801 (which adrian doesn't entirely think is needed)
• if we use e801, don't use the heap as-is; use the end of bios_extmem instead as if we didn't get anything from E820;
• add a boot loader command "biosmem" which gives details about how memory was detected

Test Plan

Boot on various machines, escape to boot loader to test biosmem command.
I tested this on the Chromebook that's in the quirks table and could
boot successfully.

[grembo]

Return actual quirk and not 1.

[grembo]

@adrian acknowledged this revision (thanks), but he also pointed out, that he thinks this needs more testing before committing. I tried to get some attention on freebsd-current, but this isn't a very interesting topic for people to test, so even though it's been only a few days, I don't think this will result in sufficient testing.

Since I don't think this is really in the spirit of a code review (once it's accepted it should be ready to get committed) and since the "dangerous" part of the patch was written by adrian himself, I reopen this request and propose two solutions:

1. Someone else accepts the revision, so I can go ahead and commit it
2. I change it, so that adrian's code is only used if a quirk is set. This way I could accomplish my primary target of making this code work on this specific hardware and leave the change for all machines to a second commit. If it then turns out the change doesn't work well, a rollback won't affect my primary goal.

Your input would be appreciated.

[jhb]

sys/boot/i386/libi386/biosmem.c

119 ↗

(On Diff #3604)

I think you don't need the 32MB check now if you have the quirk check.

If you do, please reformat the comment. It should be a block comment instead of two line comments, but also fix the punctuation / capitalization while you are there and add a blank before the comment block. (I'd probably just drop the 32MB check though.)

171 ↗

(On Diff #3604)

Please add a blank line here before the comment.

172 ↗

(On Diff #3604)

Please use '%cx' for register names.

241 ↗

(On Diff #3604)

Why "smap"? This should probably be 'biosmem' instead. I think it is just used to name the internal structure stored in the linker set, but it would be more consistent to have the name here match the command name (or at least be relevant).

[grembo]

Changes like requested, I will add a few more inline comments.

[grembo]

sys/boot/i386/libi386/biosmem.c

122 ↗

(On Diff #3736)

I kept the check in here, the idea is that we might want to be a bit bolder in the quirks table in the future (let's say: all coreboot machines), but only use the fallback code, in case the numbers returned aren't good.

Does this make sense to you or do you think that's not a realistic scenario anyway?

176 ↗

(On Diff #3736)

I hope I got this right (%cx)

[grembo]

Removed 32MiB check from code like suggested (reverted comment changes).

There might be another compatible way to improve memory detection:
Instead of the explicit quirk BQ_DISTRUST_E820_EXTMEM, introduce one
called BQ_REQ_SUFFICIENT_EXTMEM.

This would only accept extmem if it's >32MBit, otherwise set it to
0 and therfore fall through to the next method.

I might be overcomplicating this though, so if you think the current
solution is ok, I can totally live with that.

[grembo]

Is there anything else I could do to move this forward?

[adrian]

I think it's ready. Thanks for perservering!

[grembo]

Closed by commit rS279222 (authored by @grembo).

[jhb]

Just to say that I think this version was ok.