diff mbox

[RFC,v2,04/10] pcie: Introduce function for DSN capability creation

Message ID 1453138550-1096-5-git-send-email-leonid.bloch@ravellosystems.com
State New
Headers show

Commit Message

Leonid Bloch Jan. 18, 2016, 5:35 p.m. UTC 
From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>

Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
---
 hw/pci/pcie.c              | 7 +++++++
 include/hw/pci/pci_regs.h  | 3 +++
 include/hw/pci/pcie.h      | 1 +
 include/hw/pci/pcie_regs.h | 4 ++++
 4 files changed, 15 insertions(+)

Comments

Marcel Apfelbaum Jan. 19, 2016, 2:26 p.m. UTC | #1 
On 01/18/2016 07:35 PM, Leonid Bloch wrote:
> From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>
> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
> ---
>   hw/pci/pcie.c              | 7 +++++++
>   include/hw/pci/pci_regs.h  | 3 +++
>   include/hw/pci/pcie.h      | 1 +
>   include/hw/pci/pcie_regs.h | 4 ++++
>   4 files changed, 15 insertions(+)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 7b8ff24..6d55f8f 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -678,3 +678,10 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn)
>                           offset, PCI_ARI_SIZEOF);
>       pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8);
>   }
> +
> +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val)
> +{
> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_DSN, PCI_DSN_VER,
> +                        offset, PCI_DSN_SIZEOF);
> +    pci_set_quad(dev->config + offset + PCI_DSN_CAP, val);
> +}
> diff --git a/include/hw/pci/pci_regs.h b/include/hw/pci/pci_regs.h
> index 2bd3ac9..4a0cc67 100644
> --- a/include/hw/pci/pci_regs.h
> +++ b/include/hw/pci/pci_regs.h
> @@ -657,6 +657,9 @@
>   #define  PCI_ARI_CTRL_ACS	0x0002	/* ACS Function Groups Enable */
>   #define  PCI_ARI_CTRL_FG(x)	(((x) >> 4) & 7) /* Function Group */
>
> +/* Device serial number */
> +#define PCI_DSN_CAP     0x04    /* Device Serial Number Register */
> +
>   /* Address Translation Service */
>   #define PCI_ATS_CAP		0x04	/* ATS Capability Register */
>   #define  PCI_ATS_CAP_QDEP(x)	((x) & 0x1f)	/* Invalidate Queue Depth */
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index cbbf0c5..83a325c 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -119,6 +119,7 @@ void pcie_add_capability(PCIDevice *dev,
>                            uint16_t offset, uint16_t size);
>
>   void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val);
>
>   extern const VMStateDescription vmstate_pcie_device;
>
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index e3c969e..d06acd1 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -80,6 +80,10 @@
>   #define PCI_ARI_VER                     1
>   #define PCI_ARI_SIZEOF                  8
>
> +/* DSN */
> +#define PCI_DSN_VER                     1
> +#define PCI_DSN_SIZEOF                  8

Hi,

Are you sure the size of DSN is 8?
Looking at PCIe spec 3, chapter 7.12 I see 12, but I might be wrong.

Thanks,
Marcel

> +
>   /* AER */
>   #define PCI_ERR_VER                     2
>   #define PCI_ERR_SIZEOF                  0x48
>
Shmulik Ladkani Jan. 19, 2016, 3:02 p.m. UTC | #2 
Hi Marcel,

On Tue, 19 Jan 2016 16:26:31 +0200, marcel.apfelbaum@gmail.com wrote:
> >
> > +/* DSN */
> > +#define PCI_DSN_VER                     1
> > +#define PCI_DSN_SIZEOF                  8
> 
> Are you sure the size of DSN is 8?
> Looking at PCIe spec 3, chapter 7.12 I see 12, but I might be wrong.

Total size (DSN ext. cap header + Serial Number register) is indeed 12.
The Serial Number register itself is 64 bits.

Newer qemu already has PCI_EXT_CAP_DSN_SIZEOF defined as 12
(in standard-headers/linux/pci_regs.h)

We may choose better naming to reflect the '8 size' refers to the
payload.
Dmitry Fleytman Jan. 19, 2016, 3:06 p.m. UTC | #3 
> On 19 Jan 2016, at 16:26 PM, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
> 
> On 01/18/2016 07:35 PM, Leonid Bloch wrote:
>> From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>> 
>> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
>> ---
>>  hw/pci/pcie.c              | 7 +++++++
>>  include/hw/pci/pci_regs.h  | 3 +++
>>  include/hw/pci/pcie.h      | 1 +
>>  include/hw/pci/pcie_regs.h | 4 ++++
>>  4 files changed, 15 insertions(+)
>> 
>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>> index 7b8ff24..6d55f8f 100644
>> --- a/hw/pci/pcie.c
>> +++ b/hw/pci/pcie.c
>> @@ -678,3 +678,10 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn)
>>                          offset, PCI_ARI_SIZEOF);
>>      pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8);
>>  }
>> +
>> +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val)
>> +{
>> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_DSN, PCI_DSN_VER,
>> +                        offset, PCI_DSN_SIZEOF);
>> +    pci_set_quad(dev->config + offset + PCI_DSN_CAP, val);
>> +}
>> diff --git a/include/hw/pci/pci_regs.h b/include/hw/pci/pci_regs.h
>> index 2bd3ac9..4a0cc67 100644
>> --- a/include/hw/pci/pci_regs.h
>> +++ b/include/hw/pci/pci_regs.h
>> @@ -657,6 +657,9 @@
>>  #define  PCI_ARI_CTRL_ACS	0x0002	/* ACS Function Groups Enable */
>>  #define  PCI_ARI_CTRL_FG(x)	(((x) >> 4) & 7) /* Function Group */
>> 
>> +/* Device serial number */
>> +#define PCI_DSN_CAP     0x04    /* Device Serial Number Register */
>> +
>>  /* Address Translation Service */
>>  #define PCI_ATS_CAP		0x04	/* ATS Capability Register */
>>  #define  PCI_ATS_CAP_QDEP(x)	((x) & 0x1f)	/* Invalidate Queue Depth */
>> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
>> index cbbf0c5..83a325c 100644
>> --- a/include/hw/pci/pcie.h
>> +++ b/include/hw/pci/pcie.h
>> @@ -119,6 +119,7 @@ void pcie_add_capability(PCIDevice *dev,
>>                           uint16_t offset, uint16_t size);
>> 
>>  void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
>> +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val);
>> 
>>  extern const VMStateDescription vmstate_pcie_device;
>> 
>> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
>> index e3c969e..d06acd1 100644
>> --- a/include/hw/pci/pcie_regs.h
>> +++ b/include/hw/pci/pcie_regs.h
>> @@ -80,6 +80,10 @@
>>  #define PCI_ARI_VER                     1
>>  #define PCI_ARI_SIZEOF                  8
>> 
>> +/* DSN */
>> +#define PCI_DSN_VER                     1
>> +#define PCI_DSN_SIZEOF                  8
> 
> Hi,
> 
> Are you sure the size of DSN is 8?
> Looking at PCIe spec 3, chapter 7.12 I see 12, but I might be wrong.

Oops, you’re right. It’s 4 bytes for header + 8 bytes serial number = 12 bytes total. Fo some reason I thought this define is length of “payload” only, without header.
Fixing it...

Thanks for reviewing this series, Marcel.

> 
> Thanks,
> Marcel
> 
>> +
>>  /* AER */
>>  #define PCI_ERR_VER                     2
>>  #define PCI_ERR_SIZEOF                  0x48
>> 
>
diff mbox

Patch

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 7b8ff24..6d55f8f 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -678,3 +678,10 @@  void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn)
                         offset, PCI_ARI_SIZEOF);
     pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8);
 }
+
+void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val)
+{
+    pcie_add_capability(dev, PCI_EXT_CAP_ID_DSN, PCI_DSN_VER,
+                        offset, PCI_DSN_SIZEOF);
+    pci_set_quad(dev->config + offset + PCI_DSN_CAP, val);
+}
diff --git a/include/hw/pci/pci_regs.h b/include/hw/pci/pci_regs.h
index 2bd3ac9..4a0cc67 100644
--- a/include/hw/pci/pci_regs.h
+++ b/include/hw/pci/pci_regs.h
@@ -657,6 +657,9 @@ 
 #define  PCI_ARI_CTRL_ACS	0x0002	/* ACS Function Groups Enable */
 #define  PCI_ARI_CTRL_FG(x)	(((x) >> 4) & 7) /* Function Group */
 
+/* Device serial number */
+#define PCI_DSN_CAP     0x04    /* Device Serial Number Register */
+
 /* Address Translation Service */
 #define PCI_ATS_CAP		0x04	/* ATS Capability Register */
 #define  PCI_ATS_CAP_QDEP(x)	((x) & 0x1f)	/* Invalidate Queue Depth */
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index cbbf0c5..83a325c 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -119,6 +119,7 @@  void pcie_add_capability(PCIDevice *dev,
                          uint16_t offset, uint16_t size);
 
 void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
+void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val);
 
 extern const VMStateDescription vmstate_pcie_device;
 
diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
index e3c969e..d06acd1 100644
--- a/include/hw/pci/pcie_regs.h
+++ b/include/hw/pci/pcie_regs.h
@@ -80,6 +80,10 @@ 
 #define PCI_ARI_VER                     1
 #define PCI_ARI_SIZEOF                  8
 
+/* DSN */
+#define PCI_DSN_VER                     1
+#define PCI_DSN_SIZEOF                  8
+
 /* AER */
 #define PCI_ERR_VER                     2
 #define PCI_ERR_SIZEOF                  0x48