smbios: handle smbios3 for arm64

[gallatin]

Summary

Get smbios working on arm64 where it seems to be exclusively smbios version 3.x.

The "interesting" thing here is that the smbios table seems to be RAM in the EFI runtime services table. This makes it owned by "ram0", and not io memory. That prevents bus_alloc_resource() from being able to claim it, since ram0 already owns it. According to @jhb this is working as intended, as bus_alloc_resource() is meant to be used with IO memory, not physical RAM. He suggested I convert the driver to simply use pmap_mapbios(), which I've done.

This is based on initial work by Allan Jude in https://reviews.freebsd.org/D28739 and is a prerequisite for getting IPMI to attach via the SSIF method on arm64. Note that all IPMI devices I've seen on arm64 use the SSIF attachment.

[imp]

sys/dev/smbios/smbios.c

89

Max of the v2 and v3 sizes?

97–102

I'd be tempted to set the map size here too...

108

And here

114

No errors?

115

I'd still che k the signature to see which one you found.

122

I'd be tempted to say it's smbv2 explicitly rather than yhe length assertion

145–146

I think this return is wrong. We are just fixing the length sometimes... but when we don't we still need to add the child.

199

I'd be tempted to store the length in the softc... to limit interface abuse for this rman that might not be quite right for the address we smuggle in.

292

Space before (
This is how you should yell the different cases apart.

[gallatin]

sys/dev/smbios/smbios.c

89

Didn't want to do that, because that assumes placement is perfect. Eg, if you have a v2 thats sizeof (*eps) from the top of ram, mapping the v3 size might fail.

97–102

Its set to the default v2 size above.

108

Its set to the default v2 size above.

114

The function returns void.. not sure what you mean.

145–146

This is old code. The only change I made was to avoid leaking the mapped address. I'd prefer not to touch it.

292

No clue what you're trying to say. The style in this file is AFU, and I tried to match it.

[gallatin]

sys/dev/smbios/smbios.c

122

The original code did a len check against a magic number that represented the mapping size, i'm doing it "better" by using the map size. There is no point to come here if the length is not equal to the mapping size, because its apparently some kind of fixup for a weird older smbios version.

[imp]

So there's one, maybe two issues. First issue is the pmap issue I raised in the BIOS case. Second is 'We were told a smbios ptr, but it was bogus' seems to be something we should whine about. The second isn't as important, but will be a silent failure. The other no error cases are for legacy BIOS where SMBIOS might not be there (though the platforms where that is still the case are maybe 25 years old).

sys/dev/smbios/smbios.c

108

fair enough (here and above).

I have a conern though about this addr vs others....

This addr is a VA that's passed through BIOS_PADDRTOVADDR already, so it doesn't need the pmap_mapbios(). Will that done below screw things up since KERNBASE is added to the PA?

114

We found an address, but couldn't map it. Shouldn't we report it?

122

OK. That makes sense.

145–146

fair enough.

292

Yea, the style is AFU... Didn't notice the rest of the file was that way.

[imp]

sys/dev/smbios/smbios.c

173

The pa here is good for the EFI case, but not the BIOS case.... So the above comment about bios_sigsearch returning a VA is more important here.

[imp]

I misssed the return((u_int32_t)(uintptr_t)BIOS_VADDRTOPADDR(sp)); at the end of bios_sigsearch, so it does return a pa, so ignore my mismatch comments.

[imp]

So I'd have done some things differently... but what's there is fine to commit...

[gallatin]

sys/dev/smbios/smbios.c

108

The code for this has not changed with this update. The old code did the same thing, so I think this comment is out of scope for these changes. Note that bios_sigsearch() returns a physical, not virtual, address.

return((u_int32_t)(uintptr_t)BIOS_VADDRTOPADDR(sp));

In any case, a single physical address can have multiple virtual mappings, so I don't think it is a problem anyway.

173

No, it returns a VA.

199

That's not a bad idea.

[gallatin]

Test signature as suggested by @imp

[imp]

Thanks for the refinement.