Hola Alex,
On 12/08/2026 21:06, Alex Williamson wrote:
On Tue, 11 Aug 2026 16:58:43 +0100 Matt Evans matt@ozlabs.org wrote:
Hi Alex,
[snip] Isn't the init-time 'event horizon' for writing the bitfield the vfio_register_group_dev() in vfio_pci_core_register_device(), after which synchronisation is needed?
The hisi_acc_vfio_pci driver's .probe calls vfio_pci_core_register_device() and _after that_ sets zap_bars_on_revoke, and that's now in the "needs synchronisation to write the bitfield" phase.
(Re-reading my comment in vfio_pci_core_register_device() I'd noted this, "Drivers can opt out after registration". Has to be done after by definition as the default's set in vfio_pci_core_register_device().)
The concern is just blatting neighbours in the bitfield, not the window of time before the flag's set. The flag's an opt-out of a safe but (for this driver) unnecessary zap, so having it unset for a short time is OK.
I still think this really should be a standalone bool, not a bit in the bitfield. It has to be set after registration and having to take a lock to do that has downsides.
You're right on the ordering, the device is live after vfio_pci_core_register_device(). However, I think that's evidence that vfio-pci-core is setting the default polarity, inferred from the mmap op, in the wrong place. It should happen in init, not register_device.
The same mmap op pointer is available in vfio_pci_core_init_dev(), which
Ahaa, they're passed into vfio_alloc_device()!
is used by all vfio-pci variant drivers in their init callback. The proposed vfio_pci_core_register_device() change just needs to be lifted into vfio_pci_core_init_dev(). hisi_acc is then modified to fix the flag after vfio_pci_core_init_dev(), something like below.
I think that's better than anticipating it being dynamic when we don't have a use case that requires it. Thanks,
Yes, that works nicely. Done. As ever, thanks for the suggestion.
Matt
PS: Series v6 has several improvements/review fixes ready, but I'm holding off posting it. It'd depend on resolving the awful deadlock I posted about on patch [4/9].
Alex
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c @@ -1564,6 +1564,7 @@ static int hisi_acc_vfio_pci_migrn_init_dev(struct vfio_device *core_vdev) struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(core_vdev); struct pci_dev *pdev = to_pci_dev(core_vdev->dev); struct hisi_qm *pf_qm = hisi_acc_get_pf_qm(pdev);
int ret;hisi_acc_vdev->vf_id = pci_iov_vf_id(pdev) + 1; hisi_acc_vdev->pf_qm = pf_qm; @@ -1575,7 +1576,9 @@ static int hisi_acc_vfio_pci_migrn_init_dev(struct vfio_device *core_vdev) core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_PRE_COPY; core_vdev->mig_ops = &hisi_acc_vfio_pci_migrn_state_ops;
return vfio_pci_core_init_dev(core_vdev);
ret = vfio_pci_core_init_dev(core_vdev);hisi_acc_vdev->core_device.zap_bars_on_revoke = false;return ret;static const struct vfio_device_ops hisi_acc_vfio_pci_migrn_ops = {