nvdimm: only enumerate present nvdimm devices

[scottph]

Summary

Not all child devices of the NVDIMM root device represent DIMM
devices which are present in the system. The spec says (ACPI 6.2,
sec 9.20.2):

For each NVDIMM present or intended to be supported by platform,
platform firmware also exposes an NVDIMM device ... under the
NVDIMM root device.

Present NVDIMM devices are found by walking all of the NFIT
table's SPA ranges, then walking the NVDIMM regions mentioned by
those SPA ranges.

A set of NFIT walking helper functions are introduced to avoid the
need to splat the enumeration logic across several disparate
callbacks.

Submitted by: D Scott Phillips <[e-mail]>
Sponsored by: Intel Corporation
MFC after: 1 week

[scottph]

FYI, this diff is on top of D18346

[scottph]

rebase diff so that it is no longer against D18346

[scottph]

sys/dev/nvdimm/nvdimm.c

170 ↗

(On Diff #51841)

Here we also need AcpiAttachData(handle, acpi_fake_objhandler, child); so that acpi_get_device() can work in nvdimm_find_by_handle()

[kib]

sys/dev/nvdimm/nvdimm.c

178 ↗

(On Diff #52523)

see note below about the loop variable name.

181 ↗

(On Diff #52523)

Do we need to flush already created children ?

185 ↗

(On Diff #52523)

Why -1 ?

205 ↗

(On Diff #52523)

Please do not name non-integer loop variable 'i'.

216 ↗

(On Diff #52523)

Do we need to clear other already created spa's ?

sys/dev/nvdimm/nvdimm_nfit.c

41 ↗

(On Diff #52523)

This '{' should be on the new line, alone.

_* are C-std reserved names, better not use them at all.

46 ↗

(On Diff #52523)

return (*a - *b);

64 ↗

(On Diff #52523)

You want to mask the value after dereferencing the pointer, right ? I would write it as

val &= mask;

on the next line.

More important, this code potentially accesses unaligned types, esp. when you do the trick with fetching uint16_t and passing UINT16_MAX for the mask. This only works on x86, and even there it works by chance.

73 ↗

(On Diff #52523)

There is no need in {} around single-line block, usually.

114 ↗

(On Diff #52523)

And there the body block is multi-line, so {} are warranted by style.

Several more {} vs. no-{} below, I did not marked them.

[scottph]

• rebase patch
• add commit message tags
• Don't use i for non-int loop variables
• Fix some style(9) issues with braces
• Don't use variable names prefixed with underscore
• Handle unaligned accesses in find_matches()
• Fix an off-by-one error in the allocation of ivars

[scottph]

sys/dev/nvdimm/nvdimm.c

181 ↗

(On Diff #52523)

the child devices and spas get destroyed on detach, which I think covers the case you mention.

185 ↗

(On Diff #52523)

You're right, I think the -1 was wrong.

170 ↗

(On Diff #51841)

This comment is obsoleted by the nvdimm root device's own ivars.

sys/dev/nvdimm/nvdimm_nfit.c

46 ↗

(On Diff #52523)

oops, messed up a space here.

64 ↗

(On Diff #52523)

changed this to bcopy to handle the unaligned access.

114 ↗

(On Diff #52523)

Think I got them all, sorry for the simple mistakes.

[scottph]

• fix one more style(9) problem