| Message ID | 20170322025945.18182-4-mikey@neuling.org |
|---|---|
| State | Superseded |
| Headers | show |
On 03/22/2017 03:59 AM, Michael Neuling wrote: > Add code to perform indirect form 1 scoms. > > POWER8 does form 0 only. POWER9 adds form 1. The form is determined > from the address only. Hardware only allows writes for form 1. > > Only hostboot uses these scoms during IPL, so they are unused by > skiboot currently. > > Signed-off-by: Michael Neuling <mikey@neuling.org> > --- > hw/xscom.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 46 insertions(+), 2 deletions(-) > > diff --git a/hw/xscom.c b/hw/xscom.c > index 1add658776..7f3e329fee 100644 > --- a/hw/xscom.c > +++ b/hw/xscom.c > @@ -317,7 +317,8 @@ static int __xscom_write(uint32_t gcid, uint32_t pcb_addr, uint64_t val) > /* > * Indirect XSCOM access functions > */ > -static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t *val) > +static int xscom_indirect_read_form0(uint32_t gcid, uint64_t pcb_addr, > + uint64_t *val) > { > uint32_t addr; > uint64_t data; > @@ -360,7 +361,18 @@ static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t *val) > return rc; > } > > -static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t val) > +static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t *val) > +{ > + uint64_t form = (pcb_addr >> 60) & 1; > + > + if ((proc_gen == proc_gen_p9) && (form == 1)) > + return OPAL_UNSUPPORTED; > + > + return xscom_indirect_read_form0(gcid, pcb_addr, val); > +} > + > +static int xscom_indirect_write_form0(uint32_t gcid, uint64_t pcb_addr, > + uint64_t val) > { > uint32_t addr; > uint64_t data; > @@ -402,6 +414,38 @@ static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t val) > return rc; > } > > +static int xscom_indirect_write_form1(uint32_t gcid, uint64_t pcb_addr, > + uint64_t val) > +{ > + uint32_t addr; > + uint64_t data; > + int rc; > + > + if (proc_gen < proc_gen_p9) > + return OPAL_UNSUPPORTED; > + if (val & 0xfff0000000000000ULL) > + return OPAL_PARAMETER; > + > + /* Mangle address and data for form1 */ > + addr = (pcb_addr & 0x000ffffffff); > + data = (pcb_addr & 0xfff00000000) << 20; some bitmask definitions would help. > + data |= val; > + rc = __xscom_write(gcid, addr, data); > + if (rc) > + val = (uint64_t)-1; who cares about val ? > + return rc; > +} > + > +static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t val) > +{ > + uint64_t form = (pcb_addr >> 60) & 1; maybe use a helper ? Cheers, C. > + > + if ((proc_gen == proc_gen_p9) && (form == 1)) > + return xscom_indirect_write_form1(gcid, pcb_addr, val); > + > + return xscom_indirect_write_form0(gcid, pcb_addr, val); > +} > + > static uint32_t xscom_decode_chiplet(uint32_t partid, uint64_t *pcb_addr) > { > uint32_t gcid = (partid & 0x0fffffff) >> 4; >
On Wed, 2017-03-22 at 16:29 +0100, Cédric Le Goater wrote: > > +static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, > > uint64_t val) > > +{ > > + uint64_t form = (pcb_addr >> 60) & 1; > > maybe use a helper ? At least use PPC_BIT definitions :) Cheers, Ben.
On Wed, 2017-03-22 at 16:29 +0100, Cédric Le Goater wrote: > On 03/22/2017 03:59 AM, Michael Neuling wrote: > > Add code to perform indirect form 1 scoms. > > > > POWER8 does form 0 only. POWER9 adds form 1. The form is determined > > from the address only. Hardware only allows writes for form 1. > > > > Only hostboot uses these scoms during IPL, so they are unused by > > skiboot currently. > > > > Signed-off-by: Michael Neuling <mikey@neuling.org> > > --- > > hw/xscom.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 46 insertions(+), 2 deletions(-) > > > > diff --git a/hw/xscom.c b/hw/xscom.c > > index 1add658776..7f3e329fee 100644 > > --- a/hw/xscom.c > > +++ b/hw/xscom.c > > @@ -317,7 +317,8 @@ static int __xscom_write(uint32_t gcid, uint32_t > > pcb_addr, uint64_t val) > > /* > > * Indirect XSCOM access functions > > */ > > -static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t > > *val) > > +static int xscom_indirect_read_form0(uint32_t gcid, uint64_t pcb_addr, > > + uint64_t *val) > > { > > uint32_t addr; > > uint64_t data; > > @@ -360,7 +361,18 @@ static int xscom_indirect_read(uint32_t gcid, uint64_t > > pcb_addr, uint64_t *val) > > return rc; > > } > > > > -static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t > > val) > > +static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t > > *val) > > +{ > > + uint64_t form = (pcb_addr >> 60) & 1; > > + > > + if ((proc_gen == proc_gen_p9) && (form == 1)) > > + return OPAL_UNSUPPORTED; > > + > > + return xscom_indirect_read_form0(gcid, pcb_addr, val); > > +} > > + > > +static int xscom_indirect_write_form0(uint32_t gcid, uint64_t pcb_addr, > > + uint64_t val) > > { > > uint32_t addr; > > uint64_t data; > > @@ -402,6 +414,38 @@ static int xscom_indirect_write(uint32_t gcid, uint64_t > > pcb_addr, uint64_t val) > > return rc; > > } > > > > +static int xscom_indirect_write_form1(uint32_t gcid, uint64_t pcb_addr, > > + uint64_t val) > > +{ > > + uint32_t addr; > > + uint64_t data; > > + int rc; > > + > > + if (proc_gen < proc_gen_p9) > > + return OPAL_UNSUPPORTED; > > + if (val & 0xfff0000000000000ULL) > > + return OPAL_PARAMETER; > > + > > + /* Mangle address and data for form1 */ > > + addr = (pcb_addr & 0x000ffffffff); > > + data = (pcb_addr & 0xfff00000000) << 20; > > some bitmask definitions would help. I added one for the val check but... I'm leaving the pcb_addr inline mangling as IMHO it's easier to read as is. It clearly shows how the pcb_addr is split and placed in addr and data. Abstracting it with bit defs will make it harder to see what's happening. > > + data |= val; > > + rc = __xscom_write(gcid, addr, data); > > + if (rc) > > + val = (uint64_t)-1; > > who cares about val ? Good point. I'll remove > > + return rc; > > +} > > + > > +static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t > > val) > > +{ > > + uint64_t form = (pcb_addr >> 60) & 1; > > maybe use a helper ? OK, I'll add. Thanks Mikey
diff --git a/hw/xscom.c b/hw/xscom.c index 1add658776..7f3e329fee 100644 --- a/hw/xscom.c +++ b/hw/xscom.c @@ -317,7 +317,8 @@ static int __xscom_write(uint32_t gcid, uint32_t pcb_addr, uint64_t val) /* * Indirect XSCOM access functions */ -static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t *val) +static int xscom_indirect_read_form0(uint32_t gcid, uint64_t pcb_addr, + uint64_t *val) { uint32_t addr; uint64_t data; @@ -360,7 +361,18 @@ static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t *val) return rc; } -static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t val) +static int xscom_indirect_read(uint32_t gcid, uint64_t pcb_addr, uint64_t *val) +{ + uint64_t form = (pcb_addr >> 60) & 1; + + if ((proc_gen == proc_gen_p9) && (form == 1)) + return OPAL_UNSUPPORTED; + + return xscom_indirect_read_form0(gcid, pcb_addr, val); +} + +static int xscom_indirect_write_form0(uint32_t gcid, uint64_t pcb_addr, + uint64_t val) { uint32_t addr; uint64_t data; @@ -402,6 +414,38 @@ static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t val) return rc; } +static int xscom_indirect_write_form1(uint32_t gcid, uint64_t pcb_addr, + uint64_t val) +{ + uint32_t addr; + uint64_t data; + int rc; + + if (proc_gen < proc_gen_p9) + return OPAL_UNSUPPORTED; + if (val & 0xfff0000000000000ULL) + return OPAL_PARAMETER; + + /* Mangle address and data for form1 */ + addr = (pcb_addr & 0x000ffffffff); + data = (pcb_addr & 0xfff00000000) << 20; + data |= val; + rc = __xscom_write(gcid, addr, data); + if (rc) + val = (uint64_t)-1; + return rc; +} + +static int xscom_indirect_write(uint32_t gcid, uint64_t pcb_addr, uint64_t val) +{ + uint64_t form = (pcb_addr >> 60) & 1; + + if ((proc_gen == proc_gen_p9) && (form == 1)) + return xscom_indirect_write_form1(gcid, pcb_addr, val); + + return xscom_indirect_write_form0(gcid, pcb_addr, val); +} + static uint32_t xscom_decode_chiplet(uint32_t partid, uint64_t *pcb_addr) { uint32_t gcid = (partid & 0x0fffffff) >> 4;
Add code to perform indirect form 1 scoms. POWER8 does form 0 only. POWER9 adds form 1. The form is determined from the address only. Hardware only allows writes for form 1. Only hostboot uses these scoms during IPL, so they are unused by skiboot currently. Signed-off-by: Michael Neuling <mikey@neuling.org> --- hw/xscom.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)