Thanks. I suspect we are fine just merging this to stable branches and leaving the libc change alone. Ideally we would perhaps have done this the other way around (updated all the callers first then made the API change a bit later to give a graceful compat but), but it's not worth the hassle now. I also didn't notice this when the commit went by either, so the behavior change was certainly subtle.

This is still missing usr.bin/ktrdump/ktrdump.c, tools/tools/ifpifa/ifpifa.c, and tools/tools/umastat/umastat.c from my initial list. I haven't checked for any others (and you've found a few I had missed).

I would maybe split out the caching of the domain into a separate commit.

Kind of weird that they would match an _ADR value, but this is correct.

I'm somewhat surprised you are doing this when enumerating the PCI bus vs doing it in pcib_attach() FWIW. If the bus numbers are available for growing, that should be just as doable in pcib_attach() time before you attach drivers to child devices, and then you don't need the manual PCI config read/write code as you will have the normal device_t objects around to use with pci_read_config/pci_write_config.

I'm also working on merging changes to iw_cxgbe that should update it btw. For mthca we can find an upstream Linux commit to pull across. I think we should probably just move consumers to the newer KPI.

It's not quite true that binutils doesn't "understand the instructions", it's more that different toolchains use different ways to enable access to optional instructions. According to the bug I posted earlier, older versions of LLVM matched binutils in that they required -march= for this particular case before the .arch_extension was added. I'm not a huge fan of hand-assembling vs trusting the assembler to do that job, but it does work, and the pattern does seem to match up quite well with how opcodes are described in the ARM ARM at least.

Please do correct the commit log to be something more like "random: Don't collect entropy when scheduling software interrupt handlers" and then maybe a summary of Conrad's comment in the body saying that it doesn't provide very much entropy in practice.

Using .arch here is not entirely future proof, but does work. The Linux kernel doesn't use .arch_extension. It figures out the highest configured -march= value, and for binutils it passes that to GNU as via -Wa,-march=<mumble> but for clang it uses .arch <mumble> via a special wrapper macro. That approach though spans the entire kernel build for GNU as, whereas this approach only affects pmap.c so will hopefully be easier to update if needed in the future.

Thanks for fixing these.

The actual commit that added this was

This does seem a bit of a large hammer that is probably hard to use safely, and not something you'd want to enable by default as I'd expect it would break various systems. It's probably ok as a hack for specific use cases, though it might be nicer to make the knob a bit more fine-grained. For example, to mav@'s point, a more useful knob for HotPlug might be a HotPlug specific knob that we use instead of '1' if the bridge is HotPlug-capable. For the SRIOV case, it would get a bit messy as you'd have to manually go scan the bus to see if it has any children that support SRIOV, but in order to do that, you first need at least one valid bus number. If you wanted an SRIOV-specific knob, I would perhaps implement this differently by still passing 1 to pcib_setup_secbus() here, then later in attach doing an explicit scan of the child bus to check for any direct children that support SRIOV and growing the bus range (if needed) to the value of a new SRIOV-specific knob.

I presume the QEMU 9pfs support doesn't depend on this?

In D54641#1344584, @bnovkov wrote:

In D54641#1344263, @jhb wrote:

When a new PCI device shows up in hotplug both for native PCI-e hotplug and other systems like the ACPI approach you are using, the new device just shows up with all decoding disabled and the BARs set to their reset values (address of 0). It is the guest OS's job to allocate address space for the BARs and manage the address space.

Thank you, this clears up a lot and I think I'll be able to simplify the whole patch series greatly since we can completely ditch vmem.
Before I start purging the vmem-related parts from this patch stack, do you think that there's a future use for this application of vmem that would justify keeping it?

I have an (untested) little patch series to cleanup BAR registration a bit, and I Think all you will have to do for hotplug is add an early return from pci_emul_alloc_bar() to avoid trying to allocate initial addresses similar to the boot ROM case for hot plugging.

A general comment from a discussion with @bnovkov today: The complicated state machine for 64-bit BARs is fairly fragile and also not really consistent with how PCI BARs work in hardware. The right protocol for updating BARs for an OS/firmware is to first disable decoding of the relevant resource type (MEM or IO) in the Command Register, then size the BAR by writing all 1's and reading back to see which bits are hardwired at zero, then writing to the BAR to assign the real address (either the original address or a new address determined by the OS/firmware), then re-enable decoding in the command register.

Switch to invalid

In D58539#1343744, @olce wrote:

While here, you could apply the same treatment to the preceding if, except the call to parse_modmetadata() needs to be preserved. The fact that the return process is the same irrespective of whether that latter call returns 0 or not looks suspect. After a very quick glance, this function does not seem to have proper error reporting anyway.

Thanks for fixing this so quickly!

In D58542#1343670, @jrtc27 wrote:

I just used their email address for Reported by because given (a) the lack of history with the project (b) no trace of them existing anywhere else online (c) the low-quality suspicious nature of the patches (d) the name being one of *the* quintessential placeholders, I am suspicious as to whether it's their real name.

In D58541#1343668, @jrtc27 wrote:

There's also lib/libc/gen/tls.c for PT_TLS

Good catch.

Have you had a chance to test this under GDB and/or LLDB? I can't imagine they'd be setting any reserved bits so this should be fine.

See D58542 (and related commits in that stack)

My testing was to boot a kernel with a ccr_load="YES" in /boot/loader.conf (ccr.ko depends on if_cxgbe.ko) and verify it still parsed the metadata correctly and auto-loaded the dependency in the loader. That and it also didn't choke on a valid kernel but still booted ok. I don't have any degenerate kernel modules lying around to test further (e.g. a kernel module with only a sysinit).

Better align with kernel linker

This still seems a bit verbose to me, but it's less verbose than the old one, so definitely in the right direction.

We don't really MFC things in doc/ (there are no branches, only release tags).