Skip to contentSkip to conversation

add pci config space struct

PATCH In review Gerd Hoffmann 3 participants 7 messages Aug 26, 2008 – Aug 27, 2008

lore
Share & tools
lore

Gerd Hoffmann

author
#0 lore raw

PATCH 1/2 add pci config space struct

Hi,

This patch adds a struct for the pci config header make it easier to deal with it. Comes from qemu-xen.

cheers,
Gerd

Quoted 1 line

From b46342e5ea1c1d47b040becc2352ea03c9b3a33d Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <[email protected]>
Date: Tue, 26 Aug 2008 12:11:53 +0200
Subject: [PATCH] pci: add config space struct (from qemu-xen).

Signed-off-by Gerd Hoffmann <[email protected]>
---
 hw/pci.h |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)
hw/pci.h +26 -0
index e870987..2b1ebba 100644
@@ -6,6 +6,32 @@
 
 /* PCI bus */
 
+struct pci_config_header {
+    uint16_t vendor_id;
+    uint16_t device_id;
+    uint16_t command;
+    uint16_t status;
+    uint8_t  revision;
+    uint8_t  api;
+    uint8_t  subclass;
+    uint8_t  class;
+    uint8_t  cache_line_size; /* Units of 32 bit words */
+    uint8_t  latency_timer;   /* In units of bus cycles */
+    uint8_t  header_type;     /* Should be 0 */
+    uint8_t  bist;            /* Built in self test */
+    uint32_t base_address_regs[6];
+    uint32_t reserved1;
+    uint16_t sub_vendor_id;
+    uint16_t sub_device_id;
+    uint32_t rom_addr;
+    uint32_t reserved3;
+    uint32_t reserved4;
+    uint8_t  interrupt_line;
+    uint8_t  interrupt_pin;
+    uint8_t  min_gnt;
+    uint8_t  max_lat;
+};
+
 extern target_phys_addr_t pci_mem_base;
 
 typedef void PCIConfigWriteFunc(PCIDevice *pci_dev,
Signature · 1 line
1.5.5.1
#1 lore raw
Quoted 23 lines · Gerd Hoffmann

Hi,

This patch adds a struct for the pci config header make it easier to deal with it. Comes from qemu-xen.

cheers,
Gerd

hw/pci.h +15 -0
+struct pci_config_header {
+    uint16_t vendor_id;
+    uint16_t device_id;
+    uint16_t command;
+    uint16_t status;
+    uint8_t  revision;
+    uint8_t  api;
+    uint8_t  subclass;
+    uint8_t  class;
+    uint8_t  cache_line_size; /* Units of 32 bit words */
+    uint8_t  latency_timer;   /* In units of bus cycles */
+    uint8_t  header_type;     /* Should be 0 */
+    uint8_t  bist;            /* Built in self test */
+    uint32_t base_address_regs[6];
+    uint32_t reserved1;

hw/pci.h line 33

Quoted 10 lines
hw/pci.h +10 -0
+    uint16_t sub_vendor_id;
+    uint16_t sub_device_id;
+    uint32_t rom_addr;
+    uint32_t reserved3;
+    uint32_t reserved4;
+    uint8_t  interrupt_line;
+    uint8_t  interrupt_pin;
+    uint8_t  min_gnt;
+    uint8_t  max_lat;
+};

Shouldn't little-endian types be used here to force the users to use little-endian accessors?

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#2 lore raw
Quoted 6 lines · Avi Kivity
Quoted 2 lines · Gerd Hoffmann

+struct pci_config_header {
+ uint16_t vendor_id;

Shouldn't little-endian types be used here to force the users to use little-endian accessors?

Didn't notice qemu has that. Which header file I should check? There is nothing in bswap.h ...

thanks,
Gerd

#3 lore raw
Quoted 7 lines · Gerd Hoffmann
Quoted 3 lines

Shouldn't little-endian types be used here to force the users to use little-endian accessors?

Didn't notice qemu has that. Which header file I should check? There is nothing in bswap.h ...

I don't think qemu has them; it should though.

I'd define le32 (and friends) as a struct to ensure it is impossible to misuse.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#4 lore raw
Quoted 9 lines · Avi Kivity
Quoted 6 lines · Gerd Hoffmann
Quoted 3 lines

Shouldn't little-endian types be used here to force the users to use little-endian accessors?

Didn't notice qemu has that. Which header file I should check? There is nothing in bswap.h ...

I don't think qemu has them; it should though.

Adding byteordered types is a completely separate issue IMHO.

Quoted 2 lines

I'd define le32 (and friends) as a struct to ensure it is impossible to misuse.

Sure, that is the usual way ;)

I expected you suggest to just use something which is present already though.

cheers,
Gerd

#5 lore raw
In reply to Gerd Hoffmann
Quoted 4 lines · Gerd Hoffmann

Hi,

This patch adds a struct for the pci config header make it easier to deal with it. Comes from qemu-xen.

Adding a nice easy-to-use structure, then requiring all devices do byteswapping themselves (negating most of the easy-to-use benefit) seems less than ideal.

Paul

author
#6 lore raw
Quoted 9 lines · Paul Brook
Quoted 4 lines · Gerd Hoffmann

Hi,

This patch adds a struct for the pci config header make it easier to deal with it. Comes from qemu-xen.

Adding a nice easy-to-use structure, then requiring all devices do byteswapping themselves (negating most of the easy-to-use benefit) seems less than ideal.

Byteswapping must be done anyway, with or without the struct. Using the struct allows to avoid casting all over the place though. I'll go switch over the config header struct to use the byteswapped types, then repost together with some pci.c functions changed to use the new struct.
IMHO it makes the code more readable due to hardcoded magic numbers and
casts going away.

cheers,
Gerd