| Message ID | 20130520170956.29259.87841.stgit@bhelgaas-glaptop |
|---|---|
| State | New |
| Headers | show |
On Mon, May 20, 2013 at 11:09:56AM -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> > --- > 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 54fcac8..f194445 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_ASPM_L0S | > + PCI_EXP_LNKCAP_ASPM_L0S | PCI_EXP_LNKCAP_ASPM_L1 | > 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_ASPMC); > + > pci_set_long(exp_cap + PCI_EXP_DEVCAP2, > PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP); > OK this is making some new bits writeable so it will break cross-version migration. Need to add a property and disable for -M 1.5 or older. > diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h > index 109f2f4..5b81b36 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 > > @@ -40,9 +40,12 @@ > > /* PCI_EXP_LINKCAP */ > #define PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* L0s supported */ > +#define PCI_EXP_LNKCAP_ASPM_L1 0x00000800 /* L1 supported */ > > #define PCI_EXP_LNKCAP_PN_SHIFT (ffs(PCI_EXP_LNKCAP_PN) - 1) > > +#define PCI_EXP_LNKCTL_ASPMC 0x0003 /* ASPM Control */ > + > #define PCI_EXP_SLTCAP_PSN_SHIFT (ffs(PCI_EXP_SLTCAP_PSN) - 1) > > #define PCI_EXP_SLTCTL_IND_RESERVED 0x0
On Mon, May 20, 2013 at 12:32 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, May 20, 2013 at 11:09:56AM -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> >> --- >> 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 54fcac8..f194445 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_ASPM_L0S | >> + PCI_EXP_LNKCAP_ASPM_L0S | PCI_EXP_LNKCAP_ASPM_L1 | >> 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_ASPMC); >> + >> pci_set_long(exp_cap + PCI_EXP_DEVCAP2, >> PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP); >> > > OK this is making some new bits writeable so it will break > cross-version migration. Need to add a property and > disable for -M 1.5 or older. I'm not a qemu hacker. Can you point me at an example? Maybe this isn't really worth it. I only added this so I could do some experimentation with how Windows handles these bits, and I'm not sure it's of general interest. I guess it is a case of gratuitous difference compared to real hardware, though -- we already advertise the L0s capability but the enable bit isn't writable. The OS will enable L0s, and lspci will show it enabled on hardware but disabled on qemu. Bjorn >> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h >> index 109f2f4..5b81b36 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 >> >> @@ -40,9 +40,12 @@ >> >> /* PCI_EXP_LINKCAP */ >> #define PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* L0s supported */ >> +#define PCI_EXP_LNKCAP_ASPM_L1 0x00000800 /* L1 supported */ >> >> #define PCI_EXP_LNKCAP_PN_SHIFT (ffs(PCI_EXP_LNKCAP_PN) - 1) >> >> +#define PCI_EXP_LNKCTL_ASPMC 0x0003 /* ASPM Control */ >> + >> #define PCI_EXP_SLTCAP_PSN_SHIFT (ffs(PCI_EXP_SLTCAP_PSN) - 1) >> >> #define PCI_EXP_SLTCTL_IND_RESERVED 0x0
On Mon, May 20, 2013 at 02:44:25PM -0600, Bjorn Helgaas wrote: > On Mon, May 20, 2013 at 12:32 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, May 20, 2013 at 11:09:56AM -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> > >> --- > >> 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 54fcac8..f194445 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_ASPM_L0S | > >> + PCI_EXP_LNKCAP_ASPM_L0S | PCI_EXP_LNKCAP_ASPM_L1 | > >> 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_ASPMC); > >> + > >> pci_set_long(exp_cap + PCI_EXP_DEVCAP2, > >> PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP); > >> > > > > OK this is making some new bits writeable so it will break > > cross-version migration. Need to add a property and > > disable for -M 1.5 or older. > > I'm not a qemu hacker. Can you point me at an example? Okay. The closest thing is when we made SERR enable writeable. You can grep for command_serr_enable and QEMU_PCI_CAP_SERR_BITNR. > Maybe this isn't really worth it. I only added this so I could do > some experimentation with how Windows handles these bits, and I'm not > sure it's of general interest. I guess it is a case of gratuitous > difference compared to real hardware, though -- we already advertise > the L0s capability but the enable bit isn't writable. The OS will > enable L0s, and lspci will show it enabled on hardware but disabled on > qemu. > > Bjorn I agree and at least for L0s, I think it's worth fixing. It looks like we could advertize L1 or ignore it, so I don't mind what happens there, it's up to you to decide. > >> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h > >> index 109f2f4..5b81b36 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 > >> > >> @@ -40,9 +40,12 @@ > >> > >> /* PCI_EXP_LINKCAP */ > >> #define PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* L0s supported */ > >> +#define PCI_EXP_LNKCAP_ASPM_L1 0x00000800 /* L1 supported */ > >> > >> #define PCI_EXP_LNKCAP_PN_SHIFT (ffs(PCI_EXP_LNKCAP_PN) - 1) > >> > >> +#define PCI_EXP_LNKCTL_ASPMC 0x0003 /* ASPM Control */ > >> + > >> #define PCI_EXP_SLTCAP_PSN_SHIFT (ffs(PCI_EXP_SLTCAP_PSN) - 1) > >> > >> #define PCI_EXP_SLTCTL_IND_RESERVED 0x0
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 54fcac8..f194445 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_ASPM_L0S | + PCI_EXP_LNKCAP_ASPM_L0S | PCI_EXP_LNKCAP_ASPM_L1 | 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_ASPMC); + 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 109f2f4..5b81b36 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 @@ -40,9 +40,12 @@ /* PCI_EXP_LINKCAP */ #define PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* L0s supported */ +#define PCI_EXP_LNKCAP_ASPM_L1 0x00000800 /* L1 supported */ #define PCI_EXP_LNKCAP_PN_SHIFT (ffs(PCI_EXP_LNKCAP_PN) - 1) +#define PCI_EXP_LNKCTL_ASPMC 0x0003 /* ASPM Control */ + #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(-)