Re: [PATCH] ppc/pnv: Set P10 core xscom region size to match hardware

[Cédric Le Goater]

On 7/6/23 04:33, Joel Stanley wrote:
> On Wed, 5 Jul 2023 at 10:02, Cédric Le Goater <clg@kaod.org> wrote:
> > On 7/5/23 04:05, Joel Stanley wrote:
> > > On Wed, 5 Jul 2023 at 01:27, Nicholas Piggin <npiggin@gmail.com> wrote:
> > > > The P10 core xscom memory regions overlap because the size is wrong.
> > > > The P10 core+L2 xscom region size is allocated as 0x1000 (with some
> > > > unused ranges). "EC" is used as a closer match, as "EX" includes L3
> > > > which has a disjoint xscom range that would require a different
> > > > region if it were implemented.
> > > >
> > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > >
> > > Nice, that looks better:
> > >
> > > 0000000100000000-00000001000fffff (prio 0, i/o): xscom-quad.0: 0x100000
> > > 0000000100108000-000000010010ffff (prio 0, i/o): xscom-core.3: 0x8000
> > > 0000000100110000-0000000100117fff (prio 0, i/o): xscom-core.2: 0x8000
> > > 0000000100120000-0000000100127fff (prio 0, i/o): xscom-core.1: 0x8000
> > > 0000000100140000-0000000100147fff (prio 0, i/o): xscom-core.0: 0x8000
> > > 0000000108000000-00000001080fffff (prio 0, i/o): xscom-quad.4: 0x100000
> > > 0000000108108000-000000010810ffff (prio 0, i/o): xscom-core.7: 0x8000
> > > 0000000108110000-0000000108117fff (prio 0, i/o): xscom-core.6: 0x8000
> > > 0000000108120000-0000000108127fff (prio 0, i/o): xscom-core.5: 0x8000
> > > 0000000108140000-0000000108147fff (prio 0, i/o): xscom-core.4: 0x8000
> > >
> > > Reviewed-by: Joel Stanley <joel@jms.id.au>
> >
> > It'd interesting to add some dummy SLW handlers to get rid of the
> > XSCOM errors at boot and shutdown on P10 :
> >
> > [ 4824.393446266,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0
> > [ 4824.393588777,5] Unable to log error
> > [ 4824.393650582,3] XSCOM: Write failed, ret =  -6
> > [ 4824.394124623,3] Could not set special wakeup on 0:0: Unable to write 
> > QME_SPWU_HYP.
> > [ 4824.394368459,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0
> > [ 4824.394382007,5] Unable to log error
> > [ 4824.394384603,3] XSCOM: Write failed, ret =  -6
>
> Yes. I was looking at this yesterday. We need to figure out how to do
> the xscom addressing for the QME. It sets (different) bits in order to
> address a given core.>
>
> For a -smp 4 machine, the P10_QME_SPWU_HYP read comes in on these addresses:
>
>      case 0x200e883c:
>      case 0x200e483c:
>      case 0x200e283c:
>      case 0x200e183c:
>
> ie, the fourth nibble selects the core.
>
> For a -smp 8 machine, the address now has bit 24 set to select the
> second quad, so we need to cover these addresses:
>
>      case 0x210e883c:
>      case 0x210e483c:
>      case 0x210e283c:
>      case 0x210e183c:
>
> I am thinking about how to map this into an address range that a model
> can claim.
 
OPAL should have the magic macros for XSCOM addresses. I included a few
in QEMU.

> Cheers,
>
> Joel
>
> PS. For reference, this is sufficient to silence xscom errors with
> skiboot and -M powernv10 -smp4. A different set of hacks is required
> for p9.

Thanks,

C.


> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -106,6 +106,26 @@ static uint64_t xscom_read_default(PnvChip *chip,
> uint32_t pcba)
>       case 0x401082a:
>       case 0x4010828:
>           return 0;
> +
> +    /* P10_QME_SPWU_HYP */
> +    case 0x200e883c:
> +    case 0x200e483c:
> +    case 0x200e283c:
> +    case 0x200e183c:
> +        return 0;
> +
> +    /* P10_QME_SSH_HYP */
> +    case 0x200e882c:
> +    case 0x200e482c:
> +    case 0x200e282c:
> +    case 0x200e182c:
> +        return 0;
> +
> +    /* XPEC_P10_PCI_CPLT_CONF1 */
> +    case 0x08000009:
> +    case 0x09000009:
> +        return 0;
> +
>       default:
>           return -1;
>       }
> @@ -152,6 +172,13 @@ static bool xscom_write_default(PnvChip *chip,
> uint32_t pcba, uint64_t val)
>       case PRD_P8_IPOLL_REG_STATUS:
>       case PRD_P9_IPOLL_REG_MASK:
>       case PRD_P9_IPOLL_REG_STATUS:
> +
> +    /* P10_QME_SPWU_HYP */
> +    case 0x200e883c:
> +    case 0x200e483c:
> +    case 0x200e283c:
> +    case 0x200e183c:
> +
>           return true;
>       default:
>           return false;