From: "Michael S. Tsirkin" <mst@redhat.com>
To: Leonid Bloch <leonid.bloch@ravellosystems.com>
Cc: Dmitry Fleytman <dmitry@daynix.com>,
Jason Wang <jasowang@redhat.com>,
Leonid Bloch <leonid@daynix.com>,
qemu-devel@nongnu.org,
Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Subject: Re: [Qemu-devel] [PATCH 03/13] pcie: Add support for PCIe CAP v1
Date: Thu, 18 Feb 2016 12:35:51 +0200 [thread overview]
Message-ID: <20160218123022-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <1455790054-1952-4-git-send-email-leonid.bloch@ravellosystems.com>
On Thu, Feb 18, 2016 at 12:07:24PM +0200, Leonid Bloch wrote:
> From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>
A bit of text here to explain what's different
from the existing APIs.
> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
> ---
> hw/pci/pcie.c | 80 +++++++++++++++++++++++++++++++++++-----------
> include/hw/pci/pcie.h | 4 +++
> include/hw/pci/pcie_regs.h | 5 +--
> 3 files changed, 69 insertions(+), 20 deletions(-)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 435a6cf..be3a318 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -42,26 +42,15 @@
> /***************************************************************************
> * pci express capability helper functions
> */
> -int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
> -{
> - int pos;
> - uint8_t *exp_cap;
> -
> - assert(pci_is_express(dev));
> -
> - pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
> - PCI_EXP_VER2_SIZEOF);
> - if (pos < 0) {
> - return pos;
> - }
> - dev->exp.exp_cap = pos;
> - exp_cap = dev->config + pos;
>
> +static void
> +_pcie_cap_v1_fill(uint8_t *exp_cap, uint8_t port, uint8_t type, uint8_t version)
Names are rather confusing. There's init, there's fill, there's
a prefix and a non prefix versions ...
Please add some comments for each function, saying what it does.
Also this one is really both for v1 and v2, correct?
Don't use _ prefix in QEMU please.
> +{
> /* capability register
> - interrupt message number defaults to 0 */
> + interrupt message number defaults to 0 */
> pci_set_word(exp_cap + PCI_EXP_FLAGS,
> ((type << PCI_EXP_FLAGS_TYPE_SHIFT) & PCI_EXP_FLAGS_TYPE) |
> - PCI_EXP_FLAGS_VER2);
> + version);
>
> /* device capability register
> * table 7-12:
> @@ -80,6 +69,23 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
>
> pci_set_word(exp_cap + PCI_EXP_LNKSTA,
> PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25 |PCI_EXP_LNKSTA_DLLLA);
> +}
> +
> +int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
> +{
> + int pos;
> + uint8_t *exp_cap;
> +
> + assert(pci_is_express(dev));
> +
> + pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, PCI_EXP_VER2_SIZEOF);
> + if (pos < 0) {
> + return pos;
> + }
> + dev->exp.exp_cap = pos;
> + exp_cap = dev->config + pos;
> +
> + _pcie_cap_v1_fill(exp_cap, port, type, PCI_EXP_FLAGS_VER2);
>
> pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
> PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
> @@ -88,7 +94,28 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
> return pos;
> }
>
> -int pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset)
> +int pcie_cap_v1_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
> +{
> + int pos;
> + uint8_t *exp_cap;
> +
> + assert(pci_is_express(dev));
> +
> + pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
> + PCI_EXP_VER1_SIZEOF);
> + if (pos < 0) {
> + return pos;
> + }
> + dev->exp.exp_cap = pos;
> + exp_cap = dev->config + pos;
> +
> + _pcie_cap_v1_fill(exp_cap, port, type, PCI_EXP_FLAGS_VER1);
> +
> + return pos;
> +}
> +
> +static int
> +_pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset, uint8_t cap_size)
> {
> uint8_t type = PCI_EXP_TYPE_ENDPOINT;
>
> @@ -101,7 +128,19 @@ int pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset)
> type = PCI_EXP_TYPE_RC_END;
> }
>
> - return pcie_cap_init(dev, offset, type, 0);
> + return (cap_size == PCI_EXP_VER1_SIZEOF)
> + ? pcie_cap_v1_init(dev, offset, type, 0)
> + : pcie_cap_init(dev, offset, type, 0);
> +}
> +
> +int pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset)
> +{
> + return _pcie_endpoint_cap_init(dev, offset, PCI_EXP_VER2_SIZEOF);
> +}
> +
> +int pcie_endpoint_cap_v1_init(PCIDevice *dev, uint8_t offset)
> +{
> + return _pcie_endpoint_cap_init(dev, offset, PCI_EXP_VER1_SIZEOF);
> }
>
> void pcie_cap_exit(PCIDevice *dev)
> @@ -109,6 +148,11 @@ void pcie_cap_exit(PCIDevice *dev)
> pci_del_capability(dev, PCI_CAP_ID_EXP, PCI_EXP_VER2_SIZEOF);
> }
>
> +void pcie_cap_v1_exit(PCIDevice *dev)
> +{
> + pci_del_capability(dev, PCI_CAP_ID_EXP, PCI_EXP_VER1_SIZEOF);
> +}
> +
> uint8_t pcie_cap_get_type(const PCIDevice *dev)
> {
> uint32_t pos = dev->exp.exp_cap;
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index b48a7a2..cbbf0c5 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -80,8 +80,12 @@ struct PCIExpressDevice {
>
> /* PCI express capability helper functions */
> int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port);
> +int pcie_cap_v1_init(PCIDevice *dev, uint8_t offset,
> + uint8_t type, uint8_t port);
> int pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset);
> void pcie_cap_exit(PCIDevice *dev);
> +int pcie_endpoint_cap_v1_init(PCIDevice *dev, uint8_t offset);
> +void pcie_cap_v1_exit(PCIDevice *dev);
> uint8_t pcie_cap_get_type(const PCIDevice *dev);
> void pcie_cap_flags_set_vector(PCIDevice *dev, uint8_t vector);
> uint8_t pcie_cap_flags_get_vector(PCIDevice *dev);
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index 6a28b33..a95522a 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -11,6 +11,7 @@
>
> /* express capability */
>
> +#define PCI_EXP_VER1_SIZEOF 0x14 /* express capability of ver. 1 */
> #define PCI_EXP_VER2_SIZEOF 0x3c /* express capability of ver. 2 */
> #define PCI_EXT_CAP_VER_SHIFT 16
> #define PCI_EXT_CAP_NEXT_SHIFT 20
> @@ -26,11 +27,11 @@
> (((x) + PCI_EXT_CAP_ALIGN - 1) & ~(PCI_EXT_CAP_ALIGN - 1))
>
> /* PCI_EXP_FLAGS */
> -#define PCI_EXP_FLAGS_VER2 2 /* for now, supports only ver. 2 */
> +#define PCI_EXP_FLAGS_VER1 1
> +#define PCI_EXP_FLAGS_VER2 2
> #define PCI_EXP_FLAGS_IRQ_SHIFT ctz32(PCI_EXP_FLAGS_IRQ)
> #define PCI_EXP_FLAGS_TYPE_SHIFT ctz32(PCI_EXP_FLAGS_TYPE)
>
> -
> /* PCI_EXP_LINK{CAP, STA} */
> /* link speed */
> #define PCI_EXP_LNK_LS_25 1
> --
> 2.5.0
next prev parent reply other threads:[~2016-02-18 10:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-18 10:07 [Qemu-devel] [PATCH 00/13] Introduce Intel 82574 GbE Controller Emulation (e1000e) Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 01/13] msix: make msix_clr_pending() visible for clients Leonid Bloch
2016-02-18 10:23 ` Michael S. Tsirkin
2016-02-18 10:07 ` [Qemu-devel] [PATCH 02/13] pci: Introduce define for PM capability version 1.1 Leonid Bloch
2016-02-18 10:27 ` Michael S. Tsirkin
2016-02-22 16:31 ` Leonid Bloch
2016-02-22 19:08 ` Michael S. Tsirkin
2016-02-18 10:07 ` [Qemu-devel] [PATCH 03/13] pcie: Add support for PCIe CAP v1 Leonid Bloch
2016-02-18 10:35 ` Michael S. Tsirkin [this message]
2016-02-22 16:36 ` Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 04/13] pcie: Introduce function for DSN capability creation Leonid Bloch
2016-02-18 10:41 ` Michael S. Tsirkin
2016-02-22 16:42 ` Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 05/13] vmxnet3: Use generic function for DSN capability definition Leonid Bloch
2016-02-18 10:46 ` Michael S. Tsirkin
2016-02-22 16:44 ` Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 06/13] net: Introduce Toeplitz hash calculator Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 07/13] net: Add macros for MAC address tracing Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 08/13] vmxnet3: Use common MAC address tracing macros Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 09/13] net_pkt: Name vmxnet3 packet abstractions more generic Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 10/13] rtl8139: Move more TCP definitions to common header Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 11/13] net_pkt: Extend packet abstraction as required by e1000e functionality Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 12/13] e1000_regs: Add definitions for Intel 82574-specific bits Leonid Bloch
2016-02-18 10:07 ` [Qemu-devel] [PATCH 13/13] net: Introduce e1000e device emulation Leonid Bloch
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160218123022-mutt-send-email-mst@redhat.com \
--to=mst@redhat.com \
--cc=dmitry@daynix.com \
--cc=jasowang@redhat.com \
--cc=leonid.bloch@ravellosystems.com \
--cc=leonid@daynix.com \
--cc=qemu-devel@nongnu.org \
--cc=shmulik.ladkani@ravellosystems.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).