| Message ID | 20130515223308.11335.86381.stgit@bhelgaas-glaptop |
|---|---|
| State | New |
| Headers | show |
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!
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
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
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(-)