xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] xen/tools: Add 64 bits big bar support
@ 2012-09-25  3:33 Unknown
  2012-09-25  8:31 ` Ian Campbell
  2012-09-25 14:13 ` Keir Fraser
  0 siblings, 2 replies; 7+ messages in thread
From: Unknown @ 2012-09-25  3:33 UTC (permalink / raw)
  To: ian.jackson
  Cc: ian.campbell, stefano.stabellini, Xudong Hao, xen-devel, jbeulich,
	Xiantao Zhang

Currently it is assumed PCI device BAR access < 4G memory. If there is such a
device whose BAR size is larger than 4G, it must access > 4G memory address.
This patch enable the 64bits big BAR support on hvmloader.

Changes from v1:
1) Set Dynamic MMIO high memory address instead of a fixed number 640G
2) Mask bar_sz earlier to avoid older code changes
3) Add bar size barrier to judge high memory resource
4) Clean up bar64_relocate code

Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r dc56a9defa30 tools/firmware/hvmloader/cacheattr.c
--- a/tools/firmware/hvmloader/cacheattr.c	Tue Aug 14 10:28:14 2012 +0200
+++ b/tools/firmware/hvmloader/cacheattr.c	Wed Sep 12 14:50:55 2012 +0800
@@ -40,17 +40,10 @@
 #define MSR_PAT              0x0277
 #define MSR_MTRRdefType      0x02ff
 
-void cacheattr_init(void)
+unsigned int cpu_phys_addr(void)
 {
     uint32_t eax, ebx, ecx, edx;
-    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
-    unsigned int i, nr_var_ranges, phys_bits = 36;
-
-    /* Does the CPU support architectural MTRRs? */
-    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
-    if ( !(edx & (1u << 12)) )
-         return;
-
+    unsigned int phys_bits = 36;
     /* Find the physical address size for this CPU. */
     cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
     if ( eax >= 0x80000008 )
@@ -59,6 +52,22 @@
         phys_bits = (uint8_t)eax;
     }
 
+    return phys_bits;
+}
+
+void cacheattr_init(void)
+{
+    uint32_t eax, ebx, ecx, edx;
+    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
+    unsigned int i, nr_var_ranges, phys_bits;
+
+    /* Does the CPU support architectural MTRRs? */
+    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
+    if ( !(edx & (1u << 12)) )
+         return;
+
+    phys_bits = cpu_phys_addr();
+
     printf("%u-bit phys ... ", phys_bits);
 
     addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
diff -r dc56a9defa30 tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h	Tue Aug 14 10:28:14 2012 +0200
+++ b/tools/firmware/hvmloader/config.h	Wed Sep 12 14:50:55 2012 +0800
@@ -53,6 +53,8 @@
 /* MMIO hole: Hardcoded defaults, which can be dynamically expanded. */
 #define PCI_MEM_START       0xf0000000
 #define PCI_MEM_END         0xfc000000
+#define PCI_MIN_BIG_BAR_SIZE          0x20000000
+
 extern unsigned long pci_mem_start, pci_mem_end;
 
 
diff -r dc56a9defa30 tools/firmware/hvmloader/pci.c
--- a/tools/firmware/hvmloader/pci.c	Tue Aug 14 10:28:14 2012 +0200
+++ b/tools/firmware/hvmloader/pci.c	Wed Sep 12 14:50:55 2012 +0800
@@ -36,19 +36,25 @@
 
 void pci_setup(void)
 {
-    uint32_t base, devfn, bar_reg, bar_data, bar_sz, cmd, mmio_total = 0;
+    uint8_t is_64bar, using_64bar, bar64_relocate = 0;
+    uint32_t devfn, bar_reg, cmd, bar_data, bar_data_upper;
+    uint64_t base, bar_sz, bar_sz_upper, mmio_total = 0;
     uint32_t vga_devfn = 256;
     uint16_t class, vendor_id, device_id;
     unsigned int bar, pin, link, isa_irq;
+    int64_t mmio_left;
 
     /* Resources assignable to PCI devices via BARs. */
     struct resource {
-        uint32_t base, max;
-    } *resource, mem_resource, io_resource;
+        uint64_t base, max;
+    } *resource, mem_resource, high_mem_resource, io_resource;
 
     /* Create a list of device BARs in descending order of size. */
     struct bars {
-        uint32_t devfn, bar_reg, bar_sz;
+        uint32_t is_64bar;
+        uint32_t devfn;
+        uint32_t bar_reg;
+        uint64_t bar_sz;
     } *bars = (struct bars *)scratch_start;
     unsigned int i, nr_bars = 0;
 
@@ -133,22 +139,34 @@
         /* Map the I/O memory and port resources. */
         for ( bar = 0; bar < 7; bar++ )
         {
+            bar_sz_upper = 0;
             bar_reg = PCI_BASE_ADDRESS_0 + 4*bar;
             if ( bar == 6 )
                 bar_reg = PCI_ROM_ADDRESS;
 
             bar_data = pci_readl(devfn, bar_reg);
+            is_64bar = !!((bar_data & (PCI_BASE_ADDRESS_SPACE |
+                         PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
+                         (PCI_BASE_ADDRESS_SPACE_MEMORY |
+                         PCI_BASE_ADDRESS_MEM_TYPE_64));
             pci_writel(devfn, bar_reg, ~0);
             bar_sz = pci_readl(devfn, bar_reg);
             pci_writel(devfn, bar_reg, bar_data);
-            if ( bar_sz == 0 )
-                continue;
 
             bar_sz &= (((bar_data & PCI_BASE_ADDRESS_SPACE) ==
                         PCI_BASE_ADDRESS_SPACE_MEMORY) ?
                        PCI_BASE_ADDRESS_MEM_MASK :
                        (PCI_BASE_ADDRESS_IO_MASK & 0xffff));
+            if (is_64bar) {
+                bar_data_upper = pci_readl(devfn, bar_reg + 4);
+                pci_writel(devfn, bar_reg + 4, ~0);
+                bar_sz_upper = pci_readl(devfn, bar_reg + 4);
+                pci_writel(devfn, bar_reg + 4, bar_data_upper);
+                bar_sz = (bar_sz_upper << 32) | bar_sz;
+            }
             bar_sz &= ~(bar_sz - 1);
+            if ( bar_sz == 0 )
+                continue;
 
             for ( i = 0; i < nr_bars; i++ )
                 if ( bars[i].bar_sz < bar_sz )
@@ -157,6 +175,7 @@
             if ( i != nr_bars )
                 memmove(&bars[i+1], &bars[i], (nr_bars-i) * sizeof(*bars));
 
+            bars[i].is_64bar = is_64bar;
             bars[i].devfn   = devfn;
             bars[i].bar_reg = bar_reg;
             bars[i].bar_sz  = bar_sz;
@@ -167,11 +186,8 @@
 
             nr_bars++;
 
-            /* Skip the upper-half of the address for a 64-bit BAR. */
-            if ( (bar_data & (PCI_BASE_ADDRESS_SPACE |
-                              PCI_BASE_ADDRESS_MEM_TYPE_MASK)) == 
-                 (PCI_BASE_ADDRESS_SPACE_MEMORY | 
-                  PCI_BASE_ADDRESS_MEM_TYPE_64) )
+            /*The upper half is already calculated, skip it! */
+            if (is_64bar)
                 bar++;
         }
 
@@ -197,6 +213,9 @@
             ((pci_mem_start << 1) != 0) )
         pci_mem_start <<= 1;
 
+    if ( (pci_mem_start << 1) != 0 )
+        bar64_relocate = 1;
+
     /* Relocate RAM that overlaps PCI space (in 64k-page chunks). */
     while ( (pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend )
     {
@@ -218,11 +237,15 @@
         hvm_info->high_mem_pgend += nr_pages;
     }
 
+    high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend) << PAGE_SHIFT; 
+    high_mem_resource.max = 1ull << cpu_phys_addr();
     mem_resource.base = pci_mem_start;
     mem_resource.max = pci_mem_end;
     io_resource.base = 0xc000;
     io_resource.max = 0x10000;
 
+    mmio_left = pci_mem_end - pci_mem_start;
+
     /* Assign iomem and ioport resources in descending order of size. */
     for ( i = 0; i < nr_bars; i++ )
     {
@@ -230,13 +253,29 @@
         bar_reg = bars[i].bar_reg;
         bar_sz  = bars[i].bar_sz;
 
+        using_64bar = bars[i].is_64bar && bar64_relocate && (mmio_left < bar_sz);
         bar_data = pci_readl(devfn, bar_reg);
 
         if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
              PCI_BASE_ADDRESS_SPACE_MEMORY )
         {
-            resource = &mem_resource;
-            bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
+            /* Mapping high memory if PCI deivce is 64 bits bar and the bar size
+               is larger than 512M */
+            if (using_64bar && (bar_sz > PCI_MIN_BIG_BAR_SIZE)) {
+                if ( high_mem_resource.base & (bar_sz - 1) )
+                    high_mem_resource.base = high_mem_resource.base - 
+                        (high_mem_resource.base & (bar_sz - 1)) + bar_sz;
+                else
+                    high_mem_resource.base = high_mem_resource.base - 
+                        (high_mem_resource.base & (bar_sz - 1));
+                resource = &high_mem_resource;
+                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
+            } 
+            else {
+                resource = &mem_resource;
+                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
+            }
+            mmio_left -= bar_sz;
         }
         else
         {
@@ -244,13 +283,14 @@
             bar_data &= ~PCI_BASE_ADDRESS_IO_MASK;
         }
 
-        base = (resource->base + bar_sz - 1) & ~(bar_sz - 1);
-        bar_data |= base;
+        base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
+        bar_data |= (uint32_t)base;
+        bar_data_upper = (uint32_t)(base >> 32);
         base += bar_sz;
 
         if ( (base < resource->base) || (base > resource->max) )
         {
-            printf("pci dev %02x:%x bar %02x size %08x: no space for "
+            printf("pci dev %02x:%x bar %02x size %llx: no space for "
                    "resource!\n", devfn>>3, devfn&7, bar_reg, bar_sz);
             continue;
         }
@@ -258,8 +298,8 @@
         resource->base = base;
 
         pci_writel(devfn, bar_reg, bar_data);
-        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
-               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
+        if (using_64bar)
+            pci_writel(devfn, bar_reg + 4, bar_data_upper);
 
         /* Now enable the memory or I/O mapping. */
         cmd = pci_readw(devfn, PCI_COMMAND);
diff -r dc56a9defa30 tools/firmware/hvmloader/util.h
--- a/tools/firmware/hvmloader/util.h	Tue Aug 14 10:28:14 2012 +0200
+++ b/tools/firmware/hvmloader/util.h	Wed Sep 12 14:50:55 2012 +0800
@@ -215,6 +215,7 @@
 uint32_t rombios_highbios_setup(void);
 
 /* Miscellaneous. */
+unsigned int cpu_phys_addr(void);
 void cacheattr_init(void);
 unsigned long create_mp_tables(void *table);
 void hvm_write_smbios_tables(

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] xen/tools: Add 64 bits big bar support
  2012-09-25  3:33 [PATCH v2] xen/tools: Add 64 bits big bar support Unknown
@ 2012-09-25  8:31 ` Ian Campbell
  2012-09-25  9:14   ` Hao, Xudong
  2012-09-25 14:13 ` Keir Fraser
  1 sibling, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2012-09-25  8:31 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Stefano Stabellini, Xudong Hao, Xiantao Zhang, jbeulich@suse.com,
	xen-devel@lists.xen.org

On Tue, 2012-09-25 at 04:33 +0100, an unknown sender wrote:
> Currently it is assumed PCI device BAR access < 4G memory. If there is such a
> device whose BAR size is larger than 4G, it must access > 4G memory address.
> This patch enable the 64bits big BAR support on hvmloader.

Does this work in both qemu-xen-traditional/ROMBIOS and qemu-xen/SeaBIOS
configurations? Or does the BIOS not care about this stuff?

Ian

> 
> Changes from v1:
> 1) Set Dynamic MMIO high memory address instead of a fixed number 640G
> 2) Mask bar_sz earlier to avoid older code changes
> 3) Add bar size barrier to judge high memory resource
> 4) Clean up bar64_relocate code
> 
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> 
> diff -r dc56a9defa30 tools/firmware/hvmloader/cacheattr.c
> --- a/tools/firmware/hvmloader/cacheattr.c	Tue Aug 14 10:28:14 2012 +0200
> +++ b/tools/firmware/hvmloader/cacheattr.c	Wed Sep 12 14:50:55 2012 +0800
> @@ -40,17 +40,10 @@
>  #define MSR_PAT              0x0277
>  #define MSR_MTRRdefType      0x02ff
>  
> -void cacheattr_init(void)
> +unsigned int cpu_phys_addr(void)
>  {
>      uint32_t eax, ebx, ecx, edx;
> -    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
> -    unsigned int i, nr_var_ranges, phys_bits = 36;
> -
> -    /* Does the CPU support architectural MTRRs? */
> -    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> -    if ( !(edx & (1u << 12)) )
> -         return;
> -
> +    unsigned int phys_bits = 36;
>      /* Find the physical address size for this CPU. */
>      cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
>      if ( eax >= 0x80000008 )
> @@ -59,6 +52,22 @@
>          phys_bits = (uint8_t)eax;
>      }
>  
> +    return phys_bits;
> +}
> +
> +void cacheattr_init(void)
> +{
> +    uint32_t eax, ebx, ecx, edx;
> +    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
> +    unsigned int i, nr_var_ranges, phys_bits;
> +
> +    /* Does the CPU support architectural MTRRs? */
> +    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> +    if ( !(edx & (1u << 12)) )
> +         return;
> +
> +    phys_bits = cpu_phys_addr();
> +
>      printf("%u-bit phys ... ", phys_bits);
>  
>      addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
> diff -r dc56a9defa30 tools/firmware/hvmloader/config.h
> --- a/tools/firmware/hvmloader/config.h	Tue Aug 14 10:28:14 2012 +0200
> +++ b/tools/firmware/hvmloader/config.h	Wed Sep 12 14:50:55 2012 +0800
> @@ -53,6 +53,8 @@
>  /* MMIO hole: Hardcoded defaults, which can be dynamically expanded. */
>  #define PCI_MEM_START       0xf0000000
>  #define PCI_MEM_END         0xfc000000
> +#define PCI_MIN_BIG_BAR_SIZE          0x20000000
> +
>  extern unsigned long pci_mem_start, pci_mem_end;
>  
> 
> diff -r dc56a9defa30 tools/firmware/hvmloader/pci.c
> --- a/tools/firmware/hvmloader/pci.c	Tue Aug 14 10:28:14 2012 +0200
> +++ b/tools/firmware/hvmloader/pci.c	Wed Sep 12 14:50:55 2012 +0800
> @@ -36,19 +36,25 @@
>  
>  void pci_setup(void)
>  {
> -    uint32_t base, devfn, bar_reg, bar_data, bar_sz, cmd, mmio_total = 0;
> +    uint8_t is_64bar, using_64bar, bar64_relocate = 0;
> +    uint32_t devfn, bar_reg, cmd, bar_data, bar_data_upper;
> +    uint64_t base, bar_sz, bar_sz_upper, mmio_total = 0;
>      uint32_t vga_devfn = 256;
>      uint16_t class, vendor_id, device_id;
>      unsigned int bar, pin, link, isa_irq;
> +    int64_t mmio_left;
>  
>      /* Resources assignable to PCI devices via BARs. */
>      struct resource {
> -        uint32_t base, max;
> -    } *resource, mem_resource, io_resource;
> +        uint64_t base, max;
> +    } *resource, mem_resource, high_mem_resource, io_resource;
>  
>      /* Create a list of device BARs in descending order of size. */
>      struct bars {
> -        uint32_t devfn, bar_reg, bar_sz;
> +        uint32_t is_64bar;
> +        uint32_t devfn;
> +        uint32_t bar_reg;
> +        uint64_t bar_sz;
>      } *bars = (struct bars *)scratch_start;
>      unsigned int i, nr_bars = 0;
>  
> @@ -133,22 +139,34 @@
>          /* Map the I/O memory and port resources. */
>          for ( bar = 0; bar < 7; bar++ )
>          {
> +            bar_sz_upper = 0;
>              bar_reg = PCI_BASE_ADDRESS_0 + 4*bar;
>              if ( bar == 6 )
>                  bar_reg = PCI_ROM_ADDRESS;
>  
>              bar_data = pci_readl(devfn, bar_reg);
> +            is_64bar = !!((bar_data & (PCI_BASE_ADDRESS_SPACE |
> +                         PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
> +                         (PCI_BASE_ADDRESS_SPACE_MEMORY |
> +                         PCI_BASE_ADDRESS_MEM_TYPE_64));
>              pci_writel(devfn, bar_reg, ~0);
>              bar_sz = pci_readl(devfn, bar_reg);
>              pci_writel(devfn, bar_reg, bar_data);
> -            if ( bar_sz == 0 )
> -                continue;
>  
>              bar_sz &= (((bar_data & PCI_BASE_ADDRESS_SPACE) ==
>                          PCI_BASE_ADDRESS_SPACE_MEMORY) ?
>                         PCI_BASE_ADDRESS_MEM_MASK :
>                         (PCI_BASE_ADDRESS_IO_MASK & 0xffff));
> +            if (is_64bar) {
> +                bar_data_upper = pci_readl(devfn, bar_reg + 4);
> +                pci_writel(devfn, bar_reg + 4, ~0);
> +                bar_sz_upper = pci_readl(devfn, bar_reg + 4);
> +                pci_writel(devfn, bar_reg + 4, bar_data_upper);
> +                bar_sz = (bar_sz_upper << 32) | bar_sz;
> +            }
>              bar_sz &= ~(bar_sz - 1);
> +            if ( bar_sz == 0 )
> +                continue;
>  
>              for ( i = 0; i < nr_bars; i++ )
>                  if ( bars[i].bar_sz < bar_sz )
> @@ -157,6 +175,7 @@
>              if ( i != nr_bars )
>                  memmove(&bars[i+1], &bars[i], (nr_bars-i) * sizeof(*bars));
>  
> +            bars[i].is_64bar = is_64bar;
>              bars[i].devfn   = devfn;
>              bars[i].bar_reg = bar_reg;
>              bars[i].bar_sz  = bar_sz;
> @@ -167,11 +186,8 @@
>  
>              nr_bars++;
>  
> -            /* Skip the upper-half of the address for a 64-bit BAR. */
> -            if ( (bar_data & (PCI_BASE_ADDRESS_SPACE |
> -                              PCI_BASE_ADDRESS_MEM_TYPE_MASK)) == 
> -                 (PCI_BASE_ADDRESS_SPACE_MEMORY | 
> -                  PCI_BASE_ADDRESS_MEM_TYPE_64) )
> +            /*The upper half is already calculated, skip it! */
> +            if (is_64bar)
>                  bar++;
>          }
>  
> @@ -197,6 +213,9 @@
>              ((pci_mem_start << 1) != 0) )
>          pci_mem_start <<= 1;
>  
> +    if ( (pci_mem_start << 1) != 0 )
> +        bar64_relocate = 1;
> +
>      /* Relocate RAM that overlaps PCI space (in 64k-page chunks). */
>      while ( (pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend )
>      {
> @@ -218,11 +237,15 @@
>          hvm_info->high_mem_pgend += nr_pages;
>      }
>  
> +    high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend) << PAGE_SHIFT; 
> +    high_mem_resource.max = 1ull << cpu_phys_addr();
>      mem_resource.base = pci_mem_start;
>      mem_resource.max = pci_mem_end;
>      io_resource.base = 0xc000;
>      io_resource.max = 0x10000;
>  
> +    mmio_left = pci_mem_end - pci_mem_start;
> +
>      /* Assign iomem and ioport resources in descending order of size. */
>      for ( i = 0; i < nr_bars; i++ )
>      {
> @@ -230,13 +253,29 @@
>          bar_reg = bars[i].bar_reg;
>          bar_sz  = bars[i].bar_sz;
>  
> +        using_64bar = bars[i].is_64bar && bar64_relocate && (mmio_left < bar_sz);
>          bar_data = pci_readl(devfn, bar_reg);
>  
>          if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
>               PCI_BASE_ADDRESS_SPACE_MEMORY )
>          {
> -            resource = &mem_resource;
> -            bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> +            /* Mapping high memory if PCI deivce is 64 bits bar and the bar size
> +               is larger than 512M */
> +            if (using_64bar && (bar_sz > PCI_MIN_BIG_BAR_SIZE)) {
> +                if ( high_mem_resource.base & (bar_sz - 1) )
> +                    high_mem_resource.base = high_mem_resource.base - 
> +                        (high_mem_resource.base & (bar_sz - 1)) + bar_sz;
> +                else
> +                    high_mem_resource.base = high_mem_resource.base - 
> +                        (high_mem_resource.base & (bar_sz - 1));
> +                resource = &high_mem_resource;
> +                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> +            } 
> +            else {
> +                resource = &mem_resource;
> +                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> +            }
> +            mmio_left -= bar_sz;
>          }
>          else
>          {
> @@ -244,13 +283,14 @@
>              bar_data &= ~PCI_BASE_ADDRESS_IO_MASK;
>          }
>  
> -        base = (resource->base + bar_sz - 1) & ~(bar_sz - 1);
> -        bar_data |= base;
> +        base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
> +        bar_data |= (uint32_t)base;
> +        bar_data_upper = (uint32_t)(base >> 32);
>          base += bar_sz;
>  
>          if ( (base < resource->base) || (base > resource->max) )
>          {
> -            printf("pci dev %02x:%x bar %02x size %08x: no space for "
> +            printf("pci dev %02x:%x bar %02x size %llx: no space for "
>                     "resource!\n", devfn>>3, devfn&7, bar_reg, bar_sz);
>              continue;
>          }
> @@ -258,8 +298,8 @@
>          resource->base = base;
>  
>          pci_writel(devfn, bar_reg, bar_data);
> -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
> -               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
> +        if (using_64bar)
> +            pci_writel(devfn, bar_reg + 4, bar_data_upper);
>  
>          /* Now enable the memory or I/O mapping. */
>          cmd = pci_readw(devfn, PCI_COMMAND);
> diff -r dc56a9defa30 tools/firmware/hvmloader/util.h
> --- a/tools/firmware/hvmloader/util.h	Tue Aug 14 10:28:14 2012 +0200
> +++ b/tools/firmware/hvmloader/util.h	Wed Sep 12 14:50:55 2012 +0800
> @@ -215,6 +215,7 @@
>  uint32_t rombios_highbios_setup(void);
>  
>  /* Miscellaneous. */
> +unsigned int cpu_phys_addr(void);
>  void cacheattr_init(void);
>  unsigned long create_mp_tables(void *table);
>  void hvm_write_smbios_tables(

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] xen/tools: Add 64 bits big bar support
  2012-09-25  8:31 ` Ian Campbell
@ 2012-09-25  9:14   ` Hao, Xudong
  2012-09-25  9:25     ` Ian Campbell
  0 siblings, 1 reply; 7+ messages in thread
From: Hao, Xudong @ 2012-09-25  9:14 UTC (permalink / raw)
  To: Ian Campbell, Ian Jackson
  Cc: Stefano Stabellini, Zhang, Xiantao, jbeulich@suse.com,
	xen-devel@lists.xen.org


> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Tuesday, September 25, 2012 4:32 PM
> To: Ian Jackson
> Cc: xen-devel@lists.xen.org; Stefano Stabellini; jbeulich@suse.com; Zhang,
> Xiantao; Hao, Xudong
> Subject: Re: [PATCH v2] xen/tools: Add 64 bits big bar support
> 
> On Tue, 2012-09-25 at 04:33 +0100, an unknown sender wrote:
> > Currently it is assumed PCI device BAR access < 4G memory. If there is such a
> > device whose BAR size is larger than 4G, it must access > 4G memory
> address.
> > This patch enable the 64bits big BAR support on hvmloader.
> 
> Does this work in both qemu-xen-traditional/ROMBIOS and qemu-xen/SeaBIOS
> configurations? Or does the BIOS not care about this stuff?
> 
It's tested pass on both qemu-xen-traditional/ROMBIOS and qemu/SeaBIOS.

-Xudong

> Ian
> 
> >
> > Changes from v1:
> > 1) Set Dynamic MMIO high memory address instead of a fixed number 640G
> > 2) Mask bar_sz earlier to avoid older code changes
> > 3) Add bar size barrier to judge high memory resource
> > 4) Clean up bar64_relocate code
> >
> > Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> >
> > diff -r dc56a9defa30 tools/firmware/hvmloader/cacheattr.c
> > --- a/tools/firmware/hvmloader/cacheattr.c	Tue Aug 14 10:28:14 2012
> +0200
> > +++ b/tools/firmware/hvmloader/cacheattr.c	Wed Sep 12 14:50:55 2012
> +0800
> > @@ -40,17 +40,10 @@
> >  #define MSR_PAT              0x0277
> >  #define MSR_MTRRdefType      0x02ff
> >
> > -void cacheattr_init(void)
> > +unsigned int cpu_phys_addr(void)
> >  {
> >      uint32_t eax, ebx, ecx, edx;
> > -    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
> > -    unsigned int i, nr_var_ranges, phys_bits = 36;
> > -
> > -    /* Does the CPU support architectural MTRRs? */
> > -    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> > -    if ( !(edx & (1u << 12)) )
> > -         return;
> > -
> > +    unsigned int phys_bits = 36;
> >      /* Find the physical address size for this CPU. */
> >      cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
> >      if ( eax >= 0x80000008 )
> > @@ -59,6 +52,22 @@
> >          phys_bits = (uint8_t)eax;
> >      }
> >
> > +    return phys_bits;
> > +}
> > +
> > +void cacheattr_init(void)
> > +{
> > +    uint32_t eax, ebx, ecx, edx;
> > +    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
> > +    unsigned int i, nr_var_ranges, phys_bits;
> > +
> > +    /* Does the CPU support architectural MTRRs? */
> > +    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> > +    if ( !(edx & (1u << 12)) )
> > +         return;
> > +
> > +    phys_bits = cpu_phys_addr();
> > +
> >      printf("%u-bit phys ... ", phys_bits);
> >
> >      addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
> > diff -r dc56a9defa30 tools/firmware/hvmloader/config.h
> > --- a/tools/firmware/hvmloader/config.h	Tue Aug 14 10:28:14 2012 +0200
> > +++ b/tools/firmware/hvmloader/config.h	Wed Sep 12 14:50:55 2012 +0800
> > @@ -53,6 +53,8 @@
> >  /* MMIO hole: Hardcoded defaults, which can be dynamically expanded. */
> >  #define PCI_MEM_START       0xf0000000
> >  #define PCI_MEM_END         0xfc000000
> > +#define PCI_MIN_BIG_BAR_SIZE          0x20000000
> > +
> >  extern unsigned long pci_mem_start, pci_mem_end;
> >
> >
> > diff -r dc56a9defa30 tools/firmware/hvmloader/pci.c
> > --- a/tools/firmware/hvmloader/pci.c	Tue Aug 14 10:28:14 2012 +0200
> > +++ b/tools/firmware/hvmloader/pci.c	Wed Sep 12 14:50:55 2012 +0800
> > @@ -36,19 +36,25 @@
> >
> >  void pci_setup(void)
> >  {
> > -    uint32_t base, devfn, bar_reg, bar_data, bar_sz, cmd, mmio_total = 0;
> > +    uint8_t is_64bar, using_64bar, bar64_relocate = 0;
> > +    uint32_t devfn, bar_reg, cmd, bar_data, bar_data_upper;
> > +    uint64_t base, bar_sz, bar_sz_upper, mmio_total = 0;
> >      uint32_t vga_devfn = 256;
> >      uint16_t class, vendor_id, device_id;
> >      unsigned int bar, pin, link, isa_irq;
> > +    int64_t mmio_left;
> >
> >      /* Resources assignable to PCI devices via BARs. */
> >      struct resource {
> > -        uint32_t base, max;
> > -    } *resource, mem_resource, io_resource;
> > +        uint64_t base, max;
> > +    } *resource, mem_resource, high_mem_resource, io_resource;
> >
> >      /* Create a list of device BARs in descending order of size. */
> >      struct bars {
> > -        uint32_t devfn, bar_reg, bar_sz;
> > +        uint32_t is_64bar;
> > +        uint32_t devfn;
> > +        uint32_t bar_reg;
> > +        uint64_t bar_sz;
> >      } *bars = (struct bars *)scratch_start;
> >      unsigned int i, nr_bars = 0;
> >
> > @@ -133,22 +139,34 @@
> >          /* Map the I/O memory and port resources. */
> >          for ( bar = 0; bar < 7; bar++ )
> >          {
> > +            bar_sz_upper = 0;
> >              bar_reg = PCI_BASE_ADDRESS_0 + 4*bar;
> >              if ( bar == 6 )
> >                  bar_reg = PCI_ROM_ADDRESS;
> >
> >              bar_data = pci_readl(devfn, bar_reg);
> > +            is_64bar = !!((bar_data & (PCI_BASE_ADDRESS_SPACE |
> > +                         PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
> > +                         (PCI_BASE_ADDRESS_SPACE_MEMORY |
> > +                         PCI_BASE_ADDRESS_MEM_TYPE_64));
> >              pci_writel(devfn, bar_reg, ~0);
> >              bar_sz = pci_readl(devfn, bar_reg);
> >              pci_writel(devfn, bar_reg, bar_data);
> > -            if ( bar_sz == 0 )
> > -                continue;
> >
> >              bar_sz &= (((bar_data & PCI_BASE_ADDRESS_SPACE) ==
> >                          PCI_BASE_ADDRESS_SPACE_MEMORY) ?
> >                         PCI_BASE_ADDRESS_MEM_MASK :
> >                         (PCI_BASE_ADDRESS_IO_MASK & 0xffff));
> > +            if (is_64bar) {
> > +                bar_data_upper = pci_readl(devfn, bar_reg + 4);
> > +                pci_writel(devfn, bar_reg + 4, ~0);
> > +                bar_sz_upper = pci_readl(devfn, bar_reg + 4);
> > +                pci_writel(devfn, bar_reg + 4, bar_data_upper);
> > +                bar_sz = (bar_sz_upper << 32) | bar_sz;
> > +            }
> >              bar_sz &= ~(bar_sz - 1);
> > +            if ( bar_sz == 0 )
> > +                continue;
> >
> >              for ( i = 0; i < nr_bars; i++ )
> >                  if ( bars[i].bar_sz < bar_sz )
> > @@ -157,6 +175,7 @@
> >              if ( i != nr_bars )
> >                  memmove(&bars[i+1], &bars[i], (nr_bars-i) *
> sizeof(*bars));
> >
> > +            bars[i].is_64bar = is_64bar;
> >              bars[i].devfn   = devfn;
> >              bars[i].bar_reg = bar_reg;
> >              bars[i].bar_sz  = bar_sz;
> > @@ -167,11 +186,8 @@
> >
> >              nr_bars++;
> >
> > -            /* Skip the upper-half of the address for a 64-bit BAR. */
> > -            if ( (bar_data & (PCI_BASE_ADDRESS_SPACE |
> > -
> PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
> > -                 (PCI_BASE_ADDRESS_SPACE_MEMORY |
> > -                  PCI_BASE_ADDRESS_MEM_TYPE_64) )
> > +            /*The upper half is already calculated, skip it! */
> > +            if (is_64bar)
> >                  bar++;
> >          }
> >
> > @@ -197,6 +213,9 @@
> >              ((pci_mem_start << 1) != 0) )
> >          pci_mem_start <<= 1;
> >
> > +    if ( (pci_mem_start << 1) != 0 )
> > +        bar64_relocate = 1;
> > +
> >      /* Relocate RAM that overlaps PCI space (in 64k-page chunks). */
> >      while ( (pci_mem_start >> PAGE_SHIFT) <
> hvm_info->low_mem_pgend )
> >      {
> > @@ -218,11 +237,15 @@
> >          hvm_info->high_mem_pgend += nr_pages;
> >      }
> >
> > +    high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend)
> << PAGE_SHIFT;
> > +    high_mem_resource.max = 1ull << cpu_phys_addr();
> >      mem_resource.base = pci_mem_start;
> >      mem_resource.max = pci_mem_end;
> >      io_resource.base = 0xc000;
> >      io_resource.max = 0x10000;
> >
> > +    mmio_left = pci_mem_end - pci_mem_start;
> > +
> >      /* Assign iomem and ioport resources in descending order of size. */
> >      for ( i = 0; i < nr_bars; i++ )
> >      {
> > @@ -230,13 +253,29 @@
> >          bar_reg = bars[i].bar_reg;
> >          bar_sz  = bars[i].bar_sz;
> >
> > +        using_64bar = bars[i].is_64bar && bar64_relocate && (mmio_left
> < bar_sz);
> >          bar_data = pci_readl(devfn, bar_reg);
> >
> >          if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
> >               PCI_BASE_ADDRESS_SPACE_MEMORY )
> >          {
> > -            resource = &mem_resource;
> > -            bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> > +            /* Mapping high memory if PCI deivce is 64 bits bar and the
> bar size
> > +               is larger than 512M */
> > +            if (using_64bar && (bar_sz > PCI_MIN_BIG_BAR_SIZE)) {
> > +                if ( high_mem_resource.base & (bar_sz - 1) )
> > +                    high_mem_resource.base =
> high_mem_resource.base -
> > +                        (high_mem_resource.base & (bar_sz - 1)) +
> bar_sz;
> > +                else
> > +                    high_mem_resource.base =
> high_mem_resource.base -
> > +                        (high_mem_resource.base & (bar_sz - 1));
> > +                resource = &high_mem_resource;
> > +                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> > +            }
> > +            else {
> > +                resource = &mem_resource;
> > +                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> > +            }
> > +            mmio_left -= bar_sz;
> >          }
> >          else
> >          {
> > @@ -244,13 +283,14 @@
> >              bar_data &= ~PCI_BASE_ADDRESS_IO_MASK;
> >          }
> >
> > -        base = (resource->base + bar_sz - 1) & ~(bar_sz - 1);
> > -        bar_data |= base;
> > +        base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
> > +        bar_data |= (uint32_t)base;
> > +        bar_data_upper = (uint32_t)(base >> 32);
> >          base += bar_sz;
> >
> >          if ( (base < resource->base) || (base > resource->max) )
> >          {
> > -            printf("pci dev %02x:%x bar %02x size %08x: no space for "
> > +            printf("pci dev %02x:%x bar %02x size %llx: no space for "
> >                     "resource!\n", devfn>>3, devfn&7, bar_reg,
> bar_sz);
> >              continue;
> >          }
> > @@ -258,8 +298,8 @@
> >          resource->base = base;
> >
> >          pci_writel(devfn, bar_reg, bar_data);
> > -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
> > -               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
> > +        if (using_64bar)
> > +            pci_writel(devfn, bar_reg + 4, bar_data_upper);
> >
> >          /* Now enable the memory or I/O mapping. */
> >          cmd = pci_readw(devfn, PCI_COMMAND);
> > diff -r dc56a9defa30 tools/firmware/hvmloader/util.h
> > --- a/tools/firmware/hvmloader/util.h	Tue Aug 14 10:28:14 2012 +0200
> > +++ b/tools/firmware/hvmloader/util.h	Wed Sep 12 14:50:55 2012 +0800
> > @@ -215,6 +215,7 @@
> >  uint32_t rombios_highbios_setup(void);
> >
> >  /* Miscellaneous. */
> > +unsigned int cpu_phys_addr(void);
> >  void cacheattr_init(void);
> >  unsigned long create_mp_tables(void *table);
> >  void hvm_write_smbios_tables(
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] xen/tools: Add 64 bits big bar support
  2012-09-25  9:14   ` Hao, Xudong
@ 2012-09-25  9:25     ` Ian Campbell
  0 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2012-09-25  9:25 UTC (permalink / raw)
  To: Hao, Xudong
  Cc: Keir Fraser, Stefano Stabellini, Ian Jackson,
	xen-devel@lists.xen.org, jbeulich@suse.com, Zhang, Xiantao

On Tue, 2012-09-25 at 10:14 +0100, Hao, Xudong wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > Sent: Tuesday, September 25, 2012 4:32 PM
> > To: Ian Jackson
> > Cc: xen-devel@lists.xen.org; Stefano Stabellini; jbeulich@suse.com; Zhang,
> > Xiantao; Hao, Xudong
> > Subject: Re: [PATCH v2] xen/tools: Add 64 bits big bar support
> > 
> > On Tue, 2012-09-25 at 04:33 +0100, an unknown sender wrote:
> > > Currently it is assumed PCI device BAR access < 4G memory. If there is such a
> > > device whose BAR size is larger than 4G, it must access > 4G memory
> > address.
> > > This patch enable the 64bits big BAR support on hvmloader.
> > 
> > Does this work in both qemu-xen-traditional/ROMBIOS and qemu-xen/SeaBIOS
> > configurations? Or does the BIOS not care about this stuff?
> > 
> It's tested pass on both qemu-xen-traditional/ROMBIOS and qemu/SeaBIOS.

Sweet, thanks!

Adding Keir to the CC. He still watches over hvmloader IIRC.

> 
> -Xudong
> 
> > Ian
> > 
> > >
> > > Changes from v1:
> > > 1) Set Dynamic MMIO high memory address instead of a fixed number 640G
> > > 2) Mask bar_sz earlier to avoid older code changes
> > > 3) Add bar size barrier to judge high memory resource
> > > 4) Clean up bar64_relocate code
> > >
> > > Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> > > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> > >
> > > diff -r dc56a9defa30 tools/firmware/hvmloader/cacheattr.c
> > > --- a/tools/firmware/hvmloader/cacheattr.c	Tue Aug 14 10:28:14 2012
> > +0200
> > > +++ b/tools/firmware/hvmloader/cacheattr.c	Wed Sep 12 14:50:55 2012
> > +0800
> > > @@ -40,17 +40,10 @@
> > >  #define MSR_PAT              0x0277
> > >  #define MSR_MTRRdefType      0x02ff
> > >
> > > -void cacheattr_init(void)
> > > +unsigned int cpu_phys_addr(void)
> > >  {
> > >      uint32_t eax, ebx, ecx, edx;
> > > -    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
> > > -    unsigned int i, nr_var_ranges, phys_bits = 36;
> > > -
> > > -    /* Does the CPU support architectural MTRRs? */
> > > -    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> > > -    if ( !(edx & (1u << 12)) )
> > > -         return;
> > > -
> > > +    unsigned int phys_bits = 36;
> > >      /* Find the physical address size for this CPU. */
> > >      cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
> > >      if ( eax >= 0x80000008 )
> > > @@ -59,6 +52,22 @@
> > >          phys_bits = (uint8_t)eax;
> > >      }
> > >
> > > +    return phys_bits;
> > > +}
> > > +
> > > +void cacheattr_init(void)
> > > +{
> > > +    uint32_t eax, ebx, ecx, edx;
> > > +    uint64_t mtrr_cap, mtrr_def, content, addr_mask;
> > > +    unsigned int i, nr_var_ranges, phys_bits;
> > > +
> > > +    /* Does the CPU support architectural MTRRs? */
> > > +    cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> > > +    if ( !(edx & (1u << 12)) )
> > > +         return;
> > > +
> > > +    phys_bits = cpu_phys_addr();
> > > +
> > >      printf("%u-bit phys ... ", phys_bits);
> > >
> > >      addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
> > > diff -r dc56a9defa30 tools/firmware/hvmloader/config.h
> > > --- a/tools/firmware/hvmloader/config.h	Tue Aug 14 10:28:14 2012 +0200
> > > +++ b/tools/firmware/hvmloader/config.h	Wed Sep 12 14:50:55 2012 +0800
> > > @@ -53,6 +53,8 @@
> > >  /* MMIO hole: Hardcoded defaults, which can be dynamically expanded. */
> > >  #define PCI_MEM_START       0xf0000000
> > >  #define PCI_MEM_END         0xfc000000
> > > +#define PCI_MIN_BIG_BAR_SIZE          0x20000000
> > > +
> > >  extern unsigned long pci_mem_start, pci_mem_end;
> > >
> > >
> > > diff -r dc56a9defa30 tools/firmware/hvmloader/pci.c
> > > --- a/tools/firmware/hvmloader/pci.c	Tue Aug 14 10:28:14 2012 +0200
> > > +++ b/tools/firmware/hvmloader/pci.c	Wed Sep 12 14:50:55 2012 +0800
> > > @@ -36,19 +36,25 @@
> > >
> > >  void pci_setup(void)
> > >  {
> > > -    uint32_t base, devfn, bar_reg, bar_data, bar_sz, cmd, mmio_total = 0;
> > > +    uint8_t is_64bar, using_64bar, bar64_relocate = 0;
> > > +    uint32_t devfn, bar_reg, cmd, bar_data, bar_data_upper;
> > > +    uint64_t base, bar_sz, bar_sz_upper, mmio_total = 0;
> > >      uint32_t vga_devfn = 256;
> > >      uint16_t class, vendor_id, device_id;
> > >      unsigned int bar, pin, link, isa_irq;
> > > +    int64_t mmio_left;
> > >
> > >      /* Resources assignable to PCI devices via BARs. */
> > >      struct resource {
> > > -        uint32_t base, max;
> > > -    } *resource, mem_resource, io_resource;
> > > +        uint64_t base, max;
> > > +    } *resource, mem_resource, high_mem_resource, io_resource;
> > >
> > >      /* Create a list of device BARs in descending order of size. */
> > >      struct bars {
> > > -        uint32_t devfn, bar_reg, bar_sz;
> > > +        uint32_t is_64bar;
> > > +        uint32_t devfn;
> > > +        uint32_t bar_reg;
> > > +        uint64_t bar_sz;
> > >      } *bars = (struct bars *)scratch_start;
> > >      unsigned int i, nr_bars = 0;
> > >
> > > @@ -133,22 +139,34 @@
> > >          /* Map the I/O memory and port resources. */
> > >          for ( bar = 0; bar < 7; bar++ )
> > >          {
> > > +            bar_sz_upper = 0;
> > >              bar_reg = PCI_BASE_ADDRESS_0 + 4*bar;
> > >              if ( bar == 6 )
> > >                  bar_reg = PCI_ROM_ADDRESS;
> > >
> > >              bar_data = pci_readl(devfn, bar_reg);
> > > +            is_64bar = !!((bar_data & (PCI_BASE_ADDRESS_SPACE |
> > > +                         PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
> > > +                         (PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > +                         PCI_BASE_ADDRESS_MEM_TYPE_64));
> > >              pci_writel(devfn, bar_reg, ~0);
> > >              bar_sz = pci_readl(devfn, bar_reg);
> > >              pci_writel(devfn, bar_reg, bar_data);
> > > -            if ( bar_sz == 0 )
> > > -                continue;
> > >
> > >              bar_sz &= (((bar_data & PCI_BASE_ADDRESS_SPACE) ==
> > >                          PCI_BASE_ADDRESS_SPACE_MEMORY) ?
> > >                         PCI_BASE_ADDRESS_MEM_MASK :
> > >                         (PCI_BASE_ADDRESS_IO_MASK & 0xffff));
> > > +            if (is_64bar) {
> > > +                bar_data_upper = pci_readl(devfn, bar_reg + 4);
> > > +                pci_writel(devfn, bar_reg + 4, ~0);
> > > +                bar_sz_upper = pci_readl(devfn, bar_reg + 4);
> > > +                pci_writel(devfn, bar_reg + 4, bar_data_upper);
> > > +                bar_sz = (bar_sz_upper << 32) | bar_sz;
> > > +            }
> > >              bar_sz &= ~(bar_sz - 1);
> > > +            if ( bar_sz == 0 )
> > > +                continue;
> > >
> > >              for ( i = 0; i < nr_bars; i++ )
> > >                  if ( bars[i].bar_sz < bar_sz )
> > > @@ -157,6 +175,7 @@
> > >              if ( i != nr_bars )
> > >                  memmove(&bars[i+1], &bars[i], (nr_bars-i) *
> > sizeof(*bars));
> > >
> > > +            bars[i].is_64bar = is_64bar;
> > >              bars[i].devfn   = devfn;
> > >              bars[i].bar_reg = bar_reg;
> > >              bars[i].bar_sz  = bar_sz;
> > > @@ -167,11 +186,8 @@
> > >
> > >              nr_bars++;
> > >
> > > -            /* Skip the upper-half of the address for a 64-bit BAR. */
> > > -            if ( (bar_data & (PCI_BASE_ADDRESS_SPACE |
> > > -
> > PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
> > > -                 (PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > -                  PCI_BASE_ADDRESS_MEM_TYPE_64) )
> > > +            /*The upper half is already calculated, skip it! */
> > > +            if (is_64bar)
> > >                  bar++;
> > >          }
> > >
> > > @@ -197,6 +213,9 @@
> > >              ((pci_mem_start << 1) != 0) )
> > >          pci_mem_start <<= 1;
> > >
> > > +    if ( (pci_mem_start << 1) != 0 )
> > > +        bar64_relocate = 1;
> > > +
> > >      /* Relocate RAM that overlaps PCI space (in 64k-page chunks). */
> > >      while ( (pci_mem_start >> PAGE_SHIFT) <
> > hvm_info->low_mem_pgend )
> > >      {
> > > @@ -218,11 +237,15 @@
> > >          hvm_info->high_mem_pgend += nr_pages;
> > >      }
> > >
> > > +    high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend)
> > << PAGE_SHIFT;
> > > +    high_mem_resource.max = 1ull << cpu_phys_addr();
> > >      mem_resource.base = pci_mem_start;
> > >      mem_resource.max = pci_mem_end;
> > >      io_resource.base = 0xc000;
> > >      io_resource.max = 0x10000;
> > >
> > > +    mmio_left = pci_mem_end - pci_mem_start;
> > > +
> > >      /* Assign iomem and ioport resources in descending order of size. */
> > >      for ( i = 0; i < nr_bars; i++ )
> > >      {
> > > @@ -230,13 +253,29 @@
> > >          bar_reg = bars[i].bar_reg;
> > >          bar_sz  = bars[i].bar_sz;
> > >
> > > +        using_64bar = bars[i].is_64bar && bar64_relocate && (mmio_left
> > < bar_sz);
> > >          bar_data = pci_readl(devfn, bar_reg);
> > >
> > >          if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
> > >               PCI_BASE_ADDRESS_SPACE_MEMORY )
> > >          {
> > > -            resource = &mem_resource;
> > > -            bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> > > +            /* Mapping high memory if PCI deivce is 64 bits bar and the
> > bar size
> > > +               is larger than 512M */
> > > +            if (using_64bar && (bar_sz > PCI_MIN_BIG_BAR_SIZE)) {
> > > +                if ( high_mem_resource.base & (bar_sz - 1) )
> > > +                    high_mem_resource.base =
> > high_mem_resource.base -
> > > +                        (high_mem_resource.base & (bar_sz - 1)) +
> > bar_sz;
> > > +                else
> > > +                    high_mem_resource.base =
> > high_mem_resource.base -
> > > +                        (high_mem_resource.base & (bar_sz - 1));
> > > +                resource = &high_mem_resource;
> > > +                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> > > +            }
> > > +            else {
> > > +                resource = &mem_resource;
> > > +                bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
> > > +            }
> > > +            mmio_left -= bar_sz;
> > >          }
> > >          else
> > >          {
> > > @@ -244,13 +283,14 @@
> > >              bar_data &= ~PCI_BASE_ADDRESS_IO_MASK;
> > >          }
> > >
> > > -        base = (resource->base + bar_sz - 1) & ~(bar_sz - 1);
> > > -        bar_data |= base;
> > > +        base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
> > > +        bar_data |= (uint32_t)base;
> > > +        bar_data_upper = (uint32_t)(base >> 32);
> > >          base += bar_sz;
> > >
> > >          if ( (base < resource->base) || (base > resource->max) )
> > >          {
> > > -            printf("pci dev %02x:%x bar %02x size %08x: no space for "
> > > +            printf("pci dev %02x:%x bar %02x size %llx: no space for "
> > >                     "resource!\n", devfn>>3, devfn&7, bar_reg,
> > bar_sz);
> > >              continue;
> > >          }
> > > @@ -258,8 +298,8 @@
> > >          resource->base = base;
> > >
> > >          pci_writel(devfn, bar_reg, bar_data);
> > > -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
> > > -               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
> > > +        if (using_64bar)
> > > +            pci_writel(devfn, bar_reg + 4, bar_data_upper);
> > >
> > >          /* Now enable the memory or I/O mapping. */
> > >          cmd = pci_readw(devfn, PCI_COMMAND);
> > > diff -r dc56a9defa30 tools/firmware/hvmloader/util.h
> > > --- a/tools/firmware/hvmloader/util.h	Tue Aug 14 10:28:14 2012 +0200
> > > +++ b/tools/firmware/hvmloader/util.h	Wed Sep 12 14:50:55 2012 +0800
> > > @@ -215,6 +215,7 @@
> > >  uint32_t rombios_highbios_setup(void);
> > >
> > >  /* Miscellaneous. */
> > > +unsigned int cpu_phys_addr(void);
> > >  void cacheattr_init(void);
> > >  unsigned long create_mp_tables(void *table);
> > >  void hvm_write_smbios_tables(
> > 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] xen/tools: Add 64 bits big bar support
  2012-09-25  3:33 [PATCH v2] xen/tools: Add 64 bits big bar support Unknown
  2012-09-25  8:31 ` Ian Campbell
@ 2012-09-25 14:13 ` Keir Fraser
  2012-09-26  0:58   ` Hao, Xudong
  1 sibling, 1 reply; 7+ messages in thread
From: Keir Fraser @ 2012-09-25 14:13 UTC (permalink / raw)
  To: xen-devel
  Cc: Xiantao Zhang, Xudong Hao, ian.campbell, jbeulich,
	Stefano Stabellini




On 25/09/2012 04:33, "" <> wrote:

> Currently it is assumed PCI device BAR access < 4G memory. If there is such a
> device whose BAR size is larger than 4G, it must access > 4G memory address.
> This patch enable the 64bits big BAR support on hvmloader.
> 
> Changes from v1:
> 1) Set Dynamic MMIO high memory address instead of a fixed number 640G
> 2) Mask bar_sz earlier to avoid older code changes
> 3) Add bar size barrier to judge high memory resource
> 4) Clean up bar64_relocate code
> 
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>

...

> @@ -258,8 +298,8 @@
>          resource->base = base;
>  
>          pci_writel(devfn, bar_reg, bar_data);
> -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
> -               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
> +        if (using_64bar)
> +            pci_writel(devfn, bar_reg + 4, bar_data_upper);

Why is the printf removed?

 -- Keir


>          /* Now enable the memory or I/O mapping. */
>          cmd = pci_readw(devfn, PCI_COMMAND);
> diff -r dc56a9defa30 tools/firmware/hvmloader/util.h
> --- a/tools/firmware/hvmloader/util.h Tue Aug 14 10:28:14 2012 +0200
> +++ b/tools/firmware/hvmloader/util.h Wed Sep 12 14:50:55 2012 +0800
> @@ -215,6 +215,7 @@
>  uint32_t rombios_highbios_setup(void);
>  
>  /* Miscellaneous. */
> +unsigned int cpu_phys_addr(void);
>  void cacheattr_init(void);
>  unsigned long create_mp_tables(void *table);
>  void hvm_write_smbios_tables(
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] xen/tools: Add 64 bits big bar support
  2012-09-25 14:13 ` Keir Fraser
@ 2012-09-26  0:58   ` Hao, Xudong
  2012-09-26  6:25     ` Keir Fraser
  0 siblings, 1 reply; 7+ messages in thread
From: Hao, Xudong @ 2012-09-26  0:58 UTC (permalink / raw)
  To: Keir Fraser, xen-devel@lists.xen.org
  Cc: Zhang, Xiantao, ian.campbell@citrix.com, jbeulich@suse.com,
	Stefano Stabellini

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com] On Behalf Of Keir Fraser
> Sent: Tuesday, September 25, 2012 10:13 PM
> To: xen-devel@lists.xen.org
> Cc: ian.campbell@citrix.com; Stefano Stabellini; Hao, Xudong;
> jbeulich@suse.com; Zhang, Xiantao
> Subject: Re: [Xen-devel] [PATCH v2] xen/tools: Add 64 bits big bar support
> 
> 
> 
> 
> On 25/09/2012 04:33, "" <> wrote:
> 
> > Currently it is assumed PCI device BAR access < 4G memory. If there is such a
> > device whose BAR size is larger than 4G, it must access > 4G memory
> address.
> > This patch enable the 64bits big BAR support on hvmloader.
> >
> > Changes from v1:
> > 1) Set Dynamic MMIO high memory address instead of a fixed number 640G
> > 2) Mask bar_sz earlier to avoid older code changes
> > 3) Add bar size barrier to judge high memory resource
> > 4) Clean up bar64_relocate code
> >
> > Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> 
> ...
> 
> > @@ -258,8 +298,8 @@
> >          resource->base = base;
> >
> >          pci_writel(devfn, bar_reg, bar_data);
> > -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
> > -               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
> > +        if (using_64bar)
> > +            pci_writel(devfn, bar_reg + 4, bar_data_upper);
> 
> Why is the printf removed?
> 

No special reason. It's a successful print, I'll modify patch if we want to remain it.

>  -- Keir
> 
> 
> >          /* Now enable the memory or I/O mapping. */
> >          cmd = pci_readw(devfn, PCI_COMMAND);
> > diff -r dc56a9defa30 tools/firmware/hvmloader/util.h
> > --- a/tools/firmware/hvmloader/util.h Tue Aug 14 10:28:14 2012 +0200
> > +++ b/tools/firmware/hvmloader/util.h Wed Sep 12 14:50:55 2012 +0800
> > @@ -215,6 +215,7 @@
> >  uint32_t rombios_highbios_setup(void);
> >
> >  /* Miscellaneous. */
> > +unsigned int cpu_phys_addr(void);
> >  void cacheattr_init(void);
> >  unsigned long create_mp_tables(void *table);
> >  void hvm_write_smbios_tables(
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] xen/tools: Add 64 bits big bar support
  2012-09-26  0:58   ` Hao, Xudong
@ 2012-09-26  6:25     ` Keir Fraser
  0 siblings, 0 replies; 7+ messages in thread
From: Keir Fraser @ 2012-09-26  6:25 UTC (permalink / raw)
  To: Hao, Xudong, xen-devel@lists.xen.org
  Cc: Zhang, Xiantao, ian.campbell@citrix.com, jbeulich@suse.com,
	Stefano Stabellini

On 26/09/2012 01:58, "Hao, Xudong" <xudong.hao@intel.com> wrote:

>>> @@ -258,8 +298,8 @@
>>>          resource->base = base;
>>> 
>>>          pci_writel(devfn, bar_reg, bar_data);
>>> -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
>>> -               devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
>>> +        if (using_64bar)
>>> +            pci_writel(devfn, bar_reg + 4, bar_data_upper);
>> 
>> Why is the printf removed?
>> 
> 
> No special reason. It's a successful print, I'll modify patch if we want to
> remain it.

Yes please.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-09-26  6:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-25  3:33 [PATCH v2] xen/tools: Add 64 bits big bar support Unknown
2012-09-25  8:31 ` Ian Campbell
2012-09-25  9:14   ` Hao, Xudong
2012-09-25  9:25     ` Ian Campbell
2012-09-25 14:13 ` Keir Fraser
2012-09-26  0:58   ` Hao, Xudong
2012-09-26  6:25     ` Keir Fraser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).