[Skiboot] [PATCH 09/12] phb4: make endian-clean
Oliver O'Halloran
oohall at gmail.com
Tue Oct 1 15:47:20 AEST 2019
On Sun, 2019-09-29 at 17:46 +1000, Nicholas Piggin wrote:
> Convert phb4 dt construction and in-memory hardware tables to use
> explicit endian conversions.
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> hw/phb4.c | 220 ++++++++++++++++++++++++++-----------------------
> include/phb4.h | 2 +-
> 2 files changed, 116 insertions(+), 106 deletions(-)
>
> diff --git a/hw/phb4.c b/hw/phb4.c
> index 3c71427ae..f94a9995e 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -273,7 +273,7 @@ static int64_t phb4_pcicfg_check(struct phb4 *p, uint32_t bdfn,
> return OPAL_HARDWARE;
>
> /* Fetch the PE# from cache */
> - *pe = p->tbl_rtt[bdfn];
> + *pe = be16_to_cpu(p->tbl_rtt[bdfn]);
>
> return OPAL_SUCCESS;
> }
> @@ -923,7 +923,7 @@ static void phb4_init_ioda_cache(struct phb4 *p)
> * and this occurs before PEs have been assigned.
> */
> for (i = 0; i < RTT_TABLE_ENTRIES; i++)
> - p->tbl_rtt[i] = PHB4_RESERVED_PE_NUM(p);
> + p->tbl_rtt[i] = cpu_to_be16(PHB4_RESERVED_PE_NUM(p));
> memset(p->tbl_peltv, 0x0, p->tbl_peltv_size);
> memset(p->tve_cache, 0x0, sizeof(p->tve_cache));
>
> @@ -1748,110 +1748,123 @@ static void phb4_err_clear(struct phb4 *p)
> static void phb4_read_phb_status(struct phb4 *p,
> struct OpalIoPhb4ErrorData *stat)
> {
> - uint16_t val = 0;
> uint32_t i;
> uint64_t *pPEST;
> + uint16_t __16;
> + uint32_t __32;
> + uint64_t __64;
> *snip*
We consume the data that's read in phb4_read_phb_status() inside of
skiboot (see phb4_read_phb_status()) so we probably want to handle any
endian conversions elsewhere. Doing that in phb4_get_diag_data() would
make sense since that's the entry point used by the OPAL API. That puts
a bit of distance between where we read it and where we convert it
though so... dunno.
> @@ -2131,7 +2144,7 @@ static int64_t phb4_set_pe(struct phb *phb,
> /* Map or unmap the RTT range */
> for (idx = 0; idx < RTT_TABLE_ENTRIES; idx++)
> if ((idx & mask) == (bdfn & mask))
> - p->tbl_rtt[idx] = pe_number;
> + p->tbl_rtt[idx] = cpu_to_be16(pe_number);
>
> /* Invalidate the RID Translation Cache (RTC) inside the PHB */
> out_be64(p->regs + PHB_RTC_INVALIDATE, PHB_RTC_INVALIDATE_ALL);
> @@ -3450,7 +3463,7 @@ static uint64_t phb4_get_pesta(struct phb4 *p, uint64_t pe_number)
> phb4_ioda_sel(p, IODA3_TBL_PESTA, pe_number, false);
> pesta = phb4_read_reg(p, PHB_IODA_DATA0);
> if (pesta & IODA3_PESTA_MMIO_FROZEN)
> - pesta |= pPEST[2*pe_number];
> + pesta |= be64_to_cpu(pPEST[2*pe_number]);
>
> return pesta;
> }
> @@ -3808,13 +3821,13 @@ static int64_t phb4_err_inject_cfg(struct phb4 *phb, uint64_t pe_number,
> ctrl = PHB_PAPR_ERR_INJ_CTL_CFG;
>
> for (bdfn = 0; bdfn < RTT_TABLE_ENTRIES; bdfn++) {
> - if (phb->tbl_rtt[bdfn] != pe_number)
> + if (be16_to_cpu(phb->tbl_rtt[bdfn]) != pe_number)
> continue;
>
> /* The PE can be associated with PCI bus or device */
> is_bus_pe = false;
> if ((bdfn + 8) < RTT_TABLE_ENTRIES &&
> - phb->tbl_rtt[bdfn + 8] == pe_number)
> + be16_to_cpu(phb->tbl_rtt[bdfn + 8]) == pe_number)
> is_bus_pe = true;
>
> /* Figure out the PCI config address */
> @@ -5344,7 +5357,7 @@ static void phb4_allocate_tables(struct phb4 *p)
> p->tbl_rtt = local_alloc(p->chip_id, RTT_TABLE_SIZE, RTT_TABLE_SIZE);
> assert(p->tbl_rtt);
> for (i = 0; i < RTT_TABLE_ENTRIES; i++)
> - p->tbl_rtt[i] = PHB4_RESERVED_PE_NUM(p);
> + p->tbl_rtt[i] = cpu_to_be16(PHB4_RESERVED_PE_NUM(p));
>
> p->tbl_peltv = local_alloc(p->chip_id, p->tbl_peltv_size, p->tbl_peltv_size);
> assert(p->tbl_peltv);
> @@ -5482,11 +5495,11 @@ static bool phb4_calculate_windows(struct phb4 *p)
> "ibm,mmio-windows", -1);
> assert(prop->len >= (2 * sizeof(uint64_t)));
>
> - p->mm0_base = ((const uint64_t *)prop->prop)[0];
> - p->mm0_size = ((const uint64_t *)prop->prop)[1];
> + p->mm0_base = be64_to_cpu(((__be64 *)prop->prop)[0]);
> + p->mm0_size = be64_to_cpu(((__be64 *)prop->prop)[1]);
> if (prop->len > 16) {
> - p->mm1_base = ((const uint64_t *)prop->prop)[2];
> - p->mm1_size = ((const uint64_t *)prop->prop)[3];
> + p->mm1_base = be64_to_cpu(((__be64 *)prop->prop)[2]);
> + p->mm1_size = be64_to_cpu(((__be64 *)prop->prop)[3]);
> }
>
> /* Sort them so that 0 is big and 1 is small */
> @@ -5558,16 +5571,12 @@ static const struct irq_source_ops phb4_lsi_ops = {
> .attributes = phb4_lsi_attributes,
> };
>
> -#ifdef HAVE_BIG_ENDIAN
> static u64 lane_eq_default[8] = {
> - 0x5454545454545454UL, 0x5454545454545454UL,
> - 0x5454545454545454UL, 0x5454545454545454UL,
> - 0x7777777777777777UL, 0x7777777777777777UL,
> - 0x7777777777777777UL, 0x7777777777777777UL
> + CPU_TO_BE64(0x5454545454545454UL), CPU_TO_BE64(0x5454545454545454UL),
> + CPU_TO_BE64(0x5454545454545454UL), CPU_TO_BE64(0x5454545454545454UL),
> + CPU_TO_BE64(0x7777777777777777UL), CPU_TO_BE64(0x7777777777777777UL),
> + CPU_TO_BE64(0x7777777777777777UL), CPU_TO_BE64(0x7777777777777777UL),
> };
> -#else
> -#error lane_eq_default needs to be big endian (device tree property)
> -#endif
>
> static void phb4_create(struct dt_node *np)
> {
> @@ -5602,11 +5611,11 @@ static void phb4_create(struct dt_node *np)
>
> /* Get the various XSCOM register bases from the device-tree */
> prop = dt_require_property(np, "ibm,xscom-bases", 5 * sizeof(uint32_t));
> - p->pe_xscom = ((const uint32_t *)prop->prop)[0];
> - p->pe_stk_xscom = ((const uint32_t *)prop->prop)[1];
> - p->pci_xscom = ((const uint32_t *)prop->prop)[2];
> - p->pci_stk_xscom = ((const uint32_t *)prop->prop)[3];
> - p->etu_xscom = ((const uint32_t *)prop->prop)[4];
> + p->pe_xscom = be32_to_cpu(((__be32 *)prop->prop)[0]);
> + p->pe_stk_xscom = be32_to_cpu(((__be32 *)prop->prop)[1]);
> + p->pci_xscom = be32_to_cpu(((__be32 *)prop->prop)[2]);
> + p->pci_stk_xscom = be32_to_cpu(((__be32 *)prop->prop)[3]);
> + p->etu_xscom = be32_to_cpu(((__be32 *)prop->prop)[4]);
This can use dt_prop_get_cell(p, <offset>). It probably should have
been using that years ago, but here we are.
>
> /*
> * We skip the initial PERST assertion requested by the generic code
> @@ -5828,13 +5837,13 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index,
> /* Build MMIO windows list */
> mmio_win_sz = 0;
> if (mmio0_bar) {
> - mmio_win[mmio_win_sz++] = mmio0_bar;
> - mmio_win[mmio_win_sz++] = mmio0_sz;
> + mmio_win[mmio_win_sz++] = cpu_to_be64(mmio0_bar);
> + mmio_win[mmio_win_sz++] = cpu_to_be64(mmio0_sz);
> bar_en |= XPEC_NEST_STK_BAR_EN_MMIO0;
> }
> if (mmio1_bar) {
> - mmio_win[mmio_win_sz++] = mmio1_bar;
> - mmio_win[mmio_win_sz++] = mmio1_sz;
> + mmio_win[mmio_win_sz++] = cpu_to_be64(mmio1_bar);
> + mmio_win[mmio_win_sz++] = cpu_to_be64(mmio1_sz);
> bar_en |= XPEC_NEST_STK_BAR_EN_MMIO1;
> }
>
> @@ -5864,12 +5873,12 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index,
> prlog_once(PR_DEBUG, "Version reg: 0x%016llx\n", in_be64(foo));
>
> /* Create PHB node */
> - reg[0] = phb_bar;
> - reg[1] = 0x1000;
> - reg[2] = irq_bar;
> - reg[3] = 0x10000000;
> + reg[0] = cpu_to_be64(phb_bar);
> + reg[1] = cpu_to_be64(0x1000);
> + reg[2] = cpu_to_be64(irq_bar);
> + reg[3] = cpu_to_be64(0x10000000);
Might as well convert it to use dt_add_property_u64s()
>
> - np = dt_new_addr(dt_root, "pciex", reg[0]);
> + np = dt_new_addr(dt_root, "pciex", phb_bar);
> if (!np)
> return;
>
> @@ -5906,6 +5915,7 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index,
> const void *leq = dt_prop_get_def_size(stk_node, "ibm,lane-eq",
> NULL, &leq_size);
> if (leq != NULL && leq_size >= 6 * 8)
> + /* XXX */
> dt_add_property(np, "ibm,lane-eq", leq, leq_size);
We're just copying the DT property so conversions shouldn't be needed.
> }
> if (dt_has_node_property(stk_node, "ibm,capp-ucode", NULL)) {
> diff --git a/include/phb4.h b/include/phb4.h
> index 1c68ec2e2..ca701a311 100644
> --- a/include/phb4.h
> +++ b/include/phb4.h
> @@ -183,7 +183,7 @@ struct phb4 {
> uint64_t creset_start_time;
>
> /* SkiBoot owned in-memory tables */
> - uint16_t *tbl_rtt;
> + __be16 *tbl_rtt;
> uint8_t *tbl_peltv;
> uint64_t tbl_peltv_size;
> uint64_t tbl_pest;
More information about the Skiboot
mailing list