qemu: make default_write_config use mask table
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Michael S. Tsirkin
maintainerPATCH v2 1/1 qemu: make default_write_config use mask table
Change much of hw/pci to use symbolic constants and a table-driven
design: add a mask table with writable bits set and readonly bits unset. Detect change by comparing original and new registers.
This makes it easy to support capabilities where read-only/writeable bit layout differs between devices, depending on capabilities present.
As a result, writing a single byte in BAR registers now works as it should. Writing to upper limit registers in the bridge also works as it should. Code is also shorter.
---
This fixes a minor bug in previous version of the patch:
pci config header size should be 0x40.
yamahata, you probably want to rebase your patch on top of this.
hw/pci.c | 145 ++++++++++++------------------------------------------------- hw/pci.h | 18 +++++++- 2 files changed, 46 insertions(+), 117 deletions(-)
hw/pci.c +29 -116
@@ -239,6 +239,17 @@ int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)return pci_parse_devaddr (devaddr, domp, busp, slotp); } +static void pci_init_mask (PCIDevice *dev) +{ +int i; + dev->mask[PCI_CACHE_LINE_SIZE ] =0xff ; + dev->mask[PCI_INTERRUPT_LINE ] =0xff ; + dev->mask[PCI_COMMAND ] =PCI_COMMAND_IO |PCI_COMMAND_MEMORY + |PCI_COMMAND_MASTER ; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + dev->mask[i] =0xff ; +} +/* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, const char *name, int devfn,
@@ -261,6 +272,7 @@ 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_set_default_subsystem_id (pci_dev); +pci_init_mask (pci_dev);if (!config_read) config_read = pci_default_read_config;
@@ -334,6 +346,7 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, {PCIIORegion *r;uint32_t addr; +uint32_t mask;if ((unsigned int )region_num >=PCI_NUM_REGIONS )return ;
@@ -349,12 +362,17 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, r->size = size; r->type = type; r->map_func = map_func; + + mask = ~(size -1 );if (region_num ==PCI_ROM_SLOT ) { addr =0x30 ; +/* ROM enable bit is writeable */ + mask |=1 ; }else { addr =0x10 + region_num *4 ; } *(uint32_t *)(pci_dev->config + addr) =cpu_to_le32 (type); + *(uint32_t *)(pci_dev->mask + addr) =cpu_to_le32 (mask); }static void pci_update_mappings (PCIDevice *d)
@@ -463,118 +481,21 @@ uint32_t pci_default_read_config(PCIDevice *d,return val; } -void pci_default_write_config (PCIDevice *d, -uint32_t address,uint32_t val,int len) +void pci_default_write_config (PCIDevice *d,uint32_t addr,uint32_t val,int l) { -int can_write, i; -uint32_t end, addr; - -if (len ==4 && ((address >=0x10 && address <0x10 +4 *6 ) || - (address >=0x30 && address <0x34 ))) { -PCIIORegion *r; -int reg; +uint8_t orig[PCI_CONFIG_SPACE_SIZE ]; +int i; -if ( address >=0x30 ) { - reg =PCI_ROM_SLOT ; - }else { - reg = (address -0x10 ) >>2 ; - } - r = &d->io_regions[reg]; -if (r->size ==0 ) - goto default_config; -/* compute the stored value */ -if (reg ==PCI_ROM_SLOT ) { -/* keep ROM enable bit */ - val &= (~(r->size -1 )) |1 ; - }else { - val &= ~(r->size -1 ); - val |= r->type; - } - *(uint32_t *)(d->config + address) =cpu_to_le32 (val); -pci_update_mappings (d); -return ; - } - default_config:/* not efficient, but simple */ - addr = address; -for (i =0 ; i < len; i++) { -/* default read/write accesses */ -switch (d->config[0x0e ]) { -case 0x00 : -case 0x80 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x10 ... 0x27: /* base */ - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x30 ... 0x33: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; -default : -case 0x01 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x38 ... 0x3b: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; - } -if (can_write) { -/* Mask out writes to reserved bits in registers */ -switch (addr) { -case 0x05 : - val &= ~PCI_COMMAND_RESERVED_MASK_HI ; -break ; -case 0x06 : - val &= ~PCI_STATUS_RESERVED_MASK_LO ; -break ; -case 0x07 : - val &= ~PCI_STATUS_RESERVED_MASK_HI ; -break ; - } - d->config[addr] = val; - } -if (++addr >0xff ) -break ; - val >>=8 ; +memcpy (orig, d->config,PCI_CONFIG_SPACE_SIZE ); +for (i =0 ; i < l && addr <PCI_CONFIG_SPACE_SIZE ; val >>=8 , ++i, ++addr) { +uint8_t mask = d->mask[addr]; + d->config[addr] = (d->config[addr] & ~mask) | (val & mask); } - - end = address + len; -if (end >PCI_COMMAND && address < (PCI_COMMAND +2 )) { -/* if the command register is modified, we must modify the mappings */ +if (memcmp (orig +PCI_BASE_ADDRESS_0 , d->config +PCI_BASE_ADDRESS_0 ,24 ) + || ((orig[PCI_COMMAND ] ^ d->config[PCI_COMMAND ]) + & (PCI_COMMAND_MEMORY |PCI_COMMAND_IO )))pci_update_mappings (d); - } }void pci_data_write (void *opaque,uint32_t addr,uint32_t val,int len)
@@ -847,16 +768,8 @@ static void pci_bridge_write_config(PCIDevice *d, {PCIBridge *s = (PCIBridge *)d; -if (address ==0x19 || (address ==0x18 && len >1 )) { -if (address ==0x19 ) - s->bus->bus_num = val &0xff ; -else - s->bus->bus_num = (val >>8 ) &0xff ; -#if defined(DEBUG_PCI ) -printf ("pci-bridge: %s: Assigned bus %d\n" , d->name, s->bus->bus_num); -#endif - }pci_default_write_config (d, address, val, len); + s->bus->bus_num = d->config[PCI_SECONDARY_BUS ]; }PCIBus *pci_find_bus (int bus_num)
hw/pci.h +17 -1
@@ -100,16 +100,24 @@ typedef struct PCIIORegion {#define PCI_COMMAND 0x04/* 16 bits */ #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_STATUS 0x06/* 16 bits */ #define PCI_REVISION_ID 0x08/* 8 bits */ #define PCI_CLASS_DEVICE 0x0a/* Device class */ +#define PCI_CACHE_LINE_SIZE 0x0c/* 8 bits */ +#define PCI_LATENCY_TIMER 0x0d/* 8 bits */ #define PCI_HEADER_TYPE 0x0e/* 8 bits */ #define PCI_HEADER_TYPE_NORMAL 0#define PCI_HEADER_TYPE_BRIDGE 1#define PCI_HEADER_TYPE_CARDBUS 2#define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80 +#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */ +#define PCI_PRIMARY_BUS 0x18/* Primary bus number */ +#define PCI_SECONDARY_BUS 0x19/* Secondary bus number */ +#define PCI_SEC_STATUS 0x1e/* Secondary status register, only bit 14 used */ #define PCI_SUBSYSTEM_VENDOR_ID 0x2c/* 16 bits */ #define PCI_SUBSYSTEM_ID 0x2e/* 16 bits */ +#define PCI_CAPABILITY_LIST 0x34/* Offset of first capability list entry */ #define PCI_INTERRUPT_LINE 0x3c/* 8 bits */ #define PCI_INTERRUPT_PIN 0x3d/* 8 bits */ #define PCI_MIN_GNT 0x3e/* 8 bits */
@@ -139,10 +147,18 @@ typedef struct PCIIORegion {#define PCI_COMMAND_RESERVED_MASK_HI (PCI_COMMAND_RESERVED >> 8) +/* Size of the standard PCI config header */ +#define PCI_CONFIG_HEADER_SIZE 0x40 +/* Size of the standard PCI config space */ +#define PCI_CONFIG_SPACE_SIZE 0x100 +struct PCIDevice {DeviceState qdev;/* PCI config space */ -uint8_t config[256 ]; +uint8_t config[PCI_CONFIG_SPACE_SIZE ]; + +/* Used to implement R/W bytes */ +uint8_t mask[PCI_CONFIG_SPACE_SIZE ];/* the following fields are read only */ PCIBus *bus;
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH v2 1/1 qemu: make default_write_config use mask table
In reply to a message not in the archiveChange much of hw/pci to use symbolic constants and a table-driven
design: add a mask table with writable bits set and readonly bits unset. Detect change by comparing original and new registers.
This makes it easy to support capabilities where read-only/writeable bit layout differs between devices, depending on capabilities present.
As a result, writing a single byte in BAR registers now works as it should. Writing to upper limit registers in the bridge also works as it should. Code is also shorter.
---
This fixes a minor bug in previous version of the patch:
pci config header size should be 0x40.
yamahata, you probably want to rebase your patch on top of this.
hw/pci.c | 145 ++++++++++++------------------------------------------------- hw/pci.h | 18 +++++++- 2 files changed, 46 insertions(+), 117 deletions(-)
hw/pci.c +29 -116
@@ -239,6 +239,17 @@ int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)return pci_parse_devaddr (devaddr, domp, busp, slotp); } +static void pci_init_mask (PCIDevice *dev) +{ +int i; + dev->mask[PCI_CACHE_LINE_SIZE ] =0xff ; + dev->mask[PCI_INTERRUPT_LINE ] =0xff ; + dev->mask[PCI_COMMAND ] =PCI_COMMAND_IO |PCI_COMMAND_MEMORY + |PCI_COMMAND_MASTER ; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + dev->mask[i] =0xff ; +} +/* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, const char *name, int devfn,
@@ -261,6 +272,7 @@ 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_set_default_subsystem_id (pci_dev); +pci_init_mask (pci_dev);if (!config_read) config_read = pci_default_read_config;
@@ -334,6 +346,7 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, {PCIIORegion *r;uint32_t addr; +uint32_t mask;if ((unsigned int )region_num >=PCI_NUM_REGIONS )return ;
@@ -349,12 +362,17 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, r->size = size; r->type = type; r->map_func = map_func; + + mask = ~(size -1 );if (region_num ==PCI_ROM_SLOT ) { addr =0x30 ; +/* ROM enable bit is writeable */ + mask |=1 ; }else { addr =0x10 + region_num *4 ; } *(uint32_t *)(pci_dev->config + addr) =cpu_to_le32 (type); + *(uint32_t *)(pci_dev->mask + addr) =cpu_to_le32 (mask); }static void pci_update_mappings (PCIDevice *d)
@@ -463,118 +481,21 @@ uint32_t pci_default_read_config(PCIDevice *d,return val; } -void pci_default_write_config (PCIDevice *d, -uint32_t address,uint32_t val,int len) +void pci_default_write_config (PCIDevice *d,uint32_t addr,uint32_t val,int l) { -int can_write, i; -uint32_t end, addr; - -if (len ==4 && ((address >=0x10 && address <0x10 +4 *6 ) || - (address >=0x30 && address <0x34 ))) { -PCIIORegion *r; -int reg; +uint8_t orig[PCI_CONFIG_SPACE_SIZE ]; +int i; -if ( address >=0x30 ) { - reg =PCI_ROM_SLOT ; - }else { - reg = (address -0x10 ) >>2 ; - } - r = &d->io_regions[reg]; -if (r->size ==0 ) - goto default_config; -/* compute the stored value */ -if (reg ==PCI_ROM_SLOT ) { -/* keep ROM enable bit */ - val &= (~(r->size -1 )) |1 ; - }else { - val &= ~(r->size -1 ); - val |= r->type; - } - *(uint32_t *)(d->config + address) =cpu_to_le32 (val); -pci_update_mappings (d); -return ; - } - default_config:/* not efficient, but simple */ - addr = address; -for (i =0 ; i < len; i++) { -/* default read/write accesses */ -switch (d->config[0x0e ]) { -case 0x00 : -case 0x80 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x10 ... 0x27: /* base */ - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x30 ... 0x33: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; -default : -case 0x01 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x38 ... 0x3b: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; - } -if (can_write) { -/* Mask out writes to reserved bits in registers */ -switch (addr) { -case 0x05 : - val &= ~PCI_COMMAND_RESERVED_MASK_HI ; -break ; -case 0x06 : - val &= ~PCI_STATUS_RESERVED_MASK_LO ; -break ; -case 0x07 : - val &= ~PCI_STATUS_RESERVED_MASK_HI ; -break ; - } - d->config[addr] = val; - } -if (++addr >0xff ) -break ; - val >>=8 ; +memcpy (orig, d->config,PCI_CONFIG_SPACE_SIZE ); +for (i =0 ; i < l && addr <PCI_CONFIG_SPACE_SIZE ; val >>=8 , ++i, ++addr) { +uint8_t mask = d->mask[addr]; + d->config[addr] = (d->config[addr] & ~mask) | (val & mask); } - - end = address + len; -if (end >PCI_COMMAND && address < (PCI_COMMAND +2 )) { -/* if the command register is modified, we must modify the mappings */ +if (memcmp (orig +PCI_BASE_ADDRESS_0 , d->config +PCI_BASE_ADDRESS_0 ,24 ) + || ((orig[PCI_COMMAND ] ^ d->config[PCI_COMMAND ]) + & (PCI_COMMAND_MEMORY |PCI_COMMAND_IO )))pci_update_mappings (d); - } }void pci_data_write (void *opaque,uint32_t addr,uint32_t val,int len)
@@ -847,16 +768,8 @@ static void pci_bridge_write_config(PCIDevice *d, {PCIBridge *s = (PCIBridge *)d; -if (address ==0x19 || (address ==0x18 && len >1 )) { -if (address ==0x19 ) - s->bus->bus_num = val &0xff ; -else - s->bus->bus_num = (val >>8 ) &0xff ; -#if defined(DEBUG_PCI ) -printf ("pci-bridge: %s: Assigned bus %d\n" , d->name, s->bus->bus_num); -#endif - }pci_default_write_config (d, address, val, len); + s->bus->bus_num = d->config[PCI_SECONDARY_BUS ]; }PCIBus *pci_find_bus (int bus_num)
hw/pci.h +17 -1
@@ -100,16 +100,24 @@ typedef struct PCIIORegion {#define PCI_COMMAND 0x04/* 16 bits */ #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_STATUS 0x06/* 16 bits */ #define PCI_REVISION_ID 0x08/* 8 bits */ #define PCI_CLASS_DEVICE 0x0a/* Device class */ +#define PCI_CACHE_LINE_SIZE 0x0c/* 8 bits */ +#define PCI_LATENCY_TIMER 0x0d/* 8 bits */ #define PCI_HEADER_TYPE 0x0e/* 8 bits */ #define PCI_HEADER_TYPE_NORMAL 0#define PCI_HEADER_TYPE_BRIDGE 1#define PCI_HEADER_TYPE_CARDBUS 2#define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80 +#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */ +#define PCI_PRIMARY_BUS 0x18/* Primary bus number */ +#define PCI_SECONDARY_BUS 0x19/* Secondary bus number */ +#define PCI_SEC_STATUS 0x1e/* Secondary status register, only bit 14 used */ #define PCI_SUBSYSTEM_VENDOR_ID 0x2c/* 16 bits */ #define PCI_SUBSYSTEM_ID 0x2e/* 16 bits */ +#define PCI_CAPABILITY_LIST 0x34/* Offset of first capability list entry */ #define PCI_INTERRUPT_LINE 0x3c/* 8 bits */ #define PCI_INTERRUPT_PIN 0x3d/* 8 bits */ #define PCI_MIN_GNT 0x3e/* 8 bits */
@@ -139,10 +147,18 @@ typedef struct PCIIORegion {#define PCI_COMMAND_RESERVED_MASK_HI (PCI_COMMAND_RESERVED >> 8) +/* Size of the standard PCI config header */ +#define PCI_CONFIG_HEADER_SIZE 0x40 +/* Size of the standard PCI config space */ +#define PCI_CONFIG_SPACE_SIZE 0x100 +struct PCIDevice {DeviceState qdev;/* PCI config space */ -uint8_t config[256 ]; +uint8_t config[PCI_CONFIG_SPACE_SIZE ]; + +/* Used to implement R/W bytes */ +uint8_t mask[PCI_CONFIG_SPACE_SIZE ];/* the following fields are read only */ PCIBus *bus;
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 1/11 qemu: make default_write_config use mask table
In reply to a message not in the archiveChange much of hw/pci to use symbolic constants and a table-driven
design: add a mask table with writable bits set and readonly bits unset. Detect change by comparing original and new registers.
This makes it easy to support capabilities where read-only/writeable bit layout differs between devices, depending on capabilities present.
As a result, writing a single byte in BAR registers now works as it should. Writing to upper limit registers in the bridge also works as it should. Code is also shorter.
---
This is the same as v2 of the patch that I sent previously. It is included here for completeness.
hw/pci.c | 145 ++++++++++++------------------------------------------------- hw/pci.h | 18 +++++++- 2 files changed, 46 insertions(+), 117 deletions(-)
hw/pci.c +29 -116
@@ -239,6 +239,17 @@ int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)return pci_parse_devaddr (devaddr, domp, busp, slotp); } +static void pci_init_mask (PCIDevice *dev) +{ +int i; + dev->mask[PCI_CACHE_LINE_SIZE ] =0xff ; + dev->mask[PCI_INTERRUPT_LINE ] =0xff ; + dev->mask[PCI_COMMAND ] =PCI_COMMAND_IO |PCI_COMMAND_MEMORY + |PCI_COMMAND_MASTER ; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + dev->mask[i] =0xff ; +} +/* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, const char *name, int devfn,
@@ -261,6 +272,7 @@ 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_set_default_subsystem_id (pci_dev); +pci_init_mask (pci_dev);if (!config_read) config_read = pci_default_read_config;
@@ -334,6 +346,7 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, {PCIIORegion *r;uint32_t addr; +uint32_t mask;if ((unsigned int )region_num >=PCI_NUM_REGIONS )return ;
@@ -349,12 +362,17 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, r->size = size; r->type = type; r->map_func = map_func; + + mask = ~(size -1 );if (region_num ==PCI_ROM_SLOT ) { addr =0x30 ; +/* ROM enable bit is writeable */ + mask |=1 ; }else { addr =0x10 + region_num *4 ; } *(uint32_t *)(pci_dev->config + addr) =cpu_to_le32 (type); + *(uint32_t *)(pci_dev->mask + addr) =cpu_to_le32 (mask); }static void pci_update_mappings (PCIDevice *d)
@@ -463,118 +481,21 @@ uint32_t pci_default_read_config(PCIDevice *d,return val; } -void pci_default_write_config (PCIDevice *d, -uint32_t address,uint32_t val,int len) +void pci_default_write_config (PCIDevice *d,uint32_t addr,uint32_t val,int l) { -int can_write, i; -uint32_t end, addr; - -if (len ==4 && ((address >=0x10 && address <0x10 +4 *6 ) || - (address >=0x30 && address <0x34 ))) { -PCIIORegion *r; -int reg; +uint8_t orig[PCI_CONFIG_SPACE_SIZE ]; +int i; -if ( address >=0x30 ) { - reg =PCI_ROM_SLOT ; - }else { - reg = (address -0x10 ) >>2 ; - } - r = &d->io_regions[reg]; -if (r->size ==0 ) - goto default_config; -/* compute the stored value */ -if (reg ==PCI_ROM_SLOT ) { -/* keep ROM enable bit */ - val &= (~(r->size -1 )) |1 ; - }else { - val &= ~(r->size -1 ); - val |= r->type; - } - *(uint32_t *)(d->config + address) =cpu_to_le32 (val); -pci_update_mappings (d); -return ; - } - default_config:/* not efficient, but simple */ - addr = address; -for (i =0 ; i < len; i++) { -/* default read/write accesses */ -switch (d->config[0x0e ]) { -case 0x00 : -case 0x80 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x10 ... 0x27: /* base */ - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x30 ... 0x33: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; -default : -case 0x01 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x38 ... 0x3b: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; - } -if (can_write) { -/* Mask out writes to reserved bits in registers */ -switch (addr) { -case 0x05 : - val &= ~PCI_COMMAND_RESERVED_MASK_HI ; -break ; -case 0x06 : - val &= ~PCI_STATUS_RESERVED_MASK_LO ; -break ; -case 0x07 : - val &= ~PCI_STATUS_RESERVED_MASK_HI ; -break ; - } - d->config[addr] = val; - } -if (++addr >0xff ) -break ; - val >>=8 ; +memcpy (orig, d->config,PCI_CONFIG_SPACE_SIZE ); +for (i =0 ; i < l && addr <PCI_CONFIG_SPACE_SIZE ; val >>=8 , ++i, ++addr) { +uint8_t mask = d->mask[addr]; + d->config[addr] = (d->config[addr] & ~mask) | (val & mask); } - - end = address + len; -if (end >PCI_COMMAND && address < (PCI_COMMAND +2 )) { -/* if the command register is modified, we must modify the mappings */ +if (memcmp (orig +PCI_BASE_ADDRESS_0 , d->config +PCI_BASE_ADDRESS_0 ,24 ) + || ((orig[PCI_COMMAND ] ^ d->config[PCI_COMMAND ]) + & (PCI_COMMAND_MEMORY |PCI_COMMAND_IO )))pci_update_mappings (d); - } }void pci_data_write (void *opaque,uint32_t addr,uint32_t val,int len)
@@ -847,16 +768,8 @@ static void pci_bridge_write_config(PCIDevice *d, {PCIBridge *s = (PCIBridge *)d; -if (address ==0x19 || (address ==0x18 && len >1 )) { -if (address ==0x19 ) - s->bus->bus_num = val &0xff ; -else - s->bus->bus_num = (val >>8 ) &0xff ; -#if defined(DEBUG_PCI ) -printf ("pci-bridge: %s: Assigned bus %d\n" , d->name, s->bus->bus_num); -#endif - }pci_default_write_config (d, address, val, len); + s->bus->bus_num = d->config[PCI_SECONDARY_BUS ]; }PCIBus *pci_find_bus (int bus_num)
hw/pci.h +17 -1
@@ -100,16 +100,24 @@ typedef struct PCIIORegion {#define PCI_COMMAND 0x04/* 16 bits */ #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_STATUS 0x06/* 16 bits */ #define PCI_REVISION_ID 0x08/* 8 bits */ #define PCI_CLASS_DEVICE 0x0a/* Device class */ +#define PCI_CACHE_LINE_SIZE 0x0c/* 8 bits */ +#define PCI_LATENCY_TIMER 0x0d/* 8 bits */ #define PCI_HEADER_TYPE 0x0e/* 8 bits */ #define PCI_HEADER_TYPE_NORMAL 0#define PCI_HEADER_TYPE_BRIDGE 1#define PCI_HEADER_TYPE_CARDBUS 2#define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80 +#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */ +#define PCI_PRIMARY_BUS 0x18/* Primary bus number */ +#define PCI_SECONDARY_BUS 0x19/* Secondary bus number */ +#define PCI_SEC_STATUS 0x1e/* Secondary status register, only bit 14 used */ #define PCI_SUBSYSTEM_VENDOR_ID 0x2c/* 16 bits */ #define PCI_SUBSYSTEM_ID 0x2e/* 16 bits */ +#define PCI_CAPABILITY_LIST 0x34/* Offset of first capability list entry */ #define PCI_INTERRUPT_LINE 0x3c/* 8 bits */ #define PCI_INTERRUPT_PIN 0x3d/* 8 bits */ #define PCI_MIN_GNT 0x3e/* 8 bits */
@@ -139,10 +147,18 @@ typedef struct PCIIORegion {#define PCI_COMMAND_RESERVED_MASK_HI (PCI_COMMAND_RESERVED >> 8) +/* Size of the standard PCI config header */ +#define PCI_CONFIG_HEADER_SIZE 0x40 +/* Size of the standard PCI config space */ +#define PCI_CONFIG_SPACE_SIZE 0x100 +struct PCIDevice {DeviceState qdev;/* PCI config space */ -uint8_t config[256 ]; +uint8_t config[PCI_CONFIG_SPACE_SIZE ]; + +/* Used to implement R/W bytes */ +uint8_t mask[PCI_CONFIG_SPACE_SIZE ];/* the following fields are read only */ PCIBus *bus;
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 1/11 qemu: make default_write_config use mask table
In reply to a message not in the archiveChange much of hw/pci to use symbolic constants and a table-driven
design: add a mask table with writable bits set and readonly bits unset. Detect change by comparing original and new registers.
This makes it easy to support capabilities where read-only/writeable bit layout differs between devices, depending on capabilities present.
As a result, writing a single byte in BAR registers now works as it should. Writing to upper limit registers in the bridge also works as it should. Code is also shorter.
---
This is the same as v2 of the patch that I sent previously. It is included here for completeness.
hw/pci.c | 145 ++++++++++++------------------------------------------------- hw/pci.h | 18 +++++++- 2 files changed, 46 insertions(+), 117 deletions(-)
hw/pci.c +29 -116
@@ -239,6 +239,17 @@ int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)return pci_parse_devaddr (devaddr, domp, busp, slotp); } +static void pci_init_mask (PCIDevice *dev) +{ +int i; + dev->mask[PCI_CACHE_LINE_SIZE ] =0xff ; + dev->mask[PCI_INTERRUPT_LINE ] =0xff ; + dev->mask[PCI_COMMAND ] =PCI_COMMAND_IO |PCI_COMMAND_MEMORY + |PCI_COMMAND_MASTER ; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + dev->mask[i] =0xff ; +} +/* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, const char *name, int devfn,
@@ -261,6 +272,7 @@ 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_set_default_subsystem_id (pci_dev); +pci_init_mask (pci_dev);if (!config_read) config_read = pci_default_read_config;
@@ -334,6 +346,7 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, {PCIIORegion *r;uint32_t addr; +uint32_t mask;if ((unsigned int )region_num >=PCI_NUM_REGIONS )return ;
@@ -349,12 +362,17 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, r->size = size; r->type = type; r->map_func = map_func; + + mask = ~(size -1 );if (region_num ==PCI_ROM_SLOT ) { addr =0x30 ; +/* ROM enable bit is writeable */ + mask |=1 ; }else { addr =0x10 + region_num *4 ; } *(uint32_t *)(pci_dev->config + addr) =cpu_to_le32 (type); + *(uint32_t *)(pci_dev->mask + addr) =cpu_to_le32 (mask); }static void pci_update_mappings (PCIDevice *d)
@@ -463,118 +481,21 @@ uint32_t pci_default_read_config(PCIDevice *d,return val; } -void pci_default_write_config (PCIDevice *d, -uint32_t address,uint32_t val,int len) +void pci_default_write_config (PCIDevice *d,uint32_t addr,uint32_t val,int l) { -int can_write, i; -uint32_t end, addr; - -if (len ==4 && ((address >=0x10 && address <0x10 +4 *6 ) || - (address >=0x30 && address <0x34 ))) { -PCIIORegion *r; -int reg; +uint8_t orig[PCI_CONFIG_SPACE_SIZE ]; +int i; -if ( address >=0x30 ) { - reg =PCI_ROM_SLOT ; - }else { - reg = (address -0x10 ) >>2 ; - } - r = &d->io_regions[reg]; -if (r->size ==0 ) - goto default_config; -/* compute the stored value */ -if (reg ==PCI_ROM_SLOT ) { -/* keep ROM enable bit */ - val &= (~(r->size -1 )) |1 ; - }else { - val &= ~(r->size -1 ); - val |= r->type; - } - *(uint32_t *)(d->config + address) =cpu_to_le32 (val); -pci_update_mappings (d); -return ; - } - default_config:/* not efficient, but simple */ - addr = address; -for (i =0 ; i < len; i++) { -/* default read/write accesses */ -switch (d->config[0x0e ]) { -case 0x00 : -case 0x80 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x10 ... 0x27: /* base */ - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x30 ... 0x33: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; -default : -case 0x01 : -switch (addr) { -case 0x00 : -case 0x01 : -case 0x02 : -case 0x03 : -case 0x06 : -case 0x07 : -case 0x08 : -case 0x09 : -case 0x0a : -case 0x0b : -case 0x0e : - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x38 ... 0x3b: /* rom */ -case 0x3d : - can_write =0 ; -break ; -default : - can_write =1 ; -break ; - } -break ; - } -if (can_write) { -/* Mask out writes to reserved bits in registers */ -switch (addr) { -case 0x05 : - val &= ~PCI_COMMAND_RESERVED_MASK_HI ; -break ; -case 0x06 : - val &= ~PCI_STATUS_RESERVED_MASK_LO ; -break ; -case 0x07 : - val &= ~PCI_STATUS_RESERVED_MASK_HI ; -break ; - } - d->config[addr] = val; - } -if (++addr >0xff ) -break ; - val >>=8 ; +memcpy (orig, d->config,PCI_CONFIG_SPACE_SIZE ); +for (i =0 ; i < l && addr <PCI_CONFIG_SPACE_SIZE ; val >>=8 , ++i, ++addr) { +uint8_t mask = d->mask[addr]; + d->config[addr] = (d->config[addr] & ~mask) | (val & mask); } - - end = address + len; -if (end >PCI_COMMAND && address < (PCI_COMMAND +2 )) { -/* if the command register is modified, we must modify the mappings */ +if (memcmp (orig +PCI_BASE_ADDRESS_0 , d->config +PCI_BASE_ADDRESS_0 ,24 ) + || ((orig[PCI_COMMAND ] ^ d->config[PCI_COMMAND ]) + & (PCI_COMMAND_MEMORY |PCI_COMMAND_IO )))pci_update_mappings (d); - } }void pci_data_write (void *opaque,uint32_t addr,uint32_t val,int len)
@@ -847,16 +768,8 @@ static void pci_bridge_write_config(PCIDevice *d, {PCIBridge *s = (PCIBridge *)d; -if (address ==0x19 || (address ==0x18 && len >1 )) { -if (address ==0x19 ) - s->bus->bus_num = val &0xff ; -else - s->bus->bus_num = (val >>8 ) &0xff ; -#if defined(DEBUG_PCI ) -printf ("pci-bridge: %s: Assigned bus %d\n" , d->name, s->bus->bus_num); -#endif - }pci_default_write_config (d, address, val, len); + s->bus->bus_num = d->config[PCI_SECONDARY_BUS ]; }PCIBus *pci_find_bus (int bus_num)
hw/pci.h +17 -1
@@ -100,16 +100,24 @@ typedef struct PCIIORegion {#define PCI_COMMAND 0x04/* 16 bits */ #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_STATUS 0x06/* 16 bits */ #define PCI_REVISION_ID 0x08/* 8 bits */ #define PCI_CLASS_DEVICE 0x0a/* Device class */ +#define PCI_CACHE_LINE_SIZE 0x0c/* 8 bits */ +#define PCI_LATENCY_TIMER 0x0d/* 8 bits */ #define PCI_HEADER_TYPE 0x0e/* 8 bits */ #define PCI_HEADER_TYPE_NORMAL 0#define PCI_HEADER_TYPE_BRIDGE 1#define PCI_HEADER_TYPE_CARDBUS 2#define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80 +#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */ +#define PCI_PRIMARY_BUS 0x18/* Primary bus number */ +#define PCI_SECONDARY_BUS 0x19/* Secondary bus number */ +#define PCI_SEC_STATUS 0x1e/* Secondary status register, only bit 14 used */ #define PCI_SUBSYSTEM_VENDOR_ID 0x2c/* 16 bits */ #define PCI_SUBSYSTEM_ID 0x2e/* 16 bits */ +#define PCI_CAPABILITY_LIST 0x34/* Offset of first capability list entry */ #define PCI_INTERRUPT_LINE 0x3c/* 8 bits */ #define PCI_INTERRUPT_PIN 0x3d/* 8 bits */ #define PCI_MIN_GNT 0x3e/* 8 bits */
@@ -139,10 +147,18 @@ typedef struct PCIIORegion {#define PCI_COMMAND_RESERVED_MASK_HI (PCI_COMMAND_RESERVED >> 8) +/* Size of the standard PCI config header */ +#define PCI_CONFIG_HEADER_SIZE 0x40 +/* Size of the standard PCI config space */ +#define PCI_CONFIG_SPACE_SIZE 0x100 +struct PCIDevice {DeviceState qdev;/* PCI config space */ -uint8_t config[256 ]; +uint8_t config[PCI_CONFIG_SPACE_SIZE ]; + +/* Used to implement R/W bytes */ +uint8_t mask[PCI_CONFIG_SPACE_SIZE ];/* the following fields are read only */ PCIBus *bus;
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 2/11 qemu: capability bits in pci save/restore
In reply to a message not in the archiveAdd support for capability bits in save/restore for pci. These will be used for MSI, where the capability might be present or not as requested by user, which does not map well into a single version number.
--- hw/pci.c | 14 ++++++++++++-- hw/pci.h | 4 ++++ 2 files changed, 16 insertions(+), 2 deletions(-)
hw/pci.c +12 -2
@@ -127,12 +127,15 @@ int pci_bus_num(PCIBus *s)void pci_device_save (PCIDevice *s,QEMUFile *f) { +int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f,2 );/* PCI device version */ +qemu_put_be32 (f, version);/* PCI device version */ qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)
@@ -141,7 +144,7 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)int i; version_id =qemu_get_be32 (f); -if (version_id >2 ) +if (version_id >3 )return -EINVAL ;qemu_get_buffer (f, s->config,256 );pci_update_mappings (s);
@@ -149,6 +152,13 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (version_id >=2 )for (i =0 ; i <4 ; i ++) s->irq_state[i] =qemu_get_be32 (f); +if (version_id >=3 ) + s->cap_present =qemu_get_be32 (f); +else + s->cap_present =0 ; + +if (s->cap_present & ~s->cap_supported) +return -EINVAL ;return 0 ; }
hw/pci.h +4 -0
@@ -178,6 +178,10 @@ struct PCIDevice {/* Current IRQ levels. Used internally by the generic PCI code. */ int irq_state[4 ]; + +/* Capability bits for save/load */ +uint32_t cap_supported; +uint32_t cap_present; }; PCIDevice *pci_register_device(PCIBus *bus, const char *name,
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 2/11 qemu: capability bits in pci save/restore
In reply to a message not in the archiveAdd support for capability bits in save/restore for pci. These will be used for MSI, where the capability might be present or not as requested by user, which does not map well into a single version number.
--- hw/pci.c | 14 ++++++++++++-- hw/pci.h | 4 ++++ 2 files changed, 16 insertions(+), 2 deletions(-)
hw/pci.c +12 -2
@@ -127,12 +127,15 @@ int pci_bus_num(PCIBus *s)void pci_device_save (PCIDevice *s,QEMUFile *f) { +int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f,2 );/* PCI device version */ +qemu_put_be32 (f, version);/* PCI device version */ qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)
@@ -141,7 +144,7 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)int i; version_id =qemu_get_be32 (f); -if (version_id >2 ) +if (version_id >3 )return -EINVAL ;qemu_get_buffer (f, s->config,256 );pci_update_mappings (s);
@@ -149,6 +152,13 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (version_id >=2 )for (i =0 ; i <4 ; i ++) s->irq_state[i] =qemu_get_be32 (f); +if (version_id >=3 ) + s->cap_present =qemu_get_be32 (f); +else + s->cap_present =0 ; + +if (s->cap_present & ~s->cap_supported) +return -EINVAL ;return 0 ; }
hw/pci.h +4 -0
@@ -178,6 +178,10 @@ struct PCIDevice {/* Current IRQ levels. Used internally by the generic PCI code. */ int irq_state[4 ]; + +/* Capability bits for save/load */ +uint32_t cap_supported; +uint32_t cap_present; }; PCIDevice *pci_register_device(PCIBus *bus, const char *name,
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 3/11 qemu: add routines to manage PCI capabilities
In reply to a message not in the archiveAdd routines to manage PCI capability list. First user will be MSI-X.
--- hw/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ hw/pci.h | 18 +++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-)
hw/pci.c +89 -9
@@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f, version);/* PCI device version */ +/* PCI device version and capabilities */ +qemu_put_be32 (f, version); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present);qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); -if (version >=3 ) -qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)
@@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) version_id =qemu_get_be32 (f);if (version_id >3 )return -EINVAL ; -qemu_get_buffer (f, s->config,256 ); -pci_update_mappings (s); - -if (version_id >=2 ) -for (i =0 ; i <4 ; i ++) - s->irq_state[i] =qemu_get_be32 (f);if (version_id >=3 ) s->cap_present =qemu_get_be32 (f);else
@@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (s->cap_present & ~s->cap_supported)return -EINVAL ; +qemu_get_buffer (f, s->config,256 ); +pci_update_mappings (s); + +if (version_id >=2 ) +for (i =0 ; i <4 ; i ++) + s->irq_state[i] =qemu_get_be32 (f); +/* Clear mask and used bits for capabilities. +Must be restored separately, since capabilities can +be placed anywhere in config space. */ +memset (s->used,0 ,PCI_CONFIG_SPACE_SIZE ); +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + s->mask[i] =0xff ;return 0 ; }
@@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)return (PCIDevice *)dev; } + +static int pci_find_space (PCIDevice *pdev,uint8_t size) +{ +int offset =PCI_CONFIG_HEADER_SIZE ; +int i; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) +if (pdev->used[i]) + offset = i +1 ; +else if (i - offset +1 == size) +return offset; +return 0 ; +} + +static uint8_t pci_find_capability_list (PCIDevice *pdev,uint8_t cap_id, +uint8_t *prev_p) +{ +uint8_t next, prev; + +if (!(pdev->config[PCI_STATUS ] &PCI_STATUS_CAP_LIST )) +return 0 ; + +for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); + prev = next +PCI_CAP_LIST_NEXT ) +if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id) +break ; + + *prev_p = prev; +return next; +} + +/* Reserve space and add capability to the linked list in pci config space */ +int pci_add_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t offset =pci_find_space (pdev, size); +uint8_t *config = pdev->config + offset; +if (!offset) +return -ENOSPC ; + config[PCI_CAP_LIST_ID ] = cap_id; + config[PCI_CAP_LIST_NEXT ] = pdev->config[PCI_CAPABILITY_LIST ]; + pdev->config[PCI_CAPABILITY_LIST ] = offset; + pdev->config[PCI_STATUS ] |=PCI_STATUS_CAP_LIST ; +memset (pdev->used + offset,0xFF , size); +/* Make capability read-only by default */ +memset (pdev->mask + offset,0 , size); +return offset; +} + +/* Unlink capability from the pci config space. */ +void pci_del_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t prev, offset =pci_find_capability_list (pdev, cap_id, &prev); +if (!offset) +return ; + pdev->config[prev] = pdev->config[offset +PCI_CAP_LIST_NEXT ]; +/* Make capability writeable again */ +memset (pdev->mask + offset,0xff , size); +memset (pdev->used + offset,0 , size); + +if (!pdev->config[PCI_CAPABILITY_LIST ]) + pdev->config[PCI_STATUS ] &= ~PCI_STATUS_CAP_LIST ; +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability (PCIDevice *pdev,uint8_t offset,uint8_t size) +{ +memset (pdev->used + offset,0xff , size); +} + +uint8_t pci_find_capability (PCIDevice *pdev,uint8_t cap_id) +{ +uint8_t prev; +return pci_find_capability_list (pdev, cap_id, &prev); +}
hw/pci.h +17 -1
@@ -123,6 +123,10 @@ typedef struct PCIIORegion {#define PCI_MIN_GNT 0x3e/* 8 bits */ #define PCI_MAX_LAT 0x3f/* 8 bits */ +/* Capability lists */ +#define PCI_CAP_LIST_ID 0/* Capability ID */ +#define PCI_CAP_LIST_NEXT 1/* Next capability in the list */ +#define PCI_REVISION 0x08/* obsolete, use PCI_REVISION_ID */ #define PCI_SUBVENDOR_ID 0x2c/* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */ #define PCI_SUBDEVICE_ID 0x2e/* obsolete, use PCI_SUBSYSTEM_ID */
@@ -130,7 +134,7 @@ typedef struct PCIIORegion {/* Bits in the PCI Status Register (PCI 2.3 spec) */ #define PCI_STATUS_RESERVED1 0x007#define PCI_STATUS_INT_STATUS 0x008 -#define PCI_STATUS_CAPABILITIES 0x010 +#define PCI_STATUS_CAP_LIST 0x010#define PCI_STATUS_66MHZ 0x020#define PCI_STATUS_RESERVED2 0x040#define PCI_STATUS_FAST_BACK 0x080
@@ -160,6 +164,9 @@ struct PCIDevice {/* Used to implement R/W bytes */ uint8_t mask[PCI_CONFIG_SPACE_SIZE ]; +/* Used to allocate config space for capabilities. */ +uint8_t used[PCI_CONFIG_SPACE_SIZE ]; +/* the following fields are read only */ PCIBus *bus;int devfn;
@@ -194,6 +201,15 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_reserve_capability (PCIDevice *pci_dev,uint8_t offset,uint8_t size); + +uint8_t pci_find_capability (PCIDevice *pci_dev,uint8_t cap_id); + +uint32_t pci_default_read_config (PCIDevice *d,uint32_t address,int len); void pci_default_write_config(PCIDevice *d,
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 3/11 qemu: add routines to manage PCI capabilities
In reply to a message not in the archiveAdd routines to manage PCI capability list. First user will be MSI-X.
--- hw/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ hw/pci.h | 18 +++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-)
hw/pci.c +89 -9
@@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f, version);/* PCI device version */ +/* PCI device version and capabilities */ +qemu_put_be32 (f, version); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present);qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); -if (version >=3 ) -qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)
@@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) version_id =qemu_get_be32 (f);if (version_id >3 )return -EINVAL ; -qemu_get_buffer (f, s->config,256 ); -pci_update_mappings (s); - -if (version_id >=2 ) -for (i =0 ; i <4 ; i ++) - s->irq_state[i] =qemu_get_be32 (f);if (version_id >=3 ) s->cap_present =qemu_get_be32 (f);else
@@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (s->cap_present & ~s->cap_supported)return -EINVAL ; +qemu_get_buffer (f, s->config,256 ); +pci_update_mappings (s); + +if (version_id >=2 ) +for (i =0 ; i <4 ; i ++) + s->irq_state[i] =qemu_get_be32 (f); +/* Clear mask and used bits for capabilities. +Must be restored separately, since capabilities can +be placed anywhere in config space. */ +memset (s->used,0 ,PCI_CONFIG_SPACE_SIZE ); +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + s->mask[i] =0xff ;return 0 ; }
@@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)return (PCIDevice *)dev; } + +static int pci_find_space (PCIDevice *pdev,uint8_t size) +{ +int offset =PCI_CONFIG_HEADER_SIZE ; +int i; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) +if (pdev->used[i]) + offset = i +1 ; +else if (i - offset +1 == size) +return offset; +return 0 ; +} + +static uint8_t pci_find_capability_list (PCIDevice *pdev,uint8_t cap_id, +uint8_t *prev_p) +{ +uint8_t next, prev; + +if (!(pdev->config[PCI_STATUS ] &PCI_STATUS_CAP_LIST )) +return 0 ; + +for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); + prev = next +PCI_CAP_LIST_NEXT ) +if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id) +break ; + + *prev_p = prev; +return next; +} + +/* Reserve space and add capability to the linked list in pci config space */ +int pci_add_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t offset =pci_find_space (pdev, size); +uint8_t *config = pdev->config + offset; +if (!offset) +return -ENOSPC ; + config[PCI_CAP_LIST_ID ] = cap_id; + config[PCI_CAP_LIST_NEXT ] = pdev->config[PCI_CAPABILITY_LIST ]; + pdev->config[PCI_CAPABILITY_LIST ] = offset; + pdev->config[PCI_STATUS ] |=PCI_STATUS_CAP_LIST ; +memset (pdev->used + offset,0xFF , size); +/* Make capability read-only by default */ +memset (pdev->mask + offset,0 , size); +return offset; +} + +/* Unlink capability from the pci config space. */ +void pci_del_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t prev, offset =pci_find_capability_list (pdev, cap_id, &prev); +if (!offset) +return ; + pdev->config[prev] = pdev->config[offset +PCI_CAP_LIST_NEXT ]; +/* Make capability writeable again */ +memset (pdev->mask + offset,0xff , size); +memset (pdev->used + offset,0 , size); + +if (!pdev->config[PCI_CAPABILITY_LIST ]) + pdev->config[PCI_STATUS ] &= ~PCI_STATUS_CAP_LIST ; +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability (PCIDevice *pdev,uint8_t offset,uint8_t size) +{ +memset (pdev->used + offset,0xff , size); +} + +uint8_t pci_find_capability (PCIDevice *pdev,uint8_t cap_id) +{ +uint8_t prev; +return pci_find_capability_list (pdev, cap_id, &prev); +}
hw/pci.h +17 -1
@@ -123,6 +123,10 @@ typedef struct PCIIORegion {#define PCI_MIN_GNT 0x3e/* 8 bits */ #define PCI_MAX_LAT 0x3f/* 8 bits */ +/* Capability lists */ +#define PCI_CAP_LIST_ID 0/* Capability ID */ +#define PCI_CAP_LIST_NEXT 1/* Next capability in the list */ +#define PCI_REVISION 0x08/* obsolete, use PCI_REVISION_ID */ #define PCI_SUBVENDOR_ID 0x2c/* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */ #define PCI_SUBDEVICE_ID 0x2e/* obsolete, use PCI_SUBSYSTEM_ID */
@@ -130,7 +134,7 @@ typedef struct PCIIORegion {/* Bits in the PCI Status Register (PCI 2.3 spec) */ #define PCI_STATUS_RESERVED1 0x007#define PCI_STATUS_INT_STATUS 0x008 -#define PCI_STATUS_CAPABILITIES 0x010 +#define PCI_STATUS_CAP_LIST 0x010#define PCI_STATUS_66MHZ 0x020#define PCI_STATUS_RESERVED2 0x040#define PCI_STATUS_FAST_BACK 0x080
@@ -160,6 +164,9 @@ struct PCIDevice {/* Used to implement R/W bytes */ uint8_t mask[PCI_CONFIG_SPACE_SIZE ]; +/* Used to allocate config space for capabilities. */ +uint8_t used[PCI_CONFIG_SPACE_SIZE ]; +/* the following fields are read only */ PCIBus *bus;int devfn;
@@ -194,6 +201,15 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_reserve_capability (PCIDevice *pci_dev,uint8_t offset,uint8_t size); + +uint8_t pci_find_capability (PCIDevice *pci_dev,uint8_t cap_id); + +uint32_t pci_default_read_config (PCIDevice *d,uint32_t address,int len); void pci_default_write_config(PCIDevice *d,
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Re: [Qemu-devel] [PATCH 03/11] qemu: add routines to manage PCI capabilities
Quoted 79 lines · Michael S. Tsirkin
Add routines to manage PCI capability list. First user will be MSI-X.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ hw/pci.h | 18 +++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-)hw/pci.c +30
-9@@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f, version);/* PCI device version */ +/* PCI device version and capabilities */ +qemu_put_be32 (f, version); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present);qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); -if (version >=3 ) -qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)@@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) version_id =qemu_get_be32 (f);if (version_id >3 )return -EINVAL ; -qemu_get_buffer (f, s->config,256 ); -pci_update_mappings (s); - -if (version_id >=2 ) -for (i =0 ; i <4 ; i ++) - s->irq_state[i] =qemu_get_be32 (f);if (version_id >=3 ) s->cap_present =qemu_get_be32 (f);else @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (s->cap_present & ~s->cap_supported)return -EINVAL ; +qemu_get_buffer (f, s->config,256 ); +pci_update_mappings (s); + +if (version_id >=2 ) +for (i =0 ; i <4 ; i ++) + s->irq_state[i] =qemu_get_be32 (f); +/* Clear mask and used bits for capabilities. +Must be restored separately, since capabilities can +be placed anywhere in config space. */ +memset (s->used,0 ,PCI_CONFIG_SPACE_SIZE ); +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + s->mask[i] =0xff ;return 0 ; }@@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)return (PCIDevice *)dev; } + +static int pci_find_space (PCIDevice *pdev,uint8_t size) +{ +int offset =PCI_CONFIG_HEADER_SIZE ; +int i; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) +if (pdev->used[i]) + offset = i +1 ; +else if (i - offset +1 == size) +return offset; +return 0 ; +} + +static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
hw/pci.c PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name) · line 903
Quoted 10 lines
hw/pci.c +10
-0+ uint8_t *prev_p) +{ +uint8_t next, prev; + +if (!(pdev->config[PCI_STATUS ] &PCI_STATUS_CAP_LIST )) +return 0 ; + +for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); + prev = next +PCI_CAP_LIST_NEXT ) +if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id)
typo? ==
Quoted 74 lines
hw/pci.c +49
-0+break ; + + *prev_p = prev; +return next; +} + +/* Reserve space and add capability to the linked list in pci config space */ +int pci_add_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t offset =pci_find_space (pdev, size); +uint8_t *config = pdev->config + offset; +if (!offset) +return -ENOSPC ; + config[PCI_CAP_LIST_ID ] = cap_id; + config[PCI_CAP_LIST_NEXT ] = pdev->config[PCI_CAPABILITY_LIST ]; + pdev->config[PCI_CAPABILITY_LIST ] = offset; + pdev->config[PCI_STATUS ] |=PCI_STATUS_CAP_LIST ; +memset (pdev->used + offset,0xFF , size); +/* Make capability read-only by default */ +memset (pdev->mask + offset,0 , size); +return offset; +} + +/* Unlink capability from the pci config space. */ +void pci_del_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t prev, offset =pci_find_capability_list (pdev, cap_id, &prev); +if (!offset) +return ; + pdev->config[prev] = pdev->config[offset +PCI_CAP_LIST_NEXT ]; +/* Make capability writeable again */ +memset (pdev->mask + offset,0xff , size); +memset (pdev->used + offset,0 , size); + +if (!pdev->config[PCI_CAPABILITY_LIST ]) + pdev->config[PCI_STATUS ] &= ~PCI_STATUS_CAP_LIST ; +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability (PCIDevice *pdev,uint8_t offset,uint8_t size) +{ +memset (pdev->used + offset,0xff , size); +} + +uint8_t pci_find_capability (PCIDevice *pdev,uint8_t cap_id) +{ +uint8_t prev; +return pci_find_capability_list (pdev, cap_id, &prev); +}hw/pci.h +5
-1@@ -123,6 +123,10 @@ typedef struct PCIIORegion {#define PCI_MIN_GNT 0x3e/* 8 bits */ #define PCI_MAX_LAT 0x3f/* 8 bits */ +/* Capability lists */ +#define PCI_CAP_LIST_ID 0/* Capability ID */ +#define PCI_CAP_LIST_NEXT 1/* Next capability in the list */ +#define PCI_REVISION 0x08/* obsolete, use PCI_REVISION_ID */ #define PCI_SUBVENDOR_ID 0x2c/* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */ #define PCI_SUBDEVICE_ID 0x2e/* obsolete, use PCI_SUBSYSTEM_ID */ @@ -130,7 +134,7 @@ typedef struct PCIIORegion {/* Bits in the PCI Status Register (PCI 2.3 spec) */ #define PCI_STATUS_RESERVED1 0x007#define PCI_STATUS_INT_STATUS 0x008 -#define PCI_STATUS_CAPABILITIES 0x010 +#define PCI_STATUS_CAP_LIST 0x010#define PCI_STATUS_66MHZ 0x020#define PCI_STATUS_RESERVED2 0x040#define PCI_STATUS_FAST_BACK 0x080
hw/pci.h struct PCIDevice { · line 169
Quoted 6 lines
hw/pci.h +3
-0@@ -160,6 +164,9 @@ struct PCIDevice {/* Used to implement R/W bytes */ uint8_t mask[PCI_CONFIG_SPACE_SIZE ]; +/* Used to allocate config space for capabilities. */ +uint8_t used[PCI_CONFIG_SPACE_SIZE ]; +
bitmap? Or will be used for other purpose?
Quoted 19 lines
hw/pci.h +9
-0/* the following fields are read only */ PCIBus *bus;int devfn;@@ -194,6 +201,15 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_reserve_capability (PCIDevice *pci_dev,uint8_t offset,uint8_t size); + +uint8_t pci_find_capability (PCIDevice *pci_dev,uint8_t cap_id); + +uint32_t pci_default_read_config (PCIDevice *d,uint32_t address,int len); void pci_default_write_config(PCIDevice *d,
Signature · 1 line
yamahata
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 03/11] qemu: add routines to manage PCI capabilities
Quoted 84 lines · Isaku Yamahata
Quoted 83 lines · Michael S. Tsirkin
Add routines to manage PCI capability list. First user will be MSI-X.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ hw/pci.h | 18 +++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-)hw/pci.c +34
-9@@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f, version);/* PCI device version */ +/* PCI device version and capabilities */ +qemu_put_be32 (f, version); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present);qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); -if (version >=3 ) -qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)@@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) version_id =qemu_get_be32 (f);if (version_id >3 )return -EINVAL ; -qemu_get_buffer (f, s->config,256 ); -pci_update_mappings (s); - -if (version_id >=2 ) -for (i =0 ; i <4 ; i ++) - s->irq_state[i] =qemu_get_be32 (f);if (version_id >=3 ) s->cap_present =qemu_get_be32 (f);else @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (s->cap_present & ~s->cap_supported)return -EINVAL ; +qemu_get_buffer (f, s->config,256 ); +pci_update_mappings (s); + +if (version_id >=2 ) +for (i =0 ; i <4 ; i ++) + s->irq_state[i] =qemu_get_be32 (f); +/* Clear mask and used bits for capabilities. +Must be restored separately, since capabilities can +be placed anywhere in config space. */ +memset (s->used,0 ,PCI_CONFIG_SPACE_SIZE ); +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + s->mask[i] =0xff ;return 0 ; }@@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)return (PCIDevice *)dev; } + +static int pci_find_space (PCIDevice *pdev,uint8_t size) +{ +int offset =PCI_CONFIG_HEADER_SIZE ; +int i; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) +if (pdev->used[i]) + offset = i +1 ; +else if (i - offset +1 == size) +return offset; +return 0 ; +} + +static uint8_t pci_find_capability_list (PCIDevice *pdev,uint8_t cap_id, +uint8_t *prev_p) +{ +uint8_t next, prev; +
Quoted 8 lines
hw/pci.c PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name) · line 903
Quoted 6 lines
hw/pci.c +6
-0+if (!(pdev->config[PCI_STATUS ] &PCI_STATUS_CAP_LIST )) +return 0 ; + +for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); + prev = next +PCI_CAP_LIST_NEXT ) +if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id)typo? ==
Fixed. Thanks.
Quoted 77 lines
Quoted 77 lines
hw/pci.c +49
-0+break ; + + *prev_p = prev; +return next; +} + +/* Reserve space and add capability to the linked list in pci config space */ +int pci_add_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t offset =pci_find_space (pdev, size); +uint8_t *config = pdev->config + offset; +if (!offset) +return -ENOSPC ; + config[PCI_CAP_LIST_ID ] = cap_id; + config[PCI_CAP_LIST_NEXT ] = pdev->config[PCI_CAPABILITY_LIST ]; + pdev->config[PCI_CAPABILITY_LIST ] = offset; + pdev->config[PCI_STATUS ] |=PCI_STATUS_CAP_LIST ; +memset (pdev->used + offset,0xFF , size); +/* Make capability read-only by default */ +memset (pdev->mask + offset,0 , size); +return offset; +} + +/* Unlink capability from the pci config space. */ +void pci_del_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t prev, offset =pci_find_capability_list (pdev, cap_id, &prev); +if (!offset) +return ; + pdev->config[prev] = pdev->config[offset +PCI_CAP_LIST_NEXT ]; +/* Make capability writeable again */ +memset (pdev->mask + offset,0xff , size); +memset (pdev->used + offset,0 , size); + +if (!pdev->config[PCI_CAPABILITY_LIST ]) + pdev->config[PCI_STATUS ] &= ~PCI_STATUS_CAP_LIST ; +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability (PCIDevice *pdev,uint8_t offset,uint8_t size) +{ +memset (pdev->used + offset,0xff , size); +} + +uint8_t pci_find_capability (PCIDevice *pdev,uint8_t cap_id) +{ +uint8_t prev; +return pci_find_capability_list (pdev, cap_id, &prev); +}hw/pci.h +5
-1@@ -123,6 +123,10 @@ typedef struct PCIIORegion {#define PCI_MIN_GNT 0x3e/* 8 bits */ #define PCI_MAX_LAT 0x3f/* 8 bits */ +/* Capability lists */ +#define PCI_CAP_LIST_ID 0/* Capability ID */ +#define PCI_CAP_LIST_NEXT 1/* Next capability in the list */ +#define PCI_REVISION 0x08/* obsolete, use PCI_REVISION_ID */ #define PCI_SUBVENDOR_ID 0x2c/* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */ #define PCI_SUBDEVICE_ID 0x2e/* obsolete, use PCI_SUBSYSTEM_ID */ @@ -130,7 +134,7 @@ typedef struct PCIIORegion {/* Bits in the PCI Status Register (PCI 2.3 spec) */ #define PCI_STATUS_RESERVED1 0x007#define PCI_STATUS_INT_STATUS 0x008 -#define PCI_STATUS_CAPABILITIES 0x010 +#define PCI_STATUS_CAP_LIST 0x010#define PCI_STATUS_66MHZ 0x020#define PCI_STATUS_RESERVED2 0x040#define PCI_STATUS_FAST_BACK 0x080@@ -160,6 +164,9 @@ struct PCIDevice {/* Used to implement R/W bytes */ uint8_t mask[PCI_CONFIG_SPACE_SIZE ];
Quoted 5 lines
hw/pci.h struct PCIDevice { · line 169
Quoted 3 lines
hw/pci.h +3
-0+/* Used to allocate config space for capabilities. */ +uint8_t used[PCI_CONFIG_SPACE_SIZE ]; +bitmap?
Could be a bitmap. But byte for byte is much easier to code for. compare
used[offset] = 1
with
used[offset / 8] = (1 << (offset & 0x7));
Quoted 1 line
Or will be used for other purpose?
We could thinkably mark separate bits as used/unused. It's not useful for capabilities which are always byte aligned, but who knows ...
Quoted 26 lines
Quoted 19 lines
hw/pci.h +9
-0/* the following fields are read only */ PCIBus *bus;int devfn;@@ -194,6 +201,15 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_reserve_capability (PCIDevice *pci_dev,uint8_t offset,uint8_t size); + +uint8_t pci_find_capability (PCIDevice *pci_dev,uint8_t cap_id); + +uint32_t pci_default_read_config (PCIDevice *d,uint32_t address,int len); void pci_default_write_config(PCIDevice *d,Signature · 5 lines
yamahata -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 03/11] qemu: add routines to manage PCI capabilities
In reply to Isaku YamahataQuoted 84 lines · Isaku Yamahata
Quoted 83 lines · Michael S. Tsirkin
Add routines to manage PCI capability list. First user will be MSI-X.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ hw/pci.h | 18 +++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-)hw/pci.c +34
-9@@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f, version);/* PCI device version */ +/* PCI device version and capabilities */ +qemu_put_be32 (f, version); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present);qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); -if (version >=3 ) -qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)@@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) version_id =qemu_get_be32 (f);if (version_id >3 )return -EINVAL ; -qemu_get_buffer (f, s->config,256 ); -pci_update_mappings (s); - -if (version_id >=2 ) -for (i =0 ; i <4 ; i ++) - s->irq_state[i] =qemu_get_be32 (f);if (version_id >=3 ) s->cap_present =qemu_get_be32 (f);else @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (s->cap_present & ~s->cap_supported)return -EINVAL ; +qemu_get_buffer (f, s->config,256 ); +pci_update_mappings (s); + +if (version_id >=2 ) +for (i =0 ; i <4 ; i ++) + s->irq_state[i] =qemu_get_be32 (f); +/* Clear mask and used bits for capabilities. +Must be restored separately, since capabilities can +be placed anywhere in config space. */ +memset (s->used,0 ,PCI_CONFIG_SPACE_SIZE ); +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + s->mask[i] =0xff ;return 0 ; }@@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)return (PCIDevice *)dev; } + +static int pci_find_space (PCIDevice *pdev,uint8_t size) +{ +int offset =PCI_CONFIG_HEADER_SIZE ; +int i; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) +if (pdev->used[i]) + offset = i +1 ; +else if (i - offset +1 == size) +return offset; +return 0 ; +} + +static uint8_t pci_find_capability_list (PCIDevice *pdev,uint8_t cap_id, +uint8_t *prev_p) +{ +uint8_t next, prev; +
Quoted 8 lines
hw/pci.c PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name) · line 903
Quoted 6 lines
hw/pci.c +6
-0+if (!(pdev->config[PCI_STATUS ] &PCI_STATUS_CAP_LIST )) +return 0 ; + +for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); + prev = next +PCI_CAP_LIST_NEXT ) +if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id)typo? ==
Fixed. Thanks.
Quoted 77 lines
Quoted 77 lines
hw/pci.c +49
-0+break ; + + *prev_p = prev; +return next; +} + +/* Reserve space and add capability to the linked list in pci config space */ +int pci_add_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t offset =pci_find_space (pdev, size); +uint8_t *config = pdev->config + offset; +if (!offset) +return -ENOSPC ; + config[PCI_CAP_LIST_ID ] = cap_id; + config[PCI_CAP_LIST_NEXT ] = pdev->config[PCI_CAPABILITY_LIST ]; + pdev->config[PCI_CAPABILITY_LIST ] = offset; + pdev->config[PCI_STATUS ] |=PCI_STATUS_CAP_LIST ; +memset (pdev->used + offset,0xFF , size); +/* Make capability read-only by default */ +memset (pdev->mask + offset,0 , size); +return offset; +} + +/* Unlink capability from the pci config space. */ +void pci_del_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t prev, offset =pci_find_capability_list (pdev, cap_id, &prev); +if (!offset) +return ; + pdev->config[prev] = pdev->config[offset +PCI_CAP_LIST_NEXT ]; +/* Make capability writeable again */ +memset (pdev->mask + offset,0xff , size); +memset (pdev->used + offset,0 , size); + +if (!pdev->config[PCI_CAPABILITY_LIST ]) + pdev->config[PCI_STATUS ] &= ~PCI_STATUS_CAP_LIST ; +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability (PCIDevice *pdev,uint8_t offset,uint8_t size) +{ +memset (pdev->used + offset,0xff , size); +} + +uint8_t pci_find_capability (PCIDevice *pdev,uint8_t cap_id) +{ +uint8_t prev; +return pci_find_capability_list (pdev, cap_id, &prev); +}hw/pci.h +5
-1@@ -123,6 +123,10 @@ typedef struct PCIIORegion {#define PCI_MIN_GNT 0x3e/* 8 bits */ #define PCI_MAX_LAT 0x3f/* 8 bits */ +/* Capability lists */ +#define PCI_CAP_LIST_ID 0/* Capability ID */ +#define PCI_CAP_LIST_NEXT 1/* Next capability in the list */ +#define PCI_REVISION 0x08/* obsolete, use PCI_REVISION_ID */ #define PCI_SUBVENDOR_ID 0x2c/* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */ #define PCI_SUBDEVICE_ID 0x2e/* obsolete, use PCI_SUBSYSTEM_ID */ @@ -130,7 +134,7 @@ typedef struct PCIIORegion {/* Bits in the PCI Status Register (PCI 2.3 spec) */ #define PCI_STATUS_RESERVED1 0x007#define PCI_STATUS_INT_STATUS 0x008 -#define PCI_STATUS_CAPABILITIES 0x010 +#define PCI_STATUS_CAP_LIST 0x010#define PCI_STATUS_66MHZ 0x020#define PCI_STATUS_RESERVED2 0x040#define PCI_STATUS_FAST_BACK 0x080@@ -160,6 +164,9 @@ struct PCIDevice {/* Used to implement R/W bytes */ uint8_t mask[PCI_CONFIG_SPACE_SIZE ];
Quoted 5 lines
hw/pci.h struct PCIDevice { · line 169
Quoted 3 lines
hw/pci.h +3
-0+/* Used to allocate config space for capabilities. */ +uint8_t used[PCI_CONFIG_SPACE_SIZE ]; +bitmap?
Could be a bitmap. But byte for byte is much easier to code for. compare
used[offset] = 1
with
used[offset / 8] = (1 << (offset & 0x7));
Quoted 1 line
Or will be used for other purpose?
We could thinkably mark separate bits as used/unused. It's not useful for capabilities which are always byte aligned, but who knows ...
Quoted 26 lines
Quoted 19 lines
hw/pci.h +9
-0/* the following fields are read only */ PCIBus *bus;int devfn;@@ -194,6 +201,15 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_reserve_capability (PCIDevice *pci_dev,uint8_t offset,uint8_t size); + +uint8_t pci_find_capability (PCIDevice *pci_dev,uint8_t cap_id); + +uint32_t pci_default_read_config (PCIDevice *d,uint32_t address,int len); void pci_default_write_config(PCIDevice *d,Signature · 5 lines
yamahata -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 03/11] qemu: add routines to manage PCI capabilities
In reply to Michael S. TsirkinQuoted 79 lines · Michael S. Tsirkin
Add routines to manage PCI capability list. First user will be MSI-X.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ hw/pci.h | 18 +++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-)hw/pci.c +30
-9@@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)int version = s->cap_present ?3 :2 ;int i; -qemu_put_be32 (f, version);/* PCI device version */ +/* PCI device version and capabilities */ +qemu_put_be32 (f, version); +if (version >=3 ) +qemu_put_be32 (f, s->cap_present);qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]); -if (version >=3 ) -qemu_put_be32 (f, s->cap_present); }int pci_device_load (PCIDevice *s,QEMUFile *f)@@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) version_id =qemu_get_be32 (f);if (version_id >3 )return -EINVAL ; -qemu_get_buffer (f, s->config,256 ); -pci_update_mappings (s); - -if (version_id >=2 ) -for (i =0 ; i <4 ; i ++) - s->irq_state[i] =qemu_get_be32 (f);if (version_id >=3 ) s->cap_present =qemu_get_be32 (f);else @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)if (s->cap_present & ~s->cap_supported)return -EINVAL ; +qemu_get_buffer (f, s->config,256 ); +pci_update_mappings (s); + +if (version_id >=2 ) +for (i =0 ; i <4 ; i ++) + s->irq_state[i] =qemu_get_be32 (f); +/* Clear mask and used bits for capabilities. +Must be restored separately, since capabilities can +be placed anywhere in config space. */ +memset (s->used,0 ,PCI_CONFIG_SPACE_SIZE ); +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) + s->mask[i] =0xff ;return 0 ; }@@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)return (PCIDevice *)dev; } + +static int pci_find_space (PCIDevice *pdev,uint8_t size) +{ +int offset =PCI_CONFIG_HEADER_SIZE ; +int i; +for (i =PCI_CONFIG_HEADER_SIZE ; i <PCI_CONFIG_SPACE_SIZE ; ++i) +if (pdev->used[i]) + offset = i +1 ; +else if (i - offset +1 == size) +return offset; +return 0 ; +} + +static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
hw/pci.c PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name) · line 903
Quoted 10 lines
hw/pci.c +10
-0+ uint8_t *prev_p) +{ +uint8_t next, prev; + +if (!(pdev->config[PCI_STATUS ] &PCI_STATUS_CAP_LIST )) +return 0 ; + +for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); + prev = next +PCI_CAP_LIST_NEXT ) +if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id)
typo? ==
Quoted 74 lines
hw/pci.c +49
-0+break ; + + *prev_p = prev; +return next; +} + +/* Reserve space and add capability to the linked list in pci config space */ +int pci_add_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t offset =pci_find_space (pdev, size); +uint8_t *config = pdev->config + offset; +if (!offset) +return -ENOSPC ; + config[PCI_CAP_LIST_ID ] = cap_id; + config[PCI_CAP_LIST_NEXT ] = pdev->config[PCI_CAPABILITY_LIST ]; + pdev->config[PCI_CAPABILITY_LIST ] = offset; + pdev->config[PCI_STATUS ] |=PCI_STATUS_CAP_LIST ; +memset (pdev->used + offset,0xFF , size); +/* Make capability read-only by default */ +memset (pdev->mask + offset,0 , size); +return offset; +} + +/* Unlink capability from the pci config space. */ +void pci_del_capability (PCIDevice *pdev,uint8_t cap_id,uint8_t size) +{ +uint8_t prev, offset =pci_find_capability_list (pdev, cap_id, &prev); +if (!offset) +return ; + pdev->config[prev] = pdev->config[offset +PCI_CAP_LIST_NEXT ]; +/* Make capability writeable again */ +memset (pdev->mask + offset,0xff , size); +memset (pdev->used + offset,0 , size); + +if (!pdev->config[PCI_CAPABILITY_LIST ]) + pdev->config[PCI_STATUS ] &= ~PCI_STATUS_CAP_LIST ; +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability (PCIDevice *pdev,uint8_t offset,uint8_t size) +{ +memset (pdev->used + offset,0xff , size); +} + +uint8_t pci_find_capability (PCIDevice *pdev,uint8_t cap_id) +{ +uint8_t prev; +return pci_find_capability_list (pdev, cap_id, &prev); +}hw/pci.h +5
-1@@ -123,6 +123,10 @@ typedef struct PCIIORegion {#define PCI_MIN_GNT 0x3e/* 8 bits */ #define PCI_MAX_LAT 0x3f/* 8 bits */ +/* Capability lists */ +#define PCI_CAP_LIST_ID 0/* Capability ID */ +#define PCI_CAP_LIST_NEXT 1/* Next capability in the list */ +#define PCI_REVISION 0x08/* obsolete, use PCI_REVISION_ID */ #define PCI_SUBVENDOR_ID 0x2c/* obsolete, use PCI_SUBSYSTEM_VENDOR_ID */ #define PCI_SUBDEVICE_ID 0x2e/* obsolete, use PCI_SUBSYSTEM_ID */ @@ -130,7 +134,7 @@ typedef struct PCIIORegion {/* Bits in the PCI Status Register (PCI 2.3 spec) */ #define PCI_STATUS_RESERVED1 0x007#define PCI_STATUS_INT_STATUS 0x008 -#define PCI_STATUS_CAPABILITIES 0x010 +#define PCI_STATUS_CAP_LIST 0x010#define PCI_STATUS_66MHZ 0x020#define PCI_STATUS_RESERVED2 0x040#define PCI_STATUS_FAST_BACK 0x080
hw/pci.h struct PCIDevice { · line 169
Quoted 6 lines
hw/pci.h +3
-0@@ -160,6 +164,9 @@ struct PCIDevice {/* Used to implement R/W bytes */ uint8_t mask[PCI_CONFIG_SPACE_SIZE ]; +/* Used to allocate config space for capabilities. */ +uint8_t used[PCI_CONFIG_SPACE_SIZE ]; +
bitmap? Or will be used for other purpose?
Quoted 19 lines
hw/pci.h +9
-0/* the following fields are read only */ PCIBus *bus;int devfn;@@ -194,6 +201,15 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size); + +void pci_reserve_capability (PCIDevice *pci_dev,uint8_t offset,uint8_t size); + +uint8_t pci_find_capability (PCIDevice *pci_dev,uint8_t cap_id); + +uint32_t pci_default_read_config (PCIDevice *d,uint32_t address,int len); void pci_default_write_config(PCIDevice *d,
Signature · 1 line
yamahata
Michael S. Tsirkin
maintainerPATCH 1/1 qemu: fix pci_find_capability for multiple caps
In reply to Michael S. Tsirkinpci_find_capability_list has a bug so it'd stop at the first capability. This only happens to work as we only support a single capability (MSI-X). Here's a fix.
---
This is a fixup for my patch "qemu: add routines to manage PCI capabilities".
hw/pci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
hw/pci.c +1 -1
@@ -918,7 +918,7 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,for (prev =PCI_CAPABILITY_LIST ; (next = pdev->config[prev]); prev = next +PCI_CAP_LIST_NEXT ) -if (pdev->config[next +PCI_CAP_LIST_ID ] != cap_id) +if (pdev->config[next +PCI_CAP_LIST_ID ] == cap_id)break ; *prev_p = prev;
Signature · 1 line
MST
Michael S. Tsirkin
maintainerPATCH 4/11 qemu: helper routines for pci access.
In reply to a message not in the archiveAdd inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
--- hw/pci.h | 30 +++++++++++++++++++++++++++--- 1 files changed, 27 insertions(+), 3 deletions(-)
hw/pci.h +27 -3
@@ -240,21 +240,45 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name);static inline void +pci_set_word (uint8_t *config,uint16_t val) +{ +cpu_to_le16wu ((uint16_t *)config, val); +} + +static inline uint16_t +pci_get_word (uint8_t *config) +{ +return le16_to_cpupu ((uint16_t *)config); +} + +static inline void +pci_set_long (uint8_t *config,uint32_t val) +{ +cpu_to_le32wu ((uint32_t *)config, val); +} + +static inline uint32_t +pci_get_long (uint8_t *config) +{ +return le32_to_cpupu ((uint32_t *)config); +} + +static inline void pci_config_set_vendor_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_VENDOR_ID ], val); +pci_set_word (&pci_config[PCI_VENDOR_ID ], val); }static inline void pci_config_set_device_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_DEVICE_ID ], val); +pci_set_word (&pci_config[PCI_DEVICE_ID ], val); }static inline void pci_config_set_class (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_CLASS_DEVICE ], val); +pci_set_word (&pci_config[PCI_CLASS_DEVICE ], val); }typedef void (*pci_qdev_initfn )(PCIDevice *dev);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 4/11 qemu: helper routines for pci access.
In reply to a message not in the archiveAdd inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
--- hw/pci.h | 30 +++++++++++++++++++++++++++--- 1 files changed, 27 insertions(+), 3 deletions(-)
hw/pci.h +27 -3
@@ -240,21 +240,45 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name);static inline void +pci_set_word (uint8_t *config,uint16_t val) +{ +cpu_to_le16wu ((uint16_t *)config, val); +} + +static inline uint16_t +pci_get_word (uint8_t *config) +{ +return le16_to_cpupu ((uint16_t *)config); +} + +static inline void +pci_set_long (uint8_t *config,uint32_t val) +{ +cpu_to_le32wu ((uint32_t *)config, val); +} + +static inline uint32_t +pci_get_long (uint8_t *config) +{ +return le32_to_cpupu ((uint32_t *)config); +} + +static inline void pci_config_set_vendor_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_VENDOR_ID ], val); +pci_set_word (&pci_config[PCI_VENDOR_ID ], val); }static inline void pci_config_set_device_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_DEVICE_ID ], val); +pci_set_word (&pci_config[PCI_DEVICE_ID ], val); }static inline void pci_config_set_class (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_CLASS_DEVICE ], val); +pci_set_word (&pci_config[PCI_CLASS_DEVICE ], val); }typedef void (*pci_qdev_initfn )(PCIDevice *dev);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?
Quoted 61 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/pci.h | 30 +++++++++++++++++++++++++++--- 1 files changed, 27 insertions(+), 3 deletions(-)hw/pci.h +27
-3@@ -240,21 +240,45 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name);static inline void +pci_set_word (uint8_t *config,uint16_t val) +{ +cpu_to_le16wu ((uint16_t *)config, val); +} + +static inline uint16_t +pci_get_word (uint8_t *config) +{ +return le16_to_cpupu ((uint16_t *)config); +} + +static inline void +pci_set_long (uint8_t *config,uint32_t val) +{ +cpu_to_le32wu ((uint32_t *)config, val); +} + +static inline uint32_t +pci_get_long (uint8_t *config) +{ +return le32_to_cpupu ((uint32_t *)config); +} + +static inline void pci_config_set_vendor_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_VENDOR_ID ], val); +pci_set_word (&pci_config[PCI_VENDOR_ID ], val); }static inline void pci_config_set_device_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_DEVICE_ID ], val); +pci_set_word (&pci_config[PCI_DEVICE_ID ], val); }static inline void pci_config_set_class (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_CLASS_DEVICE ], val); +pci_set_word (&pci_config[PCI_CLASS_DEVICE ], val); }typedef void (*pci_qdev_initfn )(PCIDevice *dev);
Signature · 1 line
yamahata
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
Quoted 6 lines · Isaku Yamahata
Quoted 2 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?
I don't see that it makes sense - pci_set_long(config, value) is shorter than *((uint32_t *)config) = cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter.
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
In reply to Isaku YamahataQuoted 6 lines · Isaku Yamahata
Quoted 2 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?
I don't see that it makes sense - pci_set_long(config, value) is shorter than *((uint32_t *)config) = cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter.
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
Quoted 7 lines · Michael S. Tsirkin
Quoted 5 lines · Isaku Yamahata
Quoted 3 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Quoted 9 lines
Quoted 3 lines
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?I don't see that it makes sense -
pci_set_long(config, value)is shorter than *((uint32_t *)config) =cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter.
It's nice to have consistent APIs though.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
In reply to Michael S. TsirkinQuoted 7 lines · Michael S. Tsirkin
Quoted 5 lines · Isaku Yamahata
Quoted 3 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Quoted 9 lines
Quoted 3 lines
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?I don't see that it makes sense -
pci_set_long(config, value)is shorter than *((uint32_t *)config) =cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter.
It's nice to have consistent APIs though.
Signature · 1 line
error compiling committee.c: too many arguments to function
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
Quoted 12 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
Quoted 8 lines · Isaku Yamahata
Quoted 3 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?
Quoted 7 lines
Quoted 5 lines
I don't see that it makes sense -
pci_set_long(config, value)is shorter than *((uint32_t *)config) =cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter.It's nice to have consistent APIs though.
Well, if enough people feel so ...
qemu: add pci_get/set_byte
Add pci_get/set_byte to keep *_word and *_long access functions company. They are unused for now.
---
hw/pci.h +12 -0
@@ -263,6 +263,18 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name);static inline void +pci_set_byte (uint8_t *config,uint8_t val) +{ + *config = val; +} + +static inline uint8_t +pci_get_byte (uint8_t *config) +{ +return *config; +} + +static inline void pci_set_word (uint8_t *config,uint16_t val) {cpu_to_le16wu ((uint16_t *)config, val);
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
In reply to Avi KivityQuoted 12 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
Quoted 8 lines · Isaku Yamahata
Quoted 3 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?
Quoted 7 lines
Quoted 5 lines
I don't see that it makes sense -
pci_set_long(config, value)is shorter than *((uint32_t *)config) =cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter.It's nice to have consistent APIs though.
Well, if enough people feel so ...
qemu: add pci_get/set_byte
Add pci_get/set_byte to keep *_word and *_long access functions company. They are unused for now.
---
hw/pci.h +12 -0
@@ -263,6 +263,18 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name);static inline void +pci_set_byte (uint8_t *config,uint8_t val) +{ + *config = val; +} + +static inline uint8_t +pci_get_byte (uint8_t *config) +{ +return *config; +} + +static inline void pci_set_word (uint8_t *config,uint16_t val) {cpu_to_le16wu ((uint16_t *)config, val);
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
In reply to Michael S. TsirkinJust a minor comment.
How about to add pci_[sg]et_byte() for consistency?
Quoted 61 lines · Michael S. Tsirkin
Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support.
Signed-off-by Michael S. Tsirkin <[email protected]>--- hw/pci.h | 30 +++++++++++++++++++++++++++--- 1 files changed, 27 insertions(+), 3 deletions(-)hw/pci.h +27
-3@@ -240,21 +240,45 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name);static inline void +pci_set_word (uint8_t *config,uint16_t val) +{ +cpu_to_le16wu ((uint16_t *)config, val); +} + +static inline uint16_t +pci_get_word (uint8_t *config) +{ +return le16_to_cpupu ((uint16_t *)config); +} + +static inline void +pci_set_long (uint8_t *config,uint32_t val) +{ +cpu_to_le32wu ((uint32_t *)config, val); +} + +static inline uint32_t +pci_get_long (uint8_t *config) +{ +return le32_to_cpupu ((uint32_t *)config); +} + +static inline void pci_config_set_vendor_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_VENDOR_ID ], val); +pci_set_word (&pci_config[PCI_VENDOR_ID ], val); }static inline void pci_config_set_device_id (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_DEVICE_ID ], val); +pci_set_word (&pci_config[PCI_DEVICE_ID ], val); }static inline void pci_config_set_class (uint8_t *pci_config,uint16_t val) { -cpu_to_le16wu ((uint16_t *)&pci_config[PCI_CLASS_DEVICE ], val); +pci_set_word (&pci_config[PCI_CLASS_DEVICE ], val); }typedef void (*pci_qdev_initfn )(PCIDevice *dev);
Signature · 1 line
yamahata
Michael S. Tsirkin
maintainerPATCH 5/11 qemu: MSI-X support functions
In reply to a message not in the archiveAdd functions implementing MSI-X support. First user will be virtio-pci. Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
--- Makefile.target | 2 +- hw/msix.c | 423 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/msix.h | 35 +++++ hw/pci.h | 20 +++ 4 files changed, 479 insertions(+), 1 deletions(-) create mode 100644 hw/msix.c create mode 100644 hw/msix.h
Makefile.target +1 -1
@@ -486,7 +486,7 @@ endif #CONFIG_BSD_USER ifndef CONFIG_USER_ONLY OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o \ - gdbstub.o gdbstub-xml.o + gdbstub.o gdbstub-xml.o msix.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly OBJS+=virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
hw/msix.c new file +423 -0
423 lines not on this page · raw message
hw/msix.h new file +35 -0
@@ -0,0 +1,35 @@ +#ifndef QEMU_MSIX_H +#define QEMU_MSIX_H + +#include "qemu-common.h" + +int msix_init (PCIDevice *pdev,unsigned short nentries, +unsigned bar_nr,unsigned bar_size); + +void msix_write_config (PCIDevice *pci_dev,uint32_t address, +uint32_t val,int len); + +void msix_mmio_map (PCIDevice *pci_dev,int region_num, +uint32_t addr,uint32_t size,int type); + +int msix_uninit (PCIDevice *d); + +void msix_save (PCIDevice *dev,QEMUFile *f); +int msix_load (PCIDevice *dev,QEMUFile *f); + +int msix_enabled (PCIDevice *dev); +int msix_present (PCIDevice *dev); + +uint32_t msix_bar_size (PCIDevice *dev); + +int msix_vector_use (PCIDevice *dev,unsigned vector); +void msix_vector_unuse (PCIDevice *dev,unsigned vector); + +void msix_notify (PCIDevice *dev,unsigned vector); + +void msix_reset (PCIDevice *dev); + +extern int msix_disable; +extern int msix_supported; + +#endif
hw/pci.h +20 -0
@@ -156,6 +156,11 @@ typedef struct PCIIORegion {/* Size of the standard PCI config space */ #define PCI_CONFIG_SPACE_SIZE 0x100 +/* Bits in cap_supported/cap_present fields. */ +enum { +QEMU_PCI_CAP_MSIX =0x1 , +}; +struct PCIDevice {DeviceState qdev;/* PCI config space */
@@ -189,6 +194,21 @@ struct PCIDevice {/* Capability bits for save/load */ uint32_t cap_supported;uint32_t cap_present; + +/* Offset of MSI-X capability in config space */ +uint8_t msix_cap; + +/* MSI-X entries */ +int msix_entries_nr; + +/* Space to store MSIX table */ +uint8_t *msix_table_page; +/* MMIO index used to map MSIX table and pending bit entries. */ +int msix_mmio_index; +/* Reference-count for entries actually in use by driver. */ +unsigned *msix_entry_used; +/* Region including the MSI-X table */ +uint32_t msix_bar_size; }; PCIDevice *pci_register_device(PCIBus *bus, const char *name,
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 5/11 qemu: MSI-X support functions
In reply to a message not in the archiveAdd functions implementing MSI-X support. First user will be virtio-pci. Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
--- Makefile.target | 2 +- hw/msix.c | 423 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/msix.h | 35 +++++ hw/pci.h | 20 +++ 4 files changed, 479 insertions(+), 1 deletions(-) create mode 100644 hw/msix.c create mode 100644 hw/msix.h
Makefile.target +1 -1
@@ -486,7 +486,7 @@ endif #CONFIG_BSD_USER ifndef CONFIG_USER_ONLY OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o \ - gdbstub.o gdbstub-xml.o + gdbstub.o gdbstub-xml.o msix.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly OBJS+=virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
hw/msix.c new file +423 -0
423 lines not on this page · raw message
hw/msix.h new file +35 -0
@@ -0,0 +1,35 @@ +#ifndef QEMU_MSIX_H +#define QEMU_MSIX_H + +#include "qemu-common.h" + +int msix_init (PCIDevice *pdev,unsigned short nentries, +unsigned bar_nr,unsigned bar_size); + +void msix_write_config (PCIDevice *pci_dev,uint32_t address, +uint32_t val,int len); + +void msix_mmio_map (PCIDevice *pci_dev,int region_num, +uint32_t addr,uint32_t size,int type); + +int msix_uninit (PCIDevice *d); + +void msix_save (PCIDevice *dev,QEMUFile *f); +int msix_load (PCIDevice *dev,QEMUFile *f); + +int msix_enabled (PCIDevice *dev); +int msix_present (PCIDevice *dev); + +uint32_t msix_bar_size (PCIDevice *dev); + +int msix_vector_use (PCIDevice *dev,unsigned vector); +void msix_vector_unuse (PCIDevice *dev,unsigned vector); + +void msix_notify (PCIDevice *dev,unsigned vector); + +void msix_reset (PCIDevice *dev); + +extern int msix_disable; +extern int msix_supported; + +#endif
hw/pci.h +20 -0
@@ -156,6 +156,11 @@ typedef struct PCIIORegion {/* Size of the standard PCI config space */ #define PCI_CONFIG_SPACE_SIZE 0x100 +/* Bits in cap_supported/cap_present fields. */ +enum { +QEMU_PCI_CAP_MSIX =0x1 , +}; +struct PCIDevice {DeviceState qdev;/* PCI config space */
@@ -189,6 +194,21 @@ struct PCIDevice {/* Capability bits for save/load */ uint32_t cap_supported;uint32_t cap_present; + +/* Offset of MSI-X capability in config space */ +uint8_t msix_cap; + +/* MSI-X entries */ +int msix_entries_nr; + +/* Space to store MSIX table */ +uint8_t *msix_table_page; +/* MMIO index used to map MSIX table and pending bit entries. */ +int msix_mmio_index; +/* Reference-count for entries actually in use by driver. */ +unsigned *msix_entry_used; +/* Region including the MSI-X table */ +uint32_t msix_bar_size; }; PCIDevice *pci_register_device(PCIBus *bus, const char *name,
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 3 lines · Michael S. Tsirkin
Add functions implementing MSI-X support. First user will be virtio-pci. Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 3 lines · Michael S. Tsirkin
Add functions implementing MSI-X support. First user will be virtio-pci. Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 9 lines · Paul Brook
Quoted 3 lines · Michael S. Tsirkin
Add functions implementing MSI-X support. First user will be virtio-pci. Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
Paul
You are right of course. In theory there's nothing that breaks if I set this flag to on, on all platforms. OTOH if qemu emulates some controller incorrectly, guest might misdetect MSI support in the controller, and things will break horribly.
It seems safer to have a flag that can be enabled by people that know about a specific platform.
What do you think?
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 9 lines · Paul Brook
Quoted 3 lines · Michael S. Tsirkin
Add functions implementing MSI-X support. First user will be virtio-pci. Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
Paul
You are right of course. In theory there's nothing that breaks if I set this flag to on, on all platforms. OTOH if qemu emulates some controller incorrectly, guest might misdetect MSI support in the controller, and things will break horribly.
It seems safer to have a flag that can be enabled by people that know about a specific platform.
What do you think?
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
Quoted 7 lines
You are right of course. In theory there's nothing that breaks if I set this flag to on, on all platforms. OTOH if qemu emulates some controller incorrectly, guest might misdetect MSI support in the controller, and things will break horribly.
It seems safer to have a flag that can be enabled by people that know about a specific platform.
No. The solution is to fix whatever is broken.
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 7 lines
Quoted 6 lines
Quoted 2 lines
Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
Quoted 7 lines
You are right of course. In theory there's nothing that breaks if I set this flag to on, on all platforms. OTOH if qemu emulates some controller incorrectly, guest might misdetect MSI support in the controller, and things will break horribly.
It seems safer to have a flag that can be enabled by people that know about a specific platform.
No. The solution is to fix whatever is broken.
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 12 lines · Paul Brook
Quoted 12 lines
Quoted 6 lines
Quoted 2 lines
Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
You are right of course. In theory there's nothing that breaks if I set this flag to on, on all platforms. OTOH if qemu emulates some controller incorrectly, guest might misdetect MSI support in the controller, and things will break horribly.
Quoted 4 lines
Quoted 2 lines
It seems safer to have a flag that can be enabled by people that know about a specific platform.
No. The solution is to fix whatever is broken.
That's easy enough then. Patch below.
Quoted 6 lines
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Paul
Why should it be done per-device?
--->
Don't add an option for platforms to disable MSI-X in all devices. Paul Brook will find and fix all platforms that have broken MSI-X emulation.
---
hw/apic.c +0 -1
@@ -945,7 +945,6 @@ int apic_init(CPUState *env) s->cpu_env = env;apic_reset (s); - msix_supported =1 ;/* XXX: mapping more APICs at the same memory location */ if (apic_io_memory ==0 ) {
hw/msix.c +1 -7
@@ -62,9 +62,6 @@/* Flag to globally disable MSI-X support */ int msix_disable; -/* Flag for interrupt controller to declare MSI-X support */ -int msix_supported; -/* Add MSI-X capability to the config space for the device. */ /* Given a bar and its size, add MSI-X table on top of it * and fill MSI-X capability in the config space.
@@ -232,10 +229,7 @@ void msix_mmio_map(PCIDevice *d, int region_num,int msix_init (struct PCIDevice *dev,unsigned short nentries,unsigned bar_nr,unsigned bar_size) { -int ret = -ENOMEM ; -/* Nothing to do if MSI is not supported by interrupt controller */ -if (!msix_supported) -return -ENOTTY ; +int ret;if (nentries >MSIX_MAX_ENTRIES )return -EINVAL ;
hw/msix.h +0 -1
@@ -30,6 +30,5 @@ void msix_notify(PCIDevice *dev, unsigned vector);void msix_reset (PCIDevice *dev);extern int msix_disable; -extern int msix_supported;#endif
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 12 lines · Paul Brook
Quoted 12 lines
Quoted 6 lines
Quoted 2 lines
Note that platform must set a flag to declare MSI supported. For PC this will be set by APIC.
This sounds wrong. The device shouldn't know or care whether the system has a MSI capable interrupt controller. That's for the guest OS to figure out.
You are right of course. In theory there's nothing that breaks if I set this flag to on, on all platforms. OTOH if qemu emulates some controller incorrectly, guest might misdetect MSI support in the controller, and things will break horribly.
Quoted 4 lines
Quoted 2 lines
It seems safer to have a flag that can be enabled by people that know about a specific platform.
No. The solution is to fix whatever is broken.
That's easy enough then. Patch below.
Quoted 6 lines
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Paul
Why should it be done per-device?
--->
Don't add an option for platforms to disable MSI-X in all devices. Paul Brook will find and fix all platforms that have broken MSI-X emulation.
---
hw/apic.c +0 -1
@@ -945,7 +945,6 @@ int apic_init(CPUState *env) s->cpu_env = env;apic_reset (s); - msix_supported =1 ;/* XXX: mapping more APICs at the same memory location */ if (apic_io_memory ==0 ) {
hw/msix.c +1 -7
@@ -62,9 +62,6 @@/* Flag to globally disable MSI-X support */ int msix_disable; -/* Flag for interrupt controller to declare MSI-X support */ -int msix_supported; -/* Add MSI-X capability to the config space for the device. */ /* Given a bar and its size, add MSI-X table on top of it * and fill MSI-X capability in the config space.
@@ -232,10 +229,7 @@ void msix_mmio_map(PCIDevice *d, int region_num,int msix_init (struct PCIDevice *dev,unsigned short nentries,unsigned bar_nr,unsigned bar_size) { -int ret = -ENOMEM ; -/* Nothing to do if MSI is not supported by interrupt controller */ -if (!msix_supported) -return -ENOTTY ; +int ret;if (nentries >MSIX_MAX_ENTRIES )return -EINVAL ;
hw/msix.h +0 -1
@@ -30,6 +30,5 @@ void msix_notify(PCIDevice *dev, unsigned vector);void msix_reset (PCIDevice *dev);extern int msix_disable; -extern int msix_supported;#endif
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 7 lines
Quoted 5 lines
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Paul
Why should it be done per-device?
Because otherwise you end up with the horrible hacks that you're currently tripping over: devices have to magically morph into a different device when you load a VM. That's seems just plain wrong to me. Loading a VM shouldn't not do anything that can't happen during normal operation.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 7 lines
Quoted 5 lines
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Paul
Why should it be done per-device?
Because otherwise you end up with the horrible hacks that you're currently tripping over: devices have to magically morph into a different device when you load a VM. That's seems just plain wrong to me. Loading a VM shouldn't not do anything that can't happen during normal operation.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 4 lines · Paul Brook
Quoted 4 lines
Quoted 4 lines
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Paul
Why should it be done per-device?
Because otherwise you end up with the horrible hacks that you're currently tripping over: devices have to magically morph into a different device when you load a VM.
No, the hacks are there so I that I can support loading and saving from non-MSI setups in a backward-compatible way.
The flag we are discussing is set at qemu startup and can't change across load/store.
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.
At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation. And if we "fix" it, and only edit BAR registers, the happy result will be that we can't add functionality to PCI devices without breaking guests and/or breaking loading from old images each time.
Quoted 1 line
Paul
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 4 lines · Paul Brook
Quoted 4 lines
Quoted 4 lines
If we really need to avoid MSI-X capable devices then that should be done explicity per-device. i.e. you have a different virtio-net device that does not use MSI-X.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Paul
Why should it be done per-device?
Because otherwise you end up with the horrible hacks that you're currently tripping over: devices have to magically morph into a different device when you load a VM.
No, the hacks are there so I that I can support loading and saving from non-MSI setups in a backward-compatible way.
The flag we are discussing is set at qemu startup and can't change across load/store.
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.
At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation. And if we "fix" it, and only edit BAR registers, the happy result will be that we can't add functionality to PCI devices without breaking guests and/or breaking loading from old images each time.
Quoted 1 line
Paul
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 7 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 7 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 4 lines · Paul Brook
Quoted 4 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.
Quoted 8 lines
Quoted 3 lines
At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Paul
Making it work this way will mean that minor changes to a device can break backwards compatibility with old images, often in surprising ways. What are the advantages?
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 4 lines · Paul Brook
Quoted 4 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.
Quoted 8 lines
Quoted 3 lines
At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Paul
Making it work this way will mean that minor changes to a device can break backwards compatibility with old images, often in surprising ways. What are the advantages?
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Paul Brook
Quoted 7 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
Quoted 6 lines
Quoted 2 lines
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Making it work this way will mean that minor changes to a device can break backwards compatibility with old images, often in surprising ways. What are the advantages?
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
It may be that the snapshot/migration code wants to include a machine config, and create a new machine from that. However this is a separate issue, and arguably something your VM manager should be handling for you.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Paul Brook
Quoted 7 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
Quoted 6 lines
Quoted 2 lines
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Making it work this way will mean that minor changes to a device can break backwards compatibility with old images, often in surprising ways. What are the advantages?
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
It may be that the snapshot/migration code wants to include a machine config, and create a new machine from that. However this is a separate issue, and arguably something your VM manager should be handling for you.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 13 lines · Paul Brook
Quoted 12 lines · Michael S. Tsirkin
Quoted 10 lines · Paul Brook
Quoted 7 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Quoted 7 lines
Quoted 3 lines
Making it work this way will mean that minor changes to a device can break backwards compatibility with old images, often in surprising ways. What are the advantages?
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
Configuration does not change until you load another image. Look here:
void msix_reset(PCIDevice *dev)
{
if (!(dev->cap_present & QEMU_PCI_CAP_MSIX))
return;...
}
So once you load and image with MSIX capability off, it will stay off across guest restarts.
Quoted 5 lines
It may be that the snapshot/migration code wants to include a machine config, and create a new machine from that. However this is a separate issue, and arguably something your VM manager should be handling for you.
Paul
Since the image already has a necessary information, duplicating it in a separate machine config will just lead to errors.
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 13 lines · Paul Brook
Quoted 12 lines · Michael S. Tsirkin
Quoted 10 lines · Paul Brook
Quoted 7 lines
Quoted 3 lines
That's seems just plain wrong to me.
Loading a VM shouldn't not
do anything that can't happen during normal operation.At least wrt pci, we are very far from this state: load just overwrites all registers, readonly or not, which can never happen during normal operation.
IMO that code is wrong. We should only be loading things that the guest can change (directly or indirectly).
Quoted 7 lines
Quoted 3 lines
Making it work this way will mean that minor changes to a device can break backwards compatibility with old images, often in surprising ways. What are the advantages?
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
Configuration does not change until you load another image. Look here:
void msix_reset(PCIDevice *dev)
{
if (!(dev->cap_present & QEMU_PCI_CAP_MSIX))
return;...
}
So once you load and image with MSIX capability off, it will stay off across guest restarts.
Quoted 5 lines
It may be that the snapshot/migration code wants to include a machine config, and create a new machine from that. However this is a separate issue, and arguably something your VM manager should be handling for you.
Paul
Since the image already has a necessary information, duplicating it in a separate machine config will just lead to errors.
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 5 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu. I hope I don't need to explain why this is bad.
As previously discussed, any guest visible changes are liable to break a guest OS, particularly guests like Windows which deliberately break when run on "different" hardware. Personally I don't particularly care, but if we support live migration we also need to support "cold" migration - i.e. shutdown and restart.
Quoted 2 lines
So once you load and image with MSIX capability off, it will stay off across guest restarts.
I'm assuming guest restart includes restarting qemu.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 5 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu. I hope I don't need to explain why this is bad.
As previously discussed, any guest visible changes are liable to break a guest OS, particularly guests like Windows which deliberately break when run on "different" hardware. Personally I don't particularly care, but if we support live migration we also need to support "cold" migration - i.e. shutdown and restart.
Quoted 2 lines
So once you load and image with MSIX capability off, it will stay off across guest restarts.
I'm assuming guest restart includes restarting qemu.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 8 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu.
It will behave identically. That's what the patch does: discover how did the device behave on old qemu, and make it behave the same way on new qemu.
Quoted 8 lines
I hope I don't need to explain why this is bad.
As previously discussed, any guest visible changes are liable to break a guest OS, particularly guests like Windows which deliberately break when run on "different" hardware. Personally I don't particularly care, but if we support live migration we also need to support "cold" migration - i.e. shutdown and restart.
Quoted 6 lines
Quoted 2 lines
So once you load and image with MSIX capability off, it will stay off across guest restarts.
I'm assuming guest restart includes restarting qemu.
Paul
If you restart qemu, and load an image, what we should do is look at the image and behave in a way consistent with how the qemu that created the image behaved. If you load an image, you switch to a VM, and should be consistent with the VM you just loaded. And we do not need a flag or a machine description file to tell us this.
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 8 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu.
It will behave identically. That's what the patch does: discover how did the device behave on old qemu, and make it behave the same way on new qemu.
Quoted 8 lines
I hope I don't need to explain why this is bad.
As previously discussed, any guest visible changes are liable to break a guest OS, particularly guests like Windows which deliberately break when run on "different" hardware. Personally I don't particularly care, but if we support live migration we also need to support "cold" migration - i.e. shutdown and restart.
Quoted 6 lines
Quoted 2 lines
So once you load and image with MSIX capability off, it will stay off across guest restarts.
I'm assuming guest restart includes restarting qemu.
Paul
If you restart qemu, and load an image, what we should do is look at the image and behave in a way consistent with how the qemu that created the image behaved. If you load an image, you switch to a VM, and should be consistent with the VM you just loaded. And we do not need a flag or a machine description file to tell us this.
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 5 lines · Michael S. Tsirkin
Quoted 4 lines · Paul Brook
Quoted 4 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu.
It will behave identically. That's what the patch does: discover how did the device behave on old qemu, and make it behave the same way on new qemu.
You're missing the point. After doing a live migration from old-qemu to new- qemu, there is no snapshot to load. We need to be able to shutdown the guest, kill qemu (without saving a snapshot), then start qemu with the exact same hardware.
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 5 lines · Michael S. Tsirkin
Quoted 4 lines · Paul Brook
Quoted 4 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu.
It will behave identically. That's what the patch does: discover how did the device behave on old qemu, and make it behave the same way on new qemu.
You're missing the point. After doing a live migration from old-qemu to new- qemu, there is no snapshot to load. We need to be able to shutdown the guest, kill qemu (without saving a snapshot), then start qemu with the exact same hardware.
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 11 lines · Paul Brook
Quoted 10 lines · Michael S. Tsirkin
Quoted 8 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu.
Quoted 8 lines
Quoted 3 lines
It will behave identically. That's what the patch does: discover how did the device behave on old qemu, and make it behave the same way on new qemu.
You're missing the point. After doing a live migration from old-qemu to new- qemu, there is no snapshot to load. We need to be able to shutdown the guest, kill qemu (without saving a snapshot), then start qemu with the exact same hardware.
Yes, I see how this would sometimes be useful. So this feature request would mean that we need a flag to disable msix in virtio net.
I think we can agree that hardware can change across reboots. It can with real hardware and it happens without guest doing anything.
Quoted 4 lines
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
Paul
OK, so I'll add an option in virtio-net to disable msi-x, and such an option will be added in any device with msi-x support. Will that address your concern?
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 11 lines · Paul Brook
Quoted 10 lines · Michael S. Tsirkin
Quoted 8 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
If you can't create an identical machine from scratch then I don't consider snapshot/migration to be a useful feature. i.e. as soon as you shutdown and restart the guest it is liable to break anyway.
Why is liable to break?
A VM booted on an old version of qemu and migrated to a new version will behave differently to a the same VM booted on a new version of qemu.
Quoted 8 lines
Quoted 3 lines
It will behave identically. That's what the patch does: discover how did the device behave on old qemu, and make it behave the same way on new qemu.
You're missing the point. After doing a live migration from old-qemu to new- qemu, there is no snapshot to load. We need to be able to shutdown the guest, kill qemu (without saving a snapshot), then start qemu with the exact same hardware.
Yes, I see how this would sometimes be useful. So this feature request would mean that we need a flag to disable msix in virtio net.
I think we can agree that hardware can change across reboots. It can with real hardware and it happens without guest doing anything.
Quoted 4 lines
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
Paul
OK, so I'll add an option in virtio-net to disable msi-x, and such an option will be added in any device with msi-x support. Will that address your concern?
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 6 lines
Quoted 2 lines
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
OK, so I'll add an option in virtio-net to disable msi-x, and such an option will be added in any device with msi-x support. Will that address your concern?
Yes, as long as migration fails when you try to migrate to the wrong kind of device.
Paul
Re: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Michael S. TsirkinQuoted 6 lines
Quoted 2 lines
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
OK, so I'll add an option in virtio-net to disable msi-x, and such an option will be added in any device with msi-x support. Will that address your concern?
Yes, as long as migration fails when you try to migrate to the wrong kind of device.
Paul
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
Quoted 11 lines · Paul Brook
Quoted 6 lines
Quoted 2 lines
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
OK, so I'll add an option in virtio-net to disable msi-x, and such an option will be added in any device with msi-x support. Will that address your concern?
Yes, as long as migration fails when you try to migrate to the wrong kind of device.
Paul
I think the right way to do this, is to make sure that standard read-only registers in PCI config space are not modified in migration (device-specific registers could have changed as a result of guest actions, so we can't make assumptions).
Signature · 1 line
MST
Michael S. Tsirkin
maintainerRe: [Qemu-devel] [PATCH 05/11] qemu: MSI-X support functions
In reply to Paul BrookQuoted 11 lines · Paul Brook
Quoted 6 lines
Quoted 2 lines
If we can't start a new qemu with the same hardware configuration then we should not be allowing migration or loading of snapshots.
OK, so I'll add an option in virtio-net to disable msi-x, and such an option will be added in any device with msi-x support. Will that address your concern?
Yes, as long as migration fails when you try to migrate to the wrong kind of device.
Paul
I think the right way to do this, is to make sure that standard read-only registers in PCI config space are not modified in migration (device-specific registers could have changed as a result of guest actions, so we can't make assumptions).
Signature · 1 line
MST
Michael S. Tsirkin
maintainerPATCH 6/11 qemu: add flag to disable MSI-X by default
In reply to a message not in the archiveAdd global flag to disable MSI-X by default. This is useful primarily to make images loadable by older qemu (without msix). Even when MSI-X is disabled by flag, you can still load images that have MSI-X enabled.
--- hw/msix.c | 3 +++ qemu-options.hx | 2 ++ vl.c | 3 +++ 3 files changed, 8 insertions(+), 0 deletions(-)
hw/msix.c +3 -0
@@ -122,6 +122,9 @@ void msix_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len) { unsigned enable_pos = dev->msix_cap + MSIX_ENABLE_OFFSET; +if (!(dev->cap_present &QEMU_PCI_CAP_MSIX )) +return ; +if (addr + len <= enable_pos || addr > enable_pos)return ;
qemu-options.hx +2 -0
@@ -1575,3 +1575,5 @@ DEF("semihosting", 0, QEMU_OPTION_semihosting, DEF("old-param", 0, QEMU_OPTION_old_param, "-old-param old param mode\n") #endif +DEF("disable-msix", 0, QEMU_OPTION_disable_msix, + "-disable-msix disable msix support for PCI devices (enabled by default)\n")
vl.c +3 -0
@@ -134,6 +134,7 @@ int main(int argc, char **argv)#include "hw/usb.h" #include "hw/pcmcia.h" #include "hw/pc.h" +#include "hw/msix.h" #include "hw/audiodev.h" #include "hw/isa.h" #include "hw/baum.h"
@@ -5557,6 +5558,8 @@ int main(int argc, char **argv, char **envp) xen_mode =XEN_ATTACH ;break ;#endif +case QEMU_OPTION_disable_msix: + msix_disable =1 ; } } }
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 6/11 qemu: add flag to disable MSI-X by default
In reply to a message not in the archiveAdd global flag to disable MSI-X by default. This is useful primarily to make images loadable by older qemu (without msix). Even when MSI-X is disabled by flag, you can still load images that have MSI-X enabled.
--- hw/msix.c | 3 +++ qemu-options.hx | 2 ++ vl.c | 3 +++ 3 files changed, 8 insertions(+), 0 deletions(-)
hw/msix.c +3 -0
@@ -122,6 +122,9 @@ void msix_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len) { unsigned enable_pos = dev->msix_cap + MSIX_ENABLE_OFFSET; +if (!(dev->cap_present &QEMU_PCI_CAP_MSIX )) +return ; +if (addr + len <= enable_pos || addr > enable_pos)return ;
qemu-options.hx +2 -0
@@ -1575,3 +1575,5 @@ DEF("semihosting", 0, QEMU_OPTION_semihosting, DEF("old-param", 0, QEMU_OPTION_old_param, "-old-param old param mode\n") #endif +DEF("disable-msix", 0, QEMU_OPTION_disable_msix, + "-disable-msix disable msix support for PCI devices (enabled by default)\n")
vl.c +3 -0
@@ -134,6 +134,7 @@ int main(int argc, char **argv)#include "hw/usb.h" #include "hw/pcmcia.h" #include "hw/pc.h" +#include "hw/msix.h" #include "hw/audiodev.h" #include "hw/isa.h" #include "hw/baum.h"
@@ -5557,6 +5558,8 @@ int main(int argc, char **argv, char **envp) xen_mode =XEN_ATTACH ;break ;#endif +case QEMU_OPTION_disable_msix: + msix_disable =1 ; } } }
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 7/11 qemu: minimal MSI/MSI-X implementation for PC
In reply to a message not in the archiveImplement MSI support in APIC. Note that MSI and MMIO APIC registers are at the same memory location, but actually not on the global bus: MSI is on PCI bus, APIC is connected directly to the CPU. We map them on the global bus at the same address which happens to work because MSI registers are reserved in APIC MMIO and vice versa.
--- hw/apic.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 46 insertions(+), 4 deletions(-)
hw/apic.c +46 -4
@@ -19,6 +19,8 @@*/ #include "hw.h" #include "pc.h" +#include "pci.h" +#include "msix.h" #include "qemu-timer.h" #include "host-utils.h"
@@ -63,6 +65,19 @@#define MAX_APICS 255#define MAX_APIC_WORDS 8 +/* Intel APIC constants: from include/asm/msidef.h */ +#define MSI_DATA_VECTOR_SHIFT 0 +#define MSI_DATA_VECTOR_MASK 0x000000ff +#define MSI_DATA_DELIVERY_MODE_SHIFT 8 +#define MSI_DATA_TRIGGER_SHIFT 15 +#define MSI_DATA_LEVEL_SHIFT 14 +#define MSI_ADDR_DEST_MODE_SHIFT 2 +#define MSI_ADDR_DEST_ID_SHIFT 12 +#define MSI_ADDR_DEST_ID_MASK 0x00ffff0 + +#define MSI_ADDR_BASE 0xfee00000 +#define MSI_ADDR_SIZE 0x100000 + typedef struct APICState { CPUState *cpu_env; uint32_t apicbase;
@@ -86,6 +101,13 @@ typedef struct APICState {QEMUTimer *timer; } APICState; +struct msi_state { +uint64_t addr; +uint32_t data; +int mask; +int pending; +}; +static int apic_io_memory;static APICState *local_apics[MAX_APICS +1 ];static int last_apic_id =0 ;
@@ -712,11 +734,31 @@ static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr)return val; } +static void apic_send_msi (target_phys_addr_t addr,uint32 data) +{ +uint8_t dest = (addr &MSI_ADDR_DEST_ID_MASK ) >>MSI_ADDR_DEST_ID_SHIFT ; +uint8_t vector = (data &MSI_DATA_VECTOR_MASK ) >>MSI_DATA_VECTOR_SHIFT ; +uint8_t dest_mode = (addr >>MSI_ADDR_DEST_MODE_SHIFT ) &0x1 ; +uint8_t trigger_mode = (data >>MSI_DATA_TRIGGER_SHIFT ) &0x1 ; +uint8_t delivery = (data >>MSI_DATA_DELIVERY_MODE_SHIFT ) &0x7 ; +/* XXX: Ignore redirection hint. */ +apic_deliver_irq (dest, dest_mode, delivery, vector,0 , trigger_mode); +} +static void apic_mem_writel (void *opaque,target_phys_addr_t addr,uint32_t val) {CPUState *env;APICState *s; -int index; +int index = (addr >>4 ) &0xff ; +if (addr >0xfff || !index) { +/* MSI and MMIO APIC are at the same memory location, +* but actually not on the global bus: MSI is on PCI bus +* APIC is connected directly to the CPU. +* Mapping them on the global bus happens to work because +* MSI registers are reserved in APIC MMIO and vice versa. */ +apic_send_msi (addr, val); +return ; + } env = cpu_single_env;if (!env)
@@ -727,7 +769,6 @@ static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)printf ("APIC write: %08x = %08x\n" , (uint32_t )addr, val);#endif - index = (addr >>4 ) &0xff ;switch (index) {case 0x02 : s->id = (val >>24 );
@@ -911,6 +952,7 @@ int apic_init(CPUState *env) s->cpu_env = env;apic_reset (s); + msix_supported =1 ;/* XXX: mapping more APICs at the same memory location */ if (apic_io_memory ==0 ) {
@@ -918,7 +960,8 @@ int apic_init(CPUState *env)on the global memory bus. */ apic_io_memory =cpu_register_io_memory (0 , apic_mem_read, apic_mem_write,NULL ); -cpu_register_physical_memory (s->apicbase & ~0xfff ,0x1000 , +/* XXX: what if the base changes? */ +cpu_register_physical_memory (MSI_ADDR_BASE ,MSI_ADDR_SIZE , apic_io_memory); } s->timer =qemu_new_timer (vm_clock, apic_timer, s);
@@ -929,4 +972,3 @@ int apic_init(CPUState *env) local_apics[s->id] = s;return 0 ; } -
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 7/11 qemu: minimal MSI/MSI-X implementation for PC
In reply to a message not in the archiveImplement MSI support in APIC. Note that MSI and MMIO APIC registers are at the same memory location, but actually not on the global bus: MSI is on PCI bus, APIC is connected directly to the CPU. We map them on the global bus at the same address which happens to work because MSI registers are reserved in APIC MMIO and vice versa.
--- hw/apic.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 46 insertions(+), 4 deletions(-)
hw/apic.c +46 -4
@@ -19,6 +19,8 @@*/ #include "hw.h" #include "pc.h" +#include "pci.h" +#include "msix.h" #include "qemu-timer.h" #include "host-utils.h"
@@ -63,6 +65,19 @@#define MAX_APICS 255#define MAX_APIC_WORDS 8 +/* Intel APIC constants: from include/asm/msidef.h */ +#define MSI_DATA_VECTOR_SHIFT 0 +#define MSI_DATA_VECTOR_MASK 0x000000ff +#define MSI_DATA_DELIVERY_MODE_SHIFT 8 +#define MSI_DATA_TRIGGER_SHIFT 15 +#define MSI_DATA_LEVEL_SHIFT 14 +#define MSI_ADDR_DEST_MODE_SHIFT 2 +#define MSI_ADDR_DEST_ID_SHIFT 12 +#define MSI_ADDR_DEST_ID_MASK 0x00ffff0 + +#define MSI_ADDR_BASE 0xfee00000 +#define MSI_ADDR_SIZE 0x100000 + typedef struct APICState { CPUState *cpu_env; uint32_t apicbase;
@@ -86,6 +101,13 @@ typedef struct APICState {QEMUTimer *timer; } APICState; +struct msi_state { +uint64_t addr; +uint32_t data; +int mask; +int pending; +}; +static int apic_io_memory;static APICState *local_apics[MAX_APICS +1 ];static int last_apic_id =0 ;
@@ -712,11 +734,31 @@ static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr)return val; } +static void apic_send_msi (target_phys_addr_t addr,uint32 data) +{ +uint8_t dest = (addr &MSI_ADDR_DEST_ID_MASK ) >>MSI_ADDR_DEST_ID_SHIFT ; +uint8_t vector = (data &MSI_DATA_VECTOR_MASK ) >>MSI_DATA_VECTOR_SHIFT ; +uint8_t dest_mode = (addr >>MSI_ADDR_DEST_MODE_SHIFT ) &0x1 ; +uint8_t trigger_mode = (data >>MSI_DATA_TRIGGER_SHIFT ) &0x1 ; +uint8_t delivery = (data >>MSI_DATA_DELIVERY_MODE_SHIFT ) &0x7 ; +/* XXX: Ignore redirection hint. */ +apic_deliver_irq (dest, dest_mode, delivery, vector,0 , trigger_mode); +} +static void apic_mem_writel (void *opaque,target_phys_addr_t addr,uint32_t val) {CPUState *env;APICState *s; -int index; +int index = (addr >>4 ) &0xff ; +if (addr >0xfff || !index) { +/* MSI and MMIO APIC are at the same memory location, +* but actually not on the global bus: MSI is on PCI bus +* APIC is connected directly to the CPU. +* Mapping them on the global bus happens to work because +* MSI registers are reserved in APIC MMIO and vice versa. */ +apic_send_msi (addr, val); +return ; + } env = cpu_single_env;if (!env)
@@ -727,7 +769,6 @@ static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)printf ("APIC write: %08x = %08x\n" , (uint32_t )addr, val);#endif - index = (addr >>4 ) &0xff ;switch (index) {case 0x02 : s->id = (val >>24 );
@@ -911,6 +952,7 @@ int apic_init(CPUState *env) s->cpu_env = env;apic_reset (s); + msix_supported =1 ;/* XXX: mapping more APICs at the same memory location */ if (apic_io_memory ==0 ) {
@@ -918,7 +960,8 @@ int apic_init(CPUState *env)on the global memory bus. */ apic_io_memory =cpu_register_io_memory (0 , apic_mem_read, apic_mem_write,NULL ); -cpu_register_physical_memory (s->apicbase & ~0xfff ,0x1000 , +/* XXX: what if the base changes? */ +cpu_register_physical_memory (MSI_ADDR_BASE ,MSI_ADDR_SIZE , apic_io_memory); } s->timer =qemu_new_timer (vm_clock, apic_timer, s);
@@ -929,4 +972,3 @@ int apic_init(CPUState *env) local_apics[s->id] = s;return 0 ; } -
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 8/11 qemu: add support for resizing regions
In reply to a message not in the archiveMake it possible to resize PCI regions. This will be used by virtio with MSI-X, where the region size depends on whether MSI-X is enabled, and can change across load/save.
--- hw/pci.c | 54 ++++++++++++++++++++++++++++++++++++------------------ hw/pci.h | 3 +++ 2 files changed, 39 insertions(+), 18 deletions(-)
hw/pci.c +36 -18
@@ -392,6 +392,41 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, *(uint32_t *)(pci_dev->mask + addr) =cpu_to_le32 (mask); } +static void pci_unmap_region (PCIDevice *d,PCIIORegion *r) +{ +if (r->addr ==-1 ) +return ; +if (r->type &PCI_ADDRESS_SPACE_IO ) { +int class; +/* NOTE: specific hack for IDE in PC case: +only one byte must be mapped. */ + class =pci_get_word (d->config +PCI_CLASS_DEVICE ); +if (class ==0x0101 && r->size ==4 ) { +isa_unassign_ioport (r->addr +2 ,1 ); + }else { +isa_unassign_ioport (r->addr, r->size); + } + }else { +cpu_register_physical_memory (pci_to_cpu_addr (r->addr), + r->size, +IO_MEM_UNASSIGNED ); +qemu_unregister_coalesced_mmio (r->addr, r->size); + } +} + +void pci_resize_io_region (PCIDevice *pci_dev,int region_num, +uint32_t size) +{ + +PCIIORegion *r = &pci_dev->io_regions[region_num]; +if (r->size == size) +return ; + r->size = size; +pci_unmap_region (pci_dev, r); + r->addr =-1 ; +pci_update_mappings (pci_dev); +} +static void pci_update_mappings (PCIDevice *d) {PCIIORegion *r;
@@ -445,24 +480,7 @@ static void pci_update_mappings(PCIDevice *d) }/* now do the real mapping */ if (new_addr != r->addr) { -if (r->addr !=-1 ) { -if (r->type &PCI_ADDRESS_SPACE_IO ) { -int class; -/* NOTE: specific hack for IDE in PC case: -only one byte must be mapped. */ - class = d->config[0x0a ] | (d->config[0x0b ] <<8 ); -if (class ==0x0101 && r->size ==4 ) { -isa_unassign_ioport (r->addr +2 ,1 ); - }else { -isa_unassign_ioport (r->addr, r->size); - } - }else { -cpu_register_physical_memory (pci_to_cpu_addr (r->addr), - r->size, -IO_MEM_UNASSIGNED ); -qemu_unregister_coalesced_mmio (r->addr, r->size); - } - } +pci_unmap_region (d, r); r->addr = new_addr;if (r->addr !=-1 ) { r->map_func (d, i, r->addr, r->size, r->type);
hw/pci.h +3 -0
@@ -221,6 +221,9 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +void pci_resize_io_region (PCIDevice *pci_dev,int region_num, +uint32_t size); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size);void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 8/11 qemu: add support for resizing regions
In reply to a message not in the archiveMake it possible to resize PCI regions. This will be used by virtio with MSI-X, where the region size depends on whether MSI-X is enabled, and can change across load/save.
--- hw/pci.c | 54 ++++++++++++++++++++++++++++++++++++------------------ hw/pci.h | 3 +++ 2 files changed, 39 insertions(+), 18 deletions(-)
hw/pci.c +36 -18
@@ -392,6 +392,41 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, *(uint32_t *)(pci_dev->mask + addr) =cpu_to_le32 (mask); } +static void pci_unmap_region (PCIDevice *d,PCIIORegion *r) +{ +if (r->addr ==-1 ) +return ; +if (r->type &PCI_ADDRESS_SPACE_IO ) { +int class; +/* NOTE: specific hack for IDE in PC case: +only one byte must be mapped. */ + class =pci_get_word (d->config +PCI_CLASS_DEVICE ); +if (class ==0x0101 && r->size ==4 ) { +isa_unassign_ioport (r->addr +2 ,1 ); + }else { +isa_unassign_ioport (r->addr, r->size); + } + }else { +cpu_register_physical_memory (pci_to_cpu_addr (r->addr), + r->size, +IO_MEM_UNASSIGNED ); +qemu_unregister_coalesced_mmio (r->addr, r->size); + } +} + +void pci_resize_io_region (PCIDevice *pci_dev,int region_num, +uint32_t size) +{ + +PCIIORegion *r = &pci_dev->io_regions[region_num]; +if (r->size == size) +return ; + r->size = size; +pci_unmap_region (pci_dev, r); + r->addr =-1 ; +pci_update_mappings (pci_dev); +} +static void pci_update_mappings (PCIDevice *d) {PCIIORegion *r;
@@ -445,24 +480,7 @@ static void pci_update_mappings(PCIDevice *d) }/* now do the real mapping */ if (new_addr != r->addr) { -if (r->addr !=-1 ) { -if (r->type &PCI_ADDRESS_SPACE_IO ) { -int class; -/* NOTE: specific hack for IDE in PC case: -only one byte must be mapped. */ - class = d->config[0x0a ] | (d->config[0x0b ] <<8 ); -if (class ==0x0101 && r->size ==4 ) { -isa_unassign_ioport (r->addr +2 ,1 ); - }else { -isa_unassign_ioport (r->addr, r->size); - } - }else { -cpu_register_physical_memory (pci_to_cpu_addr (r->addr), - r->size, -IO_MEM_UNASSIGNED ); -qemu_unregister_coalesced_mmio (r->addr, r->size); - } - } +pci_unmap_region (d, r); r->addr = new_addr;if (r->addr !=-1 ) { r->map_func (d, i, r->addr, r->size, r->type);
hw/pci.h +3 -0
@@ -221,6 +221,9 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, uint32_t size, int type, PCIMapIORegionFunc *map_func); +void pci_resize_io_region (PCIDevice *pci_dev,int region_num, +uint32_t size); +int pci_add_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size);void pci_del_capability (PCIDevice *pci_dev,uint8_t cap_id,uint8_t cap_size);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 9/11 qemu: virtio support for many interrupt vectors
In reply to a message not in the archiveExtend virtio to support many interrupt vectors, and rearrange code in preparation for multi-vector support (mostly move reset out to bindings, because we will have to reset the vectors in transport-specific code). Actual bindings in pci, and use in net, to follow.
Load and save are not connected to bindings yet, so they are left stubbed out for now.
--- hw/syborg_virtio.c | 13 ++++++++-- hw/virtio-pci.c | 24 +++++++++++++++---- hw/virtio.c | 63 ++++++++++++++++++++++++++++++++++++++------------- hw/virtio.h | 10 ++++++- 4 files changed, 84 insertions(+), 26 deletions(-)
hw/syborg_virtio.c +10 -3
@@ -134,7 +134,10 @@ static void syborg_virtio_writel(void *opaque, target_phys_addr_t offset, vdev->features = value;break ;case SYBORG_VIRTIO_QUEUE_BASE : -virtio_queue_set_addr (vdev, vdev->queue_sel, value); +if (value ==0 ) +virtio_reset (vdev); +else +virtio_queue_set_addr (vdev, vdev->queue_sel, value);break ;case SYBORG_VIRTIO_QUEUE_SEL :if (value <VIRTIO_PCI_QUEUE_MAX )
@@ -228,7 +231,7 @@ static CPUWriteMemoryFunc *syborg_virtio_writefn[] = { syborg_virtio_writel }; -static void syborg_virtio_update_irq (void *opaque) +static void syborg_virtio_update_irq (void *opaque,uint16_t vector) {SyborgVirtIOProxy *proxy = opaque;int level;
@@ -239,7 +242,7 @@ static void syborg_virtio_update_irq(void *opaque) }static VirtIOBindings syborg_virtio_bindings = { - .update_irq = syborg_virtio_update_irq + .notify = syborg_virtio_update_irq };static void syborg_virtio_init (SyborgVirtIOProxy *proxy,VirtIODevice *vdev)
@@ -248,6 +251,8 @@ static void syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev) proxy->vdev = vdev; +/* Don't support multiple vectors */ + proxy->vdev->nvectors =0 ;sysbus_init_irq (&proxy->busdev, &proxy->irq); iomemtype =cpu_register_io_memory (0 , syborg_virtio_readfn, syborg_virtio_writefn, proxy);
@@ -255,6 +260,8 @@ static void syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev) proxy->id = ((uint32_t )0x1af4 <<16 ) | vdev->device_id; +qemu_register_reset (virtio_reset,0 , vdev); +virtio_bind_device (vdev, &syborg_virtio_bindings, proxy); }
hw/virtio-pci.c +19 -5
@@ -78,13 +78,19 @@ typedef struct {/* virtio device */ -static void virtio_pci_update_irq (void *opaque) +static void virtio_pci_notify (void *opaque,uint16_t vector) {VirtIOPCIProxy *proxy = opaque;qemu_set_irq (proxy->pci_dev.irq[0 ], proxy->vdev->isr &1 ); } +static void virtio_pci_reset (void *opaque) +{ +VirtIOPCIProxy *proxy = opaque; +virtio_reset (proxy->vdev); +} +static void virtio_ioport_write (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque;
@@ -108,7 +114,10 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)break ;case VIRTIO_PCI_QUEUE_PFN : pa = (target_phys_addr_t )val <<VIRTIO_PCI_QUEUE_ADDR_SHIFT ; -virtio_queue_set_addr (vdev, vdev->queue_sel, pa); +if (pa ==0 ) +virtio_pci_reset (proxy); +else +virtio_queue_set_addr (vdev, vdev->queue_sel, pa);break ;case VIRTIO_PCI_QUEUE_SEL :if (val <VIRTIO_PCI_QUEUE_MAX )
@@ -120,7 +129,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)case VIRTIO_PCI_STATUS : vdev->status = val &0xFF ;if (vdev->status ==0 ) -virtio_reset (vdev); +virtio_pci_reset (proxy);break ; } }
@@ -158,7 +167,7 @@ static uint32_t virtio_ioport_read(void *opaque, uint32_t addr)/* reading from the ISR also clears it. */ ret = vdev->isr; vdev->isr =0 ; -virtio_update_irq (vdev); +qemu_set_irq (proxy->pci_dev.irq[0 ],0 );break ;default :break ;
@@ -243,7 +252,7 @@ static void virtio_map(PCIDevice *pci_dev, int region_num, }static const VirtIOBindings virtio_pci_bindings = { - .update_irq = virtio_pci_update_irq + .notify = virtio_pci_notify }; static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
@@ -255,6 +264,9 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, proxy->vdev = vdev; +/* No support for multiple vectors yet. */ + proxy->vdev->nvectors =0 ; + config = proxy->pci_dev.config;pci_config_set_vendor_id (config, vendor);pci_config_set_device_id (config, device);
@@ -279,6 +291,8 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,pci_register_io_region (&proxy->pci_dev,0 , size,PCI_ADDRESS_SPACE_IO , virtio_map); +qemu_register_reset (virtio_pci_reset,0 , proxy); +virtio_bind_device (vdev, &virtio_pci_bindings, proxy); }
hw/virtio.c +47 -16
166 lines not on this page · raw message
hw/virtio.h +8 -2
@@ -71,11 +71,13 @@ typedef struct VirtQueueElement } VirtQueueElement;typedef struct { -void (*update_irq)(void * opaque); +void (*notify)(void * opaque,uint16_t vector); }VirtIOBindings ;#define VIRTIO_PCI_QUEUE_MAX 16 +#define VIRTIO_NO_VECTOR 0xffff +struct VirtIODevice {const char *name;
@@ -85,6 +87,8 @@ struct VirtIODeviceuint32_t features;size_t config_len;void *config; +uint16_t config_vector; +int nvectors;uint32_t (*get_features)(VirtIODevice *vdev);uint32_t (*bad_features)(VirtIODevice *vdev);void (*set_features)(VirtIODevice *vdev,uint32_t val);
@@ -114,7 +118,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);void virtio_save (VirtIODevice *vdev,QEMUFile *f); -void virtio_load (VirtIODevice *vdev,QEMUFile *f); +int virtio_load (VirtIODevice *vdev,QEMUFile *f);void virtio_cleanup (VirtIODevice *vdev);
@@ -140,6 +144,8 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, target_phys_addr_t addr);target_phys_addr_t virtio_queue_get_addr (VirtIODevice *vdev,int n);int virtio_queue_get_num (VirtIODevice *vdev,int n);void virtio_queue_notify (VirtIODevice *vdev,int n); +uint16_t virtio_queue_vector (VirtIODevice *vdev,int n); +void virtio_queue_set_vector (VirtIODevice *vdev,int n,uint16_t vector);void virtio_reset (void *opaque);void virtio_update_irq (VirtIODevice *vdev);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 9/11 qemu: virtio support for many interrupt vectors
In reply to a message not in the archiveExtend virtio to support many interrupt vectors, and rearrange code in preparation for multi-vector support (mostly move reset out to bindings, because we will have to reset the vectors in transport-specific code). Actual bindings in pci, and use in net, to follow.
Load and save are not connected to bindings yet, so they are left stubbed out for now.
--- hw/syborg_virtio.c | 13 ++++++++-- hw/virtio-pci.c | 24 +++++++++++++++---- hw/virtio.c | 63 ++++++++++++++++++++++++++++++++++++++------------- hw/virtio.h | 10 ++++++- 4 files changed, 84 insertions(+), 26 deletions(-)
hw/syborg_virtio.c +10 -3
@@ -134,7 +134,10 @@ static void syborg_virtio_writel(void *opaque, target_phys_addr_t offset, vdev->features = value;break ;case SYBORG_VIRTIO_QUEUE_BASE : -virtio_queue_set_addr (vdev, vdev->queue_sel, value); +if (value ==0 ) +virtio_reset (vdev); +else +virtio_queue_set_addr (vdev, vdev->queue_sel, value);break ;case SYBORG_VIRTIO_QUEUE_SEL :if (value <VIRTIO_PCI_QUEUE_MAX )
@@ -228,7 +231,7 @@ static CPUWriteMemoryFunc *syborg_virtio_writefn[] = { syborg_virtio_writel }; -static void syborg_virtio_update_irq (void *opaque) +static void syborg_virtio_update_irq (void *opaque,uint16_t vector) {SyborgVirtIOProxy *proxy = opaque;int level;
@@ -239,7 +242,7 @@ static void syborg_virtio_update_irq(void *opaque) }static VirtIOBindings syborg_virtio_bindings = { - .update_irq = syborg_virtio_update_irq + .notify = syborg_virtio_update_irq };static void syborg_virtio_init (SyborgVirtIOProxy *proxy,VirtIODevice *vdev)
@@ -248,6 +251,8 @@ static void syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev) proxy->vdev = vdev; +/* Don't support multiple vectors */ + proxy->vdev->nvectors =0 ;sysbus_init_irq (&proxy->busdev, &proxy->irq); iomemtype =cpu_register_io_memory (0 , syborg_virtio_readfn, syborg_virtio_writefn, proxy);
@@ -255,6 +260,8 @@ static void syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev) proxy->id = ((uint32_t )0x1af4 <<16 ) | vdev->device_id; +qemu_register_reset (virtio_reset,0 , vdev); +virtio_bind_device (vdev, &syborg_virtio_bindings, proxy); }
hw/virtio-pci.c +19 -5
@@ -78,13 +78,19 @@ typedef struct {/* virtio device */ -static void virtio_pci_update_irq (void *opaque) +static void virtio_pci_notify (void *opaque,uint16_t vector) {VirtIOPCIProxy *proxy = opaque;qemu_set_irq (proxy->pci_dev.irq[0 ], proxy->vdev->isr &1 ); } +static void virtio_pci_reset (void *opaque) +{ +VirtIOPCIProxy *proxy = opaque; +virtio_reset (proxy->vdev); +} +static void virtio_ioport_write (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque;
@@ -108,7 +114,10 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)break ;case VIRTIO_PCI_QUEUE_PFN : pa = (target_phys_addr_t )val <<VIRTIO_PCI_QUEUE_ADDR_SHIFT ; -virtio_queue_set_addr (vdev, vdev->queue_sel, pa); +if (pa ==0 ) +virtio_pci_reset (proxy); +else +virtio_queue_set_addr (vdev, vdev->queue_sel, pa);break ;case VIRTIO_PCI_QUEUE_SEL :if (val <VIRTIO_PCI_QUEUE_MAX )
@@ -120,7 +129,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)case VIRTIO_PCI_STATUS : vdev->status = val &0xFF ;if (vdev->status ==0 ) -virtio_reset (vdev); +virtio_pci_reset (proxy);break ; } }
@@ -158,7 +167,7 @@ static uint32_t virtio_ioport_read(void *opaque, uint32_t addr)/* reading from the ISR also clears it. */ ret = vdev->isr; vdev->isr =0 ; -virtio_update_irq (vdev); +qemu_set_irq (proxy->pci_dev.irq[0 ],0 );break ;default :break ;
@@ -243,7 +252,7 @@ static void virtio_map(PCIDevice *pci_dev, int region_num, }static const VirtIOBindings virtio_pci_bindings = { - .update_irq = virtio_pci_update_irq + .notify = virtio_pci_notify }; static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
@@ -255,6 +264,9 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, proxy->vdev = vdev; +/* No support for multiple vectors yet. */ + proxy->vdev->nvectors =0 ; + config = proxy->pci_dev.config;pci_config_set_vendor_id (config, vendor);pci_config_set_device_id (config, device);
@@ -279,6 +291,8 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,pci_register_io_region (&proxy->pci_dev,0 , size,PCI_ADDRESS_SPACE_IO , virtio_map); +qemu_register_reset (virtio_pci_reset,0 , proxy); +virtio_bind_device (vdev, &virtio_pci_bindings, proxy); }
hw/virtio.c +47 -16
166 lines not on this page · raw message
hw/virtio.h +8 -2
@@ -71,11 +71,13 @@ typedef struct VirtQueueElement } VirtQueueElement;typedef struct { -void (*update_irq)(void * opaque); +void (*notify)(void * opaque,uint16_t vector); }VirtIOBindings ;#define VIRTIO_PCI_QUEUE_MAX 16 +#define VIRTIO_NO_VECTOR 0xffff +struct VirtIODevice {const char *name;
@@ -85,6 +87,8 @@ struct VirtIODeviceuint32_t features;size_t config_len;void *config; +uint16_t config_vector; +int nvectors;uint32_t (*get_features)(VirtIODevice *vdev);uint32_t (*bad_features)(VirtIODevice *vdev);void (*set_features)(VirtIODevice *vdev,uint32_t val);
@@ -114,7 +118,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);void virtio_save (VirtIODevice *vdev,QEMUFile *f); -void virtio_load (VirtIODevice *vdev,QEMUFile *f); +int virtio_load (VirtIODevice *vdev,QEMUFile *f);void virtio_cleanup (VirtIODevice *vdev);
@@ -140,6 +144,8 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, target_phys_addr_t addr);target_phys_addr_t virtio_queue_get_addr (VirtIODevice *vdev,int n);int virtio_queue_get_num (VirtIODevice *vdev,int n);void virtio_queue_notify (VirtIODevice *vdev,int n); +uint16_t virtio_queue_vector (VirtIODevice *vdev,int n); +void virtio_queue_set_vector (VirtIODevice *vdev,int n,uint16_t vector);void virtio_reset (void *opaque);void virtio_update_irq (VirtIODevice *vdev);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 10/11 qemu: MSI-X support in virtio PCI
In reply to a message not in the archiveThis enables actual support for MSI-X in virtio PCI. First user will be virtio-net.
--- hw/virtio-pci.c | 152 ++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 112 insertions(+), 40 deletions(-)
hw/virtio-pci.c +112 -40
@@ -18,6 +18,7 @@#include "virtio.h" #include "pci.h" //#include "sysemu.h" +#include "msix.h" /* from Linux's linux/virtio_pci.h */
@@ -47,7 +48,24 @@* a read-and-acknowledge. */ #define VIRTIO_PCI_ISR 19 -#define VIRTIO_PCI_CONFIG 20 +/* MSI-X registers: only enabled if MSI-X is enabled. */ +/* A 16-bit vector for configuration changes. */ +#define VIRTIO_MSI_CONFIG_VECTOR 20 +/* A 16-bit vector for selected queue notifications. */ +#define VIRTIO_MSI_QUEUE_VECTOR 22 + +/* Config space size */ +#define VIRTIO_PCI_CONFIG_NOMSI 20 +#define VIRTIO_PCI_CONFIG_MSI 24 +#define VIRTIO_PCI_REGION_SIZE (dev) (msix_present(dev) ? \ + VIRTIO_PCI_CONFIG_MSI : \ + VIRTIO_PCI_CONFIG_NOMSI) + +/* The remaining space is defined by each driver as the per-driver +* configuration space */ +#define VIRTIO_PCI_CONFIG (dev) (msix_enabled(dev) ? \ + VIRTIO_PCI_CONFIG_MSI : \ + VIRTIO_PCI_CONFIG_NOMSI)/* Virtio ABI version, if we increment this, we break the guest driver. */ #define VIRTIO_PCI_ABI_VERSION 0
@@ -81,14 +99,17 @@ typedef struct {static void virtio_pci_notify (void *opaque,uint16_t vector) {VirtIOPCIProxy *proxy = opaque; - -qemu_set_irq (proxy->pci_dev.irq[0 ], proxy->vdev->isr &1 ); +if (msix_enabled (&proxy->pci_dev)) +msix_notify (&proxy->pci_dev, vector); +else +qemu_set_irq (proxy->pci_dev.irq[0 ], proxy->vdev->isr &1 ); }static void virtio_pci_reset (void *opaque) {VirtIOPCIProxy *proxy = opaque;virtio_reset (proxy->vdev); +msix_reset (&proxy->pci_dev); }static void virtio_ioport_write (void *opaque,uint32_t addr,uint32_t val)
@@ -97,8 +118,6 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)VirtIODevice *vdev = proxy->vdev;target_phys_addr_t pa; - addr -= proxy->addr; -switch (addr) {case VIRTIO_PCI_GUEST_FEATURES :/* Guest does not negotiate properly? We have to assume nothing. */
@@ -131,17 +150,33 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)if (vdev->status ==0 )virtio_pci_reset (proxy);break ; +case VIRTIO_MSI_CONFIG_VECTOR : +msix_vector_unuse (&proxy->pci_dev, vdev->config_vector); +/* Make it possible for guest to discover an error took place. */ +if (msix_vector_use (&proxy->pci_dev, val) <0 ) + val =VIRTIO_NO_VECTOR ; + vdev->config_vector = val; +break ; +case VIRTIO_MSI_QUEUE_VECTOR : +msix_vector_unuse (&proxy->pci_dev, +virtio_queue_vector (vdev, vdev->queue_sel)); +/* Make it possible for guest to discover an error took place. */ +if (msix_vector_use (&proxy->pci_dev, val) <0 ) + val =VIRTIO_NO_VECTOR ; +virtio_queue_set_vector (vdev, vdev->queue_sel, val); +break ; +default : +fprintf (stderr,"%s: unexpected address 0x%x value 0x%x\n" , + __func__, addr, val); +break ; } } -static uint32_t virtio_ioport_read (void *opaque,uint32_t addr) +static uint32_t virtio_ioport_read (VirtIOPCIProxy *proxy,uint32_t addr) { -VirtIOPCIProxy *proxy = opaque;VirtIODevice *vdev = proxy->vdev;uint32_t ret =0xFFFFFFFF ; - addr -= proxy->addr; -switch (addr) {case VIRTIO_PCI_HOST_FEATURES : ret = vdev->get_features (vdev);
@@ -169,6 +204,12 @@ static uint32_t virtio_ioport_read(void *opaque, uint32_t addr) vdev->isr =0 ;qemu_set_irq (proxy->pci_dev.irq[0 ],0 );break ; +case VIRTIO_MSI_CONFIG_VECTOR : + ret = vdev->config_vector; +break ; +case VIRTIO_MSI_QUEUE_VECTOR : + ret =virtio_queue_vector (vdev, vdev->queue_sel); +break ;default :break ; }
@@ -179,42 +220,72 @@ static uint32_t virtio_ioport_read(void *opaque, uint32_t addr)static uint32_t virtio_pci_config_readb (void *opaque,uint32_t addr) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) +return virtio_ioport_read (proxy, addr); + addr -= config;return virtio_config_readb (proxy->vdev, addr); }static uint32_t virtio_pci_config_readw (void *opaque,uint32_t addr) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) +return virtio_ioport_read (proxy, addr); + addr -= config;return virtio_config_readw (proxy->vdev, addr); }static uint32_t virtio_pci_config_readl (void *opaque,uint32_t addr) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) +return virtio_ioport_read (proxy, addr); + addr -= config;return virtio_config_readl (proxy->vdev, addr); }static void virtio_pci_config_writeb (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) { +virtio_ioport_write (proxy, addr, val); +return ; + } + addr -= config;virtio_config_writeb (proxy->vdev, addr, val); }static void virtio_pci_config_writew (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) { +virtio_ioport_write (proxy, addr, val); +return ; + } + addr -= config;virtio_config_writew (proxy->vdev, addr, val); }static void virtio_pci_config_writel (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) { +virtio_ioport_write (proxy, addr, val); +return ; + } + addr -= config;virtio_config_writel (proxy->vdev, addr, val); }
@@ -223,32 +294,26 @@ static void virtio_map(PCIDevice *pci_dev, int region_num, {VirtIOPCIProxy *proxy =container_of (pci_dev, VirtIOPCIProxy, pci_dev);VirtIODevice *vdev = proxy->vdev; -int i; +unsigned config_len =VIRTIO_PCI_REGION_SIZE (pci_dev) + vdev->config_len; proxy->addr = addr; -for (i =0 ; i <3 ; i++) { -register_ioport_write (addr,VIRTIO_PCI_CONFIG ,1 << i, - virtio_ioport_write, proxy); -register_ioport_read (addr,VIRTIO_PCI_CONFIG ,1 << i, - virtio_ioport_read, proxy); - } -if (vdev->config_len) { -register_ioport_write (addr +VIRTIO_PCI_CONFIG , vdev->config_len,1 , - virtio_pci_config_writeb, proxy); -register_ioport_write (addr +VIRTIO_PCI_CONFIG , vdev->config_len,2 , - virtio_pci_config_writew, proxy); -register_ioport_write (addr +VIRTIO_PCI_CONFIG , vdev->config_len,4 , - virtio_pci_config_writel, proxy); -register_ioport_read (addr +VIRTIO_PCI_CONFIG , vdev->config_len,1 , - virtio_pci_config_readb, proxy); -register_ioport_read (addr +VIRTIO_PCI_CONFIG , vdev->config_len,2 , - virtio_pci_config_readw, proxy); -register_ioport_read (addr +VIRTIO_PCI_CONFIG , vdev->config_len,4 , - virtio_pci_config_readl, proxy); +register_ioport_write (addr, config_len,1 , virtio_pci_config_writeb, proxy); +register_ioport_write (addr, config_len,2 , virtio_pci_config_writew, proxy); +register_ioport_write (addr, config_len,4 , virtio_pci_config_writel, proxy); +register_ioport_read (addr, config_len,1 , virtio_pci_config_readb, proxy); +register_ioport_read (addr, config_len,2 , virtio_pci_config_readw, proxy); +register_ioport_read (addr, config_len,4 , virtio_pci_config_readl, proxy); +if (vdev->config_len) vdev->get_config (vdev, vdev->config); - } +} + +static void virtio_write_config (PCIDevice *pci_dev,uint32_t address, +uint32_t val,int len) +{ +pci_default_write_config (pci_dev, address, val, len); +msix_write_config (pci_dev, address, val, len); }static const VirtIOBindings virtio_pci_bindings = {
@@ -264,9 +329,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, proxy->vdev = vdev; -/* No support for multiple vectors yet. */ - proxy->vdev->nvectors =0 ; - config = proxy->pci_dev.config;pci_config_set_vendor_id (config, vendor);pci_config_set_device_id (config, device);
@@ -284,7 +346,17 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, config[0x3d ] =1 ; - size =20 + vdev->config_len; +if (vdev->nvectors && !msix_init (&proxy->pci_dev, vdev->nvectors,1 ,0 )) { +pci_register_io_region (&proxy->pci_dev,1 , +msix_bar_size (&proxy->pci_dev), +PCI_ADDRESS_SPACE_MEM , + msix_mmio_map); + proxy->pci_dev.config_write = virtio_write_config; + proxy->pci_dev.unregister = msix_uninit; + }else + vdev->nvectors =0 ; + + size =VIRTIO_PCI_REGION_SIZE (&proxy->pci_dev) + vdev->config_len;if (size & (size-1 )) size =1 <<qemu_fls (size);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 10/11 qemu: MSI-X support in virtio PCI
In reply to a message not in the archiveThis enables actual support for MSI-X in virtio PCI. First user will be virtio-net.
--- hw/virtio-pci.c | 152 ++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 112 insertions(+), 40 deletions(-)
hw/virtio-pci.c +112 -40
@@ -18,6 +18,7 @@#include "virtio.h" #include "pci.h" //#include "sysemu.h" +#include "msix.h" /* from Linux's linux/virtio_pci.h */
@@ -47,7 +48,24 @@* a read-and-acknowledge. */ #define VIRTIO_PCI_ISR 19 -#define VIRTIO_PCI_CONFIG 20 +/* MSI-X registers: only enabled if MSI-X is enabled. */ +/* A 16-bit vector for configuration changes. */ +#define VIRTIO_MSI_CONFIG_VECTOR 20 +/* A 16-bit vector for selected queue notifications. */ +#define VIRTIO_MSI_QUEUE_VECTOR 22 + +/* Config space size */ +#define VIRTIO_PCI_CONFIG_NOMSI 20 +#define VIRTIO_PCI_CONFIG_MSI 24 +#define VIRTIO_PCI_REGION_SIZE (dev) (msix_present(dev) ? \ + VIRTIO_PCI_CONFIG_MSI : \ + VIRTIO_PCI_CONFIG_NOMSI) + +/* The remaining space is defined by each driver as the per-driver +* configuration space */ +#define VIRTIO_PCI_CONFIG (dev) (msix_enabled(dev) ? \ + VIRTIO_PCI_CONFIG_MSI : \ + VIRTIO_PCI_CONFIG_NOMSI)/* Virtio ABI version, if we increment this, we break the guest driver. */ #define VIRTIO_PCI_ABI_VERSION 0
@@ -81,14 +99,17 @@ typedef struct {static void virtio_pci_notify (void *opaque,uint16_t vector) {VirtIOPCIProxy *proxy = opaque; - -qemu_set_irq (proxy->pci_dev.irq[0 ], proxy->vdev->isr &1 ); +if (msix_enabled (&proxy->pci_dev)) +msix_notify (&proxy->pci_dev, vector); +else +qemu_set_irq (proxy->pci_dev.irq[0 ], proxy->vdev->isr &1 ); }static void virtio_pci_reset (void *opaque) {VirtIOPCIProxy *proxy = opaque;virtio_reset (proxy->vdev); +msix_reset (&proxy->pci_dev); }static void virtio_ioport_write (void *opaque,uint32_t addr,uint32_t val)
@@ -97,8 +118,6 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)VirtIODevice *vdev = proxy->vdev;target_phys_addr_t pa; - addr -= proxy->addr; -switch (addr) {case VIRTIO_PCI_GUEST_FEATURES :/* Guest does not negotiate properly? We have to assume nothing. */
@@ -131,17 +150,33 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)if (vdev->status ==0 )virtio_pci_reset (proxy);break ; +case VIRTIO_MSI_CONFIG_VECTOR : +msix_vector_unuse (&proxy->pci_dev, vdev->config_vector); +/* Make it possible for guest to discover an error took place. */ +if (msix_vector_use (&proxy->pci_dev, val) <0 ) + val =VIRTIO_NO_VECTOR ; + vdev->config_vector = val; +break ; +case VIRTIO_MSI_QUEUE_VECTOR : +msix_vector_unuse (&proxy->pci_dev, +virtio_queue_vector (vdev, vdev->queue_sel)); +/* Make it possible for guest to discover an error took place. */ +if (msix_vector_use (&proxy->pci_dev, val) <0 ) + val =VIRTIO_NO_VECTOR ; +virtio_queue_set_vector (vdev, vdev->queue_sel, val); +break ; +default : +fprintf (stderr,"%s: unexpected address 0x%x value 0x%x\n" , + __func__, addr, val); +break ; } } -static uint32_t virtio_ioport_read (void *opaque,uint32_t addr) +static uint32_t virtio_ioport_read (VirtIOPCIProxy *proxy,uint32_t addr) { -VirtIOPCIProxy *proxy = opaque;VirtIODevice *vdev = proxy->vdev;uint32_t ret =0xFFFFFFFF ; - addr -= proxy->addr; -switch (addr) {case VIRTIO_PCI_HOST_FEATURES : ret = vdev->get_features (vdev);
@@ -169,6 +204,12 @@ static uint32_t virtio_ioport_read(void *opaque, uint32_t addr) vdev->isr =0 ;qemu_set_irq (proxy->pci_dev.irq[0 ],0 );break ; +case VIRTIO_MSI_CONFIG_VECTOR : + ret = vdev->config_vector; +break ; +case VIRTIO_MSI_QUEUE_VECTOR : + ret =virtio_queue_vector (vdev, vdev->queue_sel); +break ;default :break ; }
@@ -179,42 +220,72 @@ static uint32_t virtio_ioport_read(void *opaque, uint32_t addr)static uint32_t virtio_pci_config_readb (void *opaque,uint32_t addr) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) +return virtio_ioport_read (proxy, addr); + addr -= config;return virtio_config_readb (proxy->vdev, addr); }static uint32_t virtio_pci_config_readw (void *opaque,uint32_t addr) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) +return virtio_ioport_read (proxy, addr); + addr -= config;return virtio_config_readw (proxy->vdev, addr); }static uint32_t virtio_pci_config_readl (void *opaque,uint32_t addr) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) +return virtio_ioport_read (proxy, addr); + addr -= config;return virtio_config_readl (proxy->vdev, addr); }static void virtio_pci_config_writeb (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) { +virtio_ioport_write (proxy, addr, val); +return ; + } + addr -= config;virtio_config_writeb (proxy->vdev, addr, val); }static void virtio_pci_config_writew (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) { +virtio_ioport_write (proxy, addr, val); +return ; + } + addr -= config;virtio_config_writew (proxy->vdev, addr, val); }static void virtio_pci_config_writel (void *opaque,uint32_t addr,uint32_t val) {VirtIOPCIProxy *proxy = opaque; - addr -= proxy->addr +VIRTIO_PCI_CONFIG ; +uint32_t config =VIRTIO_PCI_CONFIG (&proxy->pci_dev); + addr -= proxy->addr; +if (addr < config) { +virtio_ioport_write (proxy, addr, val); +return ; + } + addr -= config;virtio_config_writel (proxy->vdev, addr, val); }
@@ -223,32 +294,26 @@ static void virtio_map(PCIDevice *pci_dev, int region_num, {VirtIOPCIProxy *proxy =container_of (pci_dev, VirtIOPCIProxy, pci_dev);VirtIODevice *vdev = proxy->vdev; -int i; +unsigned config_len =VIRTIO_PCI_REGION_SIZE (pci_dev) + vdev->config_len; proxy->addr = addr; -for (i =0 ; i <3 ; i++) { -register_ioport_write (addr,VIRTIO_PCI_CONFIG ,1 << i, - virtio_ioport_write, proxy); -register_ioport_read (addr,VIRTIO_PCI_CONFIG ,1 << i, - virtio_ioport_read, proxy); - } -if (vdev->config_len) { -register_ioport_write (addr +VIRTIO_PCI_CONFIG , vdev->config_len,1 , - virtio_pci_config_writeb, proxy); -register_ioport_write (addr +VIRTIO_PCI_CONFIG , vdev->config_len,2 , - virtio_pci_config_writew, proxy); -register_ioport_write (addr +VIRTIO_PCI_CONFIG , vdev->config_len,4 , - virtio_pci_config_writel, proxy); -register_ioport_read (addr +VIRTIO_PCI_CONFIG , vdev->config_len,1 , - virtio_pci_config_readb, proxy); -register_ioport_read (addr +VIRTIO_PCI_CONFIG , vdev->config_len,2 , - virtio_pci_config_readw, proxy); -register_ioport_read (addr +VIRTIO_PCI_CONFIG , vdev->config_len,4 , - virtio_pci_config_readl, proxy); +register_ioport_write (addr, config_len,1 , virtio_pci_config_writeb, proxy); +register_ioport_write (addr, config_len,2 , virtio_pci_config_writew, proxy); +register_ioport_write (addr, config_len,4 , virtio_pci_config_writel, proxy); +register_ioport_read (addr, config_len,1 , virtio_pci_config_readb, proxy); +register_ioport_read (addr, config_len,2 , virtio_pci_config_readw, proxy); +register_ioport_read (addr, config_len,4 , virtio_pci_config_readl, proxy); +if (vdev->config_len) vdev->get_config (vdev, vdev->config); - } +} + +static void virtio_write_config (PCIDevice *pci_dev,uint32_t address, +uint32_t val,int len) +{ +pci_default_write_config (pci_dev, address, val, len); +msix_write_config (pci_dev, address, val, len); }static const VirtIOBindings virtio_pci_bindings = {
@@ -264,9 +329,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, proxy->vdev = vdev; -/* No support for multiple vectors yet. */ - proxy->vdev->nvectors =0 ; - config = proxy->pci_dev.config;pci_config_set_vendor_id (config, vendor);pci_config_set_device_id (config, device);
@@ -284,7 +346,17 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, config[0x3d ] =1 ; - size =20 + vdev->config_len; +if (vdev->nvectors && !msix_init (&proxy->pci_dev, vdev->nvectors,1 ,0 )) { +pci_register_io_region (&proxy->pci_dev,1 , +msix_bar_size (&proxy->pci_dev), +PCI_ADDRESS_SPACE_MEM , + msix_mmio_map); + proxy->pci_dev.config_write = virtio_write_config; + proxy->pci_dev.unregister = msix_uninit; + }else + vdev->nvectors =0 ; + + size =VIRTIO_PCI_REGION_SIZE (&proxy->pci_dev) + vdev->config_len;if (size & (size-1 )) size =1 <<qemu_fls (size);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 11/11 qemu: request 3 vectors in virtio-net
In reply to a message not in the archiveRequest up to 3 vectors in virtio-net. Actual bindings might supply less.
--- hw/virtio-net.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
hw/virtio-net.c +1 -0
@@ -621,6 +621,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev) n->mac_table.macs =qemu_mallocz (MAC_TABLE_ENTRIES *ETH_ALEN ); n->vlans =qemu_mallocz (MAX_VLAN >>3 ); + n->vdev.nvectors =3 ;register_savevm ("virtio-net" , virtio_net_id++,VIRTIO_NET_VM_VERSION , virtio_net_save, virtio_net_load, n);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
Michael S. Tsirkin
maintainerPATCH 11/11 qemu: request 3 vectors in virtio-net
In reply to a message not in the archiveRequest up to 3 vectors in virtio-net. Actual bindings might supply less.
--- hw/virtio-net.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
hw/virtio-net.c +1 -0
@@ -621,6 +621,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev) n->mac_table.macs =qemu_mallocz (MAC_TABLE_ENTRIES *ETH_ALEN ); n->vlans =qemu_mallocz (MAX_VLAN >>3 ); + n->vdev.nvectors =3 ;register_savevm ("virtio-net" , virtio_net_id++,VIRTIO_NET_VM_VERSION , virtio_net_save, virtio_net_load, n);
Signature · 1 line
1.6.3.1.56.g79e1.dirty