Re: [Qemu-devel] [PATCH 5/9] pci: 64bit bar support.

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jul 15, 2009 at 08:15:05PM +0900, Isaku Yamahata wrote:
> implemented pci 64bit bar support.

Minor style suggestions below.
Acked-by: Michael S. Tsirkin <mst@redhat.com>


> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/pci.c |   46 ++++++++++++++++++++++++++++++++++++++++------
>  hw/pci.h |    9 +++++++++
>  2 files changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 2d985f0..9639a32 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -429,9 +429,15 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>      } else {
>          addr = 0x10 + region_num * 4;
>      }
> +

don't add empty lines after } - block stands out enough already.

>      *(uint32_t *)(pci_dev->config + addr) = cpu_to_le32(type);
> -    *(uint32_t *)(pci_dev->wmask + addr) = cpu_to_le32(wmask & 0xffffffff);
> -    *(uint32_t *)(pci_dev->cmask + addr) = 0xffffffff;
> +    if (pci_bar_is_64bit(r)) {
> +        *(uint64_t *)(pci_dev->wmask + addr) = cpu_to_le64(wmask);
> +        *(uint64_t *)(pci_dev->cmask + addr) = ~0ULL;
> +    } else {
> +        *(uint32_t *)(pci_dev->wmask + addr) = cpu_to_le32(wmask & 0xffffffff);
> +        *(uint32_t *)(pci_dev->cmask + addr) = 0xffffffff;
> +    }
>  }
>  
>  static void pci_update_mappings(PCIDevice *d)
> @@ -466,8 +472,14 @@ static void pci_update_mappings(PCIDevice *d)
>                  }
>              } else {
>                  if (cmd & PCI_COMMAND_MEMORY) {
> -                    new_addr = le32_to_cpu(*(uint32_t *)(d->config +
> -                                                         config_ofs));
> +
> +                    if (pci_bar_is_64bit(r)) {
> +                        new_addr = le64_to_cpu(*(uint64_t *)(d->config +
> +                                                             config_ofs));

Use pci_get_quad here?

> +                    } else {
> +                        new_addr = le32_to_cpu(*(uint32_t *)(d->config +
> +                                                             config_ofs));
> +                    }
>                      /* the ROM slot has a specific enable bit */
>                      if (i == PCI_ROM_SLOT && !(new_addr & 1))
>                          goto no_mem_map;
> @@ -482,7 +494,7 @@ static void pci_update_mappings(PCIDevice *d)
>  
>                          /* keep old behaviour
>                           * without this, PC ide doesn't work well. */
> -                        last_addr >= UINT32_MAX) {
> +                        (!pci_bar_is_64bit(r) && last_addr >= UINT32_MAX)) {
>                          new_addr = PCI_BAR_UNMAPPED;
>                      }
>                  } else {
> @@ -741,7 +753,29 @@ static void pci_info_device(PCIDevice *d)
>                  monitor_printf(mon, "I/O at 0x%04"PRIx64" [0x%04"PRIx64"].\n",
>                                 r->addr, r->addr + r->size - 1);
>              } else {
> -                monitor_printf(mon, "32 bit memory at 0x%08"PRIx64" [0x%08"PRIx64"].\n",
> +                const char *type;
> +                const char* prefetch;
> +
> +                switch (r->type & PCI_ADDRESS_SPACE_MEM_TYPE_MASK) {
> +                case PCI_ADDRESS_SPACE_MEM:
> +                    type = "32 bit";
> +                    break;
> +                case PCI_ADDRESS_SPACE_MEM_64:
> +                    type = "64 bit";
> +                    break;
> +                default:
> +                    type = "unknown";
> +                    break;
> +                }
> +
> +                prefetch = "";
> +                if (r->type & PCI_ADDRESS_SPACE_MEM_PREFETCH) {
> +                    prefetch = " prefetchable";
> +                }
> +
> +                monitor_printf(mon, "%s%s memory at "
> +                               "0x%08"PRIx64" [0x%08"PRIx64"].\n",
> +                               type, prefetch,
>                                 r->addr, r->addr + r->size - 1);
>              }
>          }
> diff --git a/hw/pci.h b/hw/pci.h
> index fee9ed6..33e2ef2 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -80,6 +80,8 @@ typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
>  
>  #define PCI_ADDRESS_SPACE_MEM		0x00
>  #define PCI_ADDRESS_SPACE_IO		0x01
> +#define PCI_ADDRESS_SPACE_MEM_64        0x04    /* 64 bit address */
> +#define PCI_ADDRESS_SPACE_MEM_TYPE_MASK 0x06
>  #define PCI_ADDRESS_SPACE_MEM_PREFETCH	0x08
>  
>  typedef struct PCIIORegion {
> @@ -90,6 +92,13 @@ typedef struct PCIIORegion {
>      PCIMapIORegionFunc *map_func;
>  } PCIIORegion;
>  
> +static inline int pci_bar_is_64bit(const PCIIORegion *r)
> +{
> +    return !(r->type & PCI_ADDRESS_SPACE_IO) &&
> +        ((r->type & PCI_ADDRESS_SPACE_MEM_TYPE_MASK) ==
> +         PCI_ADDRESS_SPACE_MEM_64);
> +}
> +
>  #define PCI_ROM_SLOT 6
>  #define PCI_NUM_REGIONS 7
>  
> -- 
> 1.6.0.2
> 
>