It is mostly moot and I don't really plan to change it currently. At some point I might instead make it just try to create a single test session for each algorithm up front and skip if it gets EOPNOTSUPP. It also should not have a hardcoded list of device names. It should really take an optional command line argument for what devices you want to test or use some method to enumerate the available devices (might have to add something to OCF to support the latter).

I think most of this shouldn't be removed. The PR is quite a bit wrong, too since it is worried about the Tiers chapter in the CG saying that pc98 was removed in 12. However, 11.x is still a supported release and we should still be documenting pc98. I think keeping the small, stub pages for removed architectures in their current state listing when they were supported is also helpful on balance. They are clearly marked as unsupported on the platforms page. To Ed's point, I wonder how current any of the sentex machine list is.

Is this a case where 'not very random' PRNG output is still better than "sequential port assignment"? That is, if we had a variant of arc4random() that would fail to "weak output" rather than blocking?

I'm not sure a per-thread flag is quite right. For example, a crypto driver might check the flag and reject attempts to create sessions in its crypto_newsession callback, but later uses of arc4random to generate IV's will occur in different threads (e.g. from a TCP timer that transmits a packet via IPsec).

I suspect almost all of the other pci-e devices in bhyve are also actually endpoints and not ports. Note that right now there are no PCI bridges in boyce's topology and everything is on PCI bus zero.

BTW, sadly all the CCM descrypt vectors use nonce lens of either 7 or 13, never 12, so none of the decrypt tests can be run with OCF currently.

I will need to get some ports person to sign off on this eventually, but that's not a big deal. BTW, I have CCM tests partially working and hope to upload that diff to phab soon.

In this case though it isn't the host arch (ARCH) but a target architecture for the binary blob. That does seem like something flavors would handle, but slave ports also work. I thought part of the point of flavors was to make it easier to not have slave ports by encapsulating all the slave ports in a single port.

Or instead of slave ports, flavors, where each target arch was a separate flavor. In that case you would keep the port name as it was and just append ARCH to the page name as it's flavor name.

Just a couple of thoughts from what I've seen flying by:

In D19833#426028, @kib wrote:

In D19833#425992, @jhb wrote:

Hmm, so scottl@ added this version of MMIO access before we looked at the MCFG table, and I'm not sure that the BIOS for these chipsets don't actually already provide the MCFG table.

At least on the machine of the person who reported the problem, there is MCFG table. Chipset MCH is E7500.

However, on systems where we rely on the MCFG table, we won't have parsed the table to setup the mapping until later anyway, so I think the change is ok. Alternatively, we could move the fallback for these chipsets into a separate function that we explicitly call at around the same time we parse MCFG and just do it later in boot once it's ok to map the table, but that's a lot more work (though maybe not if we just call it from the pmap code at the time that pmap_initialized is set?) Reading the commit message was a bit confusing to me at first, so I think it might be useful to say that this makes those chipsets act more like other chipsets where we use MCFG as with MCFG we don't enable mmio access until part-way through the boot.

In fact, in the later response I proposed to remove the fallback at all. We require ACPI on amd64, and it seems that at least one BIOS is not so much broken to not provide MCFG.

Hmm, so scottl@ added this version of MMIO access before we looked at the MCFG table, and I'm not sure that the BIOS for these chipsets don't actually already provide the MCFG table. However, on systems where we rely on the MCFG table, we won't have parsed the table to setup the mapping until later anyway, so I think the change is ok. Alternatively, we could move the fallback for these chipsets into a separate function that we explicitly call at around the same time we parse MCFG and just do it later in boot once it's ok to map the table, but that's a lot more work (though maybe not if we just call it from the pmap code at the time that pmap_initialized is set?) Reading the commit message was a bit confusing to me at first, so I think it might be useful to say that this makes those chipsets act more like other chipsets where we use MCFG as with MCFG we don't enable mmio access until part-way through the boot.

BTW, this booted fine on my X1 today (I just pulled the git branch).

In D19831#425341, @kib wrote:

If you return whole XSTATE from ptrace(PT_GETREGSET, NT_X86_XSTATE), what is the point of the new API ? It would make much more sense to me if there was an API to get components like YMM/ZMM/PKRU instead of returning whole blob and then requiring the caller to parse it combining XCR0 bitmask with either prior knowledge of the component sizes, or querying the offsets on live machine.

Hmm, do you have plans for a PT_SETREGSET as well? To replace the ARM VFP, and PPC VMX, etc. sets we need set, not just get. Also, as I said previously on IRC I would like to have a PT_GETREGSET_INFO where the iovec in addr points to a structure. The first field would always be a uint64_t size member, but the rest of the fields could vary by note type. For XSTATE the structure would also hold the xcr0 mask to replace PT_GETXSTATE_INFO, so something like:

In D19602#424888, @swills wrote:

You could get around the 'g' prefix issue with PORTEPOCH but if you don't want to do that I guess it's fine. LGTM.

Overall looks good, just some minor nits/suggestions.

The patches check __FreeBSD_version already, so it should compile on everything except for 13-head in between clang 8 and r345196. It doesn't seem worth it to me to add an explicit BROKEN just for a small range of OSVERSION values on HEAD.

I would probably do anish's change as a separate commit, maybe before the idle bit commit.

In general this looks great to me. The only other caveat that perhaps might just need to be documented somewhere, is that if you have detached or suspended a child device explicitly and then do a reset of the parent, the child device will end up attached. That isn't a new problem though (a system-wide suspend doesn't preserve a device being suspended by devctl suspend). Eventually we might need to track the "desired administrative state" of a device_t kind of like IFF_UP vs IFF_RUNNING in ifnet flags as the proper fix for both cases.

It's really @np who should sign off on this. I can blacklist the driver on my dev boxes since since there's now an rc.conf variable to exempt drivers from devmatch which would be sufficient for my needs.

FYI, as an update I found that this did break a cross-build that didn't use an explicit CROSS_TOOLCHAIN, e.g. `make buildworld TARGET_ARCH=i386```. I haven't committed this yet or the port changes to add the new files. I'll have to think a bit more about what the right solution is. I'd really like this to be seamless in terms of the base toolchain just being used without having to have an explicit CROSS_TOOLCHAIN.

@brnrd I've rebased this patchset to be relative to OpenSSL 1.1.1 rather than 1.0.2. It is now much closer to the 1.1.1 port. It installs into a dedicated directory so it does not conflict with other openssl ports. It matches the 12.x base shared library version so that it could be used with LD_LIBRARY_PATH, but in theory a user could also use libmap to use it with ports built against the openssl111 port. It generally borrows all of the patches other than the extra chelsio patch from the openssl111 port. I didn't make this a slave port b/c I didn't want it to be a drag on updating the main openssl port in case the patches didn't apply out-of-the-box to newer versions, etc.

I've now tested this via netperf using RATELIMIT using the various lagg protocols under an INVARIANTS kernel.

In D19482#422744, @bdrewery wrote:

Technically seems fine.
Feels like a POLA issue though. If I happened to have some package installed then buildworld behaved differently.
Maybe CROSS_TOOLCHAIN=yes or CROSS_TOOLCHAIN=base?
I don't really care either way; default on like you have is fine too as I don't fully understand the use case.

I had labored under the incorrect assumption that libcxxrt was somewhat abandoned and that all other libc++ consumers used libc++abi, however, it seems that it's only Apple and OpenBSD that use libc++abi. NetBSD still uses libcxxrt. Also, when I compared the repositories both libcxxrt and libcxxabi had basically no real activity in the last two years. libcxxabi had more commits, but they were all due to relicensing and moving to monorepo.

Tiny style nits can be fixed when this is committed.

In D19580#422248, @chuck wrote:

In D19580#422125, @jhb wrote:

Hmm, due to what this bit actually does, I'd probably be inclined to not set it if we don't need it. When the role based error reporting is set, it means we also support at least one other register to configure the mask of fatal vs non-fatal errors IIRC which I'm pretty sure we don't do. I had assumed Windows was failing to boot without this bit set. If Windows is actually fine then I think I'd prefer we drop it until we find a need for it. A PCIe 1.0 device can have this bit clear and still be compliant. The other thing about being a special endpoint when directly hung off the RC would be required even for 1.0 compliance.

I'm happy to revert this change but wanted to double check the "PCIe 1.0" comment. Even though the code advertises Gen1 speeds, the PCIe Capability Version field is set to 0x2 which I don't think a 1.0 device would do. Should this be changed to 0x1?

In D19580#421672, @chuck wrote:

One of @jhb requests was a comment on why the RBER change was needed. Here is the thought process:

qemu in their PCIe emulation (hw/pci/pcie.c) has the comment

/*
 * Windows guests will report Code 10, device cannot start, if
 * a regular Endpoint type is exposed on a root complex.  These
 * should instead be Root Complex Integrated Endpoints.
 */

If true, and Windows is actually validating PCIe capability structures to this degree, then it seemed prudent to set other bits in this area as required by the PCIe spec.

As to where to go from here, I'm conflicted. On the one hand, the change doesn't seem to fix anything from my testing. On the other hand, given this comment in qemu and the recent experience of Windows validating the size of BAR0 in NVMe devices, I'm tempted to go back through the PCIe spec and make sure that bhyve's PCIe capability structure is compliant.

I'm curious which hypervisor you have tested this under and did you consider using one of the virtualized clocks if one was available? I know that there are some early patches to add kvmclock support to bhyve for example.