[Skiboot] [PATCH v2 2/2] hw/phys_map: Use GCIDs as a chip index
Andrew Donnellan
andrew.donnellan at au1.ibm.com
Fri Jun 16 11:11:02 AEST 2017
On 15/06/17 16:00, Oliver O'Halloran wrote:
> Currently we pass in a proc_chip structure to phys_map_get(). All we we
> really need from this structure is the Global Chip ID (GCID). This
> patch reworks the function so that we only need to pass the GCID which
> allows us to use it before the proc_chip structures have been
> initialised (i.e in the HDAT parser).
>
> Cc: Michael Neuling <mikey at neuling.org>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
> ---
> v1 -> v2: Converted NVLink usages
> ---
> hdata/fsp.c | 2 +-
> hw/npu2.c | 10 +++++-----
> hw/nx.c | 2 +-
> hw/phb4.c | 10 ++++------
> hw/phys-map.c | 9 +++++----
> hw/test/phys-map-test.c | 10 +++++-----
> hw/xive.c | 12 +++++-------
> include/phys-map.h | 2 +-
> 8 files changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/hdata/fsp.c b/hdata/fsp.c
> index 5487f33191ea..6953d97b5e2a 100644
> --- a/hdata/fsp.c
> +++ b/hdata/fsp.c
> @@ -325,7 +325,7 @@ static void bmc_create_node(const struct HDIF_common_hdr *sp)
> */
> chip_id = pcid_to_chip_id(be32_to_cpu(iopath->lpc.chip_id));
>
> - phys_map_get(get_chip(chip_id), LPC_BUS, 0, &lpcm_base, NULL);
> + phys_map_get(chip_id, LPC_BUS, 0, &lpcm_base, NULL);
> lpcm = dt_new_addr(dt_root, "lpcm-opb", lpcm_base);
> assert(lpcm);
>
> diff --git a/hw/npu2.c b/hw/npu2.c
> index 040ab8839c89..8786cc1e0e67 100644
> --- a/hw/npu2.c
> +++ b/hw/npu2.c
> @@ -521,7 +521,7 @@ static void npu2_get_gpu_base(struct npu2_dev *ndev, uint64_t *addr, uint64_t *s
> int group;
>
> group = (ndev->bdfn >> 3) & 0x1f;
> - phys_map_get(get_chip(ndev->npu->chip_id), GPU_MEM, group, addr, size);
> + phys_map_get(ndev->npu->chip_id, GPU_MEM, group, addr, size);
> }
>
> static void npu2_dn_fixup_gmb(struct dt_node *pd_dn, struct npu2_dev *ndev)
> @@ -732,7 +732,6 @@ static void npu2_hw_init(struct npu2 *p)
> {
> int i;
> uint64_t val, size, addr, gpu_min_addr, gpu_max_addr, total_size;
> - struct proc_chip *chip = get_chip(p->chip_id);
>
> npu2_ioda_reset(&p->phb, false);
>
> @@ -741,15 +740,16 @@ static void npu2_hw_init(struct npu2 *p)
> npu2_write(p, NPU2_XTS_CFG, val | NPU2_XTS_CFG_MMIOSD | NPU2_XTS_CFG_TRY_ATR_RO);
>
> /* Init memory cache directory (MCD) registers. */
> - phys_map_get(chip, GPU_MEM, NPU2_LINKS_PER_CHIP - 1, &gpu_min_addr, NULL);
> - phys_map_get(chip, GPU_MEM, 0, &gpu_max_addr, &size);
> + phys_map_get(p->chip_id, GPU_MEM, NPU2_LINKS_PER_CHIP - 1,
> + &gpu_min_addr, NULL);
> + phys_map_get(p->chip_id, GPU_MEM, 0, &gpu_max_addr, &size);
> gpu_max_addr += size;
>
> /* We assume GPU memory is contiguous from the first possible GPU to the
> * last and that the size is the same so best to check that. */
> for (i = 0; i < NPU2_LINKS_PER_CHIP; i++) {
> uint64_t tmp;
> - phys_map_get(chip, GPU_MEM, i, &addr, &tmp);
> + phys_map_get(p->chip_id, GPU_MEM, i, &addr, &tmp);
> assert((addr >= gpu_min_addr) && (addr + tmp <= gpu_max_addr));
> assert(tmp == size);
> }
> diff --git a/hw/nx.c b/hw/nx.c
> index d962b01a7535..1c269292b160 100644
> --- a/hw/nx.c
> +++ b/hw/nx.c
> @@ -50,7 +50,7 @@ void nx_p9_rng_init(void)
> */
> for_each_chip(chip) {
> /* 1) NX RNG BAR */
> - phys_map_get(chip, NX_RNG, 0, &bar, NULL);
> + phys_map_get(chip->id, NX_RNG, 0, &bar, NULL);
> xscom_write(chip->id, P9X_NX_MMIO_BAR,
> bar | P9X_NX_MMIO_BAR_EN);
> /* Read config register for pace info */
> diff --git a/hw/phb4.c b/hw/phb4.c
> index 7bcbc931fc19..fdc843aeef1a 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -3661,13 +3661,11 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index,
> uint64_t mmio_win[4];
> unsigned int mmio_win_sz;
> struct dt_node *np;
> - struct proc_chip *chip;
> char *path;
> uint64_t capp_ucode_base;
> unsigned int max_link_speed;
>
> gcid = dt_get_chip_id(stk_node);
> - chip = get_chip(gcid);
> stk_index = dt_prop_get_u32(stk_node, "reg");
> phb_num = dt_prop_get_u32(stk_node, "ibm,phb-index");
> path = dt_get_path(stk_node);
> @@ -3686,24 +3684,24 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index,
> bar_en = 0;
>
> /* Initialize PHB register BAR */
> - phys_map_get(chip, PHB4_REG_SPC, phb_num, &phb_bar, NULL);
> + phys_map_get(gcid, PHB4_REG_SPC, phb_num, &phb_bar, NULL);
> xscom_write(gcid, nest_stack + XPEC_NEST_STK_PHB_REG_BAR, phb_bar << 8);
> bar_en |= XPEC_NEST_STK_BAR_EN_PHB;
>
>
> /* Same with INT BAR (ESB) */
> - phys_map_get(chip, PHB4_XIVE_ESB, phb_num, &irq_bar, NULL);
> + phys_map_get(gcid, PHB4_XIVE_ESB, phb_num, &irq_bar, NULL);
> xscom_write(gcid, nest_stack + XPEC_NEST_STK_IRQ_BAR, irq_bar << 8);
> bar_en |= XPEC_NEST_STK_BAR_EN_INT;
>
>
> /* Same with MMIO windows */
> - phys_map_get(chip, PHB4_64BIT_MMIO, phb_num, &mmio0_bar, &mmio0_sz);
> + phys_map_get(gcid, PHB4_64BIT_MMIO, phb_num, &mmio0_bar, &mmio0_sz);
> mmio0_bmask = (~(mmio0_sz - 1)) & 0x00FFFFFFFFFFFFFFULL;
> xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0, mmio0_bar << 8);
> xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0_MASK, mmio0_bmask << 8);
>
> - phys_map_get(chip, PHB4_32BIT_MMIO, phb_num, &mmio1_bar, &mmio1_sz);
> + phys_map_get(gcid, PHB4_32BIT_MMIO, phb_num, &mmio1_bar, &mmio1_sz);
> mmio1_bmask = (~(mmio1_sz - 1)) & 0x00FFFFFFFFFFFFFFULL;
> xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1, mmio1_bar << 8);
> xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1_MASK, mmio1_bmask << 8);
> diff --git a/hw/phys-map.c b/hw/phys-map.c
> index cab23f6c9ede..e99983f62909 100644
> --- a/hw/phys-map.c
> +++ b/hw/phys-map.c
> @@ -135,10 +135,11 @@ static inline bool phys_map_entry_null(const struct phys_map_entry *e)
> return false;
> }
>
> +
> /* This crashes skiboot on error as any bad calls here are almost
> * certainly a developer error
> */
> -void phys_map_get(struct proc_chip *chip, enum phys_map_type type,
> +void phys_map_get(uint64_t gcid, enum phys_map_type type,
> int index, uint64_t *addr, uint64_t *size) {
> const struct phys_map_entry *e;
> uint64_t a;
> @@ -163,16 +164,16 @@ void phys_map_get(struct proc_chip *chip, enum phys_map_type type,
> break;
> }
> a = e->addr;
> - a += (uint64_t)chip->id << phys_map->chip_select_shift;
> + a += gcid << phys_map->chip_select_shift;
>
> if (addr)
> *addr = a;
> if (size)
> *size = e->size;
>
> - prlog(PR_TRACE, "Assigning BAR [%x] type:%02i index:%x "
> + prlog(PR_TRACE, "Assigning BAR [%"PRIx64"] type:%02i index:%x "
> "0x%016"PRIx64" for 0x%016"PRIx64"\n",
> - chip->id, type, index, a, e->size);
> + gcid, type, index, a, e->size);
>
> return;
>
> diff --git a/hw/test/phys-map-test.c b/hw/test/phys-map-test.c
> index 33810745f99e..ab75a1e6e0be 100644
> --- a/hw/test/phys-map-test.c
> +++ b/hw/test/phys-map-test.c
> @@ -84,9 +84,12 @@ static inline bool map_call_entry_null(const struct map_call_entry *t)
> /* Check calls to map to see if they overlap.
> * Creates a new table for each of the entries it gets to check against
> */
> +
> +/* Pick a chip ID, any ID. */
> +#define FAKE_CHIP_ID 8
> +
> static void check_map_call(void)
> {
> - struct proc_chip c;
> uint64_t start, size, end;
> const struct phys_map_entry *e;
> struct map_call_entry *tbl, *t, *tnext;
> @@ -102,12 +105,9 @@ static void check_map_call(void)
> assert(tbl != NULL);
> memset(tbl, 0, tbl_size);
>
> - /* Fake we are chip id 8. Any id will do */
> - c.id = 8;
> -
> /* Loop over table entries ... */
> for (e = phys_map->table; !phys_map_entry_null(e); e++) {
> - phys_map_get(&c, e->type, e->index, &start, &size);
> + phys_map_get(FAKE_CHIP_ID, e->type, e->index, &start, &size);
>
> /* Check for alignment */
> if ((e->type != SYSTEM_MEM) && (e->type != RESV)) {
> diff --git a/hw/xive.c b/hw/xive.c
> index 6db4d42aa10c..eac8acbce6fc 100644
> --- a/hw/xive.c
> +++ b/hw/xive.c
> @@ -1548,12 +1548,11 @@ static bool xive_read_bars(struct xive *x)
>
> static bool xive_configure_bars(struct xive *x)
> {
> - struct proc_chip chip;
> - struct proc_chip *c = get_chip(x->chip_id);
> + uint64_t chip_id = x->chip_id;
> uint64_t val;
>
> /* IC BAR */
> - phys_map_get(c, XIVE_IC, 0, (uint64_t *)&x->ic_base, &x->ic_size);
> + phys_map_get(chip_id, XIVE_IC, 0, (uint64_t *)&x->ic_base, &x->ic_size);
> val = (uint64_t)x->ic_base | CQ_IC_BAR_VALID;
> if (IC_PAGE_SIZE == 0x10000) {
> val |= CQ_IC_BAR_64K;
> @@ -1568,8 +1567,7 @@ static bool xive_configure_bars(struct xive *x)
> * for each chip !!! Hence we create a fake chip 0 and use that for
> * all phys_map_get(XIVE_TM) calls.
> */
> - chip.id = 0;
> - phys_map_get(&chip, XIVE_TM, 0, (uint64_t *)&x->tm_base, &x->tm_size);
> + phys_map_get(0, XIVE_TM, 0, (uint64_t *)&x->tm_base, &x->tm_size);
> val = (uint64_t)x->tm_base | CQ_TM_BAR_VALID;
> if (TM_PAGE_SIZE == 0x10000) {
> x->tm_shift = 16;
> @@ -1584,7 +1582,7 @@ static bool xive_configure_bars(struct xive *x)
> return false;
>
> /* PC BAR. Clear first, write mask, then write value */
> - phys_map_get(c, XIVE_PC, 0, (uint64_t *)&x->pc_base, &x->pc_size);
> + phys_map_get(chip_id, XIVE_PC, 0, (uint64_t *)&x->pc_base, &x->pc_size);
> xive_regwx(x, CQ_PC_BAR, 0);
> if (x->last_reg_error)
> return false;
> @@ -1598,7 +1596,7 @@ static bool xive_configure_bars(struct xive *x)
> return false;
>
> /* VC BAR. Clear first, write mask, then write value */
> - phys_map_get(c, XIVE_VC, 0, (uint64_t *)&x->vc_base, &x->vc_size);
> + phys_map_get(chip_id, XIVE_VC, 0, (uint64_t *)&x->vc_base, &x->vc_size);
> xive_regwx(x, CQ_VC_BAR, 0);
> if (x->last_reg_error)
> return false;
> diff --git a/include/phys-map.h b/include/phys-map.h
> index 50ff0c42241a..434b159e8ae8 100644
> --- a/include/phys-map.h
> +++ b/include/phys-map.h
> @@ -54,7 +54,7 @@ enum phys_map_type {
> RESV
> };
>
> -extern void phys_map_get(struct proc_chip *chip, enum phys_map_type type,
> +extern void phys_map_get(uint64_t gcid, enum phys_map_type type,
> int index, uint64_t *addr, uint64_t *size);
>
> extern void phys_map_init(void);
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com IBM Australia Limited
More information about the Skiboot
mailing list