[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 02/14] pci: consolidate pci_add_capability_at_of
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH 02/14] pci: consolidate pci_add_capability_at_offset() into pci_add_capability(). |
Date: |
Mon, 6 Sep 2010 11:10:33 +0300 |
User-agent: |
Mutt/1.5.20 (2009-12-10) |
On Mon, Sep 06, 2010 at 04:46:16PM +0900, Isaku Yamahata wrote:
> By making pci_add_capability() the special case of
> pci_add_capability_at_offset() of offset = 0,
> consolidate pci_add_capability_at_offset() into pci_add_capability().
>
> Cc: Stefan Weil <address@hidden>
> Cc: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Isaku Yamahata <address@hidden>
Good stuff.
Here's an idea: Long term I think we should just give up on the ability
to allocate capabilities dynamically, and always pass a valid offset.
In particular, this would help us get rid of the used mask, and we won't
have to pass in capability size.
virtio would then have
#define VIRTIO_PCI_MSIX_CAP_OFF 0x40
and msix.h would be changed to get the offset.
The reason is that we need to preserve the offsets for migration
to work, anyway.
Have said all this, this is just an idea, it is not a must
and can be a separate patch, or I might do this change myself.
> ---
> hw/eepro100.c | 4 ++--
> hw/msix.c | 3 ++-
> hw/pci.c | 33 ++++++++++++++++++---------------
> hw/pci.h | 5 ++---
> 4 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 2b75c8f..8cbc3aa 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -539,8 +539,8 @@ static void e100_pci_reset(EEPRO100State * s,
> E100PCIDeviceInfo *e100_device)
> if (e100_device->power_management) {
> /* Power Management Capabilities */
> int cfg_offset = 0xdc;
> - int r = pci_add_capability_at_offset(&s->dev, PCI_CAP_ID_PM,
> - cfg_offset, PCI_PM_SIZEOF);
> + int r = pci_add_capability(&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. */
> diff --git a/hw/msix.c b/hw/msix.c
> index d99403a..7ce63eb 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -73,7 +73,8 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned
> short nentries,
> }
>
> pdev->msix_bar_size = new_size;
> - config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
> MSIX_CAP_LENGTH);
> + config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
> + 0, MSIX_CAP_LENGTH);
> if (config_offset < 0)
> return config_offset;
> config = pdev->config + config_offset;
> diff --git a/hw/pci.c b/hw/pci.c
> index 2dc1577..754ffb3 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1682,11 +1682,25 @@ static void pci_del_option_rom(PCIDevice *pdev)
> pdev->rom_offset = 0;
> }
>
> -/* Reserve space and add capability to the linked list in pci config space */
> -int pci_add_capability_at_offset(PCIDevice *pdev, uint8_t cap_id,
> - uint8_t offset, uint8_t size)
> +/*
> + * if !offset
> + * Reserve space and add capability to the linked list in pci config space
> + *
> + * if offset = 0,
> + * 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 offset, uint8_t size)
> {
> - uint8_t *config = pdev->config + offset;
> + uint8_t *config;
> + if (!offset) {
> + offset = pci_find_space(pdev, size);
> + if (!offset) {
> + return -ENOSPC;
> + }
> + }
> +
> + config = pdev->config + offset;
> config[PCI_CAP_LIST_ID] = cap_id;
> config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
> pdev->config[PCI_CAPABILITY_LIST] = offset;
> @@ -1699,17 +1713,6 @@ int pci_add_capability_at_offset(PCIDevice *pdev,
> uint8_t cap_id,
> 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 c551f96..2ddba59 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -183,9 +183,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pcibus_t size, int type,
> 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);
> +int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> + uint8_t offset, uint8_t size);
>
> void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t
> cap_size);
>
> --
> 1.7.1.1
- [Qemu-devel] [PATCH 06/14] pci_ids.h: add vendor id of Texus Intesruments., (continued)
- [Qemu-devel] [PATCH 06/14] pci_ids.h: add vendor id of Texus Intesruments., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 14/14] pcie/aer: glue aer error injection into qemu monitor., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 03/14] pci bridge: add helper function for ssvid capability., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 05/14] pci: make pci_parse_devfn() aware of func., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 07/14] msi: implemented msi., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 13/14] pcie/hotplug: glue pushing attention button command. pcie_abp, Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 02/14] pci: consolidate pci_add_capability_at_offset() into pci_add_capability()., Isaku Yamahata, 2010/09/06
- [Qemu-devel] Re: [PATCH 02/14] pci: consolidate pci_add_capability_at_offset() into pci_add_capability().,
Michael S. Tsirkin <=
- [Qemu-devel] [PATCH 11/14] pcie upstream port: pci express switch upstream port., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 12/14] pcie downstream port: pci express switch downstream port., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 08/14] pcie: helper functions for pcie extended capability., Isaku Yamahata, 2010/09/06
- [Qemu-devel] [PATCH 09/14] pcie port: define struct PCIEPort/PCIESlot and helper functions, Isaku Yamahata, 2010/09/06
- [Qemu-devel] Re: [PATCH 00/14] pcie port switch emulators, Wei Xu, 2010/09/06
- [Qemu-devel] Re: [PATCH 00/14] pcie port switch emulators, Michael S. Tsirkin, 2010/09/07