Just committed this. I agree about 13.0 -- I was planning to merge to stable/13 after 13.0 has branched and smoke-test this in HEAD for a bit in the meantime.

Now tested and working, with UFS and ZFS, including multi-disk ZFS setups. In principle ready for commit.

Now does ZFS (including redundant setups) as well. Still not fully tested.

@imp any thoughts on the gmirror solution for ZFS? I think it solves 95% of the update issues and shouldn't be too bad to implement.

Just to be pedantic: We actually *do* have /boot as separate on some systems (e.g. recent powerpc64), but those systems are mutually exclusive with EFI systems. I have a mild preference /boot/efi, but it doesn't really matter much so long as we pick something -- the status quo is it not being mounted *at all*, so reliability of mounting or whatever is obviously a second-order issue.

In D15243#545983, @jhibbits wrote:

So, HTM is removed in PowerISA 3.1 and it's broken on POWER9. Is it even worth doing at this point? I'm inclined to abandon this.

Do we know all supported 32-bit AIM CPUs do the same things with DSISR? Otherwise, it looks good to me.

I have pretty mixed feelings about this approach. It only works for ofwbus children, not simplebus children, and canonicalizes a bunch of behaviors that I don't believe are standards (I'm in an airport departure lounge and don't have the spec handy). I'm also not really sure how it interacts with multipass etc. Could you elaborate a little more on the mechanism?

Fix fat-fginering; this is the right diff. It also sets Python to use Python 2, since a handful of scripts cared.

Updates from review.

OK, sounds good, thanks for testing!

Just make the page table size really small. There's a spill counter in the statistics; when it gets small enough, you should see it moving. If spill-handling doesn't work, and you have spills, everything breaks fast.

Does this break PTE spills? This code path historically was involved in re-adding PVOs to the hashed page table on faults if they got spilled.

That's a fair point. I've used them to assess hash-table spill rates when performance is not great, but not for a long time. We could also just delete them.

Yeah, counter(9) seems like the actual solution here.

Looks good to me; do you know how much this actually helps?

Should we panic if freeze_timebase somehow doesn't get set? Or, alternately, fall back on the old lame thing?

Longer-term, it might be better to avoid a proliferation of special cases like this to set LOADER_MSDOS_SUPPORT for the U-boot loader and make loader flexible enough to find the kernel etc. in the root rather than /boot. But this is fine for now.

Aside from one inline note, this looks OK for non-SPE.

Can we make the check the presence of a PHB4 node in the device tree rather than an #ifdef? It might also be best to do this in platform_powernv.c, but that's a bit larger of a project.

In D16794#361563, @jhb wrote:

On closer examination, the loader doesn't actually modify the kernel or modules in place at all. Instead, this routine is used to apply relocations on copies of data. So for example, the code to parse module metadata copies individual records out of the kernel into a local variable on the stack, then asks the relocation to be applied to that local copy. This is a really good reason to honor the the 'data' and 'len' bounds as otherwise it allows writing to random crap on the stack. Also, we do explicitly ignore all relocations for the kernel itself, so it must be some relocation in tcp_rack.ko that wasn't handled before that is being handled now perhaps. I would have expected that to trigger a warning if so in the existing code though for an "unhandled relocation type".

In D16794#361523, @jhb wrote:

I believe since ppc is using Elf_Rela and that all of the relocations calculated absolute values to store at *where or *hwhere (vs using += or the like), the loader relocations just get overwritten by the kernel. I think when using Elf_Rel you might use the original value of *where as the addend in which case multiple passes of relocations is indeed destructive, but I don't think it is for Elf_Rela.

I just looked at the actual patch. The kernel sometimes (almost always on ppc64, less often on ppc32) relocates itself again post-loader to an address that loader does not know, so I think this does not solve the actual problem, or at least solves it in only a subset of cases. Processing these relocations in the kernel would solve the issue in all cases.

In D16794#357383, @jhb wrote:

I added Justin and Nathan (who I meant to include from the start).

I believe PPC kernels are relocatable. I think they use ET_DYN instead of ET_EXEC (and there is a pending PR to fix that issue in libkvm as libkvm doesn't work on it currently as it only expects ET_EXEC for a kernel). Justin and/or Nathan can confirm.

Looks OK to me. One question: Do we want the various instances of fdt_check_header() in our code to be spelled FDT_RO_PROBE() now?

Since I'm not familiar with the clock code, where is EXT_RESOURCES defined? Can we just use that code unconditionally?

Assuming you have tested this and it works, looks great.

Looks good to me.

Approved if you fix the #if 0 bit.

Don't you want strcmp() == 0?

It's mildly stupid, since the universe of pci* isn't big, but I think I would prefer a check for "pci" || "pciex" to just checking the first three characters.

In D15229#333064, @imp wrote:

In D15229#333057, @nwhitehorn wrote:

In D15229#333038, @imp wrote:

The final option would be to do a variation on the prior option, but institutionalize it in DEVICE_ATTACH_LATE. It would be called just after interrupts were enabled for deferred work like this. It would cover the vast majority of driver independencies by giving a 'last' point. At that point, you return an error if you still weren't ready, and we'd loop while the number of devices returning an error was declining. It could largely replace config_intrhook.

I think this would work reasonably well for the general case here; the other suggestions would solve the case of smu(4), but there are other, simpler, ways to do that.

That said, I'm not sure what the advantage of a DEVICE_ATTACH_LATE() is over the mechanism originally proposed here or something like it except that it, as a side effect, delays until after interrupts -- which might not even be wanted, for example if the device in question is an interrupt controller.

If the device is an interrupt controller, you need to put it on a different pass. There's no way around that. But this isn't an interrupt controller, and interrupt controllers are special beasts that already have special handling in many places. Having a delayed attach for them is likely completely unacceptable so none of the proposals is appropriate for them.

In D15229#333038, @imp wrote:

The final option would be to do a variation on the prior option, but institutionalize it in DEVICE_ATTACH_LATE. It would be called just after interrupts were enabled for deferred work like this. It would cover the vast majority of driver independencies by giving a 'last' point. At that point, you return an error if you still weren't ready, and we'd loop while the number of devices returning an error was declining. It could largely replace config_intrhook.

It's been another couple weeks. Any objections or approvals?

This looks good to me, though I haven't tested on P8.

On POWER9, I think the lock is unnecessary (the ISA spec doesn't mention it).

Any other thoughts? I'd like to get this in soon since it is blocking other things.

Fix style and spelling issues in man pages.

Probe for pending devices at the end of each pass and after modules loaded. Add documentation.

See the comment on line 467 about powernv_smp_ap_init(). Otherwise, looks good.

Approved if you also install it to EXC_HEA.

In D15482#326710, @breno.leitao_gmail.com wrote:

1. Is there a document describing this that we can reference?

I was not able to find any documentation. Looking at the web, I found a patch[1] that documents it quickly in Linux, but it seems it was never accepted. I will ask internally.

This looks good, even if the deice tree situation is a bit lamentable. Two questions:

1. Is there a document describing this that we can reference?
2. If this is just part of the PowerNV platform, maybe the code belongs in platform_powernv.c instead?

Thanks -- I'll tweak the implementation to handle KLDs properly and write a man page, then update this diff.