Re: [Xen-devel] [PATCH v3] xen/tools: Add 64 bits big bar support

["Hao, Xudong" <xudong.hao@xxxxxxxxx>]

Oh, I got it, I miss a latest update. Thanks!

Thanks,
-Xudong


> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> Sent: Monday, October 08, 2012 8:35 PM
> To: Hao, Xudong
> Cc: Keir (Xen.org); xen-devel@xxxxxxxxxxxxx; Stefano Stabellini;
> jbeulich@xxxxxxxx; Zhang, Xiantao
> Subject: Re: [PATCH v3] xen/tools: Add 64 bits big bar support
> 
> Didn't Keir already check this in last week?
> 
> On Mon, 2012-10-08 at 13:31 +0100, Hao, Xudong wrote:
> > Hi, Ian and Keir
> >
> > Do you have other comments for this patch?
> >
> > Thanks,
> > -Xudong
> >
> > > -----Original Message-----
> > > From: Hao, Xudong
> > > Sent: Wednesday, September 26, 2012 4:36 PM
> > > To: ian.campbell@xxxxxxxxxx; keir@xxxxxxx
> > > Cc: xen-devel@xxxxxxxxxxxxx; Stefano.Stabellini@xxxxxxxxxxxxx;
> > > jbeulich@xxxxxxxx; Zhang, Xiantao; Hao, Xudong
> > > Subject: [PATCH v3] xen/tools: Add 64 bits big bar support
> > >
> > > 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.
> > >
> > > v3 changes from v2:
> > > - Remain original print information
> > >
> > > v2 changes from v1 as comments by Jan.
> > > 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@xxxxxxxxx>
> > > Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx>
> > > ---
> > >  tools/firmware/hvmloader/cacheattr.c |   25 +++++++----
> > >  tools/firmware/hvmloader/config.h    |    2 +
> > >  tools/firmware/hvmloader/pci.c       |   77
> > > ++++++++++++++++++++++++++-------
> > >  tools/firmware/hvmloader/util.h      |    1 +
> > >  4 files changed, 80 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/tools/firmware/hvmloader/cacheattr.c
> > > b/tools/firmware/hvmloader/cacheattr.c
> > > index 646f07f..592b455 100644
> > > --- a/tools/firmware/hvmloader/cacheattr.c
> > > +++ b/tools/firmware/hvmloader/cacheattr.c
> > > @@ -40,24 +40,33 @@
> > >  #define MSR_PAT              0x0277
> > >  #define MSR_MTRRdefType      0x02ff
> > >
> > > +unsigned int cpu_phys_addr(void)
> > > +{
> > > +    uint32_t eax, ebx, ecx, edx;
> > > +    unsigned int phys_bits = 36;
> > > +    /* Find the physical address size for this CPU. */
> > > +    cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
> > > +    if ( eax >= 0x80000008 )
> > > +    {
> > > +        cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
> > > +        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 = 36;
> > > +    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;
> > >
> > > -    /* Find the physical address size for this CPU. */
> > > -    cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
> > > -    if ( eax >= 0x80000008 )
> > > -    {
> > > -        cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
> > > -        phys_bits = (uint8_t)eax;
> > > -    }
> > > +    phys_bits = cpu_phys_addr();
> > >
> > >      printf("%u-bit phys ... ", phys_bits);
> > >
> > > diff --git a/tools/firmware/hvmloader/config.h
> > > b/tools/firmware/hvmloader/config.h
> > > index 7c0180d..bbf2993 100644
> > > --- a/tools/firmware/hvmloader/config.h
> > > +++ b/tools/firmware/hvmloader/config.h
> > > @@ -53,6 +53,8 @@ extern struct bios_config ovmf_config;
> > >  /* 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 --git a/tools/firmware/hvmloader/pci.c
> b/tools/firmware/hvmloader/pci.c
> > > index fd56e50..0500db5 100644
> > > --- a/tools/firmware/hvmloader/pci.c
> > > +++ b/tools/firmware/hvmloader/pci.c
> > > @@ -36,19 +36,25 @@ unsigned long igd_opregion_pgbase = 0;
> > >
> > >  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 @@ void pci_setup(void)
> > >          /* 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 @@ void pci_setup(void)
> > >              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 @@ void pci_setup(void)
> > >
> > >              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 @@ void pci_setup(void)
> > >              ((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 @@ void pci_setup(void)
> > >          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 @@ void pci_setup(void)
> > >          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 @@ void pci_setup(void)
> > >              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,11 @@ void pci_setup(void)
> > >          resource->base = base;
> > >
> > >          pci_writel(devfn, bar_reg, bar_data);
> > > -        printf("pci dev %02x:%x bar %02x size %08x: %08x\n",
> > > +        if (using_64bar)
> > > +            pci_writel(devfn, bar_reg + 4, bar_data_upper);
> > > +        printf("pci dev %02x:%x bar %02x size %llx: %08x\n",
> > >                 devfn>>3, devfn&7, bar_reg, bar_sz, bar_data);
> > > +
> > >
> > >          /* Now enable the memory or I/O mapping. */
> > >          cmd = pci_readw(devfn, PCI_COMMAND);
> > > diff --git a/tools/firmware/hvmloader/util.h
> b/tools/firmware/hvmloader/util.h
> > > index 07a9d42..ff06071 100644
> > > --- a/tools/firmware/hvmloader/util.h
> > > +++ b/tools/firmware/hvmloader/util.h
> > > @@ -215,6 +215,7 @@ void pci_setup(void);
> > >  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(
> > > --
> > > 1.5.5
> >
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel