[Skiboot] [PATCH v2 2/2] hw/phys_map: Use GCIDs as a chip index

Michael Neuling mikey at neuling.org
Fri May 19 15:50:58 AEST 2017


On Fri, 2017-05-19 at 12:40 +1000, 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>

Thanks...

Acked-By: Michael Neuling <mikey at neuling.org>

> ---
>  hdata/fsp.c             |  2 +-
>  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 +-
>  7 files changed, 22 insertions(+), 25 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/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 1d6d5ccb9601..957473346a33 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -3437,13 +3437,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);
> @@ -3462,24 +3460,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);


More information about the Skiboot mailing list