Message ID | 20240401055503.1880587-1-adityag@linux.ibm.com |
---|---|
Headers | show |
Series | P11 support for QEMU | expand |
Hello Aditya, Please run ./scripts/get_maintainer.pl when sending a series. qemu-ppc should be in Cc: Briefly looking at this, please separate the changes using one patch per model, that is : first CPU (target), LPC, OCC, PSI, SBE, PnvCore, SpaprCore. Last the PnvChip and the machines, powernv11 and pseries. A minimum commit log describing the HW is required. I don't see PHB6 or XIVE3. Why ? Also, you will need an OPAL update. The above changes are pointless without it. The minimum for now is a git commit from the opal repo, then you will need to update QEMU with a binary. Thanks, C. On 4/1/24 07:55, Aditya Gupta wrote: > This patch series adds support for Power11 pseries and powernv machine targets > to emulate VMs running on Power11. > > Most of the P11 support code has been taken from P10 code in QEMU. > And has been tested in pseries, powernv, with and without compat mode. > > Git Tree for Testing: https://github.com/adi-g15-ibm/qemu/tree/p11 > > Aditya Gupta (2): > ppc: pseries: add P11 cpu type > ppc: powernv11: add base support for P11 PowerNV > > docs/system/ppc/pseries.rst | 6 +- > hw/ppc/pnv.c | 409 ++++++++++++++++++++++++++++++++++++ > hw/ppc/pnv_core.c | 94 +++++++++ > hw/ppc/pnv_homer.c | 64 ++++++ > hw/ppc/pnv_lpc.c | 14 ++ > hw/ppc/pnv_occ.c | 14 ++ > hw/ppc/pnv_psi.c | 21 ++ > hw/ppc/pnv_sbe.c | 19 ++ > hw/ppc/spapr_cpu_core.c | 1 + > include/hw/ppc/pnv.h | 51 +++++ > include/hw/ppc/pnv_chip.h | 30 +++ > include/hw/ppc/pnv_homer.h | 3 + > include/hw/ppc/pnv_lpc.h | 4 + > include/hw/ppc/pnv_occ.h | 2 + > include/hw/ppc/pnv_psi.h | 2 + > include/hw/ppc/pnv_sbe.h | 2 + > include/hw/ppc/pnv_xscom.h | 55 +++++ > target/ppc/compat.c | 7 + > target/ppc/cpu-models.c | 2 + > target/ppc/cpu-models.h | 2 + > target/ppc/cpu_init.c | 162 ++++++++++++++ > 21 files changed, 961 insertions(+), 3 deletions(-) >
Hello Cédric, Thanks for reviewing this. On Mon, Apr 01, 2024 at 10:25:31AM +0200, Cédric Le Goater wrote: > Hello Aditya, > > Please run ./scripts/get_maintainer.pl when sending a series. qemu-ppc should be > in Cc: Tried it now, For some reason, get_maintainer.pl shows no maintainers: $ ./scripts/get_maintainer.pl -f 0002-ppc-powernv11-add-base-support-for-P11-PowerNV.patch get_maintainer.pl: No maintainers found, printing recent contributors. get_maintainer.pl: Do not blindly cc: them on patches! Use common sense. qemu-devel@nongnu.org (open list:All patches CC here) I checked the MAINTAINERS file, will add maintainers in Cc, thanks. > > Briefly looking at this, please separate the changes using one patch per model, > that is : first CPU (target), LPC, OCC, PSI, SBE, PnvCore, SpaprCore. Last the > PnvChip and the machines, powernv11 and pseries. A minimum commit log describing > the HW is required. Sure, I will split the changes and improve my commit descriptions. > I don't see PHB6 or XIVE3. Why ? Power11 core is same as Power10, so it supports till PHB5 and XIVE2, same as P10. That's why I have not added any code for them. > > Also, you will need an OPAL update. The above changes are pointless without it. > The minimum for now is a git commit from the opal repo, then you will need to > update QEMU with a binary. Agreed. I will consult when we push it to public. Will update this in next series. There might be some days delay in the next patch series. Thanks > > Thanks, > > C. > > On 4/1/24 07:55, Aditya Gupta wrote: > > This patch series adds support for Power11 pseries and powernv machine targets > > to emulate VMs running on Power11. > > > > Most of the P11 support code has been taken from P10 code in QEMU. > > And has been tested in pseries, powernv, with and without compat mode. > > > > Git Tree for Testing: https://github.com/adi-g15-ibm/qemu/tree/p11 > > > > Aditya Gupta (2): > > ppc: pseries: add P11 cpu type > > ppc: powernv11: add base support for P11 PowerNV > > > > docs/system/ppc/pseries.rst | 6 +- > > hw/ppc/pnv.c | 409 ++++++++++++++++++++++++++++++++++++ > > hw/ppc/pnv_core.c | 94 +++++++++ > > hw/ppc/pnv_homer.c | 64 ++++++ > > hw/ppc/pnv_lpc.c | 14 ++ > > hw/ppc/pnv_occ.c | 14 ++ > > hw/ppc/pnv_psi.c | 21 ++ > > hw/ppc/pnv_sbe.c | 19 ++ > > hw/ppc/spapr_cpu_core.c | 1 + > > include/hw/ppc/pnv.h | 51 +++++ > > include/hw/ppc/pnv_chip.h | 30 +++ > > include/hw/ppc/pnv_homer.h | 3 + > > include/hw/ppc/pnv_lpc.h | 4 + > > include/hw/ppc/pnv_occ.h | 2 + > > include/hw/ppc/pnv_psi.h | 2 + > > include/hw/ppc/pnv_sbe.h | 2 + > > include/hw/ppc/pnv_xscom.h | 55 +++++ > > target/ppc/compat.c | 7 + > > target/ppc/cpu-models.c | 2 + > > target/ppc/cpu-models.h | 2 + > > target/ppc/cpu_init.c | 162 ++++++++++++++ > > 21 files changed, 961 insertions(+), 3 deletions(-) > > >
Hello Aditya, On 4/2/24 08:39, Aditya Gupta wrote: > Hello Cédric, > > Thanks for reviewing this. > > On Mon, Apr 01, 2024 at 10:25:31AM +0200, Cédric Le Goater wrote: >> Hello Aditya, >> >> Please run ./scripts/get_maintainer.pl when sending a series. qemu-ppc should be >> in Cc: > > Tried it now, For some reason, get_maintainer.pl shows no maintainers: > > $ ./scripts/get_maintainer.pl -f 0002-ppc-powernv11-add-base-support-for-P11-PowerNV.patch > get_maintainer.pl: No maintainers found, printing recent contributors. > get_maintainer.pl: Do not blindly cc: them on patches! Use common sense. > > qemu-devel@nongnu.org (open list:All patches CC here) Weird. I downloaded your series with b4 and ran the get_maintainer.pl script : $ ./scripts/get_maintainer.pl 20240401_adityag_p11_support_for_qemu.patches/0001_ppc_pseries_add_p11_cpu_type.patch 20240401_adityag_p11_support_for_qemu.patches/0002_ppc_powernv11_add_base_support_for_p11_powernv.patch Nicholas Piggin <npiggin@gmail.com> (odd fixer:sPAPR (pseries)) Daniel Henrique Barboza <danielhb413@gmail.com> (reviewer:sPAPR (pseries)) David Gibson <david@gibson.dropbear.id.au> (reviewer:sPAPR (pseries)) Harsh Prateek Bora <harshpb@linux.ibm.com> (reviewer:sPAPR (pseries)) "Cédric Le Goater" <clg@kaod.org> (odd fixer:PowerNV Non-Virt...) "Frédéric Barrat" <fbarrat@linux.ibm.com> (reviewer:PowerNV Non-Virt...) qemu-ppc@nongnu.org (open list:sPAPR (pseries)) qemu-devel@nongnu.org (open list:All patches CC here) > I checked the MAINTAINERS file, will add maintainers in Cc, thanks. > >> >> Briefly looking at this, please separate the changes using one patch per model, >> that is : first CPU (target), LPC, OCC, PSI, SBE, PnvCore, SpaprCore. Last the >> PnvChip and the machines, powernv11 and pseries. A minimum commit log describing >> the HW is required. > > Sure, I will split the changes and improve my commit descriptions. > >> I don't see PHB6 or XIVE3. Why ? > > Power11 core is same as Power10, so it supports till PHB5 and XIVE2, > same as P10. That's why I have not added any code for them. ok. That's typically the info the commit log should have. >> Also, you will need an OPAL update. The above changes are pointless without it. >> The minimum for now is a git commit from the opal repo, then you will need to >> update QEMU with a binary. > > Agreed. I will consult when we push it to public. Will update this in > next series. > > There might be some days delay in the next patch series. We have entered the QEMU 9.1 cycle. There is time. I will comment more the next respin. Thanks, C.
Hello Cédric, > > > <...snip...> > > > > > > Please run ./scripts/get_maintainer.pl when sending a series. qemu-ppc should be > > > in Cc: > > > > Tried it now, For some reason, get_maintainer.pl shows no maintainers: > > > > $ ./scripts/get_maintainer.pl -f 0002-ppc-powernv11-add-base-support-for-P11-PowerNV.patch > > get_maintainer.pl: No maintainers found, printing recent contributors. > > get_maintainer.pl: Do not blindly cc: them on patches! Use common sense. > > qemu-devel@nongnu.org (open list:All patches CC here) > > Weird. I downloaded your series with b4 and ran the get_maintainer.pl script : > > $ ./scripts/get_maintainer.pl 20240401_adityag_p11_support_for_qemu.patches/0001_ppc_pseries_add_p11_cpu_type.patch 20240401_adityag_p11_support_for_qemu.patches/0002_ppc_powernv11_add_base_support_for_p11_powernv.patch > > Nicholas Piggin <npiggin@gmail.com> (odd fixer:sPAPR (pseries)) > Daniel Henrique Barboza <danielhb413@gmail.com> (reviewer:sPAPR (pseries)) > David Gibson <david@gibson.dropbear.id.au> (reviewer:sPAPR (pseries)) > Harsh Prateek Bora <harshpb@linux.ibm.com> (reviewer:sPAPR (pseries)) > "Cédric Le Goater" <clg@kaod.org> (odd fixer:PowerNV Non-Virt...) > "Frédéric Barrat" <fbarrat@linux.ibm.com> (reviewer:PowerNV Non-Virt...) > qemu-ppc@nongnu.org (open list:sPAPR (pseries)) > qemu-devel@nongnu.org (open list:All patches CC here) So, it should have worked, I will check if I can get it to work. > > > I checked the MAINTAINERS file, will add maintainers in Cc, thanks. > > > > > > > > Briefly looking at this, please separate the changes using one patch per model, > > > that is : first CPU (target), LPC, OCC, PSI, SBE, PnvCore, SpaprCore. Last the > > > PnvChip and the machines, powernv11 and pseries. A minimum commit log describing > > > the HW is required. > > > > Sure, I will split the changes and improve my commit descriptions. > > > > > I don't see PHB6 or XIVE3. Why ? > > > > Power11 core is same as Power10, so it supports till PHB5 and XIVE2, > > same as P10. That's why I have not added any code for them. > > ok. That's typically the info the commit log should have. Okay, I will add these details also. > > > > Also, you will need an OPAL update. The above changes are pointless without it. > > > The minimum for now is a git commit from the opal repo, then you will need to > > > update QEMU with a binary. > > > > Agreed. I will consult when we push it to public. Will update this in > > next series. > > > > There might be some days delay in the next patch series. > > We have entered the QEMU 9.1 cycle. There is time. I will comment more > the next respin. Thanks Cédric - Aditya Gupta > > Thanks, > > C. >
On 4/2/24 09:00, Aditya Gupta wrote: > Hello Cédric, > >>>> <...snip...> >>>> >>>> Please run ./scripts/get_maintainer.pl when sending a series. qemu-ppc should be >>>> in Cc: >>> >>> Tried it now, For some reason, get_maintainer.pl shows no maintainers: >>> >>> $ ./scripts/get_maintainer.pl -f 0002-ppc-powernv11-add-base-support-for-P11-PowerNV.patch >>> get_maintainer.pl: No maintainers found, printing recent contributors. >>> get_maintainer.pl: Do not blindly cc: them on patches! Use common sense. >>> qemu-devel@nongnu.org (open list:All patches CC here) >> >> Weird. I downloaded your series with b4 and ran the get_maintainer.pl script : >> >> $ ./scripts/get_maintainer.pl 20240401_adityag_p11_support_for_qemu.patches/0001_ppc_pseries_add_p11_cpu_type.patch 20240401_adityag_p11_support_for_qemu.patches/0002_ppc_powernv11_add_base_support_for_p11_powernv.patch >> >> Nicholas Piggin <npiggin@gmail.com> (odd fixer:sPAPR (pseries)) >> Daniel Henrique Barboza <danielhb413@gmail.com> (reviewer:sPAPR (pseries)) >> David Gibson <david@gibson.dropbear.id.au> (reviewer:sPAPR (pseries)) >> Harsh Prateek Bora <harshpb@linux.ibm.com> (reviewer:sPAPR (pseries)) >> "Cédric Le Goater" <clg@kaod.org> (odd fixer:PowerNV Non-Virt...) >> "Frédéric Barrat" <fbarrat@linux.ibm.com> (reviewer:PowerNV Non-Virt...) >> qemu-ppc@nongnu.org (open list:sPAPR (pseries)) >> qemu-devel@nongnu.org (open list:All patches CC here) > > So, it should have worked, I will check if I can get it to work. > >> >>> I checked the MAINTAINERS file, will add maintainers in Cc, thanks. >>> >>>> >>>> Briefly looking at this, please separate the changes using one patch per model, >>>> that is : first CPU (target), LPC, OCC, PSI, SBE, PnvCore, SpaprCore. Last the >>>> PnvChip and the machines, powernv11 and pseries. A minimum commit log describing >>>> the HW is required. >>> >>> Sure, I will split the changes and improve my commit descriptions. >>> >>>> I don't see PHB6 or XIVE3. Why ? >>> >>> Power11 core is same as Power10, so it supports till PHB5 and XIVE2, >>> same as P10. That's why I have not added any code for them. >> >> ok. That's typically the info the commit log should have. > > Okay, I will add these details also. Forgot to add, please update : docs/system/ppc/powernv.rst and the relevant tests under tests/qtest and tests/avocado. It helps maintenance and CI. Make sure the code is based on the HEAD of the QEMU tree. This proposal isn't and so does not compile. Thanks, C.
On Mon Apr 1, 2024 at 3:55 PM AEST, Aditya Gupta wrote: > This patch series adds support for Power11 pseries and powernv machine targets > to emulate VMs running on Power11. > > Most of the P11 support code has been taken from P10 code in QEMU. > And has been tested in pseries, powernv, with and without compat mode. Thanks for this. I wonder if we could try to get rid of some of the code / structure duplication for creating a new machine. I don't want to add a bunch of CPP generator macros or squash too much code together with lots of flags, but maybe there's something we can do. Since this is a very small change from P10, it might be a good time to work out some refactoring. Even a hw/ppc/pnv_powernv10.c and hw/ppc/pnv_powernv11.c, and target/ppc/cpu_init_power10.c and cpu_init_power11.c might be an improvement because you could easily diff them (hopefully we could do better than that, but just a thought). Thanks, Nick > > Git Tree for Testing: https://github.com/adi-g15-ibm/qemu/tree/p11 > > Aditya Gupta (2): > ppc: pseries: add P11 cpu type > ppc: powernv11: add base support for P11 PowerNV