[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vfio/pci: fix out-of-sync BAR information on re
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH] vfio/pci: fix out-of-sync BAR information on reset |
Date: |
Fri, 21 Oct 2016 12:03:21 -0600 |
On Fri, 21 Oct 2016 13:00:33 -0400
Ido Yariv <address@hidden> wrote:
> When a PCI device is reset, pci_do_device_reset resets all BAR addresses
> in the relevant PCIDevice's config buffer.
>
> The VFIO configuration space stays untouched, so the guest OS may choose
> to skip restoring the BAR addresses as they would seem intact. The PCI
> device may be left non-operational.
> One example of such a scenario is when the guest exits S3.
>
> Fix this by resetting the BAR addresses in the VFIO configuration space
> as well.
>
> Signed-off-by: Ido Yariv <address@hidden>
> ---
> hw/vfio/pci.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 65d30fd..9e1dee0 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1922,11 +1922,41 @@ static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
> static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
> {
> Error *err = NULL;
> + int nr;
>
> vfio_intx_enable(vdev, &err);
> if (err) {
> error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
> }
> +
> + for (nr = 0; nr < PCI_NUM_REGIONS - 1; ++nr) {
> + VFIOBAR *bar = &vdev->bars[nr];
> + off_t addr = vdev->config_offset + PCI_BASE_ADDRESS_0 + (4 * nr);
> + uint32_t org = 0;
> + uint64_t val = 0;
> + uint32_t len;
> +
> + if (!bar->region.size) {
> + continue;
> + }
> +
> + if (pread(vdev->vbasedev.fd, &org, sizeof(org), addr) < 0) {
> + error_report("%s(%s) read bar %d failed: %m", __func__,
> + vdev->vbasedev.name, nr);
> + continue;
> + }
> +
> + val = le32_to_cpu(org);
> + val &= (bar->ioport ? ~PCI_BASE_ADDRESS_IO_MASK :
> + ~PCI_BASE_ADDRESS_MEM_MASK);
> + val = cpu_to_le64(val);
> +
> + len = bar->mem64 ? sizeof(uint64_t) : sizeof(uint32_t);
> + if (pwrite(vdev->vbasedev.fd, &val, len, addr) != len) {
> + error_report("%s(%s) reset bar %d failed: %m", __func__,
> + vdev->vbasedev.name, nr);
> + }
> + }
Why do we care to be so precise, can't we just blindly do 4-byte writes
of zero to each of the 6 BAR registers? The bits you're trying to
preserve should be read-only anyway. Nothing is saved by trying to do
an 8-byte write for 64-bit BARs. Thanks,
Alex
> }
>
> static bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)