Re: [PATCH 0/5] ppc/pnv: fix Homer/Occ mappings on multichip systems

[Balamuruhan S]

On Wed, Nov 20, 2019 at 08:46:30AM +0100, Cédric Le Goater wrote:
> Hello,
> 
> On 19/11/2019 18:50, Balamuruhan S wrote:
> > Hi All,
> > 
> > PowerNV fails to boot in multichip systems due to some misinterpretation
> > and mapping in Homer/Occ device models, this patchset fixes the
> > following,
> > 
> >  - Homer size is 4MB per chip and Occ common area size is 8MB
> >  - Bar masks are used to calculate sizes of Homer/Occ in skiboot so
> >    return appropriate value
> >  - Occ common area is in BAR 3 on Power8 but wrongly mapped to BAR 2
> >    currently
> >  - OCC common area is shared across chips and should be mapped only once
> >    for multichip systems
> 
> The first thing to address is the HOMER XSCOM region. 
> 
> Introduce an empty skeleton for P8 and P9 with different mem ops handers
> because the registers have a different layout. From there, add the support
> for the different PBA* regs and move them out from the default XSCOM
> handlers. That should fix most of the current problems and it will provide 
> a nice framework for future extensions.

sure, I will work on it.

> 
> Why not add the associated HOMER MMIO region while we are it ? the PBA
> registers have all the definitions we need and it will gives us access
> to the pstate table.

so, idea is to have HOMER MMIO for us to use it accessing pstate table / data
and HOMER XSCOM for homer associated xscom access for PBA* registers to
P8 and P9 respectively.

> 
> 
> Second is the OCC region. Do we need a XSCOM *or* a MMIO region ? This is 
> not clear. Please check skiboot. I think a MMIO region should be enough
> because this is how sensor data from the OCC is accessed.

Okay, I will do the change for OCC to use MMIO, and will check skiboot
for making it better.

> 
> On that topic, we could define properties on the PnvOCC model for each 
> sensor and tune the value from the QEMU monitor. It really shouldn't be
> too complex.

How can we tune value from QEMU monitor ? This is new to me and will
need to check it. I remember you have advised this with the error
injection framework patches and Rashmica's patch that provides way to
use Qemu monitor to feed data, but I need to do some study.

> 
> Also the same address is used, we should only map it once but we need 
> to invent something to know from which chip it is accessed.

This is something need to check how real hardware handles it while
accessing shared occ region from different chip and think how to make it
for us.

Thanks a lot!

-- Bala

> 
> 
> C.
> 
> 
> > 
> > Request for your review and suggestions to make it better. I would like to
> > thank Cedric for his time and help to figure out the issues.
> >
> > Balamuruhan S (5):
> >   hw/ppc/pnv: incorrect homer and occ common area size
> >   hw/ppc/pnv_xscom: PBA bar mask values are incorrect with homer/occ
> >     sizes
> >   hw/ppc/pnv_xscom: Power8 occ common area is in PBA BAR 3
> >   hw/ppc/pnv_xscom: occ common area to be mapped only once
> >   hw/ppc/pnv_xscom: add PBA BARs for Power8 slw image
> > 
> >  hw/ppc/pnv_occ.c     |  2 +-
> >  hw/ppc/pnv_xscom.c   | 37 +++++++++++++++++++++++++++----------
> >  include/hw/ppc/pnv.h | 12 ++++++++----
> >  3 files changed, 36 insertions(+), 15 deletions(-)
> > 
>