On Fri, Feb 17, 2012 at 04:10:33PM +0100, Gerd Hoffmann wrote:
> This patch adds an 64bit pci bar for vram. It is turned off by default.
> It can be enabled by setting the size of the 64bit bar to be larger than
> the 32bit bar. Both 32bit and 64bit bar refer to the same memory. Only
> the first part of the memory is available via 32bit bar.
>
> The intention is to allow large vram sizes for 64bit guests, by allowing
> the vram bar being mapped above 4G, so we don't have to squeeze it into
> the pci I/O window below 4G.
>
> With vram_size_mb=16 and vram64_size_mb=256 it looks like this:
>
The vram variables are a little confusing, but I guess I can live with
it, and it will be more natural going forward.
I guess you will s/4/QXL_VRAM64_RANGE_INDEX/ when you send the
spice-protocol patch?
Other then one more note below, looks good.
> 00:02.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 02)
> (prog-if 00 [VGA controller])
> Subsystem: Red Hat, Inc Device 1100
> Physical Slot: 2
> Flags: fast devsel, IRQ 10
> Memory at f8000000 (32-bit, non-prefetchable) [size=64M]
> Memory at fc000000 (32-bit, non-prefetchable) [size=16M]
> Memory at fd020000 (32-bit, non-prefetchable) [size=8K]
> I/O ports at c5a0 [size=32]
> Memory at ffe0000000 (64-bit, prefetchable) [size=256M]
> Expansion ROM at fd000000 [disabled] [size=64K]
>
> [ needs patched seabios ]
> ---
> hw/qxl.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
> hw/qxl.h | 3 +++
> 2 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 87ad49a..e38bb29 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -914,6 +914,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t
> slot_id, uint64_t delta,
> static const int regions[] = {
> QXL_RAM_RANGE_INDEX,
> QXL_VRAM_RANGE_INDEX,
> + 4 /* vram 64bit */,
> };
> uint64_t guest_start;
> uint64_t guest_end;
> @@ -960,6 +961,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t
> slot_id, uint64_t delta,
> virt_start = (intptr_t)memory_region_get_ram_ptr(&d->vga.vram);
> break;
> case QXL_VRAM_RANGE_INDEX:
> + case 4 /* vram 64bit */:
> virt_start = (intptr_t)memory_region_get_ram_ptr(&d->vram_bar);
> break;
> default:
> @@ -1564,18 +1566,28 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl,
> uint32_t ram_min_mb)
> qxl->vga.vram_size = ram_min_mb * 1024 * 1024;
> }
>
> - /* vram (surfaces, bar 1) */
> + /* vram32 (surfaces, 32bit, bar 1) */
> + if (qxl->vram32_size_mb != -1) {
> + qxl->vram32_size = qxl->vram32_size_mb * 1024 * 1024;
> + }
> + if (qxl->vram32_size < 4096) {
> + qxl->vram32_size = 4096;
> + }
> +
> + /* vram (surfaces, 64bit, bar 4+5) */
> if (qxl->vram_size_mb != -1) {
> qxl->vram_size = qxl->vram_size_mb * 1024 * 1024;
> }
> - if (qxl->vram_size < 4096) {
> - qxl->vram_size = 4096;
> + if (qxl->vram_size < qxl->vram32_size) {
> + qxl->vram_size = qxl->vram32_size;
Am I reading correctly that you want the 64bit bar to be at least the
size of the 32bit bar? why?
> }
> +
> if (qxl->revision == 1) {
> + qxl->vram32_size = 4096;
> qxl->vram_size = 4096;
> }
> -
> qxl->vga.vram_size = msb_mask(qxl->vga.vram_size * 2 - 1);
> + qxl->vram32_size = msb_mask(qxl->vram32_size * 2 - 1);
> qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1);
> }
>
> @@ -1619,6 +1631,8 @@ static int qxl_init_common(PCIQXLDevice *qxl)
>
> memory_region_init_ram(&qxl->vram_bar, "qxl.vram", qxl->vram_size);
> vmstate_register_ram(&qxl->vram_bar, &qxl->pci.qdev);
> + memory_region_init_alias(&qxl->vram32_bar, "qxl.vram32", &qxl->vram_bar,
> + 0, qxl->vram32_size);
>
> io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
> if (qxl->revision == 1) {
> @@ -1642,7 +1656,29 @@ static int qxl_init_common(PCIQXLDevice *qxl)
> PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->vga.vram);
>
> pci_register_bar(&qxl->pci, QXL_VRAM_RANGE_INDEX,
> - PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->vram_bar);
> + PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->vram32_bar);
> +
> + if (qxl->vram32_size < qxl->vram_size) {
> + /*
> + * Make the 64bit vram bar show up only in case it is
> + * configured to be larger than the 32bit vram bar.
> + */
> + pci_register_bar(&qxl->pci, 4,
> + PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_TYPE_64 |
> + PCI_BASE_ADDRESS_MEM_PREFETCH,
> + &qxl->vram_bar);
> + }
> +
> + /* print pci bar details */
> + dprint(qxl, 1, "ram/%s: %d MB [region 0]\n",
> + qxl->id == 0 ? "pri" : "sec",
> + qxl->vga.vram_size / (1024*1024));
> + dprint(qxl, 1, "vram/32: %d MB [region 1]\n",
> + qxl->vram32_size / (1024*1024));
> + dprint(qxl, 1, "vram/64: %d MB %s\n",
> + qxl->vram_size / (1024*1024),
> + qxl->vram32_size < qxl->vram_size ? "[region 4]" : "[unmapped]");
>
> qxl->ssd.qxl.base.sif = &qxl_interface.base;
> qxl->ssd.qxl.id = qxl->id;
> @@ -1859,7 +1895,7 @@ static VMStateDescription qxl_vmstate = {
> static Property qxl_properties[] = {
> DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size,
> 64 * 1024 * 1024),
> - DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size,
> + DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram32_size,
> 64 * 1024 * 1024),
> DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision,
> QXL_DEFAULT_REVISION),
> @@ -1867,7 +1903,8 @@ static Property qxl_properties[] = {
> DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
> DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
> DEFINE_PROP_UINT32("ram_size_mb", PCIQXLDevice, ram_size_mb, -1),
> - DEFINE_PROP_UINT32("vram_size_mb", PCIQXLDevice, vram_size_mb, -1),
> + DEFINE_PROP_UINT32("vram_size_mb", PCIQXLDevice, vram32_size_mb, 0),
> + DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, 0),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/qxl.h b/hw/qxl.h
> index d062991..a1b1240 100644
> --- a/hw/qxl.h
> +++ b/hw/qxl.h
> @@ -86,6 +86,8 @@ typedef struct PCIQXLDevice {
> /* vram pci bar */
> uint32_t vram_size;
> MemoryRegion vram_bar;
> + uint32_t vram32_size;
> + MemoryRegion vram32_bar;
>
> /* io bar */
> MemoryRegion io_bar;
> @@ -93,6 +95,7 @@ typedef struct PCIQXLDevice {
> /* user-friendly properties (in megabytes) */
> uint32_t ram_size_mb;
> uint32_t vram_size_mb;
> + uint32_t vram32_size_mb;
> } PCIQXLDevice;
>
> #define PANIC_ON(x) if ((x)) { \
> --
> 1.7.1
>
> _______________________________________________
> Spice-devel mailing list
> spice-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
