That's a very big hole. We should do this more dynamically.
-- Keir
On 18/1/08 21:02, "David Stone" <unclestoner@xxxxxxxxx> wrote:
> Keir,
> Here's a first patch to address the issue with rolling over to
> guest-physical address 0x00000000 when assigning address regions to
> PCI BARs during HVM boot. For now, this:
> - Makes the hole bigger: 0xC0000000-0xF5000000. This might be
> overkill...but it should only matter for 32-bit guest OSes assigned
> more than 3GB of RAM.
> - Prevents addresses from above 0xF50000000 from being assigned to
> PCI devices by the HVM BIOS (and prevents rollover to 0x00000000)
>
> I also have some code to parse the DSDT's ASL code to more dynamically
> assign the memory hole for PCI BARs. Do you think it's worth worrying
> about this, since again it should only come in handy if you have more
> than 3GB of RAM aassigned to a 32-bit guest OS? I tried the approach
> of having two different chunks of ASL code (in different SSDTs or
> whatever) but couldn't get that to work, so now I have code to modify
> the ASL code directly.
>
> Also, this is my first attempt at submitting code to Xen so please let
> me know of any faux-pas I might be committing.
>
> Thanks,
> Dave
>
> # HG changeset patch
> # User root@xxxxxxxxxxxxxxxxxxxxx
> # Date 1200689221 18000
> # Node ID dba8f029e22298b4d5777499d14524de58fc89f0
> # Parent 62fc84adc8ed30477759461fe5ade47f19b084f4
> Make the memory hole in guest-physical memory for use by guest's PCI
> BARs bigger.
> Also prevent roll-over if the PCI BARs don't fit into the hole.
> Signed-off-by: David Stone david.stone@xxxxxxxxxx
>
> diff -r 62fc84adc8ed -r dba8f029e222 tools/firmware/hvmloader/acpi/dsdt.asl
> --- a/tools/firmware/hvmloader/acpi/dsdt.asl Fri Jan 18 13:43:26 2008 +0000
> +++ b/tools/firmware/hvmloader/acpi/dsdt.asl Fri Jan 18 15:47:01 2008 -0500
> @@ -115,15 +115,21 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2,
> 0x000BFFFF,
> 0x00000000,
> 0x00020000)
> -
> +
> + /* This hole for pci devices is made pretty big here, in
> + * case there is a device that needs a lot of address space
> + * (like a modern graphics card). This is only a potential
> + * issue on a 32-bit guest if you are assigning it more than
> + * ~3G of RAM
> + */
> DWordMemory(
> ResourceConsumer, PosDecode, MinFixed, MaxFixed,
> Cacheable, ReadWrite,
> 0x00000000,
> - 0xF0000000,
> + 0xC0000000,
> 0xF4FFFFFF,
> 0x00000000,
> - 0x05000000)
> + 0x35000000)
> })
> Return (PRT0)
> }
> diff -r 62fc84adc8ed -r dba8f029e222 tools/firmware/hvmloader/acpi/dsdt.c
> --- a/tools/firmware/hvmloader/acpi/dsdt.c Fri Jan 18 13:43:26 2008 +0000
> +++ b/tools/firmware/hvmloader/acpi/dsdt.c Fri Jan 18 15:47:01 2008 -0500
> @@ -1,11 +1,11 @@
> /*
> *
> * Intel ACPI Component Architecture
> - * ASL Optimizing Compiler version 20060707 [Feb 16 2007]
> + * ASL Optimizing Compiler version 20060707 [Jan 18 2008]
> * Copyright (C) 2000 - 2006 Intel Corporation
> * Supports ACPI Specification Revision 3.0a
> *
> - * Compilation of "dsdt.asl" - Tue Sep 11 16:12:28 2007
> + * Compilation of "dsdt.asl" - Fri Jan 18 15:31:56 2008
> *
> * C source code output
> *
> @@ -60,8 +60,8 @@ unsigned char AmlCode[] =
> 0xFF,0xFF,0x0B,0x00,0x00,0x00,0x00,0x00, /* 00000168 "........" */
> 0x00,0x00,0x02,0x00,0x87,0x17,0x00,0x00, /* 00000170 "........" */
> 0x0D,0x03,0x00,0x00,0x00,0x00,0x00,0x00, /* 00000178 "........" */
> - 0x00,0xF0,0xFF,0xFF,0xFF,0xF4,0x00,0x00, /* 00000180 "........" */
> - 0x00,0x00,0x00,0x00,0x00,0x05,0x79,0x00, /* 00000188 "......y." */
> + 0x00,0xC0,0xFF,0xFF,0xFF,0xF4,0x00,0x00, /* 00000180 "........" */
> + 0x00,0x00,0x00,0x00,0x00,0x35,0x79,0x00, /* 00000188 ".....5y." */
> 0xA4,0x50,0x52,0x54,0x30,0x08,0x42,0x55, /* 00000190 ".PRT0.BU" */
> 0x46,0x41,0x11,0x09,0x0A,0x06,0x23,0x20, /* 00000198 "FA....# " */
> 0x0C,0x18,0x79,0x00,0x08,0x42,0x55,0x46, /* 000001A0 "..y..BUF" */
> diff -r 62fc84adc8ed -r dba8f029e222 tools/firmware/hvmloader/hvmloader.c
> --- a/tools/firmware/hvmloader/hvmloader.c Fri Jan 18 13:43:26 2008 +0000
> +++ b/tools/firmware/hvmloader/hvmloader.c Fri Jan 18 15:47:01 2008 -0500
> @@ -183,7 +183,7 @@ static void apic_setup(void)
>
> static void pci_setup(void)
> {
> - uint32_t devfn, bar_reg, bar_data, bar_sz, cmd;
> + uint32_t devfn, bar_reg, bar_data, bar_sz, cmd, mem_base_test;
> uint32_t *base, io_base = 0xc000, mem_base = HVM_BELOW_4G_MMIO_START;
> uint16_t class, vendor_id, device_id;
> unsigned int bar, pin, link, isa_irq;
> @@ -254,16 +254,34 @@ static void pci_setup(void)
> base = &mem_base;
> bar_sz &= PCI_BASE_ADDRESS_MEM_MASK;
> bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> + bar_sz &= ~(bar_sz - 1);
> + mem_base_test = (*base + bar_sz - 1) & ~(bar_sz - 1);
> + if ( (mem_base_test < HVM_BELOW_4G_MMIO_START) ||
> + ( (mem_base_test+bar_sz) < HVM_BELOW_4G_MMIO_START)
> ||
> + ( (mem_base_test+bar_sz) > HVM_BELOW_4G_MMIO_END))
> + {
> + /*
> + * The PCI device's BAR won't fit into the
> hole (it might
> + * even have wrapped around). So, don't
> enable this BAR!
> + */
> + cmd = pci_readw(devfn, PCI_COMMAND);
> + cmd &= ~PCI_COMMAND_MEMORY;
> + pci_writew(devfn, PCI_COMMAND, cmd);
> + printf("pci dev %02x:%x bar %02x (size: %08x)
> is too big!\n",
> + devfn>>3, devfn&7, bar_reg, bar_sz);
> + break;
> + }
> + *base = mem_base_test;
> }
> else
> {
> base = &io_base;
> bar_sz &= PCI_BASE_ADDRESS_IO_MASK & 0xffff;
> bar_data &= ~PCI_BASE_ADDRESS_IO_MASK;
> + bar_sz &= ~(bar_sz - 1);
> + *base = (*base + bar_sz - 1) & ~(bar_sz - 1);
> }
> - bar_sz &= ~(bar_sz - 1);
> -
> - *base = (*base + bar_sz - 1) & ~(bar_sz - 1);
> +
> bar_data |= *base;
> *base += bar_sz;
>
> diff -r 62fc84adc8ed -r dba8f029e222 xen/include/public/hvm/e820.h
> --- a/xen/include/public/hvm/e820.h Fri Jan 18 13:43:26 2008 +0000
> +++ b/xen/include/public/hvm/e820.h Fri Jan 18 15:47:01 2008 -0500
> @@ -27,8 +27,14 @@
> #define HVM_E820_NR_OFFSET 0x000001E8
> #define HVM_E820_OFFSET 0x000002D0
>
> -#define HVM_BELOW_4G_RAM_END 0xF0000000
> +/*
> + * These limits define where in guest-physical address space the guest's PCI
> + * devices get their BARs mapped to. They should match the values given
> + * by the guest ACPI (in dsdt.asl).
> + */
> +#define HVM_BELOW_4G_RAM_END 0xC0000000
> #define HVM_BELOW_4G_MMIO_START HVM_BELOW_4G_RAM_END
> -#define HVM_BELOW_4G_MMIO_LENGTH ((1ULL << 32) - HVM_BELOW_4G_MMIO_START)
> +#define HVM_BELOW_4G_MMIO_END 0xF5000000
> +#define HVM_BELOW_4G_MMIO_LENGTH (HVM_BELOW_4G_MMIO_END -
> HVM_BELOW_4G_MMIO_START)
>
> #endif /* __XEN_PUBLIC_HVM_E820_H__ */
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home