Re: [Qemu-devel] [RFC PATCH v2 02/10] pci: Introduce function for PCI PM capability creation

[Marcel Apfelbaum]

On 01/18/2016 07:35 PM, Leonid Bloch wrote:
> From: Dmitry Fleytman <address@hidden>
>
> Signed-off-by: Dmitry Fleytman <address@hidden>
> Signed-off-by: Leonid Bloch <address@hidden>
> ---
>   hw/pci/pci.c              | 21 +++++++++++++++++++++
>   include/hw/pci/pci.h      |  2 ++
>   include/hw/pci/pci_regs.h |  1 +
>   3 files changed, 24 insertions(+)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b3d5100..3aaf86c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2050,6 +2050,27 @@ static void pci_del_option_rom(PCIDevice *pdev)
>       pdev->has_rom = false;
>   }
>
> +int pci_add_pm_capability(PCIDevice *pdev, uint8_t offset, uint16_t pmc)
> +{
> +    int ret = pci_add_capability(pdev, PCI_CAP_ID_PM, offset, PCI_PM_SIZEOF);
> +
> +    if (ret >= 0) {
> +        pci_set_word(pdev->config + offset + PCI_PM_PMC,
> +                     PCI_PM_CAP_VER_1_1 |

Hi,

Why not ver 1.2 ? just wondering

> +                     pmc);
> +
> +        pci_set_word(pdev->wmask + offset + PCI_PM_CTRL,
> +                     PCI_PM_CTRL_STATE_MASK |
> +                     PCI_PM_CTRL_PME_ENABLE |

PME_ENABLE and PME_STATUS are writable only if the function supports PME# 
generation from D3cold


> +                     PCI_PM_CTRL_DATA_SEL_MASK

And Data_Select is writable only if the data register is implemented.

My point is this seems to be a standard capability function, but it depends
on optional function features.

Thanks,
Marcel

);
> +
> +        pci_set_word(pdev->w1cmask + offset + PCI_PM_CTRL,
> +                     PCI_PM_CTRL_PME_STATUS);
> +    }
> +
> +    return ret;
> +}
> +
>   /*
>    * if !offset
>    * Reserve space and add capability to the linked list in pci config space
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b97c295..cec7234 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -319,6 +319,8 @@ int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
>                          uint8_t offset, uint8_t size,
>                          Error **errp);
>
> +int pci_add_pm_capability(PCIDevice *pdev, uint8_t offset, uint16_t pmc);
> +
>   void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>
>   uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id);
> diff --git a/include/hw/pci/pci_regs.h b/include/hw/pci/pci_regs.h
> index 56a404b..2bd3ac9 100644
> --- a/include/hw/pci/pci_regs.h
> +++ b/include/hw/pci/pci_regs.h
> @@ -221,6 +221,7 @@
>
>   #define PCI_PM_PMC            2       /* PM Capabilities Register */
>   #define  PCI_PM_CAP_VER_MASK  0x0007  /* Version */
> +#define  PCI_PM_CAP_VER_1_1     0x0002  /* PCI PM spec ver. 1.1 */
>   #define  PCI_PM_CAP_PME_CLOCK 0x0008  /* PME clock required */
>   #define  PCI_PM_CAP_RESERVED    0x0010  /* Reserved field */
>   #define  PCI_PM_CAP_DSI               0x0020  /* Device specific 
> initialization */