Re: [PATCH v6 03/42] powerpc/powernv: Enable M64 on P7IOC

Alexey Kardashevskiy
2015-08-10T06:30:09Z

On 08/06/2015 02:11 PM, Gavin Shan wrote:
> The patch 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, each P7IOC PHB has 16 M64 BARs and each
> of them are divided into 8 segments.

Is this a limitation of POWER7 chip or it is from IODA1?

> So each P7IOC PHB can support
> 128 M64 segments only. Also, 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 similar logic to support M64 for PHB3
> and P7IOC, we just provide 128 M64 (16 BARs) segments and fixed
> mapping between PE# and M64 segment# on P7IOC. In turn, we just
> need different phb->init_m64() hooks for P7IOC and PHB3 to support
> M64.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 116 ++++++++++++++++++++++++++----
> 1 file changed, 104 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 38b5405..e4ac703 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -172,6 +172,69 @@ static void pnv_ioda_free_pe(struct pnv_phb *phb, int pe)
> clear_bit(pe, phb->ioda.pe_alloc);
> }
>
> +static int pnv_ioda1_init_m64(struct pnv_phb *phb)
> +{
> + struct resource *r;
> + int seg;
> +
> + /* There are as many M64 segments as the maximum number
> + * of PEs, which is 128.
> + */
> + for (seg = 0; seg < phb->ioda.total_pe; seg += 8) {

This "8" is used a lot across the patch, please make it a macro
(PNV_PHB_P7IOC_SEGNUM or PNV_PHB_IODA1_SEGNUM or whatever you think it is)
with a short comment why it is "8". Or a pnv_phb member.

> + unsigned long base;
> + int64_t rc;
> +
> + base = phb->ioda.m64_base + seg * phb->ioda.m64_segsize;
> + rc = opal_pci_set_phb_mem_window(phb->opal_id,
> + OPAL_M64_WINDOW_TYPE,
> + seg / 8,
> + base,
> + 0, /* unused */
> + 8 * phb->ioda.m64_segsize);
> + if (rc != OPAL_SUCCESS) {
> + pr_warn(" Error %lld setting M64 PHB#%d-BAR#%d\n",
> + rc, phb->hose->global_number, seg / 8);
> + goto fail;
> + }
> +
> + rc = opal_pci_phb_mmio_enable(phb->opal_id,
> + OPAL_M64_WINDOW_TYPE,
> + seg / 8,
> + OPAL_ENABLE_M64_SPLIT);
> + if (rc != OPAL_SUCCESS) {
> + pr_warn(" Error %lld enabling M64 PHB#%d-BAR#%d\n",
> + rc, phb->hose->global_number, seg / 8);
> + goto fail;
> + }
> + }
> +
> + /* Strip off the segment used by the reserved PE, which

What is this reserved PE on P7IOC? "Strip off" means "exclude" here?

> + * is expected to be 0 or last supported PE#. The PHB's
> + * first memory window traces the 32-bits MMIO range

s/traces/filters/ ? Or I did not understand this comment...

> + * while the second one traces the 64-bits prefetchable
> + * MMIO range that the PHB supports.

32/64 ranges comment seems irrelevant here.

> + */
> + r = &phb->hose->mem_resources[1];
> + if (phb->ioda.reserved_pe == 0)
> + r->start += phb->ioda.m64_segsize;
> + else if (phb->ioda.reserved_pe == (phb->ioda.total_pe - 1))
> + r->end -= phb->ioda.m64_segsize;
> + else
> + pr_warn(" Cannot strip M64 segment for reserved PE#%d\n",
> + phb->ioda.reserved_pe);
> +
> + return 0;
> +
> +fail:
> + for ( ; seg >= 0; seg -= 8)
> + opal_pci_phb_mmio_enable(phb->opal_id,
> + OPAL_M64_WINDOW_TYPE,
> + seg / 8,
> + OPAL_DISABLE_M64);
> +
> + return -EIO;
> +}
> +
> /* The default M64 BAR is shared by all PEs */
> static int pnv_ioda2_init_m64(struct pnv_phb *phb)
> {
> @@ -256,9 +319,9 @@ static void pnv_ioda2_reserve_dev_m64_pe(struct pci_dev *pdev,
> }
> }
>
> -static void pnv_ioda2_reserve_m64_pe(struct pci_bus *bus,
> - unsigned long *pe_bitmap,
> - bool all)
> +static void pnv_ioda_reserve_m64_pe(struct pci_bus *bus,
> + unsigned long *pe_bitmap,
> + bool all)
> {
> struct pci_dev *pdev;
>
> @@ -266,12 +329,12 @@ static void pnv_ioda2_reserve_m64_pe(struct pci_bus *bus,
> pnv_ioda2_reserve_dev_m64_pe(pdev, pe_bitmap);
>
> if (all && pdev->subordinate)
> - pnv_ioda2_reserve_m64_pe(pdev->subordinate,
> - pe_bitmap, all);
> + pnv_ioda_reserve_m64_pe(pdev->subordinate,
> + pe_bitmap, all);
> }
> }
>
> -static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all)
> +static int pnv_ioda_pick_m64_pe(struct pci_bus *bus, bool all)
> {
> struct pci_controller *hose = pci_bus_to_host(bus);
> struct pnv_phb *phb = hose->private_data;
> @@ -293,7 +356,7 @@ static int pnv_ioda2_pick_m64_pe(struct pci_bus *bus, bool all)
> }
>
> /* Figure out reserved PE numbers by the PE */
> - pnv_ioda2_reserve_m64_pe(bus, pe_alloc, all);
> + pnv_ioda_reserve_m64_pe(bus, pe_alloc, all);
>
> /*
> * the current bus might not own M64 window and that's all
> @@ -324,6 +387,26 @@ static int pnv_ioda2_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 / 8,
> + pe->pe_number % 8);
> + 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);
> @@ -338,8 +421,8 @@ 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;

You are adding P7IOC support so at least "fixme" should go. Also,
pnv_ioda_parse_m64_window() is only called from pnv_pci_init_ioda_phb()
which is called only with PNV_PHB_IODA1 and PNV_PHB_IODA2 (no other value
is passed there a type) so the check above will never succeed, just remove it.

> }
> @@ -372,9 +455,18 @@ 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;
> - phb->reserve_m64_pe = pnv_ioda2_reserve_m64_pe;
> - phb->pick_m64_pe = pnv_ioda2_pick_m64_pe;
> + phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe;
> + phb->pick_m64_pe = pnv_ioda_pick_m64_pe;
> + switch (phb->type) {
> + case PNV_PHB_IODA1:
> + phb->init_m64 = pnv_ioda1_init_m64;
> + break;
> + case PNV_PHB_IODA2:
> + phb->init_m64 = pnv_ioda2_init_m64;
> + break;
> + default:
> + pr_debug(" M64 not supported\n");
> + }
> }
>
> static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no)
>

--
Alexey