| Message ID | 20210430184047.81653-1-lucas.araujo@eldorado.org.br (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/ppc: code motion to compile without TCG | expand |
Patchew URL: https://patchew.org/QEMU/20210430184047.81653-1-lucas.araujo@eldorado.org.br/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210430184047.81653-1-lucas.araujo@eldorado.org.br
Subject: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210430184047.81653-1-lucas.araujo@eldorado.org.br -> patchew/20210430184047.81653-1-lucas.araujo@eldorado.org.br
Switched to a new branch 'test'
1cd2718 hw/ppc: Moved TCG code to spapr_hcall_tcg
99b0292 target/ppc: Moved functions out of mmu-hash64
=== OUTPUT BEGIN ===
1/2 Checking commit 99b02924c4ea (target/ppc: Moved functions out of mmu-hash64)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#245:
new file mode 100644
total: 0 errors, 1 warnings, 214 lines checked
Patch 1/2 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/2 Checking commit 1cd27189f2ae (hw/ppc: Moved TCG code to spapr_hcall_tcg)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#378:
new file mode 100644
WARNING: Block comments use a leading /* on a separate line
#462: FILE: hw/ppc/spapr_hcall_tcg.c:80:
+ /* FIXME: What WIMG combinations could be sensible for IO?
WARNING: Block comments use a trailing */ on a separate line
#463: FILE: hw/ppc/spapr_hcall_tcg.c:81:
+ * For now we allow WIMG=010x, but are there others? */
ERROR: spaces required around that '*' (ctx:VxV)
#601: FILE: hw/ppc/spapr_hcall_tcg.c:219:
+ target_ulong *tsh = &args[i*2];
^
ERROR: spaces required around that '*' (ctx:VxV)
#602: FILE: hw/ppc/spapr_hcall_tcg.c:220:
+ target_ulong tsl = args[i*2 + 1];
^
total: 2 errors, 3 warnings, 673 lines checked
Patch 2/2 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210430184047.81653-1-lucas.araujo@eldorado.org.br/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
"Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br> writes: > After the feedback from v1 I reworked the patch with suggested ideas and > this version has less duplicated code and is overall simpler. > > This patch series is still a WIP, there are still 2 main problems I am > trying to solve, I'll mention them in their respective patches. > > The aim of these patches is to progress toward enabling disable-tcg on > PPC by solving errors in hw/ppc with that option. > > As a WIP comments are welcome. > > Lucas Mateus Castro (alqotel) (2): > target/ppc: Moved functions out of mmu-hash64 > hw/ppc: Moved TCG code to spapr_hcall_tcg > > hw/ppc/meson.build | 3 + > hw/ppc/spapr.c | 1 + > hw/ppc/spapr_caps.c | 1 + > hw/ppc/spapr_cpu_core.c | 1 + > hw/ppc/spapr_hcall.c | 301 ++-------------------------------- > hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++ > hw/ppc/spapr_rtas.c | 1 + > target/ppc/meson.build | 1 + > target/ppc/mmu-hash64.c | 81 +-------- > target/ppc/mmu-hash64.h | 6 - > target/ppc/mmu-misc.c | 86 ++++++++++ > target/ppc/mmu-misc.h | 22 +++ > 12 files changed, 478 insertions(+), 369 deletions(-) > create mode 100644 hw/ppc/spapr_hcall_tcg.c > create mode 100644 target/ppc/mmu-misc.c > create mode 100644 target/ppc/mmu-misc.h This is the list of hypercalls registered with spapr_register_hypercall and whether they are implemented by KVM HV, KVM PR or none. I also list whether the KVM hcall uses the QEMU implementation as a fallback. Maybe it will be helpful to this discussion. (This is from just looking at the code, so take it with a grain of salt) H_ADD_LOGICAL_LAN_BUFFER - not impl. by KVM H_CHANGE_LOGICAL_LAN_MAC - not impl. by KVM H_ENABLE_CRQ - not impl. by KVM H_FREE_CRQ - not impl. by KVM H_FREE_LOGICAL_LAN - not impl. by KVM H_GET_CPU_CHARACTERISTICS - not impl. by KVM H_GET_TERM_CHAR - not impl. by KVM H_HOME_NODE_ASSOCIATIVITY - not impl. by KVM H_INT_ESB - not impl. by KVM H_INT_GET_QUEUE_INFO - not impl. by KVM H_INT_GET_SOURCE_CONFIG - not impl. by KVM H_INT_GET_SOURCE_INFO - not impl. by KVM H_INT_RESET - not impl. by KVM H_INT_SET_QUEUE_CONFIG - not impl. by KVM H_INT_SET_SOURCE_CONFIG - not impl. by KVM H_INT_SYNC - not impl. by KVM H_JOIN - not impl. by KVM H_LOGICAL_CACHE_LOAD - not impl. by KVM H_LOGICAL_CACHE_STORE - not impl. by KVM H_LOGICAL_DCBF - not impl. by KVM H_LOGICAL_ICBI - not impl. by KVM H_MULTICAST_CTRL - not impl. by KVM H_PUT_TERM_CHAR - not impl. by KVM H_REGISTER_LOGICAL_LAN - not impl. by KVM H_REGISTER_PROC_TBL - not impl. by KVM H_REG_CRQ - not impl. by KVM H_RESIZE_HPT_COMMIT - not impl. by KVM H_RESIZE_HPT_PREPARE - not impl. by KVM H_SCM_BIND_MEM - not impl. by KVM H_SCM_READ_METADATA - not impl. by KVM H_SCM_UNBIND_ALL - not impl. by KVM H_SCM_WRITE_METADATA - not impl. by KVM H_SEND_CRQ - not impl. by KVM H_SEND_LOGICAL_LAN - not impl. by KVM H_SET_SPRG0 - not impl. by KVM H_SIGNAL_SYS_RESET - not impl. by KVM H_VIO_SIGNAL - not impl. by KVM H_CAS - not impl. by KVM | called by SLOF only H_LOGICAL_MEMOP - not impl. by KVM | called by SLOF only H_TPM_COMM - not impl. by KVM | called by UV only H_UPDATE_DT - not impl. by KVM | called by SLOF only H_INT_GET_OS_REPORTING_LINE - not impl. by KVM | not called by linux/SLOF/UV H_INT_GET_QUEUE_CONFIG - not impl. by KVM | not called by linux/SLOF/UV H_INT_SET_OS_REPORTING_LINE - not impl. by KVM | not called by linux/SLOF/UV H_SCM_UNBIND_MEM - not impl. by KVM | not called by linux/SLOF/UV H_GET_TCE - HV | not impl. by PR | QEMU fallback H_SET_MODE - HV | not impl. by PR | QEMU fallback H_CONFER - HV | not impl. by PR H_PAGE_INIT - HV | not impl. by PR H_PROD - HV | not impl. by PR H_RANDOM - HV | not impl. by PR H_READ - HV | not impl. by PR H_REGISTER_VPA - HV | not impl. by PR H_SET_DABR - HV | not impl. by PR H_SET_XDABR - HV | not impl. by PR H_CPPR - HV | PR | QEMU fallback H_EOI - HV | PR | QEMU fallback H_IPI - HV | PR | QEMU fallback H_IPOLL - HV | PR | QEMU fallback H_LOGICAL_CI_LOAD - HV | PR | QEMU fallback H_LOGICAL_CI_STORE - HV | PR | QEMU fallback H_PUT_TCE - HV | PR | QEMU fallback H_PUT_TCE_INDIRECT - HV | PR | QEMU fallback H_RTAS - HV | PR | QEMU fallback H_STUFF_TCE - HV | PR | QEMU fallback H_XIRR - HV | PR | QEMU fallback H_XIRR_X - HV | PR | QEMU fallback H_BULK_REMOVE - HV | PR H_CEDE - HV | PR H_ENTER - HV | PR H_PROTECT - HV | PR H_REMOVE - HV | PR H_CLEAN_SLB - never called/implemented, added along with H_REGISTER_PROC_TBL H_INVALIDATE_PID - never called/implemented, added along with H_REGISTER_PROC_TBL PS: we could perhaps use this information to annotate qemu/include/spapr.h. I can send a patch if people find it useful.
On 03/05/2021 19:21, Fabiano Rosas wrote: > "Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br> writes: > >> After the feedback from v1 I reworked the patch with suggested ideas and >> this version has less duplicated code and is overall simpler. >> >> This patch series is still a WIP, there are still 2 main problems I am >> trying to solve, I'll mention them in their respective patches. >> >> The aim of these patches is to progress toward enabling disable-tcg on >> PPC by solving errors in hw/ppc with that option. >> >> As a WIP comments are welcome. >> >> Lucas Mateus Castro (alqotel) (2): >> target/ppc: Moved functions out of mmu-hash64 >> hw/ppc: Moved TCG code to spapr_hcall_tcg >> >> hw/ppc/meson.build | 3 + >> hw/ppc/spapr.c | 1 + >> hw/ppc/spapr_caps.c | 1 + >> hw/ppc/spapr_cpu_core.c | 1 + >> hw/ppc/spapr_hcall.c | 301 ++-------------------------------- >> hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++ >> hw/ppc/spapr_rtas.c | 1 + >> target/ppc/meson.build | 1 + >> target/ppc/mmu-hash64.c | 81 +-------- >> target/ppc/mmu-hash64.h | 6 - >> target/ppc/mmu-misc.c | 86 ++++++++++ >> target/ppc/mmu-misc.h | 22 +++ >> 12 files changed, 478 insertions(+), 369 deletions(-) >> create mode 100644 hw/ppc/spapr_hcall_tcg.c >> create mode 100644 target/ppc/mmu-misc.c >> create mode 100644 target/ppc/mmu-misc.h > This is the list of hypercalls registered with spapr_register_hypercall > and whether they are implemented by KVM HV, KVM PR or none. I also list > whether the KVM hcall uses the QEMU implementation as a fallback. Maybe > it will be helpful to this discussion. > > (This is from just looking at the code, so take it with a grain of salt) > > H_ADD_LOGICAL_LAN_BUFFER - not impl. by KVM > H_CHANGE_LOGICAL_LAN_MAC - not impl. by KVM > H_ENABLE_CRQ - not impl. by KVM > H_FREE_CRQ - not impl. by KVM > H_FREE_LOGICAL_LAN - not impl. by KVM > H_GET_CPU_CHARACTERISTICS - not impl. by KVM > H_GET_TERM_CHAR - not impl. by KVM > H_HOME_NODE_ASSOCIATIVITY - not impl. by KVM > H_INT_ESB - not impl. by KVM > H_INT_GET_QUEUE_INFO - not impl. by KVM > H_INT_GET_SOURCE_CONFIG - not impl. by KVM > H_INT_GET_SOURCE_INFO - not impl. by KVM > H_INT_RESET - not impl. by KVM > H_INT_SET_QUEUE_CONFIG - not impl. by KVM > H_INT_SET_SOURCE_CONFIG - not impl. by KVM > H_INT_SYNC - not impl. by KVM > H_JOIN - not impl. by KVM > H_LOGICAL_CACHE_LOAD - not impl. by KVM > H_LOGICAL_CACHE_STORE - not impl. by KVM > H_LOGICAL_DCBF - not impl. by KVM > H_LOGICAL_ICBI - not impl. by KVM > H_MULTICAST_CTRL - not impl. by KVM > H_PUT_TERM_CHAR - not impl. by KVM > H_REGISTER_LOGICAL_LAN - not impl. by KVM > H_REGISTER_PROC_TBL - not impl. by KVM > H_REG_CRQ - not impl. by KVM > H_RESIZE_HPT_COMMIT - not impl. by KVM > H_RESIZE_HPT_PREPARE - not impl. by KVM > H_SCM_BIND_MEM - not impl. by KVM > H_SCM_READ_METADATA - not impl. by KVM > H_SCM_UNBIND_ALL - not impl. by KVM > H_SCM_WRITE_METADATA - not impl. by KVM > H_SEND_CRQ - not impl. by KVM > H_SEND_LOGICAL_LAN - not impl. by KVM > H_SET_SPRG0 - not impl. by KVM > H_SIGNAL_SYS_RESET - not impl. by KVM > H_VIO_SIGNAL - not impl. by KVM > > H_CAS - not impl. by KVM | called by SLOF only > H_LOGICAL_MEMOP - not impl. by KVM | called by SLOF only > H_TPM_COMM - not impl. by KVM | called by UV only > H_UPDATE_DT - not impl. by KVM | called by SLOF only > > H_INT_GET_OS_REPORTING_LINE - not impl. by KVM | not called by linux/SLOF/UV > H_INT_GET_QUEUE_CONFIG - not impl. by KVM | not called by linux/SLOF/UV > H_INT_SET_OS_REPORTING_LINE - not impl. by KVM | not called by linux/SLOF/UV > H_SCM_UNBIND_MEM - not impl. by KVM | not called by linux/SLOF/UV > > H_GET_TCE - HV | not impl. by PR | QEMU fallback > H_SET_MODE - HV | not impl. by PR | QEMU fallback > H_CONFER - HV | not impl. by PR > H_PAGE_INIT - HV | not impl. by PR > H_PROD - HV | not impl. by PR > H_RANDOM - HV | not impl. by PR > H_READ - HV | not impl. by PR > H_REGISTER_VPA - HV | not impl. by PR > H_SET_DABR - HV | not impl. by PR > H_SET_XDABR - HV | not impl. by PR > > H_CPPR - HV | PR | QEMU fallback > H_EOI - HV | PR | QEMU fallback > H_IPI - HV | PR | QEMU fallback > H_IPOLL - HV | PR | QEMU fallback > H_LOGICAL_CI_LOAD - HV | PR | QEMU fallback > H_LOGICAL_CI_STORE - HV | PR | QEMU fallback > H_PUT_TCE - HV | PR | QEMU fallback > H_PUT_TCE_INDIRECT - HV | PR | QEMU fallback > H_RTAS - HV | PR | QEMU fallback > H_STUFF_TCE - HV | PR | QEMU fallback > H_XIRR - HV | PR | QEMU fallback > H_XIRR_X - HV | PR | QEMU fallback > > H_BULK_REMOVE - HV | PR > H_CEDE - HV | PR > H_ENTER - HV | PR > H_PROTECT - HV | PR > H_REMOVE - HV | PR > > H_CLEAN_SLB - never called/implemented, added along with H_REGISTER_PROC_TBL > H_INVALIDATE_PID - never called/implemented, added along with H_REGISTER_PROC_TBL > > PS: we could perhaps use this information to annotate > qemu/include/spapr.h. I can send a patch if people find it useful. That would be very good, I think. I'm always in favor of more documentation
Thanks, it will be quite helpful. Also, I agree with Bruno including this information somewhere would be quite good in my opinion.
On Mon, May 03, 2021 at 07:21:11PM -0300, Fabiano Rosas wrote: > "Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br> writes: > > > After the feedback from v1 I reworked the patch with suggested ideas and > > this version has less duplicated code and is overall simpler. > > > > This patch series is still a WIP, there are still 2 main problems I am > > trying to solve, I'll mention them in their respective patches. > > > > The aim of these patches is to progress toward enabling disable-tcg on > > PPC by solving errors in hw/ppc with that option. > > > > As a WIP comments are welcome. > > > > Lucas Mateus Castro (alqotel) (2): > > target/ppc: Moved functions out of mmu-hash64 > > hw/ppc: Moved TCG code to spapr_hcall_tcg > > > > hw/ppc/meson.build | 3 + > > hw/ppc/spapr.c | 1 + > > hw/ppc/spapr_caps.c | 1 + > > hw/ppc/spapr_cpu_core.c | 1 + > > hw/ppc/spapr_hcall.c | 301 ++-------------------------------- > > hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++ > > hw/ppc/spapr_rtas.c | 1 + > > target/ppc/meson.build | 1 + > > target/ppc/mmu-hash64.c | 81 +-------- > > target/ppc/mmu-hash64.h | 6 - > > target/ppc/mmu-misc.c | 86 ++++++++++ > > target/ppc/mmu-misc.h | 22 +++ > > 12 files changed, 478 insertions(+), 369 deletions(-) > > create mode 100644 hw/ppc/spapr_hcall_tcg.c > > create mode 100644 target/ppc/mmu-misc.c > > create mode 100644 target/ppc/mmu-misc.h > > This is the list of hypercalls registered with spapr_register_hypercall > and whether they are implemented by KVM HV, KVM PR or none. I also list > whether the KVM hcall uses the QEMU implementation as a fallback. Maybe > it will be helpful to this discussion. > > (This is from just looking at the code, so take it with a grain of salt) > > H_ADD_LOGICAL_LAN_BUFFER - not impl. by KVM > H_CHANGE_LOGICAL_LAN_MAC - not impl. by KVM > H_ENABLE_CRQ - not impl. by KVM > H_FREE_CRQ - not impl. by KVM > H_FREE_LOGICAL_LAN - not impl. by KVM > H_GET_CPU_CHARACTERISTICS - not impl. by KVM > H_GET_TERM_CHAR - not impl. by KVM > H_HOME_NODE_ASSOCIATIVITY - not impl. by KVM > H_INT_ESB - not impl. by KVM > H_INT_GET_QUEUE_INFO - not impl. by KVM > H_INT_GET_SOURCE_CONFIG - not impl. by KVM > H_INT_GET_SOURCE_INFO - not impl. by KVM > H_INT_RESET - not impl. by KVM > H_INT_SET_QUEUE_CONFIG - not impl. by KVM > H_INT_SET_SOURCE_CONFIG - not impl. by KVM > H_INT_SYNC - not impl. by KVM > H_JOIN - not impl. by KVM > H_LOGICAL_CACHE_LOAD - not impl. by KVM > H_LOGICAL_CACHE_STORE - not impl. by KVM > H_LOGICAL_DCBF - not impl. by KVM > H_LOGICAL_ICBI - not impl. by KVM > H_MULTICAST_CTRL - not impl. by KVM > H_PUT_TERM_CHAR - not impl. by KVM > H_REGISTER_LOGICAL_LAN - not impl. by KVM > H_REGISTER_PROC_TBL - not impl. by KVM > H_REG_CRQ - not impl. by KVM > H_RESIZE_HPT_COMMIT - not impl. by KVM > H_RESIZE_HPT_PREPARE - not impl. by KVM > H_SCM_BIND_MEM - not impl. by KVM > H_SCM_READ_METADATA - not impl. by KVM > H_SCM_UNBIND_ALL - not impl. by KVM > H_SCM_WRITE_METADATA - not impl. by KVM > H_SEND_CRQ - not impl. by KVM > H_SEND_LOGICAL_LAN - not impl. by KVM > H_SET_SPRG0 - not impl. by KVM > H_SIGNAL_SYS_RESET - not impl. by KVM > H_VIO_SIGNAL - not impl. by KVM > > H_CAS - not impl. by KVM | called by SLOF only > H_LOGICAL_MEMOP - not impl. by KVM | called by SLOF only > H_TPM_COMM - not impl. by KVM | called by UV only > H_UPDATE_DT - not impl. by KVM | called by SLOF only > > H_INT_GET_OS_REPORTING_LINE - not impl. by KVM | not called by linux/SLOF/UV > H_INT_GET_QUEUE_CONFIG - not impl. by KVM | not called by linux/SLOF/UV > H_INT_SET_OS_REPORTING_LINE - not impl. by KVM | not called by linux/SLOF/UV > H_SCM_UNBIND_MEM - not impl. by KVM | not called by linux/SLOF/UV > > H_GET_TCE - HV | not impl. by PR | QEMU fallback > H_SET_MODE - HV | not impl. by PR | QEMU fallback > H_CONFER - HV | not impl. by PR > H_PAGE_INIT - HV | not impl. by PR > H_PROD - HV | not impl. by PR > H_RANDOM - HV | not impl. by PR > H_READ - HV | not impl. by PR > H_REGISTER_VPA - HV | not impl. by PR > H_SET_DABR - HV | not impl. by PR > H_SET_XDABR - HV | not impl. by PR > > H_CPPR - HV | PR | QEMU fallback > H_EOI - HV | PR | QEMU fallback > H_IPI - HV | PR | QEMU fallback > H_IPOLL - HV | PR | QEMU fallback > H_LOGICAL_CI_LOAD - HV | PR | QEMU fallback > H_LOGICAL_CI_STORE - HV | PR | QEMU fallback > H_PUT_TCE - HV | PR | QEMU fallback > H_PUT_TCE_INDIRECT - HV | PR | QEMU fallback > H_RTAS - HV | PR | QEMU fallback > H_STUFF_TCE - HV | PR | QEMU fallback > H_XIRR - HV | PR | QEMU fallback > H_XIRR_X - HV | PR | QEMU fallback > > H_BULK_REMOVE - HV | PR > H_CEDE - HV | PR > H_ENTER - HV | PR > H_PROTECT - HV | PR > H_REMOVE - HV | PR > > H_CLEAN_SLB - never called/implemented, added along with H_REGISTER_PROC_TBL > H_INVALIDATE_PID - never called/implemented, added along with H_REGISTER_PROC_TBL Thanks for summarizing this, Fabiano. > PS: we could perhaps use this information to annotate > qemu/include/spapr.h. I can send a patch if people find it useful. I don't want to include the whole set of information here in qemu, since exactly what's implemented in KVM is subject to change, and in most cases qemu doesn't care about that. It would be worth annotating those hcalls which qemu *relies* on being provided by KVM. As I've noted this is basically just the hash MMU calls: H_ENTER H_REMOVE H_BULK_REMOVE H_PROTECT H_READ H_RESIZE_HPT_PREPARE H_RESIZE_HPT_COMMIT Secondarily there's a handful of extra hypercalls which are probably pointless but harmless to be implemented in qemu for a KVM guest. They shouldn't be as intimately tied to TCG or the softmmu code as the above, so dealing with them can wait until a later: H_CEDE H_CONFER H_PROD H_REGISTER_VPA