In D28260#632737, @cem wrote:

In D28260#632588, @markj wrote:

Hmm, the kldxref change doesn't fully fix the problem as I understand it (not that my commit does). The real problem is that the pnp info structure doesn't contain the module name, so devmatch(8) doesn't know which PNP info goes with which module. The kldxref change at least ensures that devmatch won't try to process a PNP info definition without having seen an MDT_MODULE record.

MDT_PNP_INFO always corresponds to the most recently traversed MDT_MODULE. That's the ordering property guaranteed by the kldxref change. I don't know what crash or bug you hit in devmatch exactly. Do you have a stack or short summary? Does it reproduce on CURRENT or with kldxref otherwise fixed?

The reason we made the kldxref change was to generally address a specific problem this revision also seems to try to address — the ordering between MDT_MODULE and MDT_PNP_INFO. So I am surprised that we still need to shuffle things around in the code. If we're missing something in kldxref or devmatch, let's fix it there. (As the kldxref commit mentions, there is no guarantee the compiler preserves the source code ordering of static data values.)

We only need the copies on FreeBSD to avoid the issue you cited. On other systems, symlinks are fine.

In D28260#632588, @markj wrote:

In D28260#632587, @markj wrote:

In D28260#632585, @cem wrote:

Wait, kldxref should be ordering these correctly since rS348309. Why isn't it?

Probably because I hit this issue on stable/12 and didn't notice that kldxref was fixed.

Hmm, the kldxref change doesn't fully fix the problem as I understand it (not that my commit does). The real problem is that the pnp info structure doesn't contain the module name, so devmatch(8) doesn't know which PNP info goes with which module. The kldxref change at least ensures that devmatch won't try to process a PNP info definition without having seen an MDT_MODULE record.

In D28260#632046, @imp wrote:

To expand a little: devmatch shouldn't dump core... I don't see how, but I've not seen the core traceback yet...

To expand a little: devmatch shouldn't dump core... I don't see how, but I've not seen the core traceback yet...

I agree that something better is needed here...
But unless we change the PNP macros to require driver name and bus name, there's no way to easily detect this at compile time.
At run time, we could emit a warning for this condition... At the time I wrote devmatch, no driver violated this rule, so I never saw the core dump....
This is fine enough for 13.0, though.

Looks good to my eye, but I'd feel better with a nod from one of the main bhyve developers

Video has had ugly escape sequences in it for too long I think..

In D28069#627164, @emaste wrote:

perhaps we should include git commit --author in the template, something like "should be set appropriately, using git commit --author... if someone"?

In that case, I'm good!

Does this survive universe? mips has no virtualization support at all, last I checked... A quick peek at virtio suggests that it's entirely platform neutral...

I believe these are good places to add the accounting. They do assume that ANY read or ANY write, even one that also have an error, counts as a message rcv or snd, but I think that's OK.

fix type

tweak description

remove cut and pasted code that was bogus

newbusly it looks good.

These look good to my eye.

In D27782#621556, @kevlo wrote:

In D27782#621399, @imp wrote:

Also, boot1.efi is obsolete. We retain it only for special needs users: those with the bogus tiny ESP partitions and some really weird edge cases that loader.efi doesn't work for.

I have Ubuntu 20.10 and FreeBSD -current installed on my thinkpad laptop.
Linux's Grub2 is unable to boot FreeBSD directly from UEFI, I have to copy
the FreeBSD UEFI loader (/boot/boot.efi) into EFI partition.

Also, boot1.efi is obsolete. We retain it only for special needs users: those with the bogus tiny ESP partitions and some really weird edge cases that loader.efi doesn't work for.

This looks good. If it is checked out of subversion on -stable, then we'll get the right uname, so it can likely be MFC'd... We're not writing new svn revs to git, but there may be something I'm missing.

In D25358#618886, @gbe wrote:

In D25358#618885, @imp wrote:

In D25358#618884, @gbe wrote:

@emaste any change that this can be committed before the switch to git?

Nope. It will have to wait. Your reward for eating, though, is that I'd be happy to do the import to test the process for real once things open again.

OK, even if I don't understand the phrase "your reward for eating", since I am not a native speaker. ;)

In D25358#618884, @gbe wrote:

@emaste any change that this can be committed before the switch to git?

This looks like it would fix things, but I wonder why this is even here... If we keep it, this change prevents a crash... Though I can't for the life of me think about the purpose this serves... It went in with a bunch of efi env var support.

looks good, with spaces I'm happy.

Can you get this accepted upstream?

In D27517#615211, @mav wrote:

We also have several 128-bit capacity values represented as arrays of bytes.

everything but the tools/build/Makefile changes are good. I'm not at all sure that I understand those... This file has become a rather twisty maze, so I've not puzzled all the way through understanding...

Separate commit is fine