All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] pcie: Add more ASPM support
@ 2013-05-15 22:33 Bjorn Helgaas
  2013-05-19 11:54 ` Michael S. Tsirkin
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2013-05-15 22:33 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel

Indicate ASPM L0s and L1 support in Link Capabilities and make the ASPM
bits in Link Control writable.  These Link Control bits don't do anything
in qemu, but having them writable means the BIOS or OS can write them as
on real hardware.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 hw/pci/pcie.c              |    4 +++-
 include/hw/pci/pcie_regs.h |    5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 62bd0b8..7336054 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -73,13 +73,15 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
 
     pci_set_long(exp_cap + PCI_EXP_LNKCAP,
                  (port << PCI_EXP_LNKCAP_PN_SHIFT) |
-                 PCI_EXP_LNKCAP_ASPMS_0S |
+                 PCI_EXP_LNKCAP_ASPMS_0S | PCI_EXP_LNKCAP_ASPMS_1 |
                  PCI_EXP_LNK_MLW_1 |
                  PCI_EXP_LNK_LS_25);
 
     pci_set_word(exp_cap + PCI_EXP_LNKSTA,
                  PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
 
+    pci_set_word(dev->wmask + pos + PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPM);
+
     pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
                  PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
 
diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
index 4d123d9..6b80a8d 100644
--- a/include/hw/pci/pcie_regs.h
+++ b/include/hw/pci/pcie_regs.h
@@ -31,7 +31,7 @@
 #define PCI_EXP_FLAGS_TYPE_SHIFT        (ffs(PCI_EXP_FLAGS_TYPE) - 1)
 
 
-/* PCI_EXP_LINK{CAP, STA} */
+/* PCI_EXP_LINK{CAP, STA, CTL} */
 /* link speed */
 #define PCI_EXP_LNK_LS_25               1
 
@@ -41,9 +41,12 @@
 /* PCI_EXP_LINKCAP */
 #define PCI_EXP_LNKCAP_ASPMS_SHIFT      (ffs(PCI_EXP_LNKCAP_ASPMS) - 1)
 #define PCI_EXP_LNKCAP_ASPMS_0S         (1 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
+#define PCI_EXP_LNKCAP_ASPMS_1          (2 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
 
 #define PCI_EXP_LNKCAP_PN_SHIFT         (ffs(PCI_EXP_LNKCAP_PN) - 1)
 
+#define PCI_EXP_LNKCTL_ASPM		0x0003
+
 #define PCI_EXP_SLTCAP_PSN_SHIFT        (ffs(PCI_EXP_SLTCAP_PSN) - 1)
 
 #define PCI_EXP_SLTCTL_IND_RESERVED     0x0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] pcie: Add more ASPM support
  2013-05-15 22:33 [Qemu-devel] [PATCH] pcie: Add more ASPM support Bjorn Helgaas
@ 2013-05-19 11:54 ` Michael S. Tsirkin
  2013-05-20 16:43   ` Bjorn Helgaas
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2013-05-19 11:54 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: qemu-devel

On Wed, May 15, 2013 at 04:33:08PM -0600, Bjorn Helgaas wrote:
> Indicate ASPM L0s and L1 support in Link Capabilities and make the ASPM
> bits in Link Control writable.  These Link Control bits don't do anything
> in qemu, but having them writable means the BIOS or OS can write them as
> on real hardware.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Out of curiousity - L1 is optional on real hardware, isn't it?
Anyway, went over spec and L1 is just like L0s
but with higher latency, so I think this is a safe change.
A minor comment below - would appreciate if you can address it.

> ---
>  hw/pci/pcie.c              |    4 +++-
>  include/hw/pci/pcie_regs.h |    5 ++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 62bd0b8..7336054 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -73,13 +73,15 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
>  
>      pci_set_long(exp_cap + PCI_EXP_LNKCAP,
>                   (port << PCI_EXP_LNKCAP_PN_SHIFT) |
> -                 PCI_EXP_LNKCAP_ASPMS_0S |
> +                 PCI_EXP_LNKCAP_ASPMS_0S | PCI_EXP_LNKCAP_ASPMS_1 |
>                   PCI_EXP_LNK_MLW_1 |
>                   PCI_EXP_LNK_LS_25);
>  
>      pci_set_word(exp_cap + PCI_EXP_LNKSTA,
>                   PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
>  
> +    pci_set_word(dev->wmask + pos + PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPM);
> +
>      pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
>                   PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
>  
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index 4d123d9..6b80a8d 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -31,7 +31,7 @@
>  #define PCI_EXP_FLAGS_TYPE_SHIFT        (ffs(PCI_EXP_FLAGS_TYPE) - 1)
>  
>  
> -/* PCI_EXP_LINK{CAP, STA} */
> +/* PCI_EXP_LINK{CAP, STA, CTL} */
>  /* link speed */
>  #define PCI_EXP_LNK_LS_25               1
>  
> @@ -41,9 +41,12 @@
>  /* PCI_EXP_LINKCAP */
>  #define PCI_EXP_LNKCAP_ASPMS_SHIFT      (ffs(PCI_EXP_LNKCAP_ASPMS) - 1)
>  #define PCI_EXP_LNKCAP_ASPMS_0S         (1 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
> +#define PCI_EXP_LNKCAP_ASPMS_1          (2 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
>  
>  #define PCI_EXP_LNKCAP_PN_SHIFT         (ffs(PCI_EXP_LNKCAP_PN) - 1)
>  
> +#define PCI_EXP_LNKCTL_ASPM		0x0003
> +
>  #define PCI_EXP_SLTCAP_PSN_SHIFT        (ffs(PCI_EXP_SLTCAP_PSN) - 1)
>  
>  #define PCI_EXP_SLTCTL_IND_RESERVED     0x0


I'd like to name them the same as you did in linux/pci_regs.h
Let's rename PCI_EXP_LNKCAP_ASPMS_0S/PCI_EXP_LNKCAP_ASPMS_1 as well.


Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] pcie: Add more ASPM support
  2013-05-19 11:54 ` Michael S. Tsirkin
@ 2013-05-20 16:43   ` Bjorn Helgaas
  0 siblings, 0 replies; 3+ messages in thread
From: Bjorn Helgaas @ 2013-05-20 16:43 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel

On Sun, May 19, 2013 at 5:54 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, May 15, 2013 at 04:33:08PM -0600, Bjorn Helgaas wrote:
>> Indicate ASPM L0s and L1 support in Link Capabilities and make the ASPM
>> bits in Link Control writable.  These Link Control bits don't do anything
>> in qemu, but having them writable means the BIOS or OS can write them as
>> on real hardware.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Out of curiousity - L1 is optional on real hardware, isn't it?

Yes.  Per PCIe spec r3.0, sec 5.4.1, "ASPM support for the L1 Link
state is optional unless specifically required by a particular form
factor."

> Anyway, went over spec and L1 is just like L0s
> but with higher latency, so I think this is a safe change.
> A minor comment below - would appreciate if you can address it.
>
>> ---
>>  hw/pci/pcie.c              |    4 +++-
>>  include/hw/pci/pcie_regs.h |    5 ++++-
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>> index 62bd0b8..7336054 100644
>> --- a/hw/pci/pcie.c
>> +++ b/hw/pci/pcie.c
>> @@ -73,13 +73,15 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
>>
>>      pci_set_long(exp_cap + PCI_EXP_LNKCAP,
>>                   (port << PCI_EXP_LNKCAP_PN_SHIFT) |
>> -                 PCI_EXP_LNKCAP_ASPMS_0S |
>> +                 PCI_EXP_LNKCAP_ASPMS_0S | PCI_EXP_LNKCAP_ASPMS_1 |
>>                   PCI_EXP_LNK_MLW_1 |
>>                   PCI_EXP_LNK_LS_25);
>>
>>      pci_set_word(exp_cap + PCI_EXP_LNKSTA,
>>                   PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
>>
>> +    pci_set_word(dev->wmask + pos + PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPM);
>> +
>>      pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
>>                   PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
>>
>> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
>> index 4d123d9..6b80a8d 100644
>> --- a/include/hw/pci/pcie_regs.h
>> +++ b/include/hw/pci/pcie_regs.h
>> @@ -31,7 +31,7 @@
>>  #define PCI_EXP_FLAGS_TYPE_SHIFT        (ffs(PCI_EXP_FLAGS_TYPE) - 1)
>>
>>
>> -/* PCI_EXP_LINK{CAP, STA} */
>> +/* PCI_EXP_LINK{CAP, STA, CTL} */
>>  /* link speed */
>>  #define PCI_EXP_LNK_LS_25               1
>>
>> @@ -41,9 +41,12 @@
>>  /* PCI_EXP_LINKCAP */
>>  #define PCI_EXP_LNKCAP_ASPMS_SHIFT      (ffs(PCI_EXP_LNKCAP_ASPMS) - 1)
>>  #define PCI_EXP_LNKCAP_ASPMS_0S         (1 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
>> +#define PCI_EXP_LNKCAP_ASPMS_1          (2 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
>>
>>  #define PCI_EXP_LNKCAP_PN_SHIFT         (ffs(PCI_EXP_LNKCAP_PN) - 1)
>>
>> +#define PCI_EXP_LNKCTL_ASPM          0x0003
>> +
>>  #define PCI_EXP_SLTCAP_PSN_SHIFT        (ffs(PCI_EXP_SLTCAP_PSN) - 1)
>>
>>  #define PCI_EXP_SLTCTL_IND_RESERVED     0x0
>
>
> I'd like to name them the same as you did in linux/pci_regs.h
> Let's rename PCI_EXP_LNKCAP_ASPMS_0S/PCI_EXP_LNKCAP_ASPMS_1 as well.

Will do.  I'll repost this shortly.

Bjorn

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-05-20 17:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-15 22:33 [Qemu-devel] [PATCH] pcie: Add more ASPM support Bjorn Helgaas
2013-05-19 11:54 ` Michael S. Tsirkin
2013-05-20 16:43   ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.