Re: [PATCH] pci: Add generic PCI device option to disable 64bit MMIO BARs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 01, 2012 at 03:57:02PM -0700, Alex Williamson wrote:
> As we start to enable 64bit I/O devices, there's a good chance
> we'll find bugs and compatibility issues.  This allows a user
> to toggle off (default on) 64bit PCI MMIO BARs, downgrading
> them to 32bit BARs.
> 
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---
> 
> Should this be an "x-mem64" option implying that it's really
> only for debugging and may go away or could this have some
> longevity?

I'd rename it x-force_32bit, for clarify.
Can this go into assigned devices?
Then the error handling below won't be a problem.

>  hw/pci.c |   18 ++++++++++++++++++
>  hw/pci.h |    4 ++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 57ec104..9afddb0 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -63,6 +63,8 @@ struct BusInfo pci_bus_info = {
>                          QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
>          DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
>                          QEMU_PCI_CAP_SERR_BITNR, true),
> +        DEFINE_PROP_BIT("mem64", PCIDevice, cap_present,
> +                        QEMU_PCI_CAP_MEM64_BITNR, true),
>          DEFINE_PROP_END_OF_LIST()
>      }
>  };
> @@ -957,6 +959,22 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>          exit(1);
>      }
>  
> +    if (!(type & PCI_BASE_ADDRESS_SPACE_IO)) {
> +        if (!(pci_dev->cap_present & QEMU_PCI_CAP_MEM64)) {
> +            type &= ~PCI_BASE_ADDRESS_MEM_TYPE_64;
> +        }
> +
> +        /* 32bit BARs are limited to 2GB */
> +        if (size >= 0x80000000U && !(type & PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> +            fprintf(stderr, "Device %04x:%02x:%02x.%x BAR %d is %ld "
> +                    "GB, 64bit memory type required\n",
> +                    pci_find_domain(pci_dev->bus), pci_bus_num(pci_dev->bus),
> +                    PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn),
> +                    region_num, (long)(size >> 30));
> +            exit(1);

That's an unfriendly way to handle errors, especially
for hotplug.  If you move the option to device itself,
then it can just fail init.

errors should also go to the monitor nowdays, so no fprintf.

> +        }
> +    }
> +
>      r = &pci_dev->io_regions[region_num];
>      r->addr = PCI_BAR_UNMAPPED;
>      r->size = size;
> diff --git a/hw/pci.h b/hw/pci.h
> index 4220151..17fd996 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -126,6 +126,10 @@ enum {
>      /* command register SERR bit enabled */
>  #define QEMU_PCI_CAP_SERR_BITNR 4
>      QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
> +
> +    /* expose 64bit MMIO BARs when available */
> +#define QEMU_PCI_CAP_MEM64_BITNR 5
> +    QEMU_PCI_CAP_MEM64 = (1 << QEMU_PCI_CAP_MEM64_BITNR),
>  };
>  
>  typedef int (*msix_mask_notifier_func)(PCIDevice *, unsigned vector,
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux