| Message ID | 1446642770-4681-15-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 11/05/2015 12:12 AM, Gavin Shan wrote: > This enables M64 window on P7IOC, which has been enabled on PHB3. > Different from PHB3 where 16 M64 BARs are supported and each of > them can be owned by one particular PE# exclusively or divided > evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each > of them are divided to 8 segments. So every P7IOC PHB supports > 128 M64 segments in total. P7IOC has M64DT, which helps mapping > one particular M64 segment# to arbitrary PE#. PHB3 doesn't have > M64DT, indicating that one M64 segment can only be pinned to the > fixed PE#. In order to have same code to support M64 on P7IOC and > PHB3, we just provide 128 M64 segments on every P7IOC PHB and each > of them is pinned to the fixed PE# by bypassing the function of > M64DT. In turn, we just need different phb->init_m64() for P7IOC > and PHB3 to support M64. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- > arch/powerpc/platforms/powernv/pci.h | 3 ++ > 2 files changed, 86 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 1f7d985..bfe69f1 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, > } > } > > +static int pnv_ioda1_init_m64(struct pnv_phb *phb) > +{ > + struct resource *r; > + int index; > + > + /* > + * There are 16 M64 BARs, each of which has 8 segments. So > + * there are as many M64 segments as the maximum number of > + * PEs, which is 128. > + */ > + for (index = 0; index < PNV_IODA1_M64_NUM; index++) { > + unsigned long base, segsz = phb->ioda.m64_segsize; > + int64_t rc; > + > + base = phb->ioda.m64_base + > + index * PNV_IODA1_M64_SEGS * segsz; > + rc = opal_pci_set_phb_mem_window(phb->opal_id, > + OPAL_M64_WINDOW_TYPE, index, base, 0, > + PNV_IODA1_M64_SEGS * segsz); > + if (rc != OPAL_SUCCESS) { > + pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", > + rc, phb->hose->global_number, index); > + goto fail; > + } > + > + rc = opal_pci_phb_mmio_enable(phb->opal_id, > + OPAL_M64_WINDOW_TYPE, index, > + OPAL_ENABLE_M64_SPLIT); > + if (rc != OPAL_SUCCESS) { > + pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", > + rc, phb->hose->global_number, index); > + goto fail; > + } > + } > + > + /* > + * Exclude the segment used by the reserved PE, which > + * is expected to be 0 or last supported PE#. > + */ > + r = &phb->hose->mem_resources[1]; > + if (phb->ioda.reserved_pe_idx == 0) > + r->start += phb->ioda.m64_segsize; > + else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) > + r->end -= phb->ioda.m64_segsize; > + else > + pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", > + phb->ioda.reserved_pe_idx); > + > + return 0; > + > +fail: > + for ( ; index >= 0; index--) > + opal_pci_phb_mmio_enable(phb->opal_id, > + OPAL_M64_WINDOW_TYPE, index, OPAL_DISABLE_M64); > + > + return -EIO; > +} > + > static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus, > unsigned long *pe_bitmap, > bool all) > @@ -325,6 +383,26 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all) > pe->master = master_pe; > list_add_tail(&pe->list, &master_pe->slaves); > } > + > + /* > + * P7IOC supports M64DT, which helps mapping M64 segment > + * to one particular PE#. However, PHB3 has fixed mapping > + * between M64 segment and PE#. In order to have same logic > + * for P7IOC and PHB3, we enforce fixed mapping between M64 > + * segment and PE# on P7IOC. > + */ > + if (phb->type == PNV_PHB_IODA1) { > + int64_t rc; > + > + rc = opal_pci_map_pe_mmio_window(phb->opal_id, > + pe->pe_number, OPAL_M64_WINDOW_TYPE, > + pe->pe_number / PNV_IODA1_M64_SEGS, > + pe->pe_number % PNV_IODA1_M64_SEGS); > + if (rc != OPAL_SUCCESS) > + pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n", > + __func__, rc, phb->hose->global_number, > + pe->pe_number); > + } > } > > kfree(pe_alloc); > @@ -339,8 +417,7 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) > const u32 *r; > u64 pci_addr; > > - /* FIXME: Support M64 for P7IOC */ > - if (phb->type != PNV_PHB_IODA2) { > + if (phb->type != PNV_PHB_IODA1 && phb->type != PNV_PHB_IODA2) { > pr_info(" Not support M64 window\n"); > return; > } > @@ -373,7 +450,10 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) > > /* Use last M64 BAR to cover M64 window */ > phb->ioda.m64_bar_idx = 15; > - phb->init_m64 = pnv_ioda2_init_m64; > + if (phb->type == PNV_PHB_IODA1) > + phb->init_m64 = pnv_ioda1_init_m64; > + else > + phb->init_m64 = pnv_ioda2_init_m64; > phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; > phb->pick_m64_pe = pnv_ioda_pick_m64_pe; Nit: the callbacks initialization does not seem to relate to parsing any window :) They could all go to where pnv_ioda_parse_m64_window() is called, no separate patch is needed. > } > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > index 671fd13..c4019ac 100644 > --- a/arch/powerpc/platforms/powernv/pci.h > +++ b/arch/powerpc/platforms/powernv/pci.h > @@ -78,6 +78,9 @@ struct pnv_ioda_pe { > struct list_head list; > }; > > +#define PNV_IODA1_M64_NUM 16 /* Number of M64 BARs */ > +#define PNV_IODA1_M64_SEGS 8 /* Segments per M64 BAR */ > + > #define PNV_PHB_FLAG_EEH (1 << 0) > > struct pnv_phb { >
On 11/05/2015 12:12 AM, Gavin Shan wrote: > This enables M64 window on P7IOC, which has been enabled on PHB3. > Different from PHB3 where 16 M64 BARs are supported and each of > them can be owned by one particular PE# exclusively or divided > evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each > of them are divided to 8 segments. So every P7IOC PHB supports > 128 M64 segments in total. P7IOC has M64DT, which helps mapping > one particular M64 segment# to arbitrary PE#. PHB3 doesn't have > M64DT, indicating that one M64 segment can only be pinned to the > fixed PE#. In order to have same code to support M64 on P7IOC and > PHB3, we just provide 128 M64 segments on every P7IOC PHB and each > of them is pinned to the fixed PE# by bypassing the function of > M64DT. In turn, we just need different phb->init_m64() for P7IOC > and PHB3 to support M64. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- > arch/powerpc/platforms/powernv/pci.h | 3 ++ > 2 files changed, 86 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 1f7d985..bfe69f1 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, > } > } > > +static int pnv_ioda1_init_m64(struct pnv_phb *phb) > +{ > + struct resource *r; > + int index; > + > + /* > + * There are 16 M64 BARs, each of which has 8 segments. So > + * there are as many M64 segments as the maximum number of > + * PEs, which is 128. > + */ > + for (index = 0; index < PNV_IODA1_M64_NUM; index++) { > + unsigned long base, segsz = phb->ioda.m64_segsize; > + int64_t rc; > + > + base = phb->ioda.m64_base + > + index * PNV_IODA1_M64_SEGS * segsz; > + rc = opal_pci_set_phb_mem_window(phb->opal_id, > + OPAL_M64_WINDOW_TYPE, index, base, 0, > + PNV_IODA1_M64_SEGS * segsz); > + if (rc != OPAL_SUCCESS) { > + pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", > + rc, phb->hose->global_number, index); > + goto fail; > + } > + > + rc = opal_pci_phb_mmio_enable(phb->opal_id, > + OPAL_M64_WINDOW_TYPE, index, > + OPAL_ENABLE_M64_SPLIT); > + if (rc != OPAL_SUCCESS) { > + pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", > + rc, phb->hose->global_number, index); > + goto fail; > + } > + } > + > + /* > + * Exclude the segment used by the reserved PE, which > + * is expected to be 0 or last supported PE#. > + */ > + r = &phb->hose->mem_resources[1]; What does "1" mean here? A bridge's 64bit prefetchable window? > + if (phb->ioda.reserved_pe_idx == 0) > + r->start += phb->ioda.m64_segsize; > + else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) > + r->end -= phb->ioda.m64_segsize; > + else > + pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", > + phb->ioda.reserved_pe_idx); > +
On 11/05/2015 12:12 AM, Gavin Shan wrote: > This enables M64 window on P7IOC, which has been enabled on PHB3. > Different from PHB3 where 16 M64 BARs are supported and each of > them can be owned by one particular PE# exclusively or divided > evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each > of them are divided to 8 segments. So every P7IOC PHB supports > 128 M64 segments in total. P7IOC has M64DT, which helps mapping > one particular M64 segment# to arbitrary PE#. PHB3 doesn't have > M64DT, indicating that one M64 segment can only be pinned to the > fixed PE#. In order to have same code to support M64 on P7IOC and > PHB3, we just provide 128 M64 segments on every P7IOC PHB and each > of them is pinned to the fixed PE# by bypassing the function of > M64DT. In turn, we just need different phb->init_m64() for P7IOC > and PHB3 to support M64. I thought we decided (Ben suggested?) not to push P7IOC code now (or ever) as there is no user for it, has this changed? btw please put ioda1/ioda2/p7ioc/etc to the subject line to make it easier to see how much work is there about particular PHB type. You rename quite many functions and I generally want to ask you to group all renaming patches first but it would also make sense to keep them close to (for example) p7ioc-related patches so having more descriptive subject lines may help. Thanks.
On Mon, Nov 16, 2015 at 07:01:46PM +1100, Alexey Kardashevskiy wrote: >On 11/05/2015 12:12 AM, Gavin Shan wrote: >>This enables M64 window on P7IOC, which has been enabled on PHB3. >>Different from PHB3 where 16 M64 BARs are supported and each of >>them can be owned by one particular PE# exclusively or divided >>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>of them are divided to 8 segments. So every P7IOC PHB supports >>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>M64DT, indicating that one M64 segment can only be pinned to the >>fixed PE#. In order to have same code to support M64 on P7IOC and >>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>of them is pinned to the fixed PE# by bypassing the function of >>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>and PHB3 to support M64. >> >>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>--- >> arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- >> arch/powerpc/platforms/powernv/pci.h | 3 ++ >> 2 files changed, 86 insertions(+), 3 deletions(-) >> >>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >>index 1f7d985..bfe69f1 100644 >>--- a/arch/powerpc/platforms/powernv/pci-ioda.c >>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c >>@@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, >> } >> } >> >>+static int pnv_ioda1_init_m64(struct pnv_phb *phb) >>+{ >>+ struct resource *r; >>+ int index; >>+ >>+ /* >>+ * There are 16 M64 BARs, each of which has 8 segments. So >>+ * there are as many M64 segments as the maximum number of >>+ * PEs, which is 128. >>+ */ >>+ for (index = 0; index < PNV_IODA1_M64_NUM; index++) { >>+ unsigned long base, segsz = phb->ioda.m64_segsize; >>+ int64_t rc; >>+ >>+ base = phb->ioda.m64_base + >>+ index * PNV_IODA1_M64_SEGS * segsz; >>+ rc = opal_pci_set_phb_mem_window(phb->opal_id, >>+ OPAL_M64_WINDOW_TYPE, index, base, 0, >>+ PNV_IODA1_M64_SEGS * segsz); >>+ if (rc != OPAL_SUCCESS) { >>+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", >>+ rc, phb->hose->global_number, index); >>+ goto fail; >>+ } >>+ >>+ rc = opal_pci_phb_mmio_enable(phb->opal_id, >>+ OPAL_M64_WINDOW_TYPE, index, >>+ OPAL_ENABLE_M64_SPLIT); >>+ if (rc != OPAL_SUCCESS) { >>+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", >>+ rc, phb->hose->global_number, index); >>+ goto fail; >>+ } >>+ } >>+ >>+ /* >>+ * Exclude the segment used by the reserved PE, which >>+ * is expected to be 0 or last supported PE#. >>+ */ >>+ r = &phb->hose->mem_resources[1]; >>+ if (phb->ioda.reserved_pe_idx == 0) >>+ r->start += phb->ioda.m64_segsize; >>+ else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) >>+ r->end -= phb->ioda.m64_segsize; >>+ else >>+ pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", >>+ phb->ioda.reserved_pe_idx); >>+ >>+ return 0; >>+ >>+fail: >>+ for ( ; index >= 0; index--) >>+ opal_pci_phb_mmio_enable(phb->opal_id, >>+ OPAL_M64_WINDOW_TYPE, index, OPAL_DISABLE_M64); >>+ >>+ return -EIO; >>+} >>+ >> static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus, >> unsigned long *pe_bitmap, >> bool all) >>@@ -325,6 +383,26 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all) >> pe->master = master_pe; >> list_add_tail(&pe->list, &master_pe->slaves); >> } >>+ >>+ /* >>+ * P7IOC supports M64DT, which helps mapping M64 segment >>+ * to one particular PE#. However, PHB3 has fixed mapping >>+ * between M64 segment and PE#. In order to have same logic >>+ * for P7IOC and PHB3, we enforce fixed mapping between M64 >>+ * segment and PE# on P7IOC. >>+ */ >>+ if (phb->type == PNV_PHB_IODA1) { >>+ int64_t rc; >>+ >>+ rc = opal_pci_map_pe_mmio_window(phb->opal_id, >>+ pe->pe_number, OPAL_M64_WINDOW_TYPE, >>+ pe->pe_number / PNV_IODA1_M64_SEGS, >>+ pe->pe_number % PNV_IODA1_M64_SEGS); >>+ if (rc != OPAL_SUCCESS) >>+ pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n", >>+ __func__, rc, phb->hose->global_number, >>+ pe->pe_number); >>+ } >> } >> >> kfree(pe_alloc); >>@@ -339,8 +417,7 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >> const u32 *r; >> u64 pci_addr; >> >>- /* FIXME: Support M64 for P7IOC */ >>- if (phb->type != PNV_PHB_IODA2) { >>+ if (phb->type != PNV_PHB_IODA1 && phb->type != PNV_PHB_IODA2) { >> pr_info(" Not support M64 window\n"); >> return; >> } >>@@ -373,7 +450,10 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >> >> /* Use last M64 BAR to cover M64 window */ >> phb->ioda.m64_bar_idx = 15; >>- phb->init_m64 = pnv_ioda2_init_m64; >>+ if (phb->type == PNV_PHB_IODA1) >>+ phb->init_m64 = pnv_ioda1_init_m64; >>+ else >>+ phb->init_m64 = pnv_ioda2_init_m64; >> phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; >> phb->pick_m64_pe = pnv_ioda_pick_m64_pe; > > >Nit: the callbacks initialization does not seem to relate to parsing any >window :) They could all go to where pnv_ioda_parse_m64_window() is called, >no separate patch is needed. > Well, what's the benifit for that? I personally prefer the way I had: initialize all callbacks in one place, not in separate places. However, if you have good reason to support your suggestion, I'll change accordingly for sure. > >> } >>diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h >>index 671fd13..c4019ac 100644 >>--- a/arch/powerpc/platforms/powernv/pci.h >>+++ b/arch/powerpc/platforms/powernv/pci.h >>@@ -78,6 +78,9 @@ struct pnv_ioda_pe { >> struct list_head list; >> }; >> >>+#define PNV_IODA1_M64_NUM 16 /* Number of M64 BARs */ >>+#define PNV_IODA1_M64_SEGS 8 /* Segments per M64 BAR */ >>+ >> #define PNV_PHB_FLAG_EEH (1 << 0) >> >> struct pnv_phb { >> Thanks, Gavin
On Mon, Nov 16, 2015 at 07:02:03PM +1100, Alexey Kardashevskiy wrote: >On 11/05/2015 12:12 AM, Gavin Shan wrote: >>This enables M64 window on P7IOC, which has been enabled on PHB3. >>Different from PHB3 where 16 M64 BARs are supported and each of >>them can be owned by one particular PE# exclusively or divided >>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>of them are divided to 8 segments. So every P7IOC PHB supports >>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>M64DT, indicating that one M64 segment can only be pinned to the >>fixed PE#. In order to have same code to support M64 on P7IOC and >>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>of them is pinned to the fixed PE# by bypassing the function of >>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>and PHB3 to support M64. >> >>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>--- >> arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- >> arch/powerpc/platforms/powernv/pci.h | 3 ++ >> 2 files changed, 86 insertions(+), 3 deletions(-) >> >>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >>index 1f7d985..bfe69f1 100644 >>--- a/arch/powerpc/platforms/powernv/pci-ioda.c >>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c >>@@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, >> } >> } >> >>+static int pnv_ioda1_init_m64(struct pnv_phb *phb) >>+{ >>+ struct resource *r; >>+ int index; >>+ >>+ /* >>+ * There are 16 M64 BARs, each of which has 8 segments. So >>+ * there are as many M64 segments as the maximum number of >>+ * PEs, which is 128. >>+ */ >>+ for (index = 0; index < PNV_IODA1_M64_NUM; index++) { >>+ unsigned long base, segsz = phb->ioda.m64_segsize; >>+ int64_t rc; >>+ >>+ base = phb->ioda.m64_base + >>+ index * PNV_IODA1_M64_SEGS * segsz; >>+ rc = opal_pci_set_phb_mem_window(phb->opal_id, >>+ OPAL_M64_WINDOW_TYPE, index, base, 0, >>+ PNV_IODA1_M64_SEGS * segsz); >>+ if (rc != OPAL_SUCCESS) { >>+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", >>+ rc, phb->hose->global_number, index); >>+ goto fail; >>+ } >>+ >>+ rc = opal_pci_phb_mmio_enable(phb->opal_id, >>+ OPAL_M64_WINDOW_TYPE, index, >>+ OPAL_ENABLE_M64_SPLIT); >>+ if (rc != OPAL_SUCCESS) { >>+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", >>+ rc, phb->hose->global_number, index); >>+ goto fail; >>+ } >>+ } >>+ >>+ /* >>+ * Exclude the segment used by the reserved PE, which >>+ * is expected to be 0 or last supported PE#. >>+ */ >>+ r = &phb->hose->mem_resources[1]; > > >What does "1" mean here? A bridge's 64bit prefetchable window? > It's PHB's M64 window. > >>+ if (phb->ioda.reserved_pe_idx == 0) >>+ r->start += phb->ioda.m64_segsize; >>+ else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) >>+ r->end -= phb->ioda.m64_segsize; >>+ else >>+ pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", >>+ phb->ioda.reserved_pe_idx); >>+ Thanks, Gavin
On Mon, Nov 16, 2015 at 07:02:18PM +1100, Alexey Kardashevskiy wrote: >On 11/05/2015 12:12 AM, Gavin Shan wrote: >>This enables M64 window on P7IOC, which has been enabled on PHB3. >>Different from PHB3 where 16 M64 BARs are supported and each of >>them can be owned by one particular PE# exclusively or divided >>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>of them are divided to 8 segments. So every P7IOC PHB supports >>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>M64DT, indicating that one M64 segment can only be pinned to the >>fixed PE#. In order to have same code to support M64 on P7IOC and >>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>of them is pinned to the fixed PE# by bypassing the function of >>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>and PHB3 to support M64. > >I thought we decided (Ben suggested?) not to push P7IOC code now (or ever) as >there is no user for it, has this changed? > Remember that the code is mixed for P7IOC/PHB3. It's not harmful to support M64 window on P7IOC, which is much larger than M32. >btw please put ioda1/ioda2/p7ioc/etc to the subject line to make it easier to >see how much work is there about particular PHB type. You rename quite many >functions and I generally want to ask you to group all renaming patches first >but it would also make sense to keep them close to (for example) >p7ioc-related patches so having more descriptive subject lines may help. >Thanks. > As the code is mixed for P7IOC/PHB3, I'm not following the line (IODA1/IODA2/p7ioc/phb3) in this patchset. Instead, the sequence of patchset is order related to: cod refactoring, IO/M32/M64, DMA, PE allocation/releaseing. Thanks, Gavin
On 11/17/2015 12:38 PM, Gavin Shan wrote: > On Mon, Nov 16, 2015 at 07:02:03PM +1100, Alexey Kardashevskiy wrote: >> On 11/05/2015 12:12 AM, Gavin Shan wrote: >>> This enables M64 window on P7IOC, which has been enabled on PHB3. >>> Different from PHB3 where 16 M64 BARs are supported and each of >>> them can be owned by one particular PE# exclusively or divided >>> evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>> of them are divided to 8 segments. So every P7IOC PHB supports >>> 128 M64 segments in total. P7IOC has M64DT, which helps mapping >>> one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>> M64DT, indicating that one M64 segment can only be pinned to the >>> fixed PE#. In order to have same code to support M64 on P7IOC and >>> PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>> of them is pinned to the fixed PE# by bypassing the function of >>> M64DT. In turn, we just need different phb->init_m64() for P7IOC >>> and PHB3 to support M64. >>> >>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- >>> arch/powerpc/platforms/powernv/pci.h | 3 ++ >>> 2 files changed, 86 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >>> index 1f7d985..bfe69f1 100644 >>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c >>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c >>> @@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, >>> } >>> } >>> >>> +static int pnv_ioda1_init_m64(struct pnv_phb *phb) >>> +{ >>> + struct resource *r; >>> + int index; >>> + >>> + /* >>> + * There are 16 M64 BARs, each of which has 8 segments. So >>> + * there are as many M64 segments as the maximum number of >>> + * PEs, which is 128. >>> + */ >>> + for (index = 0; index < PNV_IODA1_M64_NUM; index++) { >>> + unsigned long base, segsz = phb->ioda.m64_segsize; >>> + int64_t rc; >>> + >>> + base = phb->ioda.m64_base + >>> + index * PNV_IODA1_M64_SEGS * segsz; >>> + rc = opal_pci_set_phb_mem_window(phb->opal_id, >>> + OPAL_M64_WINDOW_TYPE, index, base, 0, >>> + PNV_IODA1_M64_SEGS * segsz); >>> + if (rc != OPAL_SUCCESS) { >>> + pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", >>> + rc, phb->hose->global_number, index); >>> + goto fail; >>> + } >>> + >>> + rc = opal_pci_phb_mmio_enable(phb->opal_id, >>> + OPAL_M64_WINDOW_TYPE, index, >>> + OPAL_ENABLE_M64_SPLIT); >>> + if (rc != OPAL_SUCCESS) { >>> + pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", >>> + rc, phb->hose->global_number, index); >>> + goto fail; >>> + } >>> + } >>> + >>> + /* >>> + * Exclude the segment used by the reserved PE, which >>> + * is expected to be 0 or last supported PE#. >>> + */ >>> + r = &phb->hose->mem_resources[1]; >> >> >> What does "1" mean here? A bridge's 64bit prefetchable window? >> > > It's PHB's M64 window. mem_resources[] of a hose are not windows of the root PCI bridge? > >> >>> + if (phb->ioda.reserved_pe_idx == 0) >>> + r->start += phb->ioda.m64_segsize; >>> + else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) >>> + r->end -= phb->ioda.m64_segsize; >>> + else >>> + pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", >>> + phb->ioda.reserved_pe_idx); >>> + > > Thanks, > Gavin >
On 11/17/2015 12:42 PM, Gavin Shan wrote: > On Mon, Nov 16, 2015 at 07:02:18PM +1100, Alexey Kardashevskiy wrote: >> On 11/05/2015 12:12 AM, Gavin Shan wrote: >>> This enables M64 window on P7IOC, which has been enabled on PHB3. >>> Different from PHB3 where 16 M64 BARs are supported and each of >>> them can be owned by one particular PE# exclusively or divided >>> evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>> of them are divided to 8 segments. So every P7IOC PHB supports >>> 128 M64 segments in total. P7IOC has M64DT, which helps mapping >>> one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>> M64DT, indicating that one M64 segment can only be pinned to the >>> fixed PE#. In order to have same code to support M64 on P7IOC and >>> PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>> of them is pinned to the fixed PE# by bypassing the function of >>> M64DT. In turn, we just need different phb->init_m64() for P7IOC >>> and PHB3 to support M64. >> >> I thought we decided (Ben suggested?) not to push P7IOC code now (or ever) as >> there is no user for it, has this changed? >> > > Remember that the code is mixed for P7IOC/PHB3. It's not harmful to support > M64 window on P7IOC, which is much larger than M32. The patchset starts with removing dead code and then adds more dead code. This is not right... >> btw please put ioda1/ioda2/p7ioc/etc to the subject line to make it easier to >> see how much work is there about particular PHB type. You rename quite many >> functions and I generally want to ask you to group all renaming patches first >> but it would also make sense to keep them close to (for example) >> p7ioc-related patches so having more descriptive subject lines may help. >> Thanks. >> > > As the code is mixed for P7IOC/PHB3, I'm not following the line (IODA1/IODA2/p7ioc/phb3) > in this patchset. But you should draw the bold line between PHB types imho. > Instead, the sequence of patchset is order related to: cod refactoring, > IO/M32/M64, DMA, PE allocation/releaseing. Some patches from this patchset are about P7IOC only. All I am asking is to say specifically in the subject line what the patch touches - IODA1/IODA2/p7ioc/phb3/all_of_them. Or I can walk through all of them, pick P7IOC's ones, evaluate the amount of code and entropy they actually add and then ask Ben what we do about it, it will just take longer rather than if you did it.
On Tue, Nov 17, 2015 at 01:11:56PM +1100, Alexey Kardashevskiy wrote: >On 11/17/2015 12:38 PM, Gavin Shan wrote: >>On Mon, Nov 16, 2015 at 07:02:03PM +1100, Alexey Kardashevskiy wrote: >>>On 11/05/2015 12:12 AM, Gavin Shan wrote: >>>>This enables M64 window on P7IOC, which has been enabled on PHB3. >>>>Different from PHB3 where 16 M64 BARs are supported and each of >>>>them can be owned by one particular PE# exclusively or divided >>>>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>>>of them are divided to 8 segments. So every P7IOC PHB supports >>>>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>>>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>>>M64DT, indicating that one M64 segment can only be pinned to the >>>>fixed PE#. In order to have same code to support M64 on P7IOC and >>>>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>>>of them is pinned to the fixed PE# by bypassing the function of >>>>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>>>and PHB3 to support M64. >>>> >>>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>>>--- >>>> arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- >>>> arch/powerpc/platforms/powernv/pci.h | 3 ++ >>>> 2 files changed, 86 insertions(+), 3 deletions(-) >>>> >>>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >>>>index 1f7d985..bfe69f1 100644 >>>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c >>>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c >>>>@@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, >>>> } >>>> } >>>> >>>>+static int pnv_ioda1_init_m64(struct pnv_phb *phb) >>>>+{ >>>>+ struct resource *r; >>>>+ int index; >>>>+ >>>>+ /* >>>>+ * There are 16 M64 BARs, each of which has 8 segments. So >>>>+ * there are as many M64 segments as the maximum number of >>>>+ * PEs, which is 128. >>>>+ */ >>>>+ for (index = 0; index < PNV_IODA1_M64_NUM; index++) { >>>>+ unsigned long base, segsz = phb->ioda.m64_segsize; >>>>+ int64_t rc; >>>>+ >>>>+ base = phb->ioda.m64_base + >>>>+ index * PNV_IODA1_M64_SEGS * segsz; >>>>+ rc = opal_pci_set_phb_mem_window(phb->opal_id, >>>>+ OPAL_M64_WINDOW_TYPE, index, base, 0, >>>>+ PNV_IODA1_M64_SEGS * segsz); >>>>+ if (rc != OPAL_SUCCESS) { >>>>+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", >>>>+ rc, phb->hose->global_number, index); >>>>+ goto fail; >>>>+ } >>>>+ >>>>+ rc = opal_pci_phb_mmio_enable(phb->opal_id, >>>>+ OPAL_M64_WINDOW_TYPE, index, >>>>+ OPAL_ENABLE_M64_SPLIT); >>>>+ if (rc != OPAL_SUCCESS) { >>>>+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", >>>>+ rc, phb->hose->global_number, index); >>>>+ goto fail; >>>>+ } >>>>+ } >>>>+ >>>>+ /* >>>>+ * Exclude the segment used by the reserved PE, which >>>>+ * is expected to be 0 or last supported PE#. >>>>+ */ >>>>+ r = &phb->hose->mem_resources[1]; >>> >>> >>>What does "1" mean here? A bridge's 64bit prefetchable window? >>> >> >>It's PHB's M64 window. > >mem_resources[] of a hose are not windows of the root PCI bridge? > No. They're windows for root bus, but not for root PCI bridge. >> >>> >>>>+ if (phb->ioda.reserved_pe_idx == 0) >>>>+ r->start += phb->ioda.m64_segsize; >>>>+ else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) >>>>+ r->end -= phb->ioda.m64_segsize; >>>>+ else >>>>+ pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", >>>>+ phb->ioda.reserved_pe_idx); >>>>+ >> Thanks, Gavin
On Tue, Nov 17, 2015 at 01:37:22PM +1100, Alexey Kardashevskiy wrote: >On 11/17/2015 12:42 PM, Gavin Shan wrote: >>On Mon, Nov 16, 2015 at 07:02:18PM +1100, Alexey Kardashevskiy wrote: >>>On 11/05/2015 12:12 AM, Gavin Shan wrote: >>>>This enables M64 window on P7IOC, which has been enabled on PHB3. >>>>Different from PHB3 where 16 M64 BARs are supported and each of >>>>them can be owned by one particular PE# exclusively or divided >>>>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>>>of them are divided to 8 segments. So every P7IOC PHB supports >>>>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>>>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>>>M64DT, indicating that one M64 segment can only be pinned to the >>>>fixed PE#. In order to have same code to support M64 on P7IOC and >>>>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>>>of them is pinned to the fixed PE# by bypassing the function of >>>>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>>>and PHB3 to support M64. >>> >>>I thought we decided (Ben suggested?) not to push P7IOC code now (or ever) as >>>there is no user for it, has this changed? >>> >> >>Remember that the code is mixed for P7IOC/PHB3. It's not harmful to support >>M64 window on P7IOC, which is much larger than M32. > > >The patchset starts with removing dead code and then adds more dead code. >This is not right... > Sorry, you mean it's fine to break the code on P7IOC as it's going to be dead. But I'm curious when it's going happen, any idea about that? >>>btw please put ioda1/ioda2/p7ioc/etc to the subject line to make it easier to >>>see how much work is there about particular PHB type. You rename quite many >>>functions and I generally want to ask you to group all renaming patches first >>>but it would also make sense to keep them close to (for example) >>>p7ioc-related patches so having more descriptive subject lines may help. >>>Thanks. >>> >> >>As the code is mixed for P7IOC/PHB3, I'm not following the line (IODA1/IODA2/p7ioc/phb3) >>in this patchset. > >But you should draw the bold line between PHB types imho. > >>Instead, the sequence of patchset is order related to: cod refactoring, >>IO/M32/M64, DMA, PE allocation/releaseing. > > >Some patches from this patchset are about P7IOC only. All I am asking is to >say specifically in the subject line what the patch touches - >IODA1/IODA2/p7ioc/phb3/all_of_them. Or I can walk through all of them, pick >P7IOC's ones, evaluate the amount of code and entropy they actually add and >then ask Ben what we do about it, it will just take longer rather than if you >did it. > Please give me a clear command what key words you need in the subject in next revision. What I understood is you want to see one of them: powerpc/powernv/ioda1: powerpc/powernv/ioda2: powerpc/powernv/all: Thanks, Gavin > >-- >Alexey >
On Tue, 2015-11-17 at 14:04 +1100, Gavin Shan wrote: > > Sorry, you mean it's fine to break the code on P7IOC as it's going to be dead. > But I'm curious when it's going happen, any idea about that? Is it ? I think it's ok to not add support for M64, hotpug etc.... for IODA1, but we can do that without *breaking* basic function that we have today. It's not completely dead, there are still a number of machines inside of IBM with P7 that we can support for a little while longer (including a few inside ozlabs). Cheers, Ben.
On 11/17/2015 02:04 PM, Gavin Shan wrote: > On Tue, Nov 17, 2015 at 01:37:22PM +1100, Alexey Kardashevskiy wrote: >> On 11/17/2015 12:42 PM, Gavin Shan wrote: >>> On Mon, Nov 16, 2015 at 07:02:18PM +1100, Alexey Kardashevskiy wrote: >>>> On 11/05/2015 12:12 AM, Gavin Shan wrote: >>>>> This enables M64 window on P7IOC, which has been enabled on PHB3. >>>>> Different from PHB3 where 16 M64 BARs are supported and each of >>>>> them can be owned by one particular PE# exclusively or divided >>>>> evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>>>> of them are divided to 8 segments. So every P7IOC PHB supports >>>>> 128 M64 segments in total. P7IOC has M64DT, which helps mapping >>>>> one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>>>> M64DT, indicating that one M64 segment can only be pinned to the >>>>> fixed PE#. In order to have same code to support M64 on P7IOC and >>>>> PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>>>> of them is pinned to the fixed PE# by bypassing the function of >>>>> M64DT. In turn, we just need different phb->init_m64() for P7IOC >>>>> and PHB3 to support M64. >>>> >>>> I thought we decided (Ben suggested?) not to push P7IOC code now (or ever) as >>>> there is no user for it, has this changed? >>>> >>> >>> Remember that the code is mixed for P7IOC/PHB3. It's not harmful to support >>> M64 window on P7IOC, which is much larger than M32. >> >> >> The patchset starts with removing dead code and then adds more dead code. >> This is not right... >> > > Sorry, you mean it's fine to break the code on P7IOC as it's going to be dead. I am saying that the _new_ code which implements PCI hotplug on P7IOC is dead, not the existing P7IOC support which needs to keep working. Reworks you make should keep P7IOC alive but they do not have to add hotplug. imho it is more likely that we drop P7IOC support in the mainline kernel in next 5 years than someone plugs a PCI device to a running P7IOC machine anywhere. > But I'm curious when it's going happen, any idea about that? > >>>> btw please put ioda1/ioda2/p7ioc/etc to the subject line to make it easier to >>>> see how much work is there about particular PHB type. You rename quite many >>>> functions and I generally want to ask you to group all renaming patches first >>>> but it would also make sense to keep them close to (for example) >>>> p7ioc-related patches so having more descriptive subject lines may help. >>>> Thanks. >>>> >>> >>> As the code is mixed for P7IOC/PHB3, I'm not following the line (IODA1/IODA2/p7ioc/phb3) >>> in this patchset. >> >> But you should draw the bold line between PHB types imho. >> >>> Instead, the sequence of patchset is order related to: cod refactoring, >>> IO/M32/M64, DMA, PE allocation/releaseing. >> >> >> Some patches from this patchset are about P7IOC only. All I am asking is to >> say specifically in the subject line what the patch touches - >> IODA1/IODA2/p7ioc/phb3/all_of_them. Or I can walk through all of them, pick >> P7IOC's ones, evaluate the amount of code and entropy they actually add and >> then ask Ben what we do about it, it will just take longer rather than if you >> did it. >> > > Please give me a clear command what key words you need in the subject in next revision. > What I understood is you want to see one of them: > > powerpc/powernv/ioda1: Yes, looks good. > powerpc/powernv/ioda2: Yes. > powerpc/powernv/all: Just "powerpc/powernv".
On Tue, Nov 17, 2015 at 03:43:28PM +1100, Alexey Kardashevskiy wrote: >On 11/17/2015 02:04 PM, Gavin Shan wrote: >>On Tue, Nov 17, 2015 at 01:37:22PM +1100, Alexey Kardashevskiy wrote: >>>On 11/17/2015 12:42 PM, Gavin Shan wrote: >>>>On Mon, Nov 16, 2015 at 07:02:18PM +1100, Alexey Kardashevskiy wrote: >>>>>On 11/05/2015 12:12 AM, Gavin Shan wrote: >>>>>>This enables M64 window on P7IOC, which has been enabled on PHB3. >>>>>>Different from PHB3 where 16 M64 BARs are supported and each of >>>>>>them can be owned by one particular PE# exclusively or divided >>>>>>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>>>>>of them are divided to 8 segments. So every P7IOC PHB supports >>>>>>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>>>>>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>>>>>M64DT, indicating that one M64 segment can only be pinned to the >>>>>>fixed PE#. In order to have same code to support M64 on P7IOC and >>>>>>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>>>>>of them is pinned to the fixed PE# by bypassing the function of >>>>>>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>>>>>and PHB3 to support M64. >>>>> >>>>>I thought we decided (Ben suggested?) not to push P7IOC code now (or ever) as >>>>>there is no user for it, has this changed? >>>>> >>>> >>>>Remember that the code is mixed for P7IOC/PHB3. It's not harmful to support >>>>M64 window on P7IOC, which is much larger than M32. >>> >>> >>>The patchset starts with removing dead code and then adds more dead code. >>>This is not right... >>> >> >>Sorry, you mean it's fine to break the code on P7IOC as it's going to be dead. > > >I am saying that the _new_ code which implements PCI hotplug on P7IOC is >dead, not the existing P7IOC support which needs to keep working. Reworks you >make should keep P7IOC alive but they do not have to add hotplug. > >imho it is more likely that we drop P7IOC support in the mainline kernel in >next 5 years than someone plugs a PCI device to a running P7IOC machine >anywhere. > This patchset isn't supporting PCI hotplug on P7IOC. At the code is mixed for PHB3/P7IOC, I have to make some changes for P7IOC in order to support PCI hotplug on PHB3. It's one of the reason that I introduced M64 support for P7IOC. Another readon is M32 has limited space size (2GB) on P7IOC, one adapter with more than 2GB memory resource will fail to work on P7IOC. So it's still nice to support M64 on P7IOC. >>But I'm curious when it's going happen, any idea about that? >> >>>>>btw please put ioda1/ioda2/p7ioc/etc to the subject line to make it easier to >>>>>see how much work is there about particular PHB type. You rename quite many >>>>>functions and I generally want to ask you to group all renaming patches first >>>>>but it would also make sense to keep them close to (for example) >>>>>p7ioc-related patches so having more descriptive subject lines may help. >>>>>Thanks. >>>>> >>>> >>>>As the code is mixed for P7IOC/PHB3, I'm not following the line (IODA1/IODA2/p7ioc/phb3) >>>>in this patchset. >>> >>>But you should draw the bold line between PHB types imho. >>> >>>>Instead, the sequence of patchset is order related to: cod refactoring, >>>>IO/M32/M64, DMA, PE allocation/releaseing. >>> >>> >>>Some patches from this patchset are about P7IOC only. All I am asking is to >>>say specifically in the subject line what the patch touches - >>>IODA1/IODA2/p7ioc/phb3/all_of_them. Or I can walk through all of them, pick >>>P7IOC's ones, evaluate the amount of code and entropy they actually add and >>>then ask Ben what we do about it, it will just take longer rather than if you >>>did it. >>> >> >>Please give me a clear command what key words you need in the subject in next revision. >>What I understood is you want to see one of them: >> >>powerpc/powernv/ioda1: > >Yes, looks good. > >>powerpc/powernv/ioda2: > > >Yes. > >>powerpc/powernv/all: > >Just "powerpc/powernv". > Thanks for confirm, I'll put them into the subject in next revision. Thanks, Gavin
On 11/17/2015 12:37 PM, Gavin Shan wrote: > On Mon, Nov 16, 2015 at 07:01:46PM +1100, Alexey Kardashevskiy wrote: >> On 11/05/2015 12:12 AM, Gavin Shan wrote: >>> This enables M64 window on P7IOC, which has been enabled on PHB3. >>> Different from PHB3 where 16 M64 BARs are supported and each of >>> them can be owned by one particular PE# exclusively or divided >>> evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>> of them are divided to 8 segments. So every P7IOC PHB supports >>> 128 M64 segments in total. P7IOC has M64DT, which helps mapping >>> one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>> M64DT, indicating that one M64 segment can only be pinned to the >>> fixed PE#. In order to have same code to support M64 on P7IOC and >>> PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>> of them is pinned to the fixed PE# by bypassing the function of >>> M64DT. In turn, we just need different phb->init_m64() for P7IOC >>> and PHB3 to support M64. >>> >>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- >>> arch/powerpc/platforms/powernv/pci.h | 3 ++ >>> 2 files changed, 86 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >>> index 1f7d985..bfe69f1 100644 >>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c >>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c >>> @@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, >>> } >>> } >>> >>> +static int pnv_ioda1_init_m64(struct pnv_phb *phb) >>> +{ >>> + struct resource *r; >>> + int index; >>> + >>> + /* >>> + * There are 16 M64 BARs, each of which has 8 segments. So >>> + * there are as many M64 segments as the maximum number of >>> + * PEs, which is 128. >>> + */ >>> + for (index = 0; index < PNV_IODA1_M64_NUM; index++) { >>> + unsigned long base, segsz = phb->ioda.m64_segsize; >>> + int64_t rc; >>> + >>> + base = phb->ioda.m64_base + >>> + index * PNV_IODA1_M64_SEGS * segsz; >>> + rc = opal_pci_set_phb_mem_window(phb->opal_id, >>> + OPAL_M64_WINDOW_TYPE, index, base, 0, >>> + PNV_IODA1_M64_SEGS * segsz); >>> + if (rc != OPAL_SUCCESS) { >>> + pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", >>> + rc, phb->hose->global_number, index); >>> + goto fail; >>> + } >>> + >>> + rc = opal_pci_phb_mmio_enable(phb->opal_id, >>> + OPAL_M64_WINDOW_TYPE, index, >>> + OPAL_ENABLE_M64_SPLIT); >>> + if (rc != OPAL_SUCCESS) { >>> + pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", >>> + rc, phb->hose->global_number, index); >>> + goto fail; >>> + } >>> + } >>> + >>> + /* >>> + * Exclude the segment used by the reserved PE, which >>> + * is expected to be 0 or last supported PE#. >>> + */ >>> + r = &phb->hose->mem_resources[1]; >>> + if (phb->ioda.reserved_pe_idx == 0) >>> + r->start += phb->ioda.m64_segsize; >>> + else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) >>> + r->end -= phb->ioda.m64_segsize; >>> + else >>> + pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", >>> + phb->ioda.reserved_pe_idx); >>> + >>> + return 0; >>> + >>> +fail: >>> + for ( ; index >= 0; index--) >>> + opal_pci_phb_mmio_enable(phb->opal_id, >>> + OPAL_M64_WINDOW_TYPE, index, OPAL_DISABLE_M64); >>> + >>> + return -EIO; >>> +} >>> + >>> static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus, >>> unsigned long *pe_bitmap, >>> bool all) >>> @@ -325,6 +383,26 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all) >>> pe->master = master_pe; >>> list_add_tail(&pe->list, &master_pe->slaves); >>> } >>> + >>> + /* >>> + * P7IOC supports M64DT, which helps mapping M64 segment >>> + * to one particular PE#. However, PHB3 has fixed mapping >>> + * between M64 segment and PE#. In order to have same logic >>> + * for P7IOC and PHB3, we enforce fixed mapping between M64 >>> + * segment and PE# on P7IOC. >>> + */ >>> + if (phb->type == PNV_PHB_IODA1) { >>> + int64_t rc; >>> + >>> + rc = opal_pci_map_pe_mmio_window(phb->opal_id, >>> + pe->pe_number, OPAL_M64_WINDOW_TYPE, >>> + pe->pe_number / PNV_IODA1_M64_SEGS, >>> + pe->pe_number % PNV_IODA1_M64_SEGS); >>> + if (rc != OPAL_SUCCESS) >>> + pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n", >>> + __func__, rc, phb->hose->global_number, >>> + pe->pe_number); >>> + } >>> } >>> >>> kfree(pe_alloc); >>> @@ -339,8 +417,7 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >>> const u32 *r; >>> u64 pci_addr; >>> >>> - /* FIXME: Support M64 for P7IOC */ >>> - if (phb->type != PNV_PHB_IODA2) { >>> + if (phb->type != PNV_PHB_IODA1 && phb->type != PNV_PHB_IODA2) { >>> pr_info(" Not support M64 window\n"); >>> return; >>> } >>> @@ -373,7 +450,10 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >>> >>> /* Use last M64 BAR to cover M64 window */ >>> phb->ioda.m64_bar_idx = 15; >>> - phb->init_m64 = pnv_ioda2_init_m64; >>> + if (phb->type == PNV_PHB_IODA1) >>> + phb->init_m64 = pnv_ioda1_init_m64; >>> + else >>> + phb->init_m64 = pnv_ioda2_init_m64; >>> phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; >>> phb->pick_m64_pe = pnv_ioda_pick_m64_pe; >> >> >> Nit: the callbacks initialization does not seem to relate to parsing any >> window :) They could all go to where pnv_ioda_parse_m64_window() is called, >> no separate patch is needed. >> > > Well, what's the benifit for that? I personally prefer the way I had: initialize > all callbacks in one place, not in separate places. One place is good, agree. Which should have been a single phv_phb_ops instance as it is done everywhere else in the kernel. And the existing pnv_phb's callbacks are already initialized in various places, not a single one. > However, if you have good > reason to support your suggestion, I'll change accordingly for sure. Nah, leave it as is for now, we will probably change this later. > >> >>> } >>> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h >>> index 671fd13..c4019ac 100644 >>> --- a/arch/powerpc/platforms/powernv/pci.h >>> +++ b/arch/powerpc/platforms/powernv/pci.h >>> @@ -78,6 +78,9 @@ struct pnv_ioda_pe { >>> struct list_head list; >>> }; >>> >>> +#define PNV_IODA1_M64_NUM 16 /* Number of M64 BARs */ >>> +#define PNV_IODA1_M64_SEGS 8 /* Segments per M64 BAR */ >>> + >>> #define PNV_PHB_FLAG_EEH (1 << 0) >>> >>> struct pnv_phb { >>> > > Thanks, > Gavin >
On Thu, Nov 19, 2015 at 11:18:46AM +1100, Alexey Kardashevskiy wrote: >On 11/17/2015 12:37 PM, Gavin Shan wrote: >>On Mon, Nov 16, 2015 at 07:01:46PM +1100, Alexey Kardashevskiy wrote: >>>On 11/05/2015 12:12 AM, Gavin Shan wrote: >>>>This enables M64 window on P7IOC, which has been enabled on PHB3. >>>>Different from PHB3 where 16 M64 BARs are supported and each of >>>>them can be owned by one particular PE# exclusively or divided >>>>evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each >>>>of them are divided to 8 segments. So every P7IOC PHB supports >>>>128 M64 segments in total. P7IOC has M64DT, which helps mapping >>>>one particular M64 segment# to arbitrary PE#. PHB3 doesn't have >>>>M64DT, indicating that one M64 segment can only be pinned to the >>>>fixed PE#. In order to have same code to support M64 on P7IOC and >>>>PHB3, we just provide 128 M64 segments on every P7IOC PHB and each >>>>of them is pinned to the fixed PE# by bypassing the function of >>>>M64DT. In turn, we just need different phb->init_m64() for P7IOC >>>>and PHB3 to support M64. >>>> >>>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>>>--- >>>> arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- >>>> arch/powerpc/platforms/powernv/pci.h | 3 ++ >>>> 2 files changed, 86 insertions(+), 3 deletions(-) >>>> >>>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >>>>index 1f7d985..bfe69f1 100644 >>>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c >>>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c >>>>@@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, >>>> } >>>> } >>>> >>>>+static int pnv_ioda1_init_m64(struct pnv_phb *phb) >>>>+{ >>>>+ struct resource *r; >>>>+ int index; >>>>+ >>>>+ /* >>>>+ * There are 16 M64 BARs, each of which has 8 segments. So >>>>+ * there are as many M64 segments as the maximum number of >>>>+ * PEs, which is 128. >>>>+ */ >>>>+ for (index = 0; index < PNV_IODA1_M64_NUM; index++) { >>>>+ unsigned long base, segsz = phb->ioda.m64_segsize; >>>>+ int64_t rc; >>>>+ >>>>+ base = phb->ioda.m64_base + >>>>+ index * PNV_IODA1_M64_SEGS * segsz; >>>>+ rc = opal_pci_set_phb_mem_window(phb->opal_id, >>>>+ OPAL_M64_WINDOW_TYPE, index, base, 0, >>>>+ PNV_IODA1_M64_SEGS * segsz); >>>>+ if (rc != OPAL_SUCCESS) { >>>>+ pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", >>>>+ rc, phb->hose->global_number, index); >>>>+ goto fail; >>>>+ } >>>>+ >>>>+ rc = opal_pci_phb_mmio_enable(phb->opal_id, >>>>+ OPAL_M64_WINDOW_TYPE, index, >>>>+ OPAL_ENABLE_M64_SPLIT); >>>>+ if (rc != OPAL_SUCCESS) { >>>>+ pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", >>>>+ rc, phb->hose->global_number, index); >>>>+ goto fail; >>>>+ } >>>>+ } >>>>+ >>>>+ /* >>>>+ * Exclude the segment used by the reserved PE, which >>>>+ * is expected to be 0 or last supported PE#. >>>>+ */ >>>>+ r = &phb->hose->mem_resources[1]; >>>>+ if (phb->ioda.reserved_pe_idx == 0) >>>>+ r->start += phb->ioda.m64_segsize; >>>>+ else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) >>>>+ r->end -= phb->ioda.m64_segsize; >>>>+ else >>>>+ pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", >>>>+ phb->ioda.reserved_pe_idx); >>>>+ >>>>+ return 0; >>>>+ >>>>+fail: >>>>+ for ( ; index >= 0; index--) >>>>+ opal_pci_phb_mmio_enable(phb->opal_id, >>>>+ OPAL_M64_WINDOW_TYPE, index, OPAL_DISABLE_M64); >>>>+ >>>>+ return -EIO; >>>>+} >>>>+ >>>> static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus, >>>> unsigned long *pe_bitmap, >>>> bool all) >>>>@@ -325,6 +383,26 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all) >>>> pe->master = master_pe; >>>> list_add_tail(&pe->list, &master_pe->slaves); >>>> } >>>>+ >>>>+ /* >>>>+ * P7IOC supports M64DT, which helps mapping M64 segment >>>>+ * to one particular PE#. However, PHB3 has fixed mapping >>>>+ * between M64 segment and PE#. In order to have same logic >>>>+ * for P7IOC and PHB3, we enforce fixed mapping between M64 >>>>+ * segment and PE# on P7IOC. >>>>+ */ >>>>+ if (phb->type == PNV_PHB_IODA1) { >>>>+ int64_t rc; >>>>+ >>>>+ rc = opal_pci_map_pe_mmio_window(phb->opal_id, >>>>+ pe->pe_number, OPAL_M64_WINDOW_TYPE, >>>>+ pe->pe_number / PNV_IODA1_M64_SEGS, >>>>+ pe->pe_number % PNV_IODA1_M64_SEGS); >>>>+ if (rc != OPAL_SUCCESS) >>>>+ pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n", >>>>+ __func__, rc, phb->hose->global_number, >>>>+ pe->pe_number); >>>>+ } >>>> } >>>> >>>> kfree(pe_alloc); >>>>@@ -339,8 +417,7 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >>>> const u32 *r; >>>> u64 pci_addr; >>>> >>>>- /* FIXME: Support M64 for P7IOC */ >>>>- if (phb->type != PNV_PHB_IODA2) { >>>>+ if (phb->type != PNV_PHB_IODA1 && phb->type != PNV_PHB_IODA2) { >>>> pr_info(" Not support M64 window\n"); >>>> return; >>>> } >>>>@@ -373,7 +450,10 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >>>> >>>> /* Use last M64 BAR to cover M64 window */ >>>> phb->ioda.m64_bar_idx = 15; >>>>- phb->init_m64 = pnv_ioda2_init_m64; >>>>+ if (phb->type == PNV_PHB_IODA1) >>>>+ phb->init_m64 = pnv_ioda1_init_m64; >>>>+ else >>>>+ phb->init_m64 = pnv_ioda2_init_m64; >>>> phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; >>>> phb->pick_m64_pe = pnv_ioda_pick_m64_pe; >>> >>> >>>Nit: the callbacks initialization does not seem to relate to parsing any >>>window :) They could all go to where pnv_ioda_parse_m64_window() is called, >>>no separate patch is needed. >>> >> >>Well, what's the benifit for that? I personally prefer the way I had: initialize >>all callbacks in one place, not in separate places. > >One place is good, agree. Which should have been a single phv_phb_ops >instance as it is done everywhere else in the kernel. And the existing >pnv_phb's callbacks are already initialized in various places, not a single >one. > Ok, but that's separate issue not related to this patchset. I guess the issue you're taling about can be fixed in future, not in this patchset. >>However, if you have good >>reason to support your suggestion, I'll change accordingly for sure. > >Nah, leave it as is for now, we will probably change this later. > Thanks. >> >>> >>>> } >>>>diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h >>>>index 671fd13..c4019ac 100644 >>>>--- a/arch/powerpc/platforms/powernv/pci.h >>>>+++ b/arch/powerpc/platforms/powernv/pci.h >>>>@@ -78,6 +78,9 @@ struct pnv_ioda_pe { >>>> struct list_head list; >>>> }; >>>> >>>>+#define PNV_IODA1_M64_NUM 16 /* Number of M64 BARs */ >>>>+#define PNV_IODA1_M64_SEGS 8 /* Segments per M64 BAR */ >>>>+ >>>> #define PNV_PHB_FLAG_EEH (1 << 0) >>>> >>>> struct pnv_phb { >>>> Thanks, Gavin
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 1f7d985..bfe69f1 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -256,6 +256,64 @@ static void pnv_ioda_reserve_dev_m64_pe(struct pci_dev *pdev, } } +static int pnv_ioda1_init_m64(struct pnv_phb *phb) +{ + struct resource *r; + int index; + + /* + * There are 16 M64 BARs, each of which has 8 segments. So + * there are as many M64 segments as the maximum number of + * PEs, which is 128. + */ + for (index = 0; index < PNV_IODA1_M64_NUM; index++) { + unsigned long base, segsz = phb->ioda.m64_segsize; + int64_t rc; + + base = phb->ioda.m64_base + + index * PNV_IODA1_M64_SEGS * segsz; + rc = opal_pci_set_phb_mem_window(phb->opal_id, + OPAL_M64_WINDOW_TYPE, index, base, 0, + PNV_IODA1_M64_SEGS * segsz); + if (rc != OPAL_SUCCESS) { + pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n", + rc, phb->hose->global_number, index); + goto fail; + } + + rc = opal_pci_phb_mmio_enable(phb->opal_id, + OPAL_M64_WINDOW_TYPE, index, + OPAL_ENABLE_M64_SPLIT); + if (rc != OPAL_SUCCESS) { + pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n", + rc, phb->hose->global_number, index); + goto fail; + } + } + + /* + * Exclude the segment used by the reserved PE, which + * is expected to be 0 or last supported PE#. + */ + r = &phb->hose->mem_resources[1]; + if (phb->ioda.reserved_pe_idx == 0) + r->start += phb->ioda.m64_segsize; + else if (phb->ioda.reserved_pe_idx == (phb->ioda.total_pe_num - 1)) + r->end -= phb->ioda.m64_segsize; + else + pr_warn(" Cannot cut M64 segment for reserved PE#%d\n", + phb->ioda.reserved_pe_idx); + + return 0; + +fail: + for ( ; index >= 0; index--) + opal_pci_phb_mmio_enable(phb->opal_id, + OPAL_M64_WINDOW_TYPE, index, OPAL_DISABLE_M64); + + return -EIO; +} + static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus, unsigned long *pe_bitmap, bool all) @@ -325,6 +383,26 @@ static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all) pe->master = master_pe; list_add_tail(&pe->list, &master_pe->slaves); } + + /* + * P7IOC supports M64DT, which helps mapping M64 segment + * to one particular PE#. However, PHB3 has fixed mapping + * between M64 segment and PE#. In order to have same logic + * for P7IOC and PHB3, we enforce fixed mapping between M64 + * segment and PE# on P7IOC. + */ + if (phb->type == PNV_PHB_IODA1) { + int64_t rc; + + rc = opal_pci_map_pe_mmio_window(phb->opal_id, + pe->pe_number, OPAL_M64_WINDOW_TYPE, + pe->pe_number / PNV_IODA1_M64_SEGS, + pe->pe_number % PNV_IODA1_M64_SEGS); + if (rc != OPAL_SUCCESS) + pr_warn("%s: Error %lld mapping M64 for PHB#%d-PE#%d\n", + __func__, rc, phb->hose->global_number, + pe->pe_number); + } } kfree(pe_alloc); @@ -339,8 +417,7 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) const u32 *r; u64 pci_addr; - /* FIXME: Support M64 for P7IOC */ - if (phb->type != PNV_PHB_IODA2) { + if (phb->type != PNV_PHB_IODA1 && phb->type != PNV_PHB_IODA2) { pr_info(" Not support M64 window\n"); return; } @@ -373,7 +450,10 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) /* Use last M64 BAR to cover M64 window */ phb->ioda.m64_bar_idx = 15; - phb->init_m64 = pnv_ioda2_init_m64; + if (phb->type == PNV_PHB_IODA1) + phb->init_m64 = pnv_ioda1_init_m64; + else + phb->init_m64 = pnv_ioda2_init_m64; phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; phb->pick_m64_pe = pnv_ioda_pick_m64_pe; } diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index 671fd13..c4019ac 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -78,6 +78,9 @@ struct pnv_ioda_pe { struct list_head list; }; +#define PNV_IODA1_M64_NUM 16 /* Number of M64 BARs */ +#define PNV_IODA1_M64_SEGS 8 /* Segments per M64 BAR */ + #define PNV_PHB_FLAG_EEH (1 << 0) struct pnv_phb {
This enables M64 window on P7IOC, which has been enabled on PHB3. Different from PHB3 where 16 M64 BARs are supported and each of them can be owned by one particular PE# exclusively or divided evenly to 256 segments, every P7IOC PHB has 16 M64 BARs and each of them are divided to 8 segments. So every P7IOC PHB supports 128 M64 segments in total. P7IOC has M64DT, which helps mapping one particular M64 segment# to arbitrary PE#. PHB3 doesn't have M64DT, indicating that one M64 segment can only be pinned to the fixed PE#. In order to have same code to support M64 on P7IOC and PHB3, we just provide 128 M64 segments on every P7IOC PHB and each of them is pinned to the fixed PE# by bypassing the function of M64DT. In turn, we just need different phb->init_m64() for P7IOC and PHB3 to support M64. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/pci-ioda.c | 86 +++++++++++++++++++++++++++++-- arch/powerpc/platforms/powernv/pci.h | 3 ++ 2 files changed, 86 insertions(+), 3 deletions(-)