* [Qemu-devel] [PATCH 0/2] pci_add_capability_at_offset @ 2010-04-07 8:04 Michael S. Tsirkin 2010-04-07 8:04 ` [Qemu-devel] [PATCH 1/2] pci: add API to add capability at a known offset Michael S. Tsirkin 2010-04-07 8:04 ` [Qemu-devel] [PATCH 2/2] eepro100: convert to new capability API Michael S. Tsirkin 0 siblings, 2 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2010-04-07 8:04 UTC (permalink / raw) To: weil, qemu-devel eepro100 has to jump through hoops to add capability at a specific offset. It's cleaner to simply add an API for that. Michael S. Tsirkin (2): pci: add API to add capability at a known offset eepro100: convert to new capability API hw/eepro100.c | 22 +++++++++------------- hw/pci.c | 16 ++++++++++++---- hw/pci.h | 2 ++ 3 files changed, 23 insertions(+), 17 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] pci: add API to add capability at a known offset 2010-04-07 8:04 [Qemu-devel] [PATCH 0/2] pci_add_capability_at_offset Michael S. Tsirkin @ 2010-04-07 8:04 ` Michael S. Tsirkin 2010-04-07 8:36 ` [Qemu-devel] " Stefan Weil 2010-04-07 8:04 ` [Qemu-devel] [PATCH 2/2] eepro100: convert to new capability API Michael S. Tsirkin 1 sibling, 1 reply; 6+ messages in thread From: Michael S. Tsirkin @ 2010-04-07 8:04 UTC (permalink / raw) To: weil, qemu-devel Unlike virtio, device emulations need to add pci capabilities at known offsets to match real hardware. Make this possible by adding an appropriate API. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/pci.c | 16 ++++++++++++---- hw/pci.h | 2 ++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 0dbca17..df03149 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1789,12 +1789,10 @@ static int pci_add_option_rom(PCIDevice *pdev) } /* 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) +int pci_add_capability_at_offset(PCIDevice *pdev, uint8_t cap_id, + uint8_t offset, 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; @@ -1807,6 +1805,16 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) return offset; } +/* Find and 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); + if (!offset) + return -ENOSPC; + return pci_add_capability_at_offset(pdev, cap_id, offset, size); +} + /* Unlink capability from the pci config space. */ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) { diff --git a/hw/pci.h b/hw/pci.h index 20c670e..625188c 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -190,6 +190,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, PCIMapIORegionFunc *map_func); int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size); +int pci_add_capability_at_offset(PCIDevice *pci_dev, uint8_t cap_id, + uint8_t cap_offset, uint8_t cap_size); void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size); -- 1.7.0.2.280.gc6f05 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH 1/2] pci: add API to add capability at a known offset 2010-04-07 8:04 ` [Qemu-devel] [PATCH 1/2] pci: add API to add capability at a known offset Michael S. Tsirkin @ 2010-04-07 8:36 ` Stefan Weil 0 siblings, 0 replies; 6+ messages in thread From: Stefan Weil @ 2010-04-07 8:36 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel Michael S. Tsirkin schrieb: > Unlike virtio, device emulations need to add pci capabilities > at known offsets to match real hardware. Make this possible > by adding an appropriate API. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/pci.c | 16 ++++++++++++---- > hw/pci.h | 2 ++ > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 0dbca17..df03149 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1789,12 +1789,10 @@ static int pci_add_option_rom(PCIDevice *pdev) > } > > /* 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) > +int pci_add_capability_at_offset(PCIDevice *pdev, uint8_t cap_id, > + uint8_t offset, 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; > @@ -1807,6 +1805,16 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) > return offset; > } > > +/* Find and 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); > + if (!offset) > + return -ENOSPC; > {} Coding style? The rest is ok. > + return pci_add_capability_at_offset(pdev, cap_id, offset, size); > +} > + > /* Unlink capability from the pci config space. */ > void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) > { > diff --git a/hw/pci.h b/hw/pci.h > index 20c670e..625188c 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -190,6 +190,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, > PCIMapIORegionFunc *map_func); > > int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size); > +int pci_add_capability_at_offset(PCIDevice *pci_dev, uint8_t cap_id, > + uint8_t cap_offset, uint8_t cap_size); > > void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size); > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] eepro100: convert to new capability API 2010-04-07 8:04 [Qemu-devel] [PATCH 0/2] pci_add_capability_at_offset Michael S. Tsirkin 2010-04-07 8:04 ` [Qemu-devel] [PATCH 1/2] pci: add API to add capability at a known offset Michael S. Tsirkin @ 2010-04-07 8:04 ` Michael S. Tsirkin 2010-04-07 8:42 ` [Qemu-devel] " Stefan Weil 1 sibling, 1 reply; 6+ messages in thread From: Michael S. Tsirkin @ 2010-04-07 8:04 UTC (permalink / raw) To: weil, qemu-devel Using new pci_add_capability_at_offset makes eepro100 code cleaner. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/eepro100.c | 22 +++++++++------------- 1 files changed, 9 insertions(+), 13 deletions(-) diff --git a/hw/eepro100.c b/hw/eepro100.c index 785a7da..a74d834 100644 --- a/hw/eepro100.c +++ b/hw/eepro100.c @@ -539,21 +539,17 @@ static void e100_pci_reset(EEPRO100State * s, E100PCIDeviceInfo *e100_device) if (e100_device->power_management) { /* Power Management Capabilities */ - int cfg_offset; - pci_reserve_capability(&s->dev, PCI_CONFIG_HEADER_SIZE, - 0xdc - PCI_CONFIG_HEADER_SIZE); - cfg_offset = pci_add_capability(&s->dev, PCI_CAP_ID_PM, PCI_PM_SIZEOF); - assert(cfg_offset == 0xdc); - if (cfg_offset > 0) { - /* Power Management Capabilities */ - pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21); + int cfg_offset = 0xdc; + int r = pci_add_capability_at_offset(&s->dev, PCI_CAP_ID_PM, + cfg_offset, PCI_PM_SIZEOF); + assert(r >= 0); + pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21); #if 0 /* TODO: replace dummy code for power management emulation. */ - /* TODO: Power Management Control / Status. */ - pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000); - /* TODO: Ethernet Power Consumption Registers (i82559 and later). */ - pci_set_byte(pci_conf + cfg_offset + PCI_PM_PPB_EXTENSIONS, 0x0000); + /* TODO: Power Management Control / Status. */ + pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000); + /* TODO: Ethernet Power Consumption Registers (i82559 and later). */ + pci_set_byte(pci_conf + cfg_offset + PCI_PM_PPB_EXTENSIONS, 0x0000); #endif - } } #if EEPROM_SIZE > 0 -- 1.7.0.2.280.gc6f05 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH 2/2] eepro100: convert to new capability API 2010-04-07 8:04 ` [Qemu-devel] [PATCH 2/2] eepro100: convert to new capability API Michael S. Tsirkin @ 2010-04-07 8:42 ` Stefan Weil 2010-04-07 11:04 ` Michael S. Tsirkin 0 siblings, 1 reply; 6+ messages in thread From: Stefan Weil @ 2010-04-07 8:42 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel Michael S. Tsirkin schrieb: > Using new pci_add_capability_at_offset makes > eepro100 code cleaner. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/eepro100.c | 22 +++++++++------------- > 1 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/hw/eepro100.c b/hw/eepro100.c > index 785a7da..a74d834 100644 > --- a/hw/eepro100.c > +++ b/hw/eepro100.c > @@ -539,21 +539,17 @@ static void e100_pci_reset(EEPRO100State * s, > E100PCIDeviceInfo *e100_device) > > if (e100_device->power_management) { > /* Power Management Capabilities */ > - int cfg_offset; > - pci_reserve_capability(&s->dev, PCI_CONFIG_HEADER_SIZE, > - 0xdc - PCI_CONFIG_HEADER_SIZE); > - cfg_offset = pci_add_capability(&s->dev, PCI_CAP_ID_PM, PCI_PM_SIZEOF); > - assert(cfg_offset == 0xdc); > - if (cfg_offset > 0) { > - /* Power Management Capabilities */ > - pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21); > + int cfg_offset = 0xdc; Suggestion: uint8_t cfg_offset = 0xdc; > + int r = pci_add_capability_at_offset(&s->dev, PCI_CAP_ID_PM, > + cfg_offset, PCI_PM_SIZEOF); > + assert(r >= 0); > + pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21); > #if 0 /* TODO: replace dummy code for power management emulation. */ > - /* TODO: Power Management Control / Status. */ > - pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000); > - /* TODO: Ethernet Power Consumption Registers (i82559 and later). */ > - pci_set_byte(pci_conf + cfg_offset + PCI_PM_PPB_EXTENSIONS, 0x0000); > + /* TODO: Power Management Control / Status. */ > + pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000); > + /* TODO: Ethernet Power Consumption Registers (i82559 and later). */ > + pci_set_byte(pci_conf + cfg_offset + PCI_PM_PPB_EXTENSIONS, 0x0000); > #endif > - } > } > > #if EEPROM_SIZE > 0 Acked-by: Stefan Weil <weil@mail.berlios.de> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH 2/2] eepro100: convert to new capability API 2010-04-07 8:42 ` [Qemu-devel] " Stefan Weil @ 2010-04-07 11:04 ` Michael S. Tsirkin 0 siblings, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2010-04-07 11:04 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-devel On Wed, Apr 07, 2010 at 10:42:07AM +0200, Stefan Weil wrote: > Michael S. Tsirkin schrieb: > > Using new pci_add_capability_at_offset makes > > eepro100 code cleaner. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > hw/eepro100.c | 22 +++++++++------------- > > 1 files changed, 9 insertions(+), 13 deletions(-) > > > > diff --git a/hw/eepro100.c b/hw/eepro100.c > > index 785a7da..a74d834 100644 > > --- a/hw/eepro100.c > > +++ b/hw/eepro100.c > > @@ -539,21 +539,17 @@ static void e100_pci_reset(EEPRO100State * s, > > E100PCIDeviceInfo *e100_device) > > > > if (e100_device->power_management) { > > /* Power Management Capabilities */ > > - int cfg_offset; > > - pci_reserve_capability(&s->dev, PCI_CONFIG_HEADER_SIZE, > > - 0xdc - PCI_CONFIG_HEADER_SIZE); > > - cfg_offset = pci_add_capability(&s->dev, PCI_CAP_ID_PM, PCI_PM_SIZEOF); > > - assert(cfg_offset == 0xdc); > > - if (cfg_offset > 0) { > > - /* Power Management Capabilities */ > > - pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21); > > + int cfg_offset = 0xdc; > > Suggestion: > > uint8_t cfg_offset = 0xdc; I don't think it matters as there's no chance of overflow or anything, all math is on small constants. > > + int r = pci_add_capability_at_offset(&s->dev, PCI_CAP_ID_PM, > > + cfg_offset, PCI_PM_SIZEOF); > > + assert(r >= 0); > > + pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21); > > #if 0 /* TODO: replace dummy code for power management emulation. */ > > - /* TODO: Power Management Control / Status. */ > > - pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000); > > - /* TODO: Ethernet Power Consumption Registers (i82559 and later). */ > > - pci_set_byte(pci_conf + cfg_offset + PCI_PM_PPB_EXTENSIONS, 0x0000); > > + /* TODO: Power Management Control / Status. */ > > + pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000); > > + /* TODO: Ethernet Power Consumption Registers (i82559 and later). */ > > + pci_set_byte(pci_conf + cfg_offset + PCI_PM_PPB_EXTENSIONS, 0x0000); > > #endif > > - } > > } > > > > #if EEPROM_SIZE > 0 > > Acked-by: Stefan Weil <weil@mail.berlios.de> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-07 11:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-07 8:04 [Qemu-devel] [PATCH 0/2] pci_add_capability_at_offset Michael S. Tsirkin 2010-04-07 8:04 ` [Qemu-devel] [PATCH 1/2] pci: add API to add capability at a known offset Michael S. Tsirkin 2010-04-07 8:36 ` [Qemu-devel] " Stefan Weil 2010-04-07 8:04 ` [Qemu-devel] [PATCH 2/2] eepro100: convert to new capability API Michael S. Tsirkin 2010-04-07 8:42 ` [Qemu-devel] " Stefan Weil 2010-04-07 11:04 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).