* [PATCH] Fix Vector table and PBA addresses print output @ 2018-01-30 10:34 Gustavo Pimentel 2018-01-30 14:07 ` Alex Williamson 0 siblings, 1 reply; 5+ messages in thread From: Gustavo Pimentel @ 2018-01-30 10:34 UTC (permalink / raw) To: linux-pci; +Cc: mj, Gustavo Pimentel Lack of 3-bit rotation to the right was leading to erroneous display of the vector table and PBA. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- ls-caps.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ls-caps.c b/ls-caps.c index 507771a..cd13a37 100644 --- a/ls-caps.c +++ b/ls-caps.c @@ -1104,10 +1104,10 @@ cap_msix(struct device *d, int where, int cap) off = get_conf_long(d, where + PCI_MSIX_TABLE); printf("\t\tVector table: BAR=%d offset=%08x\n", - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); off = get_conf_long(d, where + PCI_MSIX_PBA); printf("\t\tPBA: BAR=%d offset=%08x\n", - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); } static void -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix Vector table and PBA addresses print output 2018-01-30 10:34 [PATCH] Fix Vector table and PBA addresses print output Gustavo Pimentel @ 2018-01-30 14:07 ` Alex Williamson 2018-01-30 16:08 ` Gustavo Pimentel 0 siblings, 1 reply; 5+ messages in thread From: Alex Williamson @ 2018-01-30 14:07 UTC (permalink / raw) To: Gustavo Pimentel; +Cc: linux-pci, mj On Tue, 30 Jan 2018 10:34:08 +0000 Gustavo Pimentel <gustavo.pimentel@synopsys.com> wrote: > Lack of 3-bit rotation to the right was leading to erroneous display of the > vector table and PBA. > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > --- > ls-caps.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ls-caps.c b/ls-caps.c > index 507771a..cd13a37 100644 > --- a/ls-caps.c > +++ b/ls-caps.c > @@ -1104,10 +1104,10 @@ cap_msix(struct device *d, int where, int cap) > > off = get_conf_long(d, where + PCI_MSIX_TABLE); > printf("\t\tVector table: BAR=%d offset=%08x\n", > - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); > + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); > off = get_conf_long(d, where + PCI_MSIX_PBA); > printf("\t\tPBA: BAR=%d offset=%08x\n", > - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); > + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); > } > > static void The code is correct as is, the register contains bits 31:3 of the offset. The spec states (PCI rev 3.0, 6.8.2.4, 6.8.2.5): The lower 3 Table BIR bits are masked off (set to zero) by software to form a 32-bit QWORD -aligned offset. The lower 3 PBA BIR bits are masked off (set to zero) by software to form a 32-bit QWORD-aligned offset. Shifting is incorrect, the mask operation fills the lower 3 bits. Thanks, Alex ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix Vector table and PBA addresses print output 2018-01-30 14:07 ` Alex Williamson @ 2018-01-30 16:08 ` Gustavo Pimentel 2018-01-30 16:45 ` Alex Williamson 0 siblings, 1 reply; 5+ messages in thread From: Gustavo Pimentel @ 2018-01-30 16:08 UTC (permalink / raw) To: Alex Williamson; +Cc: linux-pci@vger.kernel.org, mj@ucw.cz Hi Alex, On 30/01/2018 14:07, Alex Williamson wrote: > On Tue, 30 Jan 2018 10:34:08 +0000 > Gustavo Pimentel <gustavo.pimentel@synopsys.com> wrote: > >> Lack of 3-bit rotation to the right was leading to erroneous display of the >> vector table and PBA. >> >> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> >> --- >> ls-caps.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/ls-caps.c b/ls-caps.c >> index 507771a..cd13a37 100644 >> --- a/ls-caps.c >> +++ b/ls-caps.c >> @@ -1104,10 +1104,10 @@ cap_msix(struct device *d, int where, int cap) >> >> off = get_conf_long(d, where + PCI_MSIX_TABLE); >> printf("\t\tVector table: BAR=%d offset=%08x\n", >> - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); >> + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); >> off = get_conf_long(d, where + PCI_MSIX_PBA); >> printf("\t\tPBA: BAR=%d offset=%08x\n", >> - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); >> + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); >> } >> >> static void > > > The code is correct as is, the register contains bits 31:3 of the > offset. The spec states (PCI rev 3.0, 6.8.2.4, 6.8.2.5): I have just downloaded now the rev 3.0 (PCI_Express_Base_r3.0_10Nov10.pdf, November 10, 2010) from PCI-SIG and the chapters that you refer don't match with the downloaded version, can you confirm the doc date? > > The lower 3 Table BIR bits are masked off (set to zero) by software > to form a 32-bit QWORD -aligned offset. > > The lower 3 PBA BIR bits are masked off (set to zero) by software to > form a 32-bit QWORD-aligned offset. > Yes, I understand the meaning but for the user the value printed should not include (in my perspective) the offset provide by the Table BIR field or the PBA BIR field. > Shifting is incorrect, the mask operation fills the lower 3 bits. The spec defines: | 31 24 | 23 16 | 15 8 | 7 3 | 2 0 | +00h | Msg Ctrl Reg | PNext ID | CapID | +04h | Table offset | Tb BIR | +08h | PBA offset | PBA BIR | +0Ch | Reserved | Msg Data Reg | Let's apply a real example here, using the info from config space: | 31 24 | 23 16 | 15 8 | 7 0 | 0b+00h | 00 | 20 | 00 | 11 | 0b+04h | 00 | 00 | 00 | 00 | 0b+08h | 00 | 00 | 18 | 00 | 0b+0Ch | 00 | 00 | 00 | 00 | If I decode manually the info from config space, I get for each field the following value: Msg Ctrl Reg = 0x20 PNext ID = 0x0 CapID = 0x11 Table offset = 0x0 Table BIR = 0x0 Table offset = 0x300 Table BIR = 0x0 Msg Data Reg = 0x0 Using lspci, I get: ... Capabilities: [b0] MSI-X: Enable+ Count=33 Masked- Vector table: BAR=0 offset=00000000 PBA: BAR=0 offset=00001800 ... In my perspective should be like: Capabilities: [b0] MSI-X: Enable+ Count=33 Masked- Vector table: BAR=0 offset=00000000 PBA: BAR=0 offset=00000300 > Thanks, > > Alex > Regards, Gustavo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix Vector table and PBA addresses print output 2018-01-30 16:08 ` Gustavo Pimentel @ 2018-01-30 16:45 ` Alex Williamson 2018-01-31 12:41 ` Martin Mares 0 siblings, 1 reply; 5+ messages in thread From: Alex Williamson @ 2018-01-30 16:45 UTC (permalink / raw) To: Gustavo Pimentel; +Cc: linux-pci@vger.kernel.org, mj@ucw.cz On Tue, 30 Jan 2018 16:08:07 +0000 Gustavo Pimentel <gustavo.pimentel@synopsys.com> wrote: > Hi Alex, > > On 30/01/2018 14:07, Alex Williamson wrote: > > On Tue, 30 Jan 2018 10:34:08 +0000 > > Gustavo Pimentel <gustavo.pimentel@synopsys.com> wrote: > > > >> Lack of 3-bit rotation to the right was leading to erroneous display of the > >> vector table and PBA. > >> > >> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > >> --- > >> ls-caps.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/ls-caps.c b/ls-caps.c > >> index 507771a..cd13a37 100644 > >> --- a/ls-caps.c > >> +++ b/ls-caps.c > >> @@ -1104,10 +1104,10 @@ cap_msix(struct device *d, int where, int cap) > >> > >> off = get_conf_long(d, where + PCI_MSIX_TABLE); > >> printf("\t\tVector table: BAR=%d offset=%08x\n", > >> - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); > >> + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); > >> off = get_conf_long(d, where + PCI_MSIX_PBA); > >> printf("\t\tPBA: BAR=%d offset=%08x\n", > >> - off & PCI_MSIX_BIR, off & ~PCI_MSIX_BIR); > >> + off & PCI_MSIX_BIR, (off & ~PCI_MSIX_BIR) >> 3); > >> } > >> > >> static void > > > > > > The code is correct as is, the register contains bits 31:3 of the > > offset. The spec states (PCI rev 3.0, 6.8.2.4, 6.8.2.5): > > I have just downloaded now the rev 3.0 (PCI_Express_Base_r3.0_10Nov10.pdf, > November 10, 2010) from PCI-SIG and the chapters that you refer don't match with > the downloaded version, can you confirm the doc date? I'm referencing the PCI Local Bus Specification Rev 3.0, Feb 3, 2004. MSI-X is compatible between PCI and PCIe, in fact my copy of the PCIe 3.1a spec refers to the PCI spec for these registers. > > > > The lower 3 Table BIR bits are masked off (set to zero) by software > > to form a 32-bit QWORD -aligned offset. > > > > The lower 3 PBA BIR bits are masked off (set to zero) by software to > > form a 32-bit QWORD-aligned offset. > > > > Yes, I understand the meaning but for the user the value printed should not > include (in my perspective) the offset provide by the Table BIR field or the PBA > BIR field. The offset is the value with the BIR field masked, shifting the value results in the (offset/8), which is not a useful thing. > > Shifting is incorrect, the mask operation fills the lower 3 bits. > The spec defines: > | 31 24 | 23 16 | 15 8 | 7 3 | 2 0 | > +00h | Msg Ctrl Reg | PNext ID | CapID | > +04h | Table offset | Tb BIR | > +08h | PBA offset | PBA BIR | > +0Ch | Reserved | Msg Data Reg | > > Let's apply a real example here, using the info from config space: > | 31 24 | 23 16 | 15 8 | 7 0 | > 0b+00h | 00 | 20 | 00 | 11 | > 0b+04h | 00 | 00 | 00 | 00 | > 0b+08h | 00 | 00 | 18 | 00 | > 0b+0Ch | 00 | 00 | 00 | 00 | > > If I decode manually the info from config space, I get for each field the > following value: > > Msg Ctrl Reg = 0x20 > PNext ID = 0x0 > CapID = 0x11 > Table offset = 0x0 > Table BIR = 0x0 > Table offset = 0x300 Correction, Table offset[31:3] = 0x300, the actual offset is 0x1800 > Table BIR = 0x0 > Msg Data Reg = 0x0 > > Using lspci, I get: > ... > Capabilities: [b0] MSI-X: Enable+ Count=33 Masked- > Vector table: BAR=0 offset=00000000 > PBA: BAR=0 offset=00001800 Which is correct > ... > In my perspective should be like: > > Capabilities: [b0] MSI-X: Enable+ Count=33 Masked- > Vector table: BAR=0 offset=00000000 > PBA: BAR=0 offset=00000300 Which is a meaningless and misleading value. The PBA lives at offset 0x1800 into BAR0 MMIO space, why is anything other than that relevant to present to the user? By your logic we should also present count as 32 and let the user figure out that it's encoded as N-1. Thanks, Alex ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix Vector table and PBA addresses print output 2018-01-30 16:45 ` Alex Williamson @ 2018-01-31 12:41 ` Martin Mares 0 siblings, 0 replies; 5+ messages in thread From: Martin Mares @ 2018-01-31 12:41 UTC (permalink / raw) To: Alex Williamson; +Cc: Gustavo Pimentel, linux-pci@vger.kernel.org Hello! > Which is a meaningless and misleading value. The PBA lives at offset > 0x1800 into BAR0 MMIO space, why is anything other than that relevant > to present to the user? By your logic we should also present > count as 32 and let the user figure out that it's encoded as N-1. I completely agree that we should display the real offset (as we do now). Have a nice fortnight -- Martin `MJ' Mares <mj@ucw.cz> http://mj.ucw.cz/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth The better the better, the better the bet. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-31 12:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-30 10:34 [PATCH] Fix Vector table and PBA addresses print output Gustavo Pimentel 2018-01-30 14:07 ` Alex Williamson 2018-01-30 16:08 ` Gustavo Pimentel 2018-01-30 16:45 ` Alex Williamson 2018-01-31 12:41 ` Martin Mares
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).