[Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV

Balamuruhan S posted 3 patches 4 years, 7 months ago
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test checkpatch passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190910071019.16689-1-bala24@linux.ibm.com
Maintainers: "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
hw/ppc/Makefile.objs       |   1 +
hw/ppc/pnv.c               |  87 ++++++++++++---
hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
hw/ppc/pnv_occ.c           |  78 ++++++++++++++
hw/ppc/pnv_xscom.c         |  34 +++++-
include/hw/ppc/pnv.h       |  21 ++++
include/hw/ppc/pnv_homer.h |  52 +++++++++
include/hw/ppc/pnv_occ.h   |   3 +
8 files changed, 513 insertions(+), 21 deletions(-)
create mode 100644 hw/ppc/pnv_homer.c
create mode 100644 include/hw/ppc/pnv_homer.h
[Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV
Posted by Balamuruhan S 4 years, 7 months ago
Hi All,

This is follow-up patch that implements HOMER and OCC SRAM device
models to emulate homer memory and occ common area access for pstate
table, occ sensors, runtime data and slw.

This version addresses review comments in previous patchset and
breaks it to have separate patch series for Homer and OCC emulation,

https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00979.html

currently skiboot disables the homer/occ code path with `QUIRK_NO_PBA`,
this quirk have to be removed in skiboot for it to use HOMER and OCC
SRAM device models along with few bug fixes,

https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0
https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173

changes from v1:
    * reuse PnvOCC device model to implement SRAM device.
    * implement PnvHomer as separate device model.
    * have core max base address as part of PnvHOMERClass.
    * reuse PNV_CHIP_INDEX() instead of introducing new `chip_num`.
    * define all the memory ops access address as macros.
    * few coding style warnings given by checkpatch.pl.

I request for review, comments and suggestions for the changes.

Balamuruhan S (3):
  hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
  hw/ppc/pnv_occ: add sram device model for occ common area
  hw/ppc/pnv_homer: add PowerNV homer device model

 hw/ppc/Makefile.objs       |   1 +
 hw/ppc/pnv.c               |  87 ++++++++++++---
 hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_occ.c           |  78 ++++++++++++++
 hw/ppc/pnv_xscom.c         |  34 +++++-
 include/hw/ppc/pnv.h       |  21 ++++
 include/hw/ppc/pnv_homer.h |  52 +++++++++
 include/hw/ppc/pnv_occ.h   |   3 +
 8 files changed, 513 insertions(+), 21 deletions(-)
 create mode 100644 hw/ppc/pnv_homer.c
 create mode 100644 include/hw/ppc/pnv_homer.h

-- 
2.14.5


Re: [Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV
Posted by Cédric Le Goater 4 years, 7 months ago
On 10/09/2019 09:10, Balamuruhan S wrote:
> Hi All,
> 
> This is follow-up patch that implements HOMER and OCC SRAM device
> models to emulate homer memory and occ common area access for pstate
> table, occ sensors, runtime data and slw.
> 
> This version addresses review comments in previous patchset and
> breaks it to have separate patch series for Homer and OCC emulation,
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00979.html
> 
> currently skiboot disables the homer/occ code path with `QUIRK_NO_PBA`,
> this quirk have to be removed in skiboot for it to use HOMER and OCC
> SRAM device models along with few bug fixes,
> 
> https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
> https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0

Can't we generate the sensors in QEMU ? I am not sure what this
patch does. Is the Header Block invalid ? 

It would be good to generate properties to control their values 
on the monitor line, like Rashmica did for GPIO model in the 
Aspeed machine.

> https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173

This one is weird .

C. 

> 
> changes from v1:
>     * reuse PnvOCC device model to implement SRAM device.
>     * implement PnvHomer as separate device model.
>     * have core max base address as part of PnvHOMERClass.
>     * reuse PNV_CHIP_INDEX() instead of introducing new `chip_num`.
>     * define all the memory ops access address as macros.
>     * few coding style warnings given by checkpatch.pl.
> 
> I request for review, comments and suggestions for the changes.
> 
> Balamuruhan S (3):
>   hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
>   hw/ppc/pnv_occ: add sram device model for occ common area
>   hw/ppc/pnv_homer: add PowerNV homer device model
> 
>  hw/ppc/Makefile.objs       |   1 +
>  hw/ppc/pnv.c               |  87 ++++++++++++---
>  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/pnv_occ.c           |  78 ++++++++++++++
>  hw/ppc/pnv_xscom.c         |  34 +++++-
>  include/hw/ppc/pnv.h       |  21 ++++
>  include/hw/ppc/pnv_homer.h |  52 +++++++++
>  include/hw/ppc/pnv_occ.h   |   3 +
>  8 files changed, 513 insertions(+), 21 deletions(-)
>  create mode 100644 hw/ppc/pnv_homer.c
>  create mode 100644 include/hw/ppc/pnv_homer.h
> 


Re: [Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV
Posted by Balamuruhan S 4 years, 7 months ago
On Tue, Sep 10, 2019 at 01:45:55PM +0200, Cédric Le Goater wrote:
> On 10/09/2019 09:10, Balamuruhan S wrote:
> > Hi All,
> > 
> > This is follow-up patch that implements HOMER and OCC SRAM device
> > models to emulate homer memory and occ common area access for pstate
> > table, occ sensors, runtime data and slw.
> > 
> > This version addresses review comments in previous patchset and
> > breaks it to have separate patch series for Homer and OCC emulation,
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00979.html
> > 
> > currently skiboot disables the homer/occ code path with `QUIRK_NO_PBA`,
> > this quirk have to be removed in skiboot for it to use HOMER and OCC
> > SRAM device models along with few bug fixes,
> > 
> > https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
> > https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0
> 
> Can't we generate the sensors in QEMU ? I am not sure what this
> patch does. Is the Header Block invalid ? 

This doesn't directly affect Qemu, this is skiboot bug where it
creates device tree node for sensor-groups and does sensor
sanity check to initialize, but in negative scenario where there
is no sensors like in Qemu the sanity check fails but still device
tree populates the sensor-groups node wrongly. The cleanup is not
handled in skiboot and this patch does that.

> 
> It would be good to generate properties to control their values 
> on the monitor line, like Rashmica did for GPIO model in the 
> Aspeed machine.

> 
> > https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173
> 
> This one is weird .

I did a miss here, in skiboot there is check whether parsed pstate id
for pmin and pmax is valid or not. In this check, pmax to pmin for P8
it is 0 to -N and for P9 0 to N. But in Qemu for the MemoryRegionOps
structure read() callback function can have only uint64_t as return
type, so for P8 I got error from skiboot as we return postive value
and misunderstood to make this skiboot change. Cedric how can we handle
this from Qemu ?

Thanks for review,

-- Bala

> 
> C. 
> 
> > 
> > changes from v1:
> >     * reuse PnvOCC device model to implement SRAM device.
> >     * implement PnvHomer as separate device model.
> >     * have core max base address as part of PnvHOMERClass.
> >     * reuse PNV_CHIP_INDEX() instead of introducing new `chip_num`.
> >     * define all the memory ops access address as macros.
> >     * few coding style warnings given by checkpatch.pl.
> > 
> > I request for review, comments and suggestions for the changes.
> > 
> > Balamuruhan S (3):
> >   hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
> >   hw/ppc/pnv_occ: add sram device model for occ common area
> >   hw/ppc/pnv_homer: add PowerNV homer device model
> > 
> >  hw/ppc/Makefile.objs       |   1 +
> >  hw/ppc/pnv.c               |  87 ++++++++++++---
> >  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
> >  hw/ppc/pnv_occ.c           |  78 ++++++++++++++
> >  hw/ppc/pnv_xscom.c         |  34 +++++-
> >  include/hw/ppc/pnv.h       |  21 ++++
> >  include/hw/ppc/pnv_homer.h |  52 +++++++++
> >  include/hw/ppc/pnv_occ.h   |   3 +
> >  8 files changed, 513 insertions(+), 21 deletions(-)
> >  create mode 100644 hw/ppc/pnv_homer.c
> >  create mode 100644 include/hw/ppc/pnv_homer.h
> > 
>