[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH V6 28/32] pci: initialize pci config headers dep
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH V6 28/32] pci: initialize pci config headers depending it pci header type. |
Date: |
Tue, 3 Nov 2009 16:27:18 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Fri, Oct 30, 2009 at 09:21:22PM +0900, Isaku Yamahata wrote:
> - Only sets default subsystem id for header type 00.(normal header type)
> because header type 01 doesn't have subsystem id, and uses the register
> for other purpose. So setting default subsystem id doesn't make sense.
>
> - initialize wmask more for header type 01.(bridge header type)
> Without those wmasks, linux was confused not boot,
> and lspci was confused not to print out expected IO/memory range.
>
> Signed-off-by: Isaku Yamahata <address@hidden>
> ---
> hw/cirrus_vga.c | 1 -
> hw/pci.c | 42 ++++++++++++++++++++++++++++++++++++++----
> hw/pci.h | 29 +++++++++++++++++++++++++++++
> 3 files changed, 67 insertions(+), 5 deletions(-)
>
> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index ef72c62..c1bafd3 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -180,7 +180,6 @@
> #define PCI_COMMAND_PALETTESNOOPING 0x0020
> #define PCI_COMMAND_PARITYDETECTION 0x0040
> #define PCI_COMMAND_ADDRESSDATASTEPPING 0x0080
> -#define PCI_COMMAND_SERR 0x0100
> #define PCI_COMMAND_BACKTOBACKTRANS 0x0200
> // PCI 0x08, 0xff000000 (0x09-0x0b:class,0x08:rev)
> #define PCI_CLASS_BASE_DISPLAY 0x03
> diff --git a/hw/pci.c b/hw/pci.c
> index 46b22ec..beefae3 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -445,6 +445,30 @@ static void pci_init_wmask(PCIDevice *dev)
> dev->wmask[i] = 0xff;
> }
>
> +static void pci_init_wmask_bridge(PCIDevice *d)
> +{
> + /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
> + PCI_SEC_LETENCY_TIMER */
> + memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
> +
> + /* base and limit */
> + d->wmask[PCI_IO_BASE] = PCI_IO_RANGE_MASK & 0xff;
> + d->wmask[PCI_IO_LIMIT] = PCI_IO_RANGE_MASK & 0xff;
> + pci_set_word(d->wmask + PCI_MEMORY_BASE,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_set_word(d->wmask + PCI_MEMORY_LIMIT,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_set_word(d->wmask + PCI_PREF_MEMORY_BASE,
> + PCI_PREF_RANGE_MASK & 0xffff);
> + pci_set_word(d->wmask + PCI_PREF_MEMORY_LIMIT,
> + PCI_PREF_RANGE_MASK & 0xffff);
> +
> + /* PCI_PREF_BASE_UPPER32 and PCI_PREF_LIMIT_UPPER32 */
> + memset(d->wmask + PCI_PREF_BASE_UPPER32, 0xff, 8);
> +
> + pci_set_word(d->wmask + PCI_BRIDGE_CONTROL, 0xffff);
> +}
> +
> static void pci_config_alloc(PCIDevice *pci_dev)
> {
> int config_size = pci_config_size(pci_dev);
> @@ -467,7 +491,8 @@ static void pci_config_free(PCIDevice *pci_dev)
> static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> const char *name, int devfn,
> PCIConfigReadFunc *config_read,
> - PCIConfigWriteFunc *config_write)
> + PCIConfigWriteFunc *config_write,
> + uint8_t header_type)
> {
> if (devfn < 0) {
> for(devfn = bus->devfn_min ; devfn < 256; devfn += 8) {
> @@ -484,9 +509,16 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
> memset(pci_dev->irq_state, 0, sizeof(pci_dev->irq_state));
> pci_config_alloc(pci_dev);
> - pci_set_default_subsystem_id(pci_dev);
> +
> + header_type &= ~PCI_HEADER_TYPE_MULTI_FUNCTION;
> + if (header_type == PCI_HEADER_TYPE_NORMAL) {
> + pci_set_default_subsystem_id(pci_dev);
> + }
> pci_init_cmask(pci_dev);
> pci_init_wmask(pci_dev);
> + if (header_type == PCI_HEADER_TYPE_BRIDGE) {
> + pci_init_wmask_bridge(pci_dev);
> + }
>
> if (!config_read)
> config_read = pci_default_read_config;
Instead of this, can we have pci_init_bridge that will simply
be called after device has been initialized?
Down the road we will be able to move it to pci_bridge.c
> @@ -509,7 +541,8 @@ PCIDevice *pci_register_device(PCIBus *bus, const char
> *name,
>
> pci_dev = qemu_mallocz(instance_size);
> pci_dev = do_pci_register_device(pci_dev, bus, name, devfn,
> - config_read, config_write);
> + config_read, config_write,
> + PCI_HEADER_TYPE_NORMAL);
> return pci_dev;
> }
> static target_phys_addr_t pci_to_cpu_addr(target_phys_addr_t addr)
> @@ -1059,7 +1092,8 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo
> *base)
> bus = FROM_QBUS(PCIBus, qdev_get_parent_bus(qdev));
> devfn = pci_dev->devfn;
> pci_dev = do_pci_register_device(pci_dev, bus, base->name, devfn,
> - info->config_read, info->config_write);
> + info->config_read, info->config_write,
> + info->header_type);
> assert(pci_dev);
> rc = info->init(pci_dev);
> if (rc != 0)
> diff --git a/hw/pci.h b/hw/pci.h
> index 7991bfd..3f50294 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -100,6 +100,14 @@ typedef struct PCIIORegion {
> #define PCI_COMMAND_IO 0x1 /* Enable response in I/O space
> */
> #define PCI_COMMAND_MEMORY 0x2 /* Enable response in Memory space */
> #define PCI_COMMAND_MASTER 0x4 /* Enable bus master */
> +#define PCI_COMMAND_SPECIAL 0x8 /* Enable response to special cycles */
> +#define PCI_COMMAND_INVALIDATE 0x10 /* Use memory write and invalidate */
> +#define PCI_COMMAND_VGA_PALETTE 0x20 /* Enable palette snooping */
> +#define PCI_COMMAND_PARITY 0x40 /* Enable parity checking */
> +#define PCI_COMMAND_WAIT 0x80 /* Enable address/data stepping */
> +#define PCI_COMMAND_SERR 0x100 /* Enable SERR */
> +#define PCI_COMMAND_FAST_BACK 0x200 /* Enable back-to-back writes */
> +#define PCI_COMMAND_INTX_DISABLE 0x400 /* INTx Emulation Disable */
> #define PCI_STATUS 0x06 /* 16 bits */
> #define PCI_REVISION_ID 0x08 /* 8 bits */
> #define PCI_CLASS_PROG 0x09 /* Reg. Level Programming
> Interface */
> @@ -119,16 +127,30 @@ typedef struct PCIIORegion {
> #define PCI_PRIMARY_BUS 0x18 /* Primary bus number */
> #define PCI_SECONDARY_BUS 0x19 /* Secondary bus number */
> #define PCI_SUBORDINATE_BUS 0x1a /* Highest bus number behind the bridge
> */
> +#define PCI_SEC_LATENCY_TIMER 0x1b /* Latency timer for secondary
> interface */
> +#define PCI_IO_BASE 0x1c /* I/O range behind the bridge */
> +#define PCI_IO_LIMIT 0x1d
> +#define PCI_IO_RANGE_MASK (~0x0fUL)
> #define PCI_SEC_STATUS 0x1e /* Secondary status register,
> only bit 14 used */
> +#define PCI_MEMORY_BASE 0x20 /* Memory range behind */
> +#define PCI_MEMORY_LIMIT 0x22
> +#define PCI_MEMORY_RANGE_MASK (~0x0fUL)
> +#define PCI_PREF_MEMORY_BASE 0x24 /* Prefetchable memory range behind
> */
> +#define PCI_PREF_MEMORY_LIMIT 0x26
> +#define PCI_PREF_RANGE_MASK (~0x0fUL)
> +#define PCI_PREF_BASE_UPPER32 0x28 /* Upper half of prefetchable memory
> range */
> #define PCI_SUBSYSTEM_VENDOR_ID 0x2c /* 16 bits */
> #define PCI_SUBSYSTEM_ID 0x2e /* 16 bits */
> #define PCI_ROM_ADDRESS 0x30 /* Bits 31..11 are address,
> 10..1 reserved */
> #define PCI_ROM_ADDRESS_ENABLE 0x01
> +#define PCI_IO_BASE_UPPER16 0x30 /* Upper half of I/O addresses */
> +#define PCI_IO_LIMIT_UPPER16 0x32
> #define PCI_CAPABILITY_LIST 0x34 /* Offset of first capability list
> entry */
> #define PCI_ROM_ADDRESS1 0x38 /* Same as PCI_ROM_ADDRESS, but for
> htype 1 */
> #define PCI_INTERRUPT_LINE 0x3c /* 8 bits */
> #define PCI_INTERRUPT_PIN 0x3d /* 8 bits */
> #define PCI_MIN_GNT 0x3e /* 8 bits */
> +#define PCI_BRIDGE_CONTROL 0x3e
> #define PCI_MAX_LAT 0x3f /* 8 bits */
>
> /* Capability lists */
Many of these are unused. I'd rather we only defined what we use.
> @@ -358,6 +380,13 @@ typedef struct {
> PCIConfigReadFunc *config_read;
> PCIConfigWriteFunc *config_write;
>
> + /* pci config header type */
> + uint8_t header_type; /* this is necessary for initialization
> + * code to know its header type before
> + * device specific code can initialize
> + * configuration space.
> + */
The text probably belongs in commit comment.
Comments that tell how field is used quickly get out of date.
Say *what* it is is enough.
> +
> /* pcie stuff */
> int is_express; /* is this device pci express?
> * initialization code needs to know this before
> --
> 1.6.0.2
- [Qemu-devel] Re: [PATCH V6 28/32] pci: initialize pci config headers depending it pci header type.,
Michael S. Tsirkin <=