Re: git: 36b855f18925 - main - amd64/vmm: Lock global PCI passthrough structures

On Fri, May 22, 2026 at 10:13:38PM +0000, Bjoern A. Zeeb wrote:
> On Fri, 6 Feb 2026, Mark Johnston wrote:
> 
> > The branch main has been updated by markj:
> > 
> > URL: https://cgit.FreeBSD.org/src/commit/?id=36b855f1892575cbfe1cd5455b989bfc8ae07502
> > 
> > commit 36b855f1892575cbfe1cd5455b989bfc8ae07502
> > Author:     Mark Johnston <markj@FreeBSD.org>
> > AuthorDate: 2026-02-06 15:29:22 +0000
> > Commit:     Mark Johnston <markj@FreeBSD.org>
> > CommitDate: 2026-02-06 15:38:51 +0000
> > 
> >    amd64/vmm: Lock global PCI passthrough structures
> > 
> >    There is a global list of ppt-claimed devices, accessed via several
> >    vmm ioctls.  The ioctls are locked by per-VM locks, but this isn't
> >    sufficient to prevent multiple VMs from trying to bind a given device.
> > 
> >    Add a sleepable lock and use that to synchronize all access to ppt
> >    devices.
> > 
> >    Reviewed by:    corvink, jhb
> >    MFC after:      2 weeks
> >    Differential Revision:  https://reviews.freebsd.org/D55065
> > ---
> > sys/amd64/vmm/io/ppt.c | 162 +++++++++++++++++++++++++++++++++----------------
> > 1 file changed, 111 insertions(+), 51 deletions(-)
> > 
> > diff --git a/sys/amd64/vmm/io/ppt.c b/sys/amd64/vmm/io/ppt.c
> > index 6feac5dcbbed..b522e18e3b24 100644
> > --- a/sys/amd64/vmm/io/ppt.c
> > +++ b/sys/amd64/vmm/io/ppt.c
> ...
> > int
> > @@ -529,10 +558,12 @@ ppt_unmap_mmio(struct vm *vm, int bus, int slot, int func,
> > 	struct pptseg *seg;
> > 	struct pptdev *ppt;
> > 
> > +	PPT_LOCK();
> > 	error = ppt_find(vm, bus, slot, func, &ppt);
> > 	if (error)
> > -		return (error);
> > +		goto out;
> > 
> > +	error = ENOENT;
> > 	for (i = 0; i < MAX_MMIOSEGS; i++) {
> > 		seg = &ppt->mmio[i];
> > 		if (seg->gpa == gpa && seg->len == len) {
> > @@ -541,9 +572,11 @@ ppt_unmap_mmio(struct vm *vm, int bus, int slot, int func,
> > 				seg->gpa = 0;
> > 				seg->len = 0;
> > 			}
> > -			return (error);
> > +			break;
> > 		}
> > 	}
> > +out:
> > +	PPT_UNLOCK();
> > 	return (ENOENT);
> > }
> 
> There's a bug here.  That should be return (error).  That's been breaking
> pci passthru since February.  I cannot imagine how no one noticed this in
> 3 months?

Fixed in commit b13335331092, thanks.

If I'm reading the bhyve code correctly, this is "cosmetic": the
libvmmapi wrapper for this operation is vm_unmap_pptdev_mmio(), called
in bhyve's passthru_mmio_map().  When the ioctl fails,
passthru_mmio_map() prints that warning and returns an error to the
caller... and all callers ignore it.  I think that was true even before
my refactoring in commit 86150ed98b790.

How exactly is passthru broken for you?