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

[Cédric Le Goater]

On 21/11/2019 10:11, Balamuruhan S wrote:
> 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.

yes. 

>> 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.


See Joel's patch which has a simple example :  

   patchwork.ozlabs.org/patch/1196519

It simply generates object properties : 


+    for (led = 0; led < s->nr_leds; led++) {
+        char *name;
+
+        name = g_strdup_printf("led%d", led);
+        object_property_add(obj, name, "bool", pca9552_get_led, 
pca9552_set_led,
+                            NULL, NULL, NULL);
+    }

with defined get and set accessors. 

We could do the same for the OCC sensors with a table describing the 
sensor layout. Accessors would just simply update the table. we could
even trigger the OCC interrupt if needed.

This is the initial table :

  https://github.com/open-power/occ/blob/master/src/occ_405/sensor/sensor_info.c

Linux should be able to grab the values through hwmon just as on real HW.
This is the case today for the DTS.

>>
>> 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.

Yes. I suppose there is some chip id in the powerbus message.

C.

  
> 
> 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(-)
>>>
>>
>