On Thu, Aug 05, 2021 at 11:08:21AM -0600, Alex Williamson wrote: > +void vfio_pci_test_and_up_write_memory_lock(struct vfio_pci_device *vdev) > +{ > + if (vdev->zapped_bars && __vfio_pci_memory_enabled(vdev)) { > + WARN_ON(vfio_device_io_remap_mapping_range(&vdev->vdev, > + VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX), > + VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX) - > + VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX))); > + vdev->zapped_bars = false; Doing actual work inside a WARN_ON is pretty nasty. Also the non-ONCE version here will lead to massive log splatter if it actually hits. I'd prefer something like: loff_t start = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX); loff_t end = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX); if (vdev->zapped_bars && __vfio_pci_memory_enabled(vdev)) { if (!vfio_device_io_remap_mapping_range(&vdev->vdev, start, end - start)) vdev->zapped_bars = false; WARN_ON_ONCE(vdev->zapped_bars); > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > - vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); What is the story with this appearing earlier and disappearing here again? > +extern void vfio_pci_test_and_up_write_memory_lock(struct vfio_pci_device > + *vdev); No need for the extern.