Hi, On 1/30/20 2:50 PM, Andre Przywara wrote: > On Thu, 23 Jan 2020 13:47:48 +0000 > Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote: > > Hi, > >> To get the size of the expansion ROM, software writes 0xfffff800 to the >> expansion ROM BAR in the PCI configuration space. PCI emulation executes >> the optional configuration space write callback that a device can >> implement before emulating this write. >> >> VFIO doesn't have support for emulating expansion ROMs. > With "VFIO doesn't have support" you mean kvmtool's VFIO implementation or the kernel's VFIO driver? > Because to me it looks like it should work in the kernel, at least for the BAR sizing on the expansion ROM BAR: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/vfio/pci/vfio_pci_config.c#n477 > > Am I missing something here? kvmtool's implementation of VFIO doesn't have support for expansion roms, sorry for the confusion. VFIO definitely has support for expansion roms, I actually have some patches that I wrote in a previous iteration of this series that enable kvmtool to use it (I'll come back to them after this series gets merged). > > QEMU seems to have code to load the ROM from the device and present that to the guest, but I am not sure exactly why. Same here, I don't know why qemu does that, I would have imagined that the KVM_MEM_READONLY flag for KVM_SET_USER_MEMORY_REGION is a perfect use case for expansion ROM emulation. To sanitize accesses? To make it possible for the user to provide its own firmware file for the device? Either way, I think this is a discussion for another time. To summarize, I'll reword the commit to remove the confusion - kvmtool's implementation of VFIO doesn't have support for expansion ROM bars and emulation. Thanks, Alex > > Cheers, > Andre > >> However, the >> callback writes the guest value to the hardware BAR, and then it reads >> it back to the BAR to make sure the write has completed successfully. >> >> After this, we return to regular PCI emulation and because the BAR is >> no longer 0, we write back to the BAR the value that the guest used to >> get the size. As a result, the guest will think that the ROM size is >> 0x800 after the subsequent read and we end up unintentionally exposing >> to the guest a BAR which we don't emulate. >> >> Let's fix this by ignoring writes to the expansion ROM BAR. >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx> >> --- >> vfio/pci.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/vfio/pci.c b/vfio/pci.c >> index 1bdc20038411..1f38f90c3ae9 100644 >> --- a/vfio/pci.c >> +++ b/vfio/pci.c >> @@ -472,6 +472,9 @@ static void vfio_pci_cfg_write(struct kvm *kvm, struct pci_device_header *pci_hd >> struct vfio_device *vdev; >> void *base = pci_hdr; >> >> + if (offset == PCI_ROM_ADDRESS) >> + return; >> + >> pdev = container_of(pci_hdr, struct vfio_pci_device, hdr); >> vdev = container_of(pdev, struct vfio_device, pci); >> info = &vdev->regions[VFIO_PCI_CONFIG_REGION_INDEX].info;
