[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH V5 16/29] pci: 64bit bar support.
From: |
Isaku Yamahata |
Subject: |
[Qemu-devel] Re: [PATCH V5 16/29] pci: 64bit bar support. |
Date: |
Tue, 13 Oct 2009 22:52:54 +0900 |
User-agent: |
Mutt/1.5.6i |
On Sat, Oct 10, 2009 at 09:39:29PM +0200, Michael S. Tsirkin wrote:
> On Fri, Oct 09, 2009 at 03:28:49PM +0900, Isaku Yamahata wrote:
> > implemented pci 64bit bar support.
> >
> > Signed-off-by: Isaku Yamahata <address@hidden>
> > ---
> > hw/pci.c | 34 +++++++++++++++++++++++++++++-----
> > hw/pci.h | 2 ++
> > 2 files changed, 31 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index e7f8fb4..ee3ab05 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -454,6 +454,13 @@ static int pci_unregister_device(DeviceState *dev)
> > return 0;
> > }
> >
> > +static inline int pci_bar_is_mem64(const PCIIORegion *r)
> > +{
> > + return !(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> > + (r->type & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
> > + PCI_BASE_ADDRESS_MEM_TYPE_64;
> > +}
> > +
>
> This should be just
> return r->type & PCI_BASE_ADDRESS_MEM_TYPE_64
> and then this can be open-coded.
> PCI_BASE_ADDRESS_MEM_TYPE_MASK will also not be needed.
r->type & PCI_BASE_ADDRESS_MEM_TYPE_64 can be true for IO space.
For example, bar of IO space base with size 4 is 0x5.
Thus PCI_BASE_ADDRESS_SPACE_IO check is necessary.
> I have a feeling I said this already, and the spec is pretty
> clear: there is exactly 1 bit that means 64 bit memory.
> No reason to check anything else.
It isn't to me because the spec says that (Table 6-4 page 226)
bit[2:3]
00 32bit
01 reserved(obsolete below 1M)
10 64bit
11 reserved
Although 11 isn't likely to be used in the future, it is safe to use mask.
> > void pci_register_bar(PCIDevice *pci_dev, int region_num,
> > pcibus_t size, int type,
> > PCIMapIORegionFunc *map_func)
> > @@ -484,8 +491,13 @@ void pci_register_bar(PCIDevice *pci_dev, int
> > region_num,
> > wmask |= PCI_ROM_ADDRESS_ENABLE;
> > }
> > pci_set_long(pci_dev->config + addr, type);
> > - pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> > - pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> > + if (pci_bar_is_mem64(r)) {
> > + pci_set_quad(pci_dev->wmask + addr, wmask);
> > + pci_set_quad(pci_dev->cmask + addr, ~0ULL);
> > + } else {
> > + pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> > + pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> > + }
> > }
> >
> > static void pci_update_mappings(PCIDevice *d)
> > @@ -513,7 +525,11 @@ static void pci_update_mappings(PCIDevice *d)
> > }
> > } else {
> > if (cmd & PCI_COMMAND_MEMORY) {
> > - new_addr = pci_get_long(d->config + pci_bar(d, i));
> > + if (pci_bar_is_mem64(r)) {
> > + new_addr = pci_get_quad(d->config + pci_bar(d, i));
> > + } else {
> > + new_addr = pci_get_long(d->config + pci_bar(d, i));
> > + }
> > /* the ROM slot has a specific enable bit */
> > if (i == PCI_ROM_SLOT && !(new_addr &
> > PCI_ROM_ADDRESS_ENABLE))
> > goto no_mem_map;
> > @@ -531,7 +547,7 @@ static void pci_update_mappings(PCIDevice *d)
> > * Without this, PC ide doesn't work well.
> > * TODO: remove this work around.
> > */
> > - last_addr >= UINT32_MAX) {
> > + (!pci_bar_is_mem64(r) && last_addr >= UINT32_MAX))
> > {
> > new_addr = PCI_BAR_UNMAPPED;
> > }
> > } else {
> > @@ -773,8 +789,16 @@ static void pci_info_device(PCIDevice *d)
> > " [0x%04"FMT_PCIBUS"].\n",
> > r->addr, r->addr + r->size - 1);
> > } else {
> > - monitor_printf(mon, "32 bit memory at 0x%08"FMT_PCIBUS
> > + const char *type = pci_bar_is_mem64(r)? "64 bit": "32 bit";
> > + const char *prefetch = "";
> > +
> > + if (r->type & PCI_BASE_ADDRESS_MEM_PREFETCH) {
> > + prefetch = " prefetchable";
> > + }
> > +
> > + monitor_printf(mon, "%s%s memory at 0x%08"FMT_PCIBUS
> > " [0x%08"FMT_PCIBUS"].\n",
> > + type, prefetch,
> > r->addr, r->addr + r->size - 1);
> > }
> > }
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 8fbd45e..269981a 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -114,6 +114,8 @@ typedef struct PCIIORegion {
> > #define PCI_BASE_ADDRESS_0 0x10 /* 32 bits */
> > #define PCI_BASE_ADDRESS_SPACE_IO 0x01
> > #define PCI_BASE_ADDRESS_SPACE_MEMORY 0x00
> > +#define PCI_BASE_ADDRESS_MEM_TYPE_MASK 0x06
> > +#define PCI_BASE_ADDRESS_MEM_TYPE_64 0x04 /* 64 bit address */
> > #define PCI_BASE_ADDRESS_MEM_PREFETCH 0x08 /* prefetchable? */
> > #define PCI_PRIMARY_BUS 0x18 /* Primary bus number */
> > #define PCI_SECONDARY_BUS 0x19 /* Secondary bus number */
>
--
yamahata
[Qemu-devel] [PATCH V5 22/29] vmstate: add VMSTATE_ARRAY_POINTER for pointer to array., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 23/29] pci: pcie host and mmcfg support., Isaku Yamahata, 2009/10/09