Changed excise_initrd_region to support both 32- and 64-bit values for linux,initrd-start and linux,initrd-end

[lffpires_ruabrasil.org]

Summary

Fix excise_initrd_region() to support 32- and 64-bit initrd params.

Changed excise_initrd_region to support both 32- and 64-bit
values for linux,initrd-start and linux,initrd-end.

This fixes the boot problem on some machines after rS334485.

[lffpires_ruabrasil.org]

After further investigation, linux,initrd-start/end are always 32-bit values

[jhibbits]

In D15667#331142, @lffpires_ruabrasil.org wrote:

After further investigation, linux,initrd-start/end are always 32-bit values

That's odd, because they're 64-bit values in the device tree:

Node 0xe38: chosen

linux,initrd-end:
  00 00 00 00 00 00 00 00
linux,initrd-start:
  00 00 00 00 00 00 00 00

sys/powerpc/ofw/ofw_machdep.c

302 ↗

(On Diff #43335)

This really should be OF_getencprop() anyway.

306 ↗

(On Diff #43335)

This as well.

[lffpires_ruabrasil.org]

Removed dummy comment

[lffpires_ruabrasil.org]

Rework after comments

[lffpires_ruabrasil.org]

In D15667#331145, @jhibbits wrote:

In D15667#331142, @lffpires_ruabrasil.org wrote:

After further investigation, linux,initrd-start/end are always 32-bit values

That's odd, because they're 64-bit values in the device tree:

Node 0xe38: chosen

linux,initrd-end:
  00 00 00 00 00 00 00 00
linux,initrd-start:
  00 00 00 00 00 00 00 00

Since they're 64-bit on power9 and 32-bit on power8, we'll have to handle both cases again.

[breno.leitao_gmail.com]

I tested it on my POWER9 and it continues to boot. This change seems to make sense, and fixes 32-bits initrd addresses, but I am not sure how it broke Justin's P9, and if it is going to fix.

[jhibbits]

sys/powerpc/ofw/ofw_machdep.c

308 ↗

(On Diff #43339)

I wouldn't make this a KASSERT(). Problems with the device tree should error appropriately, not panic. It's possible the device tree may have a 12-byte entry here, or longer. It's a broken tree, but not a broken kernel.

KASSERT() is for an invariant violation, not an external dependency violation. Just return early here. An error is appropriate, though.

318 ↗

(On Diff #43339)

Same here as above.

[lffpires_ruabrasil.org]

sys/powerpc/ofw/ofw_machdep.c

308 ↗

(On Diff #43339)

What is an appropriate way to do error handling here? Should I printf() an error message and return (asz), allowing the boot process to continue?

[jhibbits]

sys/powerpc/ofw/ofw_machdep.c

308 ↗

(On Diff #43339)

Yes. Even if there is an initrd, if it's not valid it can't be used, so don't try, and warn the user.

[lffpires_ruabrasil.org]

• Improved linux,initrd-{start,end} length checking

[lffpires_ruabrasil.org]

• Removed extra printf param

[breno.leitao_gmail.com]

sys/powerpc/ofw/ofw_machdep.c

304 ↗

(On Diff #43374)

Why do you have 3 entries for cell? Shouldn't it be 2?

[jhibbits]

sys/powerpc/ofw/ofw_machdep.c

304 ↗

(On Diff #43374)

You only need 2 cells. OF_getencprop() returns the actual size, not the size read. It will only write up to the size passed in.

[lffpires_ruabrasil.org]

sys/powerpc/ofw/ofw_machdep.c

304 ↗

(On Diff #43374)

Why do you have 3 entries for cell? Shouldn't it be 2?

I used 3 to be able to detect errors in the device tree where initrd-{start,end} are longer than 8 bytes. If I used only 2 cells here, I wouldn't be able to tell that.
I could also do separate calls to OF_getproplen(), but I thought it would be simpler this way.

You only need 2 cells. OF_getencprop() returns the actual size, not the size read. It will only write up to the size passed in.

OF_getencprop() is returning the size read, only limited to the size passed in.

Looking at sys/dev/ofw/openfirm.c, it returns the value it got from OF_getprop(). And ofw_fdt_getprop() in sys/dev/ofw/ofw_fdt.c trims the length returned from the fdt to the buffer length that was passed in.

I double-checked this by trying to read another value (ibm,architecture-vec-5) whose length is 6, and verified that OF_getencprop() returns 6 when called with a buffer with 2 cells, but 4 when the buffer had just 1.

[jhibbits]

sys/powerpc/ofw/ofw_machdep.c

304 ↗

(On Diff #43374)

OF_getencprop() is returning the size read, only limited to the size passed in.

That's contrary to the man page, which states that it returns the actual size in bytes, not the size copied. Does it behave this way with a real OF machine (Apple PowerMac, or a different IBM environment)? If so, the man page needs updated, and your change is appropriate. If not, ofw_fdt_getprop() needs to be fixed in sys/dev/ofw/ofw_fdt.c

[lffpires_ruabrasil.org]

Changed to reflect the correct behavior of OF_getencprop().
A patch to fix ofw_fdt_getprop() was submitted as D15680.

[lffpires_ruabrasil.org]

• Improved size checking