WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [Patch] Make memory hole for PCI Express bigger and prev

To: David Stone <unclestoner@xxxxxxxxx>, Xen Developers <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [Patch] Make memory hole for PCI Express bigger and prevent roll-over
From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Date: Fri, 18 Jan 2008 21:31:00 +0000
Delivery-date: Fri, 18 Jan 2008 13:31:29 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1a74a8410801181302k1923c547t3eb76edb3fc2fbc8@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AchaGWs+qcOfksYMEdyX+wAWy6hiGQ==
Thread-topic: [Xen-devel] [Patch] Make memory hole for PCI Express bigger and prevent roll-over
User-agent: Microsoft-Entourage/11.3.6.070618
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