On Mon, Apr 10, 2017 at 03:53:28PM +1000, Russell Currey wrote:
>Dumping the PE State Tables (PEST) can be highly verbose if a number of PEs
>are affected, especially in the case where the whole PHB is frozen and 255
>lines get printed. Check for duplicates when dumping the PEST to reduce
>useless output.
>
>For example:
>
> PE[f8] A/B: 9700002600000000 80000080d00000f8
> PE[f9] A/B: 8000000000000000 0000000000000000
> PE[..fe] A/B: as above
> PE[ff] A/B: 8440002b00000000 0000000000000000
>
>instead of:
>
> PE[f8] A/B: 9700002600000000 80000080d00000f8
> PE[f9] A/B: 8000000000000000 0000000000000000
> PE[fa] A/B: 8000000000000000 0000000000000000
> PE[fb] A/B: 8000000000000000 0000000000000000
> PE[fc] A/B: 8000000000000000 0000000000000000
> PE[fd] A/B: 8000000000000000 0000000000000000
> PE[fe] A/B: 8000000000000000 0000000000000000
> PE[ff] A/B: 8440002b00000000 0000000000000000
>
>and you can imagine how much worse it can get for 255 PEs.
>
Russell, PHB3 does have 256 PEs while P7IOC has 128 PEs. None of them
has 255 PEs :) It really depends on the possibility (how often) all
PESTA/B are dumped. I think it should be very very rare because only
one error case (link-down-freeze-all) can cause this. So it's fine to
keep current implementation and code. However, it's nice to make the
code smart though.
With below comments resolved:
Reviewed-by: Gavin Shan <gws...@linux.vnet.ibm.com>
>Signed-off-by: Russell Currey <rus...@russell.cc>
>---
> arch/powerpc/platforms/powernv/pci.c | 52 ++++++++++++++++++++++--------------
> 1 file changed, 32 insertions(+), 20 deletions(-)
>
>diff --git a/arch/powerpc/platforms/powernv/pci.c
>b/arch/powerpc/platforms/powernv/pci.c
>index eb835e977e33..303c9d84d3d4 100644
>--- a/arch/powerpc/platforms/powernv/pci.c
>+++ b/arch/powerpc/platforms/powernv/pci.c
>@@ -227,11 +227,40 @@ void pnv_teardown_msi_irqs(struct pci_dev *pdev)
> }
> #endif /* CONFIG_PCI_MSI */
>
>+/* Nicely print the contents of the PE State Tables (PEST). */
>+static void pnv_pci_dump_pest(__be64 pestA[], __be64 pestB[], int pest_size)
>+{
>+ int i;
>+ __be64 prevA, prevB;
>+ bool dup = false;
>+ prevA = prevB = ~0;
@prevA and @prevB can be initialized while they're declared. Also,
it would be nicer to have UL suffix as they are 64-bits in width.
__be64 prevA = ULONG_MAX, prevB = ULONG_MAX;
>+
>+ for (i = 0; i < pest_size; i++) {
>+ __be64 peA = be64_to_cpu(pestA[i]);
>+ __be64 peB = be64_to_cpu(pestB[i]);
>+
>+ if (peA != prevA || peB != prevB) {
>+ if (dup) {
>+ pr_info("PE[..%x] A/B: as above\n", i-1);
>+ dup = false;
>+ }
>+ prevA = peA;
>+ prevB = peB;
>+ if (peA || peB)
This condition isn't match with original implementation where the Bit#63
are check on PESTA/B and the corresponding entry is skipped if none of
them is set.
>+ pr_info("PE[%2x] A/B: %016llx %016llx\n",
>+ i, peA, peB);
>+ } else {
>+ /* Don't need to track zeroes */
>+ if (!dup && (peA || peB))
Same as above. Also, the else/if can be combined to be "else if" to
avoid nested the extra ifdef.
>+ dup = true;
>+ }
>+ }
>+}
>+
> static void pnv_pci_dump_p7ioc_diag_data(struct pci_controller *hose,
> struct OpalIoPhbErrorCommon *common)
> {
> struct OpalIoP7IOCPhbErrorData *data;
>- int i;
>
> data = (struct OpalIoP7IOCPhbErrorData *)common;
> pr_info("P7IOC PHB#%x Diag-data (Version: %d)\n",
>@@ -308,22 +337,13 @@ static void pnv_pci_dump_p7ioc_diag_data(struct
>pci_controller *hose,
> be64_to_cpu(data->dma1ErrorLog0),
> be64_to_cpu(data->dma1ErrorLog1));
>
>- for (i = 0; i < OPAL_P7IOC_NUM_PEST_REGS; i++) {
>- if ((be64_to_cpu(data->pestA[i]) >> 63) == 0 &&
>- (be64_to_cpu(data->pestB[i]) >> 63) == 0)
>- continue;
>-
>- pr_info("PE[%3d] A/B: %016llx %016llx\n",
>- i, be64_to_cpu(data->pestA[i]),
>- be64_to_cpu(data->pestB[i]));
>- }
>+ pnv_pci_dump_pest(data->pestA, data->pestB, OPAL_P7IOC_NUM_PEST_REGS);
> }
>
> static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose,
> struct OpalIoPhbErrorCommon *common)
> {
> struct OpalIoPhb3ErrorData *data;
>- int i;
>
> data = (struct OpalIoPhb3ErrorData*)common;
> pr_info("PHB3 PHB#%x Diag-data (Version: %d)\n",
>@@ -404,15 +424,7 @@ static void pnv_pci_dump_phb3_diag_data(struct
>pci_controller *hose,
> be64_to_cpu(data->dma1ErrorLog0),
> be64_to_cpu(data->dma1ErrorLog1));
>
>- for (i = 0; i < OPAL_PHB3_NUM_PEST_REGS; i++) {
>- if ((be64_to_cpu(data->pestA[i]) >> 63) == 0 &&
>- (be64_to_cpu(data->pestB[i]) >> 63) == 0)
>- continue;
>-
>- pr_info("PE[%3d] A/B: %016llx %016llx\n",
>- i, be64_to_cpu(data->pestA[i]),
>- be64_to_cpu(data->pestB[i]));
>- }
>+ pnv_pci_dump_pest(data->pestA, data->pestB, OPAL_PHB3_NUM_PEST_REGS);
> }
>
> void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
>--
>2.12.2
>
