* [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
@ 2021-04-30 18:40 Lucas Mateus Castro (alqotel)
2021-04-30 18:40 ` [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64 Lucas Mateus Castro (alqotel)
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Lucas Mateus Castro (alqotel) @ 2021-04-30 18:40 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), farosas, david
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
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64
2021-04-30 18:40 [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG Lucas Mateus Castro (alqotel)
@ 2021-04-30 18:40 ` Lucas Mateus Castro (alqotel)
2021-04-30 20:19 ` Fabiano Rosas
2021-05-03 4:24 ` David Gibson
2021-04-30 18:40 ` [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg Lucas Mateus Castro (alqotel)
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Lucas Mateus Castro (alqotel) @ 2021-04-30 18:40 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), farosas, david
The functions ppc_store_lpcr, ppc_hash64_filter_pagesizes and
ppc_hash64_unmap_hptes have been moved to mmu-misc.h since they are
not needed in a !TCG context and mmu-hash64 should not be compiled
in such situation.
ppc_store_lpcr and ppc_hash64_filter_pagesizes are used by multiple
functions, while ppc_hash64_unmap_hptes is used by rehash_hpt (in
spapr_hcall.c).
Also I've put the functions in mmu-misc as I am unsure in which file
this functions should go, so I just created a new one for now, any
suggestion which file to put them (considering it's a file that must be
compiled in a !TCG situation)?
Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
---
hw/ppc/spapr.c | 1 +
hw/ppc/spapr_caps.c | 1 +
hw/ppc/spapr_cpu_core.c | 1 +
hw/ppc/spapr_hcall.c | 1 +
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 +++++++++++
10 files changed, 115 insertions(+), 86 deletions(-)
create mode 100644 target/ppc/mmu-misc.c
create mode 100644 target/ppc/mmu-misc.h
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e4be00b732..61f8f150c2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -53,6 +53,7 @@
#include "mmu-book3s-v3.h"
#include "cpu-models.h"
#include "hw/core/cpu.h"
+#include "mmu-misc.h"
#include "hw/boards.h"
#include "hw/ppc/ppc.h"
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 9ea7ddd1e9..22352ff018 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -34,6 +34,7 @@
#include "kvm_ppc.h"
#include "migration/vmstate.h"
#include "sysemu/tcg.h"
+#include "mmu-misc.h"
#include "hw/ppc/spapr.h"
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 4f316a6f9d..f4d93999e5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -24,6 +24,7 @@
#include "sysemu/reset.h"
#include "sysemu/hw_accel.h"
#include "qemu/error-report.h"
+#include "mmu-misc.h"
static void spapr_reset_vcpu(PowerPCCPU *cpu)
{
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7b5cd3553c..4b0ba69841 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -13,6 +13,7 @@
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_cpu_core.h"
#include "mmu-hash64.h"
+#include "mmu-misc.h"
#include "cpu-models.h"
#include "trace.h"
#include "kvm_ppc.h"
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 8a79f9c628..8935b75d1c 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -35,6 +35,7 @@
#include "sysemu/hw_accel.h"
#include "sysemu/runstate.h"
#include "kvm_ppc.h"
+#include "mmu-misc.h"
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index bbfef90e08..7a97648803 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -31,6 +31,7 @@ ppc_softmmu_ss.add(when: 'TARGET_PPC64', if_true: files(
'mmu-book3s-v3.c',
'mmu-hash64.c',
'mmu-radix64.c',
+ 'mmu-misc.c',
))
target_arch += {'ppc': ppc_ss}
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 0fabc10302..919a3e9f51 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -30,6 +30,7 @@
#include "exec/log.h"
#include "hw/hw.h"
#include "mmu-book3s-v3.h"
+#include "mmu-misc.h"
/* #define DEBUG_SLB */
@@ -499,20 +500,6 @@ const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
return hptes;
}
-void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
- hwaddr ptex, int n)
-{
- if (cpu->vhyp) {
- PPCVirtualHypervisorClass *vhc =
- PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
- vhc->unmap_hptes(cpu->vhyp, hptes, ptex, n);
- return;
- }
-
- address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
- false, n * HASH_PTE_SIZE_64);
-}
-
static unsigned hpte_page_shift(const PPCHash64SegmentPageSizes *sps,
uint64_t pte0, uint64_t pte1)
{
@@ -1119,14 +1106,6 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
cpu->env.tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
}
-void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
-{
- PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
- CPUPPCState *env = &cpu->env;
-
- env->spr[SPR_LPCR] = val & pcc->lpcr_mask;
-}
-
void helper_store_lpcr(CPUPPCState *env, target_ulong val)
{
PowerPCCPU *cpu = env_archcpu(env);
@@ -1197,61 +1176,3 @@ const PPCHash64Options ppc_hash64_opts_POWER7 = {
}
};
-void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
- bool (*cb)(void *, uint32_t, uint32_t),
- void *opaque)
-{
- PPCHash64Options *opts = cpu->hash64_opts;
- int i;
- int n = 0;
- bool ci_largepage = false;
-
- assert(opts);
-
- n = 0;
- for (i = 0; i < ARRAY_SIZE(opts->sps); i++) {
- PPCHash64SegmentPageSizes *sps = &opts->sps[i];
- int j;
- int m = 0;
-
- assert(n <= i);
-
- if (!sps->page_shift) {
- break;
- }
-
- for (j = 0; j < ARRAY_SIZE(sps->enc); j++) {
- PPCHash64PageSize *ps = &sps->enc[j];
-
- assert(m <= j);
- if (!ps->page_shift) {
- break;
- }
-
- if (cb(opaque, sps->page_shift, ps->page_shift)) {
- if (ps->page_shift >= 16) {
- ci_largepage = true;
- }
- sps->enc[m++] = *ps;
- }
- }
-
- /* Clear rest of the row */
- for (j = m; j < ARRAY_SIZE(sps->enc); j++) {
- memset(&sps->enc[j], 0, sizeof(sps->enc[j]));
- }
-
- if (m) {
- n++;
- }
- }
-
- /* Clear the rest of the table */
- for (i = n; i < ARRAY_SIZE(opts->sps); i++) {
- memset(&opts->sps[i], 0, sizeof(opts->sps[i]));
- }
-
- if (!ci_largepage) {
- opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
- }
-}
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 87729d48b3..562602b466 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -15,12 +15,8 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
target_ulong pte0, target_ulong pte1);
unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
uint64_t pte0, uint64_t pte1);
-void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
void ppc_hash64_init(PowerPCCPU *cpu);
void ppc_hash64_finalize(PowerPCCPU *cpu);
-void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
- bool (*cb)(void *, uint32_t, uint32_t),
- void *opaque);
#endif
/*
@@ -112,8 +108,6 @@ struct ppc_hash_pte64 {
const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
hwaddr ptex, int n);
-void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
- hwaddr ptex, int n);
static inline uint64_t ppc_hash64_hpte0(PowerPCCPU *cpu,
const ppc_hash_pte64_t *hptes, int i)
diff --git a/target/ppc/mmu-misc.c b/target/ppc/mmu-misc.c
new file mode 100644
index 0000000000..8abda66547
--- /dev/null
+++ b/target/ppc/mmu-misc.c
@@ -0,0 +1,86 @@
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "mmu-hash64.h"
+#include "fpu/softfloat-helpers.h"
+#include "mmu-misc.h"
+
+void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
+{
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ CPUPPCState *env = &cpu->env;
+
+ env->spr[SPR_LPCR] = val & pcc->lpcr_mask;
+}
+
+void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
+ bool (*cb)(void *, uint32_t, uint32_t),
+ void *opaque)
+{
+ PPCHash64Options *opts = cpu->hash64_opts;
+ int i;
+ int n = 0;
+ bool ci_largepage = false;
+
+ assert(opts);
+
+ n = 0;
+ for (i = 0; i < ARRAY_SIZE(opts->sps); i++) {
+ PPCHash64SegmentPageSizes *sps = &opts->sps[i];
+ int j;
+ int m = 0;
+
+ assert(n <= i);
+
+ if (!sps->page_shift) {
+ break;
+ }
+
+ for (j = 0; j < ARRAY_SIZE(sps->enc); j++) {
+ PPCHash64PageSize *ps = &sps->enc[j];
+
+ assert(m <= j);
+ if (!ps->page_shift) {
+ break;
+ }
+
+ if (cb(opaque, sps->page_shift, ps->page_shift)) {
+ if (ps->page_shift >= 16) {
+ ci_largepage = true;
+ }
+ sps->enc[m++] = *ps;
+ }
+ }
+
+ /* Clear rest of the row */
+ for (j = m; j < ARRAY_SIZE(sps->enc); j++) {
+ memset(&sps->enc[j], 0, sizeof(sps->enc[j]));
+ }
+
+ if (m) {
+ n++;
+ }
+ }
+
+ /* Clear the rest of the table */
+ for (i = n; i < ARRAY_SIZE(opts->sps); i++) {
+ memset(&opts->sps[i], 0, sizeof(opts->sps[i]));
+ }
+
+ if (!ci_largepage) {
+ opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
+ }
+}
+
+void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n)
+{
+ if (cpu->vhyp) {
+ PPCVirtualHypervisorClass *vhc =
+ PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ vhc->unmap_hptes(cpu->vhyp, hptes, ptex, n);
+ return;
+ }
+
+ address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
+ false, n * HASH_PTE_SIZE_64);
+}
diff --git a/target/ppc/mmu-misc.h b/target/ppc/mmu-misc.h
new file mode 100644
index 0000000000..7be6bf7b44
--- /dev/null
+++ b/target/ppc/mmu-misc.h
@@ -0,0 +1,22 @@
+#ifndef MMU_MISC_H
+#define MMU_MISC_H
+#include "qemu/osdep.h"
+#include "cpu.h"
+
+#ifndef CONFIG_USER_ONLY
+
+#ifdef TARGET_PPC64
+
+void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
+void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
+ bool (*cb)(void *, uint32_t, uint32_t),
+ void *opaque);
+
+#endif
+
+void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n);
+
+#endif
+
+#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg
2021-04-30 18:40 [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG Lucas Mateus Castro (alqotel)
2021-04-30 18:40 ` [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64 Lucas Mateus Castro (alqotel)
@ 2021-04-30 18:40 ` Lucas Mateus Castro (alqotel)
2021-04-30 23:38 ` Fabiano Rosas
2021-05-03 4:34 ` David Gibson
2021-04-30 18:54 ` [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG no-reply
2021-05-03 22:21 ` Fabiano Rosas
3 siblings, 2 replies; 17+ messages in thread
From: Lucas Mateus Castro (alqotel) @ 2021-04-30 18:40 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), farosas, david
Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
environment in spapr_hcall.c and changed build options to only build
spapr_hcall_tcg.c when CONFIG_TCG is enabled.
Added the function h_tcg_only to be used for hypercalls that should be
called only in TCG environment but have been called in a TCG-less one.
Right now, a #ifndef is used to know if there is a need of a h_tcg_only
function to be implemented and used as hypercalls, I initially thought
of always having that option turned on and having spapr_hcall_tcg.c
overwrite those hypercalls when TCG is enabled, but
spapr_register_hypercalls checks if a hypercall already exists for that
opcode so that wouldn't work, so if anyone has any suggestions I'm
interested.
Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
is_ram_address), what is the advised way to deal with these
duplications?
Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
---
hw/ppc/meson.build | 3 +
hw/ppc/spapr_hcall.c | 300 ++--------------------------------
hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++
3 files changed, 363 insertions(+), 283 deletions(-)
create mode 100644 hw/ppc/spapr_hcall_tcg.c
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 218631c883..3c7f2f08b7 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -29,6 +29,9 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
'spapr_numa.c',
'pef.c',
))
+ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
+ 'spapr_hcall_tcg.c'
+))
ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
'spapr_pci_vfio.c',
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4b0ba69841..b37fbdc32e 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -22,6 +22,15 @@
#include "mmu-book3s-v3.h"
#include "hw/mem/memory-device.h"
+#ifndef CONFIG_TCG
+static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ g_assert_not_reached();
+ return 0;
+}
+#endif /* !CONFIG_TCG */
+
static bool has_spr(PowerPCCPU *cpu, int spr)
{
/* We can test whether the SPR is defined by checking for a valid name */
@@ -55,284 +64,6 @@ static bool is_ram_address(SpaprMachineState *spapr, hwaddr addr)
return false;
}
-static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
- target_ulong opcode, target_ulong *args)
-{
- target_ulong flags = args[0];
- target_ulong ptex = args[1];
- target_ulong pteh = args[2];
- target_ulong ptel = args[3];
- unsigned apshift;
- target_ulong raddr;
- target_ulong slot;
- const ppc_hash_pte64_t *hptes;
-
- apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
- if (!apshift) {
- /* Bad page size encoding */
- return H_PARAMETER;
- }
-
- raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
-
- if (is_ram_address(spapr, raddr)) {
- /* Regular RAM - should have WIMG=0010 */
- if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
- return H_PARAMETER;
- }
- } else {
- target_ulong wimg_flags;
- /* Looks like an IO address */
- /* FIXME: What WIMG combinations could be sensible for IO?
- * For now we allow WIMG=010x, but are there others? */
- /* FIXME: Should we check against registered IO addresses? */
- wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
-
- if (wimg_flags != HPTE64_R_I &&
- wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
- return H_PARAMETER;
- }
- }
-
- pteh &= ~0x60ULL;
-
- if (!valid_ptex(cpu, ptex)) {
- return H_PARAMETER;
- }
-
- slot = ptex & 7ULL;
- ptex = ptex & ~7ULL;
-
- if (likely((flags & H_EXACT) == 0)) {
- hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
- for (slot = 0; slot < 8; slot++) {
- if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
- break;
- }
- }
- ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
- if (slot == 8) {
- return H_PTEG_FULL;
- }
- } else {
- hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
- if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
- ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
- return H_PTEG_FULL;
- }
- ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
- }
-
- spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
-
- args[0] = ptex + slot;
- return H_SUCCESS;
-}
-
-typedef enum {
- REMOVE_SUCCESS = 0,
- REMOVE_NOT_FOUND = 1,
- REMOVE_PARM = 2,
- REMOVE_HW = 3,
-} RemoveResult;
-
-static RemoveResult remove_hpte(PowerPCCPU *cpu
- , target_ulong ptex,
- target_ulong avpn,
- target_ulong flags,
- target_ulong *vp, target_ulong *rp)
-{
- const ppc_hash_pte64_t *hptes;
- target_ulong v, r;
-
- if (!valid_ptex(cpu, ptex)) {
- return REMOVE_PARM;
- }
-
- hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
- v = ppc_hash64_hpte0(cpu, hptes, 0);
- r = ppc_hash64_hpte1(cpu, hptes, 0);
- ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
-
- if ((v & HPTE64_V_VALID) == 0 ||
- ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
- ((flags & H_ANDCOND) && (v & avpn) != 0)) {
- return REMOVE_NOT_FOUND;
- }
- *vp = v;
- *rp = r;
- spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
- ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
- return REMOVE_SUCCESS;
-}
-
-static target_ulong h_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
- target_ulong opcode, target_ulong *args)
-{
- CPUPPCState *env = &cpu->env;
- target_ulong flags = args[0];
- target_ulong ptex = args[1];
- target_ulong avpn = args[2];
- RemoveResult ret;
-
- ret = remove_hpte(cpu, ptex, avpn, flags,
- &args[0], &args[1]);
-
- switch (ret) {
- case REMOVE_SUCCESS:
- check_tlb_flush(env, true);
- return H_SUCCESS;
-
- case REMOVE_NOT_FOUND:
- return H_NOT_FOUND;
-
- case REMOVE_PARM:
- return H_PARAMETER;
-
- case REMOVE_HW:
- return H_HARDWARE;
- }
-
- g_assert_not_reached();
-}
-
-#define H_BULK_REMOVE_TYPE 0xc000000000000000ULL
-#define H_BULK_REMOVE_REQUEST 0x4000000000000000ULL
-#define H_BULK_REMOVE_RESPONSE 0x8000000000000000ULL
-#define H_BULK_REMOVE_END 0xc000000000000000ULL
-#define H_BULK_REMOVE_CODE 0x3000000000000000ULL
-#define H_BULK_REMOVE_SUCCESS 0x0000000000000000ULL
-#define H_BULK_REMOVE_NOT_FOUND 0x1000000000000000ULL
-#define H_BULK_REMOVE_PARM 0x2000000000000000ULL
-#define H_BULK_REMOVE_HW 0x3000000000000000ULL
-#define H_BULK_REMOVE_RC 0x0c00000000000000ULL
-#define H_BULK_REMOVE_FLAGS 0x0300000000000000ULL
-#define H_BULK_REMOVE_ABSOLUTE 0x0000000000000000ULL
-#define H_BULK_REMOVE_ANDCOND 0x0100000000000000ULL
-#define H_BULK_REMOVE_AVPN 0x0200000000000000ULL
-#define H_BULK_REMOVE_PTEX 0x00ffffffffffffffULL
-
-#define H_BULK_REMOVE_MAX_BATCH 4
-
-static target_ulong h_bulk_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
- target_ulong opcode, target_ulong *args)
-{
- CPUPPCState *env = &cpu->env;
- int i;
- target_ulong rc = H_SUCCESS;
-
- for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
- target_ulong *tsh = &args[i*2];
- target_ulong tsl = args[i*2 + 1];
- target_ulong v, r, ret;
-
- if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
- break;
- } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
- return H_PARAMETER;
- }
-
- *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
- *tsh |= H_BULK_REMOVE_RESPONSE;
-
- if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
- *tsh |= H_BULK_REMOVE_PARM;
- return H_PARAMETER;
- }
-
- ret = remove_hpte(cpu, *tsh & H_BULK_REMOVE_PTEX, tsl,
- (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
- &v, &r);
-
- *tsh |= ret << 60;
-
- switch (ret) {
- case REMOVE_SUCCESS:
- *tsh |= (r & (HPTE64_R_C | HPTE64_R_R)) << 43;
- break;
-
- case REMOVE_PARM:
- rc = H_PARAMETER;
- goto exit;
-
- case REMOVE_HW:
- rc = H_HARDWARE;
- goto exit;
- }
- }
- exit:
- check_tlb_flush(env, true);
-
- return rc;
-}
-
-static target_ulong h_protect(PowerPCCPU *cpu, SpaprMachineState *spapr,
- target_ulong opcode, target_ulong *args)
-{
- CPUPPCState *env = &cpu->env;
- target_ulong flags = args[0];
- target_ulong ptex = args[1];
- target_ulong avpn = args[2];
- const ppc_hash_pte64_t *hptes;
- target_ulong v, r;
-
- if (!valid_ptex(cpu, ptex)) {
- return H_PARAMETER;
- }
-
- hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
- v = ppc_hash64_hpte0(cpu, hptes, 0);
- r = ppc_hash64_hpte1(cpu, hptes, 0);
- ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
-
- if ((v & HPTE64_V_VALID) == 0 ||
- ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
- return H_NOT_FOUND;
- }
-
- r &= ~(HPTE64_R_PP0 | HPTE64_R_PP | HPTE64_R_N |
- HPTE64_R_KEY_HI | HPTE64_R_KEY_LO);
- r |= (flags << 55) & HPTE64_R_PP0;
- r |= (flags << 48) & HPTE64_R_KEY_HI;
- r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
- spapr_store_hpte(cpu, ptex,
- (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
- ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
- /* Flush the tlb */
- check_tlb_flush(env, true);
- /* Don't need a memory barrier, due to qemu's global lock */
- spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
- return H_SUCCESS;
-}
-
-static target_ulong h_read(PowerPCCPU *cpu, SpaprMachineState *spapr,
- target_ulong opcode, target_ulong *args)
-{
- target_ulong flags = args[0];
- target_ulong ptex = args[1];
- int i, ridx, n_entries = 1;
- const ppc_hash_pte64_t *hptes;
-
- if (!valid_ptex(cpu, ptex)) {
- return H_PARAMETER;
- }
-
- if (flags & H_READ_4) {
- /* Clear the two low order bits */
- ptex &= ~(3ULL);
- n_entries = 4;
- }
-
- hptes = ppc_hash64_map_hptes(cpu, ptex, n_entries);
- for (i = 0, ridx = 0; i < n_entries; i++) {
- args[ridx++] = ppc_hash64_hpte0(cpu, hptes, i);
- args[ridx++] = ppc_hash64_hpte1(cpu, hptes, i);
- }
- ppc_hash64_unmap_hptes(cpu, hptes, ptex, n_entries);
-
- return H_SUCCESS;
-}
-
struct SpaprPendingHpt {
/* These fields are read-only after initialization */
int shift;
@@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
static void hypercall_register_types(void)
{
+
+#ifndef CONFIG_TCG
/* hcall-pft */
- spapr_register_hypercall(H_ENTER, h_enter);
- spapr_register_hypercall(H_REMOVE, h_remove);
- spapr_register_hypercall(H_PROTECT, h_protect);
- spapr_register_hypercall(H_READ, h_read);
+ spapr_register_hypercall(H_ENTER, h_tcg_only);
+ spapr_register_hypercall(H_REMOVE, h_tcg_only);
+ spapr_register_hypercall(H_PROTECT, h_tcg_only);
+ spapr_register_hypercall(H_READ, h_tcg_only);
/* hcall-bulk */
- spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
+ spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
+#endif /* !CONFIG_TCG */
/* hcall-hpt-resize */
spapr_register_hypercall(H_RESIZE_HPT_PREPARE, h_resize_hpt_prepare);
diff --git a/hw/ppc/spapr_hcall_tcg.c b/hw/ppc/spapr_hcall_tcg.c
new file mode 100644
index 0000000000..92ff24c8dc
--- /dev/null
+++ b/hw/ppc/spapr_hcall_tcg.c
@@ -0,0 +1,343 @@
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "sysemu/hw_accel.h"
+#include "sysemu/runstate.h"
+#include "qemu/log.h"
+#include "qemu/main-loop.h"
+#include "qemu/module.h"
+#include "qemu/error-report.h"
+#include "cpu.h"
+#include "exec/exec-all.h"
+#include "helper_regs.h"
+#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_cpu_core.h"
+#include "mmu-hash64.h"
+#include "mmu-misc.h"
+#include "cpu-models.h"
+#include "trace.h"
+#include "kvm_ppc.h"
+#include "hw/ppc/fdt.h"
+#include "hw/ppc/spapr_ovec.h"
+#include "mmu-book3s-v3.h"
+#include "hw/mem/memory-device.h"
+
+static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
+{
+ /*
+ * hash value/pteg group index is normalized by HPT mask
+ */
+ if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~ppc_hash64_hpt_mask(cpu)) {
+ return false;
+ }
+ return true;
+}
+
+static bool is_ram_address(SpaprMachineState *spapr, hwaddr addr)
+{
+ MachineState *machine = MACHINE(spapr);
+ DeviceMemoryState *dms = machine->device_memory;
+
+ if (addr < machine->ram_size) {
+ return true;
+ }
+ if ((addr >= dms->base)
+ && ((addr - dms->base) < memory_region_size(&dms->mr))) {
+ return true;
+ }
+
+ return false;
+}
+
+static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ target_ulong flags = args[0];
+ target_ulong ptex = args[1];
+ target_ulong pteh = args[2];
+ target_ulong ptel = args[3];
+ unsigned apshift;
+ target_ulong raddr;
+ target_ulong slot;
+ const ppc_hash_pte64_t *hptes;
+
+ apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
+ if (!apshift) {
+ /* Bad page size encoding */
+ return H_PARAMETER;
+ }
+
+ raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
+
+ if (is_ram_address(spapr, raddr)) {
+ /* Regular RAM - should have WIMG=0010 */
+ if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
+ return H_PARAMETER;
+ }
+ } else {
+ target_ulong wimg_flags;
+ /* Looks like an IO address */
+ /* FIXME: What WIMG combinations could be sensible for IO?
+ * For now we allow WIMG=010x, but are there others? */
+ /* FIXME: Should we check against registered IO addresses? */
+ wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
+
+ if (wimg_flags != HPTE64_R_I &&
+ wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
+ return H_PARAMETER;
+ }
+ }
+
+ pteh &= ~0x60ULL;
+
+ if (!valid_ptex(cpu, ptex)) {
+ return H_PARAMETER;
+ }
+
+ slot = ptex & 7ULL;
+ ptex = ptex & ~7ULL;
+
+ if (likely((flags & H_EXACT) == 0)) {
+ hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
+ for (slot = 0; slot < 8; slot++) {
+ if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
+ break;
+ }
+ }
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
+ if (slot == 8) {
+ return H_PTEG_FULL;
+ }
+ } else {
+ hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
+ if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
+ return H_PTEG_FULL;
+ }
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
+ }
+
+ spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
+
+ args[0] = ptex + slot;
+ return H_SUCCESS;
+}
+
+typedef enum {
+ REMOVE_SUCCESS = 0,
+ REMOVE_NOT_FOUND = 1,
+ REMOVE_PARM = 2,
+ REMOVE_HW = 3,
+} RemoveResult;
+
+static RemoveResult remove_hpte(PowerPCCPU *cpu
+ , target_ulong ptex,
+ target_ulong avpn,
+ target_ulong flags,
+ target_ulong *vp, target_ulong *rp)
+{
+ const ppc_hash_pte64_t *hptes;
+ target_ulong v, r;
+
+ if (!valid_ptex(cpu, ptex)) {
+ return REMOVE_PARM;
+ }
+
+ hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
+ v = ppc_hash64_hpte0(cpu, hptes, 0);
+ r = ppc_hash64_hpte1(cpu, hptes, 0);
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
+
+ if ((v & HPTE64_V_VALID) == 0 ||
+ ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
+ ((flags & H_ANDCOND) && (v & avpn) != 0)) {
+ return REMOVE_NOT_FOUND;
+ }
+ *vp = v;
+ *rp = r;
+ spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
+ ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
+ return REMOVE_SUCCESS;
+}
+
+static target_ulong h_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ CPUPPCState *env = &cpu->env;
+ target_ulong flags = args[0];
+ target_ulong ptex = args[1];
+ target_ulong avpn = args[2];
+ RemoveResult ret;
+
+ ret = remove_hpte(cpu, ptex, avpn, flags,
+ &args[0], &args[1]);
+
+ switch (ret) {
+ case REMOVE_SUCCESS:
+ check_tlb_flush(env, true);
+ return H_SUCCESS;
+
+ case REMOVE_NOT_FOUND:
+ return H_NOT_FOUND;
+
+ case REMOVE_PARM:
+ return H_PARAMETER;
+
+ case REMOVE_HW:
+ return H_HARDWARE;
+ }
+
+ g_assert_not_reached();
+}
+
+#define H_BULK_REMOVE_TYPE 0xc000000000000000ULL
+#define H_BULK_REMOVE_REQUEST 0x4000000000000000ULL
+#define H_BULK_REMOVE_RESPONSE 0x8000000000000000ULL
+#define H_BULK_REMOVE_END 0xc000000000000000ULL
+#define H_BULK_REMOVE_CODE 0x3000000000000000ULL
+#define H_BULK_REMOVE_SUCCESS 0x0000000000000000ULL
+#define H_BULK_REMOVE_NOT_FOUND 0x1000000000000000ULL
+#define H_BULK_REMOVE_PARM 0x2000000000000000ULL
+#define H_BULK_REMOVE_HW 0x3000000000000000ULL
+#define H_BULK_REMOVE_RC 0x0c00000000000000ULL
+#define H_BULK_REMOVE_FLAGS 0x0300000000000000ULL
+#define H_BULK_REMOVE_ABSOLUTE 0x0000000000000000ULL
+#define H_BULK_REMOVE_ANDCOND 0x0100000000000000ULL
+#define H_BULK_REMOVE_AVPN 0x0200000000000000ULL
+#define H_BULK_REMOVE_PTEX 0x00ffffffffffffffULL
+
+#define H_BULK_REMOVE_MAX_BATCH 4
+
+static target_ulong h_bulk_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ CPUPPCState *env = &cpu->env;
+ int i;
+ target_ulong rc = H_SUCCESS;
+
+ for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
+ target_ulong *tsh = &args[i*2];
+ target_ulong tsl = args[i*2 + 1];
+ target_ulong v, r, ret;
+
+ if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
+ break;
+ } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
+ return H_PARAMETER;
+ }
+
+ *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
+ *tsh |= H_BULK_REMOVE_RESPONSE;
+
+ if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
+ *tsh |= H_BULK_REMOVE_PARM;
+ return H_PARAMETER;
+ }
+
+ ret = remove_hpte(cpu, *tsh & H_BULK_REMOVE_PTEX, tsl,
+ (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
+ &v, &r);
+
+ *tsh |= ret << 60;
+
+ switch (ret) {
+ case REMOVE_SUCCESS:
+ *tsh |= (r & (HPTE64_R_C | HPTE64_R_R)) << 43;
+ break;
+
+ case REMOVE_PARM:
+ rc = H_PARAMETER;
+ goto exit;
+
+ case REMOVE_HW:
+ rc = H_HARDWARE;
+ goto exit;
+ }
+ }
+ exit:
+ check_tlb_flush(env, true);
+
+ return rc;
+}
+
+static target_ulong h_protect(PowerPCCPU *cpu, SpaprMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ CPUPPCState *env = &cpu->env;
+ target_ulong flags = args[0];
+ target_ulong ptex = args[1];
+ target_ulong avpn = args[2];
+ const ppc_hash_pte64_t *hptes;
+ target_ulong v, r;
+
+ if (!valid_ptex(cpu, ptex)) {
+ return H_PARAMETER;
+ }
+
+ hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
+ v = ppc_hash64_hpte0(cpu, hptes, 0);
+ r = ppc_hash64_hpte1(cpu, hptes, 0);
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
+
+ if ((v & HPTE64_V_VALID) == 0 ||
+ ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
+ return H_NOT_FOUND;
+ }
+
+ r &= ~(HPTE64_R_PP0 | HPTE64_R_PP | HPTE64_R_N |
+ HPTE64_R_KEY_HI | HPTE64_R_KEY_LO);
+ r |= (flags << 55) & HPTE64_R_PP0;
+ r |= (flags << 48) & HPTE64_R_KEY_HI;
+ r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
+ spapr_store_hpte(cpu, ptex,
+ (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
+ ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
+ /* Flush the tlb */
+ check_tlb_flush(env, true);
+ /* Don't need a memory barrier, due to qemu's global lock */
+ spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
+ return H_SUCCESS;
+}
+
+static target_ulong h_read(PowerPCCPU *cpu, SpaprMachineState *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ target_ulong flags = args[0];
+ target_ulong ptex = args[1];
+ int i, ridx, n_entries = 1;
+ const ppc_hash_pte64_t *hptes;
+
+ if (!valid_ptex(cpu, ptex)) {
+ return H_PARAMETER;
+ }
+
+ if (flags & H_READ_4) {
+ /* Clear the two low order bits */
+ ptex &= ~(3ULL);
+ n_entries = 4;
+ }
+
+ hptes = ppc_hash64_map_hptes(cpu, ptex, n_entries);
+ for (i = 0, ridx = 0; i < n_entries; i++) {
+ args[ridx++] = ppc_hash64_hpte0(cpu, hptes, i);
+ args[ridx++] = ppc_hash64_hpte1(cpu, hptes, i);
+ }
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, n_entries);
+
+ return H_SUCCESS;
+}
+
+
+static void hypercall_register_types(void)
+{
+ /* hcall-pft */
+ spapr_register_hypercall(H_ENTER, h_enter);
+ spapr_register_hypercall(H_REMOVE, h_remove);
+ spapr_register_hypercall(H_PROTECT, h_protect);
+ spapr_register_hypercall(H_READ, h_read);
+
+ /* hcall-bulk */
+ spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
+}
+
+type_init(hypercall_register_types)
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
2021-04-30 18:40 [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG Lucas Mateus Castro (alqotel)
2021-04-30 18:40 ` [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64 Lucas Mateus Castro (alqotel)
2021-04-30 18:40 ` [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg Lucas Mateus Castro (alqotel)
@ 2021-04-30 18:54 ` no-reply
2021-05-03 22:21 ` Fabiano Rosas
3 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2021-04-30 18:54 UTC (permalink / raw)
To: lucas.araujo
Cc: farosas, qemu-devel, lucas.araujo, qemu-ppc, bruno.larsen, david
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64
2021-04-30 18:40 ` [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64 Lucas Mateus Castro (alqotel)
@ 2021-04-30 20:19 ` Fabiano Rosas
2021-05-03 4:24 ` David Gibson
1 sibling, 0 replies; 17+ messages in thread
From: Fabiano Rosas @ 2021-04-30 20:19 UTC (permalink / raw)
To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), david
"Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br> writes:
> The functions ppc_store_lpcr, ppc_hash64_filter_pagesizes and
> ppc_hash64_unmap_hptes have been moved to mmu-misc.h since they are
> not needed in a !TCG context and mmu-hash64 should not be compiled
> in such situation.
What TCG code do the mmu-* files use? Could we move the TCG-only parts
out of them instead? It just occured to me that you cannot really
exclude mmu-hash64.c and mmu-radix64.c from the KVM build because they
are needed by GDB. At least I'm sure ppc64_v3_get_phys_page_debug is
needed and I suspect that function will end up pulling the whole file
in.
So we might need a different strategy for them.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg
2021-04-30 18:40 ` [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg Lucas Mateus Castro (alqotel)
@ 2021-04-30 23:38 ` Fabiano Rosas
2021-05-03 4:35 ` David Gibson
2021-05-03 4:34 ` David Gibson
1 sibling, 1 reply; 17+ messages in thread
From: Fabiano Rosas @ 2021-04-30 23:38 UTC (permalink / raw)
To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), david
"Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br> writes:
> Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
> is_ram_address), what is the advised way to deal with these
> duplications?
valid_ptex is only needed by the TCG hcalls isn't it?
is_ram_address could in theory stay where it is but be exposed via
hw/ppc/spapr.h since spapr_hcall.c will always be present.
> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> ---
> hw/ppc/meson.build | 3 +
> hw/ppc/spapr_hcall.c | 300 ++--------------------------------
> hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 363 insertions(+), 283 deletions(-)
> create mode 100644 hw/ppc/spapr_hcall_tcg.c
<snip>
> @@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>
> static void hypercall_register_types(void)
> {
> +
> +#ifndef CONFIG_TCG
> /* hcall-pft */
> - spapr_register_hypercall(H_ENTER, h_enter);
> - spapr_register_hypercall(H_REMOVE, h_remove);
> - spapr_register_hypercall(H_PROTECT, h_protect);
> - spapr_register_hypercall(H_READ, h_read);
> + spapr_register_hypercall(H_ENTER, h_tcg_only);
> + spapr_register_hypercall(H_REMOVE, h_tcg_only);
> + spapr_register_hypercall(H_PROTECT, h_tcg_only);
> + spapr_register_hypercall(H_READ, h_tcg_only);
>
> /* hcall-bulk */
> - spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> + spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
> +#endif /* !CONFIG_TCG */
My suggestion for this was:
#ifndef CONFIG_TCG
static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
g_assert_not_reached();
}
static void hypercall_register_tcg(void)
{
spapr_register_hypercall(H_ENTER, h_tcg_only);
spapr_register_hypercall(H_REMOVE, h_tcg_only);
spapr_register_hypercall(H_PROTECT, h_tcg_only);
spapr_register_hypercall(H_READ, h_tcg_only);
(...)
}
#endif
static void hypercall_register_types(void)
{
hypercall_register_tcg();
<register KVM hcalls>
}
type_init(hypercall_register_types);
> +static void hypercall_register_types(void)
> +{
> + /* hcall-pft */
> + spapr_register_hypercall(H_ENTER, h_enter);
> + spapr_register_hypercall(H_REMOVE, h_remove);
> + spapr_register_hypercall(H_PROTECT, h_protect);
> + spapr_register_hypercall(H_READ, h_read);
> +
> + /* hcall-bulk */
> + spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> +}
> +
> +type_init(hypercall_register_types)
And here:
void hypercall_register_tcg(void)
{
/* hcall-pft */
spapr_register_hypercall(H_ENTER, h_enter);
spapr_register_hypercall(H_REMOVE, h_remove);
spapr_register_hypercall(H_PROTECT, h_protect);
spapr_register_hypercall(H_READ, h_read);
(...)
}
Because the TCG and KVM builds are not mutually exlusive, so you would
end up calling type_init twice (which I don't know much about but I
assume is not allowed).
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64
2021-04-30 18:40 ` [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64 Lucas Mateus Castro (alqotel)
2021-04-30 20:19 ` Fabiano Rosas
@ 2021-05-03 4:24 ` David Gibson
2021-05-05 17:30 ` Lucas Mateus Martins Araujo e Castro
1 sibling, 1 reply; 17+ messages in thread
From: David Gibson @ 2021-05-03 4:24 UTC (permalink / raw)
To: Lucas Mateus Castro (alqotel); +Cc: bruno.larsen, qemu-ppc, qemu-devel, farosas
[-- Attachment #1: Type: text/plain, Size: 12764 bytes --]
On Fri, Apr 30, 2021 at 03:40:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
> The functions ppc_store_lpcr, ppc_hash64_filter_pagesizes and
> ppc_hash64_unmap_hptes have been moved to mmu-misc.h since they are
> not needed in a !TCG context and mmu-hash64 should not be compiled
> in such situation.
>
> ppc_store_lpcr and ppc_hash64_filter_pagesizes are used by multiple
> functions, while ppc_hash64_unmap_hptes is used by rehash_hpt (in
> spapr_hcall.c).
Hmm.. looking at it, ppc_store_lpcr() (and helper_store_lpcr()) don't
really belong in this file at all. The LPCR has some things related
to the hash MMU, but plenty of others that don't. So, maybe
misc_helper.c? That might have to be moved again, since misc_helper
itself should probably mostly not be used for !TCG. But.. one thing
at a time.
AFAICT the only user of ppc_hash64_filter_pagesizes() is in
spapr_caps.c. For now you can just move it next to the caller, it's
debatable whether it belongs more to PAPR or MMU code.
ppc_hash64_unmap_hptes() is definitely TCG only and should stay where
it is. The call from rehash_hpt() can be solved because rehash_hpt()
itself is TCG only. I've already suggested splitting the TCG (well,
softmmu) only things out from spapr_hcall.c, so it might simplify
things to tackle that first.
> Also I've put the functions in mmu-misc as I am unsure in which file
> this functions should go, so I just created a new one for now, any
> suggestion which file to put them (considering it's a file that must be
> compiled in a !TCG situation)?
>
> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> ---
> hw/ppc/spapr.c | 1 +
> hw/ppc/spapr_caps.c | 1 +
> hw/ppc/spapr_cpu_core.c | 1 +
> hw/ppc/spapr_hcall.c | 1 +
> 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 +++++++++++
> 10 files changed, 115 insertions(+), 86 deletions(-)
> create mode 100644 target/ppc/mmu-misc.c
> create mode 100644 target/ppc/mmu-misc.h
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e4be00b732..61f8f150c2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -53,6 +53,7 @@
> #include "mmu-book3s-v3.h"
> #include "cpu-models.h"
> #include "hw/core/cpu.h"
> +#include "mmu-misc.h"
>
> #include "hw/boards.h"
> #include "hw/ppc/ppc.h"
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9ea7ddd1e9..22352ff018 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -34,6 +34,7 @@
> #include "kvm_ppc.h"
> #include "migration/vmstate.h"
> #include "sysemu/tcg.h"
> +#include "mmu-misc.h"
>
> #include "hw/ppc/spapr.h"
>
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 4f316a6f9d..f4d93999e5 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -24,6 +24,7 @@
> #include "sysemu/reset.h"
> #include "sysemu/hw_accel.h"
> #include "qemu/error-report.h"
> +#include "mmu-misc.h"
>
> static void spapr_reset_vcpu(PowerPCCPU *cpu)
> {
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7b5cd3553c..4b0ba69841 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -13,6 +13,7 @@
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/spapr_cpu_core.h"
> #include "mmu-hash64.h"
> +#include "mmu-misc.h"
> #include "cpu-models.h"
> #include "trace.h"
> #include "kvm_ppc.h"
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 8a79f9c628..8935b75d1c 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -35,6 +35,7 @@
> #include "sysemu/hw_accel.h"
> #include "sysemu/runstate.h"
> #include "kvm_ppc.h"
> +#include "mmu-misc.h"
>
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/spapr_vio.h"
> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
> index bbfef90e08..7a97648803 100644
> --- a/target/ppc/meson.build
> +++ b/target/ppc/meson.build
> @@ -31,6 +31,7 @@ ppc_softmmu_ss.add(when: 'TARGET_PPC64', if_true: files(
> 'mmu-book3s-v3.c',
> 'mmu-hash64.c',
> 'mmu-radix64.c',
> + 'mmu-misc.c',
> ))
>
> target_arch += {'ppc': ppc_ss}
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 0fabc10302..919a3e9f51 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -30,6 +30,7 @@
> #include "exec/log.h"
> #include "hw/hw.h"
> #include "mmu-book3s-v3.h"
> +#include "mmu-misc.h"
>
> /* #define DEBUG_SLB */
>
> @@ -499,20 +500,6 @@ const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
> return hptes;
> }
>
> -void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
> - hwaddr ptex, int n)
> -{
> - if (cpu->vhyp) {
> - PPCVirtualHypervisorClass *vhc =
> - PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> - vhc->unmap_hptes(cpu->vhyp, hptes, ptex, n);
> - return;
> - }
> -
> - address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
> - false, n * HASH_PTE_SIZE_64);
> -}
> -
> static unsigned hpte_page_shift(const PPCHash64SegmentPageSizes *sps,
> uint64_t pte0, uint64_t pte1)
> {
> @@ -1119,14 +1106,6 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
> cpu->env.tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
> }
>
> -void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> -{
> - PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> - CPUPPCState *env = &cpu->env;
> -
> - env->spr[SPR_LPCR] = val & pcc->lpcr_mask;
> -}
> -
> void helper_store_lpcr(CPUPPCState *env, target_ulong val)
> {
> PowerPCCPU *cpu = env_archcpu(env);
> @@ -1197,61 +1176,3 @@ const PPCHash64Options ppc_hash64_opts_POWER7 = {
> }
> };
>
> -void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
> - bool (*cb)(void *, uint32_t, uint32_t),
> - void *opaque)
> -{
> - PPCHash64Options *opts = cpu->hash64_opts;
> - int i;
> - int n = 0;
> - bool ci_largepage = false;
> -
> - assert(opts);
> -
> - n = 0;
> - for (i = 0; i < ARRAY_SIZE(opts->sps); i++) {
> - PPCHash64SegmentPageSizes *sps = &opts->sps[i];
> - int j;
> - int m = 0;
> -
> - assert(n <= i);
> -
> - if (!sps->page_shift) {
> - break;
> - }
> -
> - for (j = 0; j < ARRAY_SIZE(sps->enc); j++) {
> - PPCHash64PageSize *ps = &sps->enc[j];
> -
> - assert(m <= j);
> - if (!ps->page_shift) {
> - break;
> - }
> -
> - if (cb(opaque, sps->page_shift, ps->page_shift)) {
> - if (ps->page_shift >= 16) {
> - ci_largepage = true;
> - }
> - sps->enc[m++] = *ps;
> - }
> - }
> -
> - /* Clear rest of the row */
> - for (j = m; j < ARRAY_SIZE(sps->enc); j++) {
> - memset(&sps->enc[j], 0, sizeof(sps->enc[j]));
> - }
> -
> - if (m) {
> - n++;
> - }
> - }
> -
> - /* Clear the rest of the table */
> - for (i = n; i < ARRAY_SIZE(opts->sps); i++) {
> - memset(&opts->sps[i], 0, sizeof(opts->sps[i]));
> - }
> -
> - if (!ci_largepage) {
> - opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
> - }
> -}
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 87729d48b3..562602b466 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -15,12 +15,8 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
> target_ulong pte0, target_ulong pte1);
> unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
> uint64_t pte0, uint64_t pte1);
> -void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
> void ppc_hash64_init(PowerPCCPU *cpu);
> void ppc_hash64_finalize(PowerPCCPU *cpu);
> -void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
> - bool (*cb)(void *, uint32_t, uint32_t),
> - void *opaque);
> #endif
>
> /*
> @@ -112,8 +108,6 @@ struct ppc_hash_pte64 {
>
> const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
> hwaddr ptex, int n);
> -void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
> - hwaddr ptex, int n);
>
> static inline uint64_t ppc_hash64_hpte0(PowerPCCPU *cpu,
> const ppc_hash_pte64_t *hptes, int i)
> diff --git a/target/ppc/mmu-misc.c b/target/ppc/mmu-misc.c
> new file mode 100644
> index 0000000000..8abda66547
> --- /dev/null
> +++ b/target/ppc/mmu-misc.c
> @@ -0,0 +1,86 @@
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "mmu-hash64.h"
> +#include "fpu/softfloat-helpers.h"
> +#include "mmu-misc.h"
> +
> +void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> +{
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> + CPUPPCState *env = &cpu->env;
> +
> + env->spr[SPR_LPCR] = val & pcc->lpcr_mask;
> +}
> +
> +void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
> + bool (*cb)(void *, uint32_t, uint32_t),
> + void *opaque)
> +{
> + PPCHash64Options *opts = cpu->hash64_opts;
> + int i;
> + int n = 0;
> + bool ci_largepage = false;
> +
> + assert(opts);
> +
> + n = 0;
> + for (i = 0; i < ARRAY_SIZE(opts->sps); i++) {
> + PPCHash64SegmentPageSizes *sps = &opts->sps[i];
> + int j;
> + int m = 0;
> +
> + assert(n <= i);
> +
> + if (!sps->page_shift) {
> + break;
> + }
> +
> + for (j = 0; j < ARRAY_SIZE(sps->enc); j++) {
> + PPCHash64PageSize *ps = &sps->enc[j];
> +
> + assert(m <= j);
> + if (!ps->page_shift) {
> + break;
> + }
> +
> + if (cb(opaque, sps->page_shift, ps->page_shift)) {
> + if (ps->page_shift >= 16) {
> + ci_largepage = true;
> + }
> + sps->enc[m++] = *ps;
> + }
> + }
> +
> + /* Clear rest of the row */
> + for (j = m; j < ARRAY_SIZE(sps->enc); j++) {
> + memset(&sps->enc[j], 0, sizeof(sps->enc[j]));
> + }
> +
> + if (m) {
> + n++;
> + }
> + }
> +
> + /* Clear the rest of the table */
> + for (i = n; i < ARRAY_SIZE(opts->sps); i++) {
> + memset(&opts->sps[i], 0, sizeof(opts->sps[i]));
> + }
> +
> + if (!ci_largepage) {
> + opts->flags &= ~PPC_HASH64_CI_LARGEPAGE;
> + }
> +}
> +
> +void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
> + hwaddr ptex, int n)
> +{
> + if (cpu->vhyp) {
> + PPCVirtualHypervisorClass *vhc =
> + PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> + vhc->unmap_hptes(cpu->vhyp, hptes, ptex, n);
> + return;
> + }
> +
> + address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
> + false, n * HASH_PTE_SIZE_64);
> +}
> diff --git a/target/ppc/mmu-misc.h b/target/ppc/mmu-misc.h
> new file mode 100644
> index 0000000000..7be6bf7b44
> --- /dev/null
> +++ b/target/ppc/mmu-misc.h
> @@ -0,0 +1,22 @@
> +#ifndef MMU_MISC_H
> +#define MMU_MISC_H
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +
> +#ifndef CONFIG_USER_ONLY
> +
> +#ifdef TARGET_PPC64
> +
> +void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
> +void ppc_hash64_filter_pagesizes(PowerPCCPU *cpu,
> + bool (*cb)(void *, uint32_t, uint32_t),
> + void *opaque);
> +
> +#endif
> +
> +void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
> + hwaddr ptex, int n);
> +
> +#endif
> +
> +#endif
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg
2021-04-30 18:40 ` [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg Lucas Mateus Castro (alqotel)
2021-04-30 23:38 ` Fabiano Rosas
@ 2021-05-03 4:34 ` David Gibson
2021-05-04 18:14 ` Lucas Mateus Martins Araujo e Castro
1 sibling, 1 reply; 17+ messages in thread
From: David Gibson @ 2021-05-03 4:34 UTC (permalink / raw)
To: Lucas Mateus Castro (alqotel); +Cc: bruno.larsen, qemu-ppc, qemu-devel, farosas
[-- Attachment #1: Type: text/plain, Size: 26058 bytes --]
On Fri, Apr 30, 2021 at 03:40:47PM -0300, Lucas Mateus Castro (alqotel) wrote:
> Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
> h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
> environment in spapr_hcall.c and changed build options to only build
> spapr_hcall_tcg.c when CONFIG_TCG is enabled.
This looks good. I'd suggest the name 'spapr_softmmu.c' instead,
which more specifically describes what's special about these
functions.
h_resize_hpt_prepare(), h_resize_hpt_commit() and the functions they
depend on belong in the softmmu set as well.
> Added the function h_tcg_only to be used for hypercalls that should be
> called only in TCG environment but have been called in a TCG-less
> one.
Again, 'h_softmmu' would be a better name.
>
> Right now, a #ifndef is used to know if there is a need of a h_tcg_only
> function to be implemented and used as hypercalls, I initially thought
> of always having that option turned on and having spapr_hcall_tcg.c
> overwrite those hypercalls when TCG is enabled, but
> spapr_register_hypercalls checks if a hypercall already exists for that
> opcode so that wouldn't work, so if anyone has any suggestions I'm
> interested.
The ifdef is fine. We don't want to litter the code with them, but a
few is fine. Especially in this context where it's pretty clearly
just excluding some things from a simple list of calls.
>
> Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
> is_ram_address), what is the advised way to deal with these
> duplications?
valid_ptex() is only used by softmmu functions that are moving, so it
should travel with them, no need for duplication. is_ram_address() is
also used by h_page_init() which is also needed in !TCG code. So,
leave it in spapr_hcall.c and just export it for use in the TCG only
code.
>
> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> ---
> hw/ppc/meson.build | 3 +
> hw/ppc/spapr_hcall.c | 300 ++--------------------------------
> hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 363 insertions(+), 283 deletions(-)
> create mode 100644 hw/ppc/spapr_hcall_tcg.c
>
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index 218631c883..3c7f2f08b7 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -29,6 +29,9 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
> 'spapr_numa.c',
> 'pef.c',
> ))
> +ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
> + 'spapr_hcall_tcg.c'
> +))
> ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
> ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
> 'spapr_pci_vfio.c',
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4b0ba69841..b37fbdc32e 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -22,6 +22,15 @@
> #include "mmu-book3s-v3.h"
> #include "hw/mem/memory-device.h"
>
> +#ifndef CONFIG_TCG
> +static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + g_assert_not_reached();
> + return 0;
> +}
> +#endif /* !CONFIG_TCG */
> +
> static bool has_spr(PowerPCCPU *cpu, int spr)
> {
> /* We can test whether the SPR is defined by checking for a valid name */
> @@ -55,284 +64,6 @@ static bool is_ram_address(SpaprMachineState *spapr, hwaddr addr)
> return false;
> }
>
> -static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
> - target_ulong opcode, target_ulong *args)
> -{
> - target_ulong flags = args[0];
> - target_ulong ptex = args[1];
> - target_ulong pteh = args[2];
> - target_ulong ptel = args[3];
> - unsigned apshift;
> - target_ulong raddr;
> - target_ulong slot;
> - const ppc_hash_pte64_t *hptes;
> -
> - apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
> - if (!apshift) {
> - /* Bad page size encoding */
> - return H_PARAMETER;
> - }
> -
> - raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
> -
> - if (is_ram_address(spapr, raddr)) {
> - /* Regular RAM - should have WIMG=0010 */
> - if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
> - return H_PARAMETER;
> - }
> - } else {
> - target_ulong wimg_flags;
> - /* Looks like an IO address */
> - /* FIXME: What WIMG combinations could be sensible for IO?
> - * For now we allow WIMG=010x, but are there others? */
> - /* FIXME: Should we check against registered IO addresses? */
> - wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
> -
> - if (wimg_flags != HPTE64_R_I &&
> - wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
> - return H_PARAMETER;
> - }
> - }
> -
> - pteh &= ~0x60ULL;
> -
> - if (!valid_ptex(cpu, ptex)) {
> - return H_PARAMETER;
> - }
> -
> - slot = ptex & 7ULL;
> - ptex = ptex & ~7ULL;
> -
> - if (likely((flags & H_EXACT) == 0)) {
> - hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
> - for (slot = 0; slot < 8; slot++) {
> - if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
> - break;
> - }
> - }
> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
> - if (slot == 8) {
> - return H_PTEG_FULL;
> - }
> - } else {
> - hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
> - if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
> - ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
> - return H_PTEG_FULL;
> - }
> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> - }
> -
> - spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
> -
> - args[0] = ptex + slot;
> - return H_SUCCESS;
> -}
> -
> -typedef enum {
> - REMOVE_SUCCESS = 0,
> - REMOVE_NOT_FOUND = 1,
> - REMOVE_PARM = 2,
> - REMOVE_HW = 3,
> -} RemoveResult;
> -
> -static RemoveResult remove_hpte(PowerPCCPU *cpu
> - , target_ulong ptex,
> - target_ulong avpn,
> - target_ulong flags,
> - target_ulong *vp, target_ulong *rp)
> -{
> - const ppc_hash_pte64_t *hptes;
> - target_ulong v, r;
> -
> - if (!valid_ptex(cpu, ptex)) {
> - return REMOVE_PARM;
> - }
> -
> - hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
> - v = ppc_hash64_hpte0(cpu, hptes, 0);
> - r = ppc_hash64_hpte1(cpu, hptes, 0);
> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> -
> - if ((v & HPTE64_V_VALID) == 0 ||
> - ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
> - ((flags & H_ANDCOND) && (v & avpn) != 0)) {
> - return REMOVE_NOT_FOUND;
> - }
> - *vp = v;
> - *rp = r;
> - spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
> - ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
> - return REMOVE_SUCCESS;
> -}
> -
> -static target_ulong h_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
> - target_ulong opcode, target_ulong *args)
> -{
> - CPUPPCState *env = &cpu->env;
> - target_ulong flags = args[0];
> - target_ulong ptex = args[1];
> - target_ulong avpn = args[2];
> - RemoveResult ret;
> -
> - ret = remove_hpte(cpu, ptex, avpn, flags,
> - &args[0], &args[1]);
> -
> - switch (ret) {
> - case REMOVE_SUCCESS:
> - check_tlb_flush(env, true);
> - return H_SUCCESS;
> -
> - case REMOVE_NOT_FOUND:
> - return H_NOT_FOUND;
> -
> - case REMOVE_PARM:
> - return H_PARAMETER;
> -
> - case REMOVE_HW:
> - return H_HARDWARE;
> - }
> -
> - g_assert_not_reached();
> -}
> -
> -#define H_BULK_REMOVE_TYPE 0xc000000000000000ULL
> -#define H_BULK_REMOVE_REQUEST 0x4000000000000000ULL
> -#define H_BULK_REMOVE_RESPONSE 0x8000000000000000ULL
> -#define H_BULK_REMOVE_END 0xc000000000000000ULL
> -#define H_BULK_REMOVE_CODE 0x3000000000000000ULL
> -#define H_BULK_REMOVE_SUCCESS 0x0000000000000000ULL
> -#define H_BULK_REMOVE_NOT_FOUND 0x1000000000000000ULL
> -#define H_BULK_REMOVE_PARM 0x2000000000000000ULL
> -#define H_BULK_REMOVE_HW 0x3000000000000000ULL
> -#define H_BULK_REMOVE_RC 0x0c00000000000000ULL
> -#define H_BULK_REMOVE_FLAGS 0x0300000000000000ULL
> -#define H_BULK_REMOVE_ABSOLUTE 0x0000000000000000ULL
> -#define H_BULK_REMOVE_ANDCOND 0x0100000000000000ULL
> -#define H_BULK_REMOVE_AVPN 0x0200000000000000ULL
> -#define H_BULK_REMOVE_PTEX 0x00ffffffffffffffULL
> -
> -#define H_BULK_REMOVE_MAX_BATCH 4
> -
> -static target_ulong h_bulk_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
> - target_ulong opcode, target_ulong *args)
> -{
> - CPUPPCState *env = &cpu->env;
> - int i;
> - target_ulong rc = H_SUCCESS;
> -
> - for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
> - target_ulong *tsh = &args[i*2];
> - target_ulong tsl = args[i*2 + 1];
> - target_ulong v, r, ret;
> -
> - if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
> - break;
> - } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
> - return H_PARAMETER;
> - }
> -
> - *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
> - *tsh |= H_BULK_REMOVE_RESPONSE;
> -
> - if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
> - *tsh |= H_BULK_REMOVE_PARM;
> - return H_PARAMETER;
> - }
> -
> - ret = remove_hpte(cpu, *tsh & H_BULK_REMOVE_PTEX, tsl,
> - (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
> - &v, &r);
> -
> - *tsh |= ret << 60;
> -
> - switch (ret) {
> - case REMOVE_SUCCESS:
> - *tsh |= (r & (HPTE64_R_C | HPTE64_R_R)) << 43;
> - break;
> -
> - case REMOVE_PARM:
> - rc = H_PARAMETER;
> - goto exit;
> -
> - case REMOVE_HW:
> - rc = H_HARDWARE;
> - goto exit;
> - }
> - }
> - exit:
> - check_tlb_flush(env, true);
> -
> - return rc;
> -}
> -
> -static target_ulong h_protect(PowerPCCPU *cpu, SpaprMachineState *spapr,
> - target_ulong opcode, target_ulong *args)
> -{
> - CPUPPCState *env = &cpu->env;
> - target_ulong flags = args[0];
> - target_ulong ptex = args[1];
> - target_ulong avpn = args[2];
> - const ppc_hash_pte64_t *hptes;
> - target_ulong v, r;
> -
> - if (!valid_ptex(cpu, ptex)) {
> - return H_PARAMETER;
> - }
> -
> - hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
> - v = ppc_hash64_hpte0(cpu, hptes, 0);
> - r = ppc_hash64_hpte1(cpu, hptes, 0);
> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> -
> - if ((v & HPTE64_V_VALID) == 0 ||
> - ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
> - return H_NOT_FOUND;
> - }
> -
> - r &= ~(HPTE64_R_PP0 | HPTE64_R_PP | HPTE64_R_N |
> - HPTE64_R_KEY_HI | HPTE64_R_KEY_LO);
> - r |= (flags << 55) & HPTE64_R_PP0;
> - r |= (flags << 48) & HPTE64_R_KEY_HI;
> - r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
> - spapr_store_hpte(cpu, ptex,
> - (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
> - ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
> - /* Flush the tlb */
> - check_tlb_flush(env, true);
> - /* Don't need a memory barrier, due to qemu's global lock */
> - spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
> - return H_SUCCESS;
> -}
> -
> -static target_ulong h_read(PowerPCCPU *cpu, SpaprMachineState *spapr,
> - target_ulong opcode, target_ulong *args)
> -{
> - target_ulong flags = args[0];
> - target_ulong ptex = args[1];
> - int i, ridx, n_entries = 1;
> - const ppc_hash_pte64_t *hptes;
> -
> - if (!valid_ptex(cpu, ptex)) {
> - return H_PARAMETER;
> - }
> -
> - if (flags & H_READ_4) {
> - /* Clear the two low order bits */
> - ptex &= ~(3ULL);
> - n_entries = 4;
> - }
> -
> - hptes = ppc_hash64_map_hptes(cpu, ptex, n_entries);
> - for (i = 0, ridx = 0; i < n_entries; i++) {
> - args[ridx++] = ppc_hash64_hpte0(cpu, hptes, i);
> - args[ridx++] = ppc_hash64_hpte1(cpu, hptes, i);
> - }
> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, n_entries);
> -
> - return H_SUCCESS;
> -}
> -
> struct SpaprPendingHpt {
> /* These fields are read-only after initialization */
> int shift;
> @@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>
> static void hypercall_register_types(void)
> {
> +
> +#ifndef CONFIG_TCG
> /* hcall-pft */
> - spapr_register_hypercall(H_ENTER, h_enter);
> - spapr_register_hypercall(H_REMOVE, h_remove);
> - spapr_register_hypercall(H_PROTECT, h_protect);
> - spapr_register_hypercall(H_READ, h_read);
> + spapr_register_hypercall(H_ENTER, h_tcg_only);
> + spapr_register_hypercall(H_REMOVE, h_tcg_only);
> + spapr_register_hypercall(H_PROTECT, h_tcg_only);
> + spapr_register_hypercall(H_READ, h_tcg_only);
>
> /* hcall-bulk */
> - spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> + spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
> +#endif /* !CONFIG_TCG */
>
> /* hcall-hpt-resize */
> spapr_register_hypercall(H_RESIZE_HPT_PREPARE, h_resize_hpt_prepare);
> diff --git a/hw/ppc/spapr_hcall_tcg.c b/hw/ppc/spapr_hcall_tcg.c
> new file mode 100644
> index 0000000000..92ff24c8dc
> --- /dev/null
> +++ b/hw/ppc/spapr_hcall_tcg.c
> @@ -0,0 +1,343 @@
> +#include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> +#include "qapi/error.h"
> +#include "sysemu/hw_accel.h"
> +#include "sysemu/runstate.h"
> +#include "qemu/log.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/module.h"
> +#include "qemu/error-report.h"
> +#include "cpu.h"
> +#include "exec/exec-all.h"
> +#include "helper_regs.h"
> +#include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_cpu_core.h"
> +#include "mmu-hash64.h"
> +#include "mmu-misc.h"
> +#include "cpu-models.h"
> +#include "trace.h"
> +#include "kvm_ppc.h"
> +#include "hw/ppc/fdt.h"
> +#include "hw/ppc/spapr_ovec.h"
> +#include "mmu-book3s-v3.h"
> +#include "hw/mem/memory-device.h"
> +
> +static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
> +{
> + /*
> + * hash value/pteg group index is normalized by HPT mask
> + */
> + if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~ppc_hash64_hpt_mask(cpu)) {
> + return false;
> + }
> + return true;
> +}
> +
> +static bool is_ram_address(SpaprMachineState *spapr, hwaddr addr)
> +{
> + MachineState *machine = MACHINE(spapr);
> + DeviceMemoryState *dms = machine->device_memory;
> +
> + if (addr < machine->ram_size) {
> + return true;
> + }
> + if ((addr >= dms->base)
> + && ((addr - dms->base) < memory_region_size(&dms->mr))) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + target_ulong flags = args[0];
> + target_ulong ptex = args[1];
> + target_ulong pteh = args[2];
> + target_ulong ptel = args[3];
> + unsigned apshift;
> + target_ulong raddr;
> + target_ulong slot;
> + const ppc_hash_pte64_t *hptes;
> +
> + apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
> + if (!apshift) {
> + /* Bad page size encoding */
> + return H_PARAMETER;
> + }
> +
> + raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
> +
> + if (is_ram_address(spapr, raddr)) {
> + /* Regular RAM - should have WIMG=0010 */
> + if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
> + return H_PARAMETER;
> + }
> + } else {
> + target_ulong wimg_flags;
> + /* Looks like an IO address */
> + /* FIXME: What WIMG combinations could be sensible for IO?
> + * For now we allow WIMG=010x, but are there others? */
> + /* FIXME: Should we check against registered IO addresses? */
> + wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
> +
> + if (wimg_flags != HPTE64_R_I &&
> + wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
> + return H_PARAMETER;
> + }
> + }
> +
> + pteh &= ~0x60ULL;
> +
> + if (!valid_ptex(cpu, ptex)) {
> + return H_PARAMETER;
> + }
> +
> + slot = ptex & 7ULL;
> + ptex = ptex & ~7ULL;
> +
> + if (likely((flags & H_EXACT) == 0)) {
> + hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
> + for (slot = 0; slot < 8; slot++) {
> + if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
> + break;
> + }
> + }
> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
> + if (slot == 8) {
> + return H_PTEG_FULL;
> + }
> + } else {
> + hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
> + if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
> + ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
> + return H_PTEG_FULL;
> + }
> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> + }
> +
> + spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
> +
> + args[0] = ptex + slot;
> + return H_SUCCESS;
> +}
> +
> +typedef enum {
> + REMOVE_SUCCESS = 0,
> + REMOVE_NOT_FOUND = 1,
> + REMOVE_PARM = 2,
> + REMOVE_HW = 3,
> +} RemoveResult;
> +
> +static RemoveResult remove_hpte(PowerPCCPU *cpu
> + , target_ulong ptex,
> + target_ulong avpn,
> + target_ulong flags,
> + target_ulong *vp, target_ulong *rp)
> +{
> + const ppc_hash_pte64_t *hptes;
> + target_ulong v, r;
> +
> + if (!valid_ptex(cpu, ptex)) {
> + return REMOVE_PARM;
> + }
> +
> + hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
> + v = ppc_hash64_hpte0(cpu, hptes, 0);
> + r = ppc_hash64_hpte1(cpu, hptes, 0);
> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> +
> + if ((v & HPTE64_V_VALID) == 0 ||
> + ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
> + ((flags & H_ANDCOND) && (v & avpn) != 0)) {
> + return REMOVE_NOT_FOUND;
> + }
> + *vp = v;
> + *rp = r;
> + spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
> + ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
> + return REMOVE_SUCCESS;
> +}
> +
> +static target_ulong h_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + CPUPPCState *env = &cpu->env;
> + target_ulong flags = args[0];
> + target_ulong ptex = args[1];
> + target_ulong avpn = args[2];
> + RemoveResult ret;
> +
> + ret = remove_hpte(cpu, ptex, avpn, flags,
> + &args[0], &args[1]);
> +
> + switch (ret) {
> + case REMOVE_SUCCESS:
> + check_tlb_flush(env, true);
> + return H_SUCCESS;
> +
> + case REMOVE_NOT_FOUND:
> + return H_NOT_FOUND;
> +
> + case REMOVE_PARM:
> + return H_PARAMETER;
> +
> + case REMOVE_HW:
> + return H_HARDWARE;
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +#define H_BULK_REMOVE_TYPE 0xc000000000000000ULL
> +#define H_BULK_REMOVE_REQUEST 0x4000000000000000ULL
> +#define H_BULK_REMOVE_RESPONSE 0x8000000000000000ULL
> +#define H_BULK_REMOVE_END 0xc000000000000000ULL
> +#define H_BULK_REMOVE_CODE 0x3000000000000000ULL
> +#define H_BULK_REMOVE_SUCCESS 0x0000000000000000ULL
> +#define H_BULK_REMOVE_NOT_FOUND 0x1000000000000000ULL
> +#define H_BULK_REMOVE_PARM 0x2000000000000000ULL
> +#define H_BULK_REMOVE_HW 0x3000000000000000ULL
> +#define H_BULK_REMOVE_RC 0x0c00000000000000ULL
> +#define H_BULK_REMOVE_FLAGS 0x0300000000000000ULL
> +#define H_BULK_REMOVE_ABSOLUTE 0x0000000000000000ULL
> +#define H_BULK_REMOVE_ANDCOND 0x0100000000000000ULL
> +#define H_BULK_REMOVE_AVPN 0x0200000000000000ULL
> +#define H_BULK_REMOVE_PTEX 0x00ffffffffffffffULL
> +
> +#define H_BULK_REMOVE_MAX_BATCH 4
> +
> +static target_ulong h_bulk_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + CPUPPCState *env = &cpu->env;
> + int i;
> + target_ulong rc = H_SUCCESS;
> +
> + for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
> + target_ulong *tsh = &args[i*2];
> + target_ulong tsl = args[i*2 + 1];
> + target_ulong v, r, ret;
> +
> + if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
> + break;
> + } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
> + return H_PARAMETER;
> + }
> +
> + *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
> + *tsh |= H_BULK_REMOVE_RESPONSE;
> +
> + if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
> + *tsh |= H_BULK_REMOVE_PARM;
> + return H_PARAMETER;
> + }
> +
> + ret = remove_hpte(cpu, *tsh & H_BULK_REMOVE_PTEX, tsl,
> + (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
> + &v, &r);
> +
> + *tsh |= ret << 60;
> +
> + switch (ret) {
> + case REMOVE_SUCCESS:
> + *tsh |= (r & (HPTE64_R_C | HPTE64_R_R)) << 43;
> + break;
> +
> + case REMOVE_PARM:
> + rc = H_PARAMETER;
> + goto exit;
> +
> + case REMOVE_HW:
> + rc = H_HARDWARE;
> + goto exit;
> + }
> + }
> + exit:
> + check_tlb_flush(env, true);
> +
> + return rc;
> +}
> +
> +static target_ulong h_protect(PowerPCCPU *cpu, SpaprMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + CPUPPCState *env = &cpu->env;
> + target_ulong flags = args[0];
> + target_ulong ptex = args[1];
> + target_ulong avpn = args[2];
> + const ppc_hash_pte64_t *hptes;
> + target_ulong v, r;
> +
> + if (!valid_ptex(cpu, ptex)) {
> + return H_PARAMETER;
> + }
> +
> + hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
> + v = ppc_hash64_hpte0(cpu, hptes, 0);
> + r = ppc_hash64_hpte1(cpu, hptes, 0);
> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> +
> + if ((v & HPTE64_V_VALID) == 0 ||
> + ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
> + return H_NOT_FOUND;
> + }
> +
> + r &= ~(HPTE64_R_PP0 | HPTE64_R_PP | HPTE64_R_N |
> + HPTE64_R_KEY_HI | HPTE64_R_KEY_LO);
> + r |= (flags << 55) & HPTE64_R_PP0;
> + r |= (flags << 48) & HPTE64_R_KEY_HI;
> + r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
> + spapr_store_hpte(cpu, ptex,
> + (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
> + ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
> + /* Flush the tlb */
> + check_tlb_flush(env, true);
> + /* Don't need a memory barrier, due to qemu's global lock */
> + spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
> + return H_SUCCESS;
> +}
> +
> +static target_ulong h_read(PowerPCCPU *cpu, SpaprMachineState *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> + target_ulong flags = args[0];
> + target_ulong ptex = args[1];
> + int i, ridx, n_entries = 1;
> + const ppc_hash_pte64_t *hptes;
> +
> + if (!valid_ptex(cpu, ptex)) {
> + return H_PARAMETER;
> + }
> +
> + if (flags & H_READ_4) {
> + /* Clear the two low order bits */
> + ptex &= ~(3ULL);
> + n_entries = 4;
> + }
> +
> + hptes = ppc_hash64_map_hptes(cpu, ptex, n_entries);
> + for (i = 0, ridx = 0; i < n_entries; i++) {
> + args[ridx++] = ppc_hash64_hpte0(cpu, hptes, i);
> + args[ridx++] = ppc_hash64_hpte1(cpu, hptes, i);
> + }
> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, n_entries);
> +
> + return H_SUCCESS;
> +}
> +
> +
> +static void hypercall_register_types(void)
> +{
> + /* hcall-pft */
> + spapr_register_hypercall(H_ENTER, h_enter);
> + spapr_register_hypercall(H_REMOVE, h_remove);
> + spapr_register_hypercall(H_PROTECT, h_protect);
> + spapr_register_hypercall(H_READ, h_read);
> +
> + /* hcall-bulk */
> + spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> +}
> +
> +type_init(hypercall_register_types)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg
2021-04-30 23:38 ` Fabiano Rosas
@ 2021-05-03 4:35 ` David Gibson
0 siblings, 0 replies; 17+ messages in thread
From: David Gibson @ 2021-05-03 4:35 UTC (permalink / raw)
To: Fabiano Rosas
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]
On Fri, Apr 30, 2021 at 08:38:05PM -0300, Fabiano Rosas wrote:
> "Lucas Mateus Castro (alqotel)" <lucas.araujo@eldorado.org.br> writes:
>
> > Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
> > is_ram_address), what is the advised way to deal with these
> > duplications?
>
> valid_ptex is only needed by the TCG hcalls isn't it?
>
> is_ram_address could in theory stay where it is but be exposed via
> hw/ppc/spapr.h since spapr_hcall.c will always be present.
>
> > Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> > ---
> > hw/ppc/meson.build | 3 +
> > hw/ppc/spapr_hcall.c | 300 ++--------------------------------
> > hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 363 insertions(+), 283 deletions(-)
> > create mode 100644 hw/ppc/spapr_hcall_tcg.c
>
> <snip>
>
> > @@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
> >
> > static void hypercall_register_types(void)
> > {
> > +
> > +#ifndef CONFIG_TCG
> > /* hcall-pft */
> > - spapr_register_hypercall(H_ENTER, h_enter);
> > - spapr_register_hypercall(H_REMOVE, h_remove);
> > - spapr_register_hypercall(H_PROTECT, h_protect);
> > - spapr_register_hypercall(H_READ, h_read);
> > + spapr_register_hypercall(H_ENTER, h_tcg_only);
> > + spapr_register_hypercall(H_REMOVE, h_tcg_only);
> > + spapr_register_hypercall(H_PROTECT, h_tcg_only);
> > + spapr_register_hypercall(H_READ, h_tcg_only);
> >
> > /* hcall-bulk */
> > - spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> > + spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
> > +#endif /* !CONFIG_TCG */
>
> My suggestion for this was:
>
> #ifndef CONFIG_TCG
> static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
> target_ulong opcode, target_ulong *args)
> {
> g_assert_not_reached();
> }
>
> static void hypercall_register_tcg(void)
> {
> spapr_register_hypercall(H_ENTER, h_tcg_only);
> spapr_register_hypercall(H_REMOVE, h_tcg_only);
> spapr_register_hypercall(H_PROTECT, h_tcg_only);
> spapr_register_hypercall(H_READ, h_tcg_only);
> (...)
> }
> #endif
>
> static void hypercall_register_types(void)
> {
> hypercall_register_tcg();
>
> <register KVM hcalls>
> }
> type_init(hypercall_register_types);
Eh, swings and roundabouts. Either of these approaches is fine.
> > +static void hypercall_register_types(void)
> > +{
> > + /* hcall-pft */
> > + spapr_register_hypercall(H_ENTER, h_enter);
> > + spapr_register_hypercall(H_REMOVE, h_remove);
> > + spapr_register_hypercall(H_PROTECT, h_protect);
> > + spapr_register_hypercall(H_READ, h_read);
> > +
> > + /* hcall-bulk */
> > + spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
> > +}
> > +
> > +type_init(hypercall_register_types)
>
> And here:
>
> void hypercall_register_tcg(void)
> {
> /* hcall-pft */
> spapr_register_hypercall(H_ENTER, h_enter);
> spapr_register_hypercall(H_REMOVE, h_remove);
> spapr_register_hypercall(H_PROTECT, h_protect);
> spapr_register_hypercall(H_READ, h_read);
> (...)
> }
>
> Because the TCG and KVM builds are not mutually exlusive, so you would
> end up calling type_init twice (which I don't know much about but I
> assume is not allowed).
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
2021-04-30 18:40 [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG Lucas Mateus Castro (alqotel)
` (2 preceding siblings ...)
2021-04-30 18:54 ` [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG no-reply
@ 2021-05-03 22:21 ` Fabiano Rosas
2021-05-04 14:43 ` Bruno Piazera Larsen
` (2 more replies)
3 siblings, 3 replies; 17+ messages in thread
From: Fabiano Rosas @ 2021-05-03 22:21 UTC (permalink / raw)
To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), david
"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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
2021-05-03 22:21 ` Fabiano Rosas
@ 2021-05-04 14:43 ` Bruno Piazera Larsen
2021-05-04 15:57 ` Lucas Mateus Martins Araujo e Castro
2021-05-05 4:42 ` David Gibson
2 siblings, 0 replies; 17+ messages in thread
From: Bruno Piazera Larsen @ 2021-05-04 14:43 UTC (permalink / raw)
To: Fabiano Rosas, Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc; +Cc: david
[-- Attachment #1: Type: text/plain, Size: 5963 bytes --]
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
--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
[-- Attachment #2: Type: text/html, Size: 6593 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
2021-05-03 22:21 ` Fabiano Rosas
2021-05-04 14:43 ` Bruno Piazera Larsen
@ 2021-05-04 15:57 ` Lucas Mateus Martins Araujo e Castro
2021-05-05 4:42 ` David Gibson
2 siblings, 0 replies; 17+ messages in thread
From: Lucas Mateus Martins Araujo e Castro @ 2021-05-04 15:57 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel, qemu-ppc; +Cc: Bruno Piazera Larsen, david
[-- Attachment #1: Type: text/plain, Size: 6123 bytes --]
Thanks, it will be quite helpful.
Also, I agree with Bruno including this information somewhere would be quite good in my opinion.
________________________________
From: Fabiano Rosas <farosas@linux.ibm.com>
Sent: Monday, May 3, 2021 7:21 PM
To: Lucas Mateus Martins Araujo e Castro <lucas.araujo@eldorado.org.br>; qemu-devel@nongnu.org <qemu-devel@nongnu.org>; qemu-ppc@nongnu.org <qemu-ppc@nongnu.org>
Cc: Bruno Piazera Larsen <bruno.larsen@eldorado.org.br>; Lucas Mateus Martins Araujo e Castro <lucas.araujo@eldorado.org.br>; david@gibson.dropbear.id.au <david@gibson.dropbear.id.au>
Subject: Re: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
"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.
[-- Attachment #2: Type: text/html, Size: 11077 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg
2021-05-03 4:34 ` David Gibson
@ 2021-05-04 18:14 ` Lucas Mateus Martins Araujo e Castro
2021-05-05 4:58 ` David Gibson
0 siblings, 1 reply; 17+ messages in thread
From: Lucas Mateus Martins Araujo e Castro @ 2021-05-04 18:14 UTC (permalink / raw)
To: David Gibson; +Cc: bruno.larsen, qemu-ppc, qemu-devel, farosas
[-- Attachment #1: Type: text/plain, Size: 26560 bytes --]
On 03/05/2021 01:34, David Gibson wrote:
> On Fri, Apr 30, 2021 at 03:40:47PM -0300, Lucas Mateus Castro (alqotel) wrote:
>> Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
>> h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
>> environment in spapr_hcall.c and changed build options to only build
>> spapr_hcall_tcg.c when CONFIG_TCG is enabled.
> This looks good. I'd suggest the name 'spapr_softmmu.c' instead,
> which more specifically describes what's special about these
> functions.
>
> h_resize_hpt_prepare(), h_resize_hpt_commit() and the functions they
> depend on belong in the softmmu set as well.
Moved these hcalls to spapr_softmmu.c as well, along with the most
functions they depend on, but 1 function (push_sregs_to_kvm_pr) is
also used by hcalls in spapr_hcall.c, so for now I'm just leaving it in
spapr_hcall.c and exporting it to be used in spapr_softmmu.c.
On a related note, from what I've seen h_resize_hpt_prepare and
h_resize_hpt_commit are not implementede in KVM, so they're only
called when there's softmmu so that's why they can be moved to
spapr_softmmu.c?
>> Added the function h_tcg_only to be used for hypercalls that should be
>> called only in TCG environment but have been called in a TCG-less
>> one.
> Again, 'h_softmmu' would be a better name.
Ok, I will use that name.
>> Right now, a #ifndef is used to know if there is a need of a h_tcg_only
>> function to be implemented and used as hypercalls, I initially thought
>> of always having that option turned on and having spapr_hcall_tcg.c
>> overwrite those hypercalls when TCG is enabled, but
>> spapr_register_hypercalls checks if a hypercall already exists for that
>> opcode so that wouldn't work, so if anyone has any suggestions I'm
>> interested.
> The ifdef is fine. We don't want to litter the code with them, but a
> few is fine. Especially in this context where it's pretty clearly
> just excluding some things from a simple list of calls.
>
>> Also spapr_hcall_tcg.c only has 2 duplicated functions (valid_ptex and
>> is_ram_address), what is the advised way to deal with these
>> duplications?
> valid_ptex() is only used by softmmu functions that are moving, so it
> should travel with them, no need for duplication. is_ram_address() is
> also used by h_page_init() which is also needed in !TCG code. So,
> leave it in spapr_hcall.c and just export it for use in the TCG only
> code.
>
On both is_ram_address and push_sregs_to_kvm_pr I exported them
by adding their prototypes to spapr.h and made them non-static.
>> Signed-off-by: Lucas Mateus Castro (alqotel)<lucas.araujo@eldorado.org.br>
>> ---
>> hw/ppc/meson.build | 3 +
>> hw/ppc/spapr_hcall.c | 300 ++--------------------------------
>> hw/ppc/spapr_hcall_tcg.c | 343 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 363 insertions(+), 283 deletions(-)
>> create mode 100644 hw/ppc/spapr_hcall_tcg.c
>>
>> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
>> index 218631c883..3c7f2f08b7 100644
>> --- a/hw/ppc/meson.build
>> +++ b/hw/ppc/meson.build
>> @@ -29,6 +29,9 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
>> 'spapr_numa.c',
>> 'pef.c',
>> ))
>> +ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
>> + 'spapr_hcall_tcg.c'
>> +))
>> ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>> ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
>> 'spapr_pci_vfio.c',
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 4b0ba69841..b37fbdc32e 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -22,6 +22,15 @@
>> #include "mmu-book3s-v3.h"
>> #include "hw/mem/memory-device.h"
>>
>> +#ifndef CONFIG_TCG
>> +static target_ulong h_tcg_only(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> + g_assert_not_reached();
>> + return 0;
>> +}
>> +#endif /* !CONFIG_TCG */
>> +
>> static bool has_spr(PowerPCCPU *cpu, int spr)
>> {
>> /* We can test whether the SPR is defined by checking for a valid name */
>> @@ -55,284 +64,6 @@ static bool is_ram_address(SpaprMachineState *spapr, hwaddr addr)
>> return false;
>> }
>>
>> -static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> - target_ulong opcode, target_ulong *args)
>> -{
>> - target_ulong flags = args[0];
>> - target_ulong ptex = args[1];
>> - target_ulong pteh = args[2];
>> - target_ulong ptel = args[3];
>> - unsigned apshift;
>> - target_ulong raddr;
>> - target_ulong slot;
>> - const ppc_hash_pte64_t *hptes;
>> -
>> - apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
>> - if (!apshift) {
>> - /* Bad page size encoding */
>> - return H_PARAMETER;
>> - }
>> -
>> - raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
>> -
>> - if (is_ram_address(spapr, raddr)) {
>> - /* Regular RAM - should have WIMG=0010 */
>> - if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
>> - return H_PARAMETER;
>> - }
>> - } else {
>> - target_ulong wimg_flags;
>> - /* Looks like an IO address */
>> - /* FIXME: What WIMG combinations could be sensible for IO?
>> - * For now we allow WIMG=010x, but are there others? */
>> - /* FIXME: Should we check against registered IO addresses? */
>> - wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
>> -
>> - if (wimg_flags != HPTE64_R_I &&
>> - wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
>> - return H_PARAMETER;
>> - }
>> - }
>> -
>> - pteh &= ~0x60ULL;
>> -
>> - if (!valid_ptex(cpu, ptex)) {
>> - return H_PARAMETER;
>> - }
>> -
>> - slot = ptex & 7ULL;
>> - ptex = ptex & ~7ULL;
>> -
>> - if (likely((flags & H_EXACT) == 0)) {
>> - hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
>> - for (slot = 0; slot < 8; slot++) {
>> - if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
>> - break;
>> - }
>> - }
>> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
>> - if (slot == 8) {
>> - return H_PTEG_FULL;
>> - }
>> - } else {
>> - hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
>> - if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
>> - ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
>> - return H_PTEG_FULL;
>> - }
>> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
>> - }
>> -
>> - spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
>> -
>> - args[0] = ptex + slot;
>> - return H_SUCCESS;
>> -}
>> -
>> -typedef enum {
>> - REMOVE_SUCCESS = 0,
>> - REMOVE_NOT_FOUND = 1,
>> - REMOVE_PARM = 2,
>> - REMOVE_HW = 3,
>> -} RemoveResult;
>> -
>> -static RemoveResult remove_hpte(PowerPCCPU *cpu
>> - , target_ulong ptex,
>> - target_ulong avpn,
>> - target_ulong flags,
>> - target_ulong *vp, target_ulong *rp)
>> -{
>> - const ppc_hash_pte64_t *hptes;
>> - target_ulong v, r;
>> -
>> - if (!valid_ptex(cpu, ptex)) {
>> - return REMOVE_PARM;
>> - }
>> -
>> - hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
>> - v = ppc_hash64_hpte0(cpu, hptes, 0);
>> - r = ppc_hash64_hpte1(cpu, hptes, 0);
>> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
>> -
>> - if ((v & HPTE64_V_VALID) == 0 ||
>> - ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
>> - ((flags & H_ANDCOND) && (v & avpn) != 0)) {
>> - return REMOVE_NOT_FOUND;
>> - }
>> - *vp = v;
>> - *rp = r;
>> - spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
>> - ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
>> - return REMOVE_SUCCESS;
>> -}
>> -
>> -static target_ulong h_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> - target_ulong opcode, target_ulong *args)
>> -{
>> - CPUPPCState *env = &cpu->env;
>> - target_ulong flags = args[0];
>> - target_ulong ptex = args[1];
>> - target_ulong avpn = args[2];
>> - RemoveResult ret;
>> -
>> - ret = remove_hpte(cpu, ptex, avpn, flags,
>> - &args[0], &args[1]);
>> -
>> - switch (ret) {
>> - case REMOVE_SUCCESS:
>> - check_tlb_flush(env, true);
>> - return H_SUCCESS;
>> -
>> - case REMOVE_NOT_FOUND:
>> - return H_NOT_FOUND;
>> -
>> - case REMOVE_PARM:
>> - return H_PARAMETER;
>> -
>> - case REMOVE_HW:
>> - return H_HARDWARE;
>> - }
>> -
>> - g_assert_not_reached();
>> -}
>> -
>> -#define H_BULK_REMOVE_TYPE 0xc000000000000000ULL
>> -#define H_BULK_REMOVE_REQUEST 0x4000000000000000ULL
>> -#define H_BULK_REMOVE_RESPONSE 0x8000000000000000ULL
>> -#define H_BULK_REMOVE_END 0xc000000000000000ULL
>> -#define H_BULK_REMOVE_CODE 0x3000000000000000ULL
>> -#define H_BULK_REMOVE_SUCCESS 0x0000000000000000ULL
>> -#define H_BULK_REMOVE_NOT_FOUND 0x1000000000000000ULL
>> -#define H_BULK_REMOVE_PARM 0x2000000000000000ULL
>> -#define H_BULK_REMOVE_HW 0x3000000000000000ULL
>> -#define H_BULK_REMOVE_RC 0x0c00000000000000ULL
>> -#define H_BULK_REMOVE_FLAGS 0x0300000000000000ULL
>> -#define H_BULK_REMOVE_ABSOLUTE 0x0000000000000000ULL
>> -#define H_BULK_REMOVE_ANDCOND 0x0100000000000000ULL
>> -#define H_BULK_REMOVE_AVPN 0x0200000000000000ULL
>> -#define H_BULK_REMOVE_PTEX 0x00ffffffffffffffULL
>> -
>> -#define H_BULK_REMOVE_MAX_BATCH 4
>> -
>> -static target_ulong h_bulk_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> - target_ulong opcode, target_ulong *args)
>> -{
>> - CPUPPCState *env = &cpu->env;
>> - int i;
>> - target_ulong rc = H_SUCCESS;
>> -
>> - for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
>> - target_ulong *tsh = &args[i*2];
>> - target_ulong tsl = args[i*2 + 1];
>> - target_ulong v, r, ret;
>> -
>> - if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
>> - break;
>> - } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
>> - return H_PARAMETER;
>> - }
>> -
>> - *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
>> - *tsh |= H_BULK_REMOVE_RESPONSE;
>> -
>> - if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
>> - *tsh |= H_BULK_REMOVE_PARM;
>> - return H_PARAMETER;
>> - }
>> -
>> - ret = remove_hpte(cpu, *tsh & H_BULK_REMOVE_PTEX, tsl,
>> - (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
>> - &v, &r);
>> -
>> - *tsh |= ret << 60;
>> -
>> - switch (ret) {
>> - case REMOVE_SUCCESS:
>> - *tsh |= (r & (HPTE64_R_C | HPTE64_R_R)) << 43;
>> - break;
>> -
>> - case REMOVE_PARM:
>> - rc = H_PARAMETER;
>> - goto exit;
>> -
>> - case REMOVE_HW:
>> - rc = H_HARDWARE;
>> - goto exit;
>> - }
>> - }
>> - exit:
>> - check_tlb_flush(env, true);
>> -
>> - return rc;
>> -}
>> -
>> -static target_ulong h_protect(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> - target_ulong opcode, target_ulong *args)
>> -{
>> - CPUPPCState *env = &cpu->env;
>> - target_ulong flags = args[0];
>> - target_ulong ptex = args[1];
>> - target_ulong avpn = args[2];
>> - const ppc_hash_pte64_t *hptes;
>> - target_ulong v, r;
>> -
>> - if (!valid_ptex(cpu, ptex)) {
>> - return H_PARAMETER;
>> - }
>> -
>> - hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
>> - v = ppc_hash64_hpte0(cpu, hptes, 0);
>> - r = ppc_hash64_hpte1(cpu, hptes, 0);
>> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
>> -
>> - if ((v & HPTE64_V_VALID) == 0 ||
>> - ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
>> - return H_NOT_FOUND;
>> - }
>> -
>> - r &= ~(HPTE64_R_PP0 | HPTE64_R_PP | HPTE64_R_N |
>> - HPTE64_R_KEY_HI | HPTE64_R_KEY_LO);
>> - r |= (flags << 55) & HPTE64_R_PP0;
>> - r |= (flags << 48) & HPTE64_R_KEY_HI;
>> - r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
>> - spapr_store_hpte(cpu, ptex,
>> - (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
>> - ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
>> - /* Flush the tlb */
>> - check_tlb_flush(env, true);
>> - /* Don't need a memory barrier, due to qemu's global lock */
>> - spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
>> - return H_SUCCESS;
>> -}
>> -
>> -static target_ulong h_read(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> - target_ulong opcode, target_ulong *args)
>> -{
>> - target_ulong flags = args[0];
>> - target_ulong ptex = args[1];
>> - int i, ridx, n_entries = 1;
>> - const ppc_hash_pte64_t *hptes;
>> -
>> - if (!valid_ptex(cpu, ptex)) {
>> - return H_PARAMETER;
>> - }
>> -
>> - if (flags & H_READ_4) {
>> - /* Clear the two low order bits */
>> - ptex &= ~(3ULL);
>> - n_entries = 4;
>> - }
>> -
>> - hptes = ppc_hash64_map_hptes(cpu, ptex, n_entries);
>> - for (i = 0, ridx = 0; i < n_entries; i++) {
>> - args[ridx++] = ppc_hash64_hpte0(cpu, hptes, i);
>> - args[ridx++] = ppc_hash64_hpte1(cpu, hptes, i);
>> - }
>> - ppc_hash64_unmap_hptes(cpu, hptes, ptex, n_entries);
>> -
>> - return H_SUCCESS;
>> -}
>> -
>> struct SpaprPendingHpt {
>> /* These fields are read-only after initialization */
>> int shift;
>> @@ -2021,14 +1752,17 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>>
>> static void hypercall_register_types(void)
>> {
>> +
>> +#ifndef CONFIG_TCG
>> /* hcall-pft */
>> - spapr_register_hypercall(H_ENTER, h_enter);
>> - spapr_register_hypercall(H_REMOVE, h_remove);
>> - spapr_register_hypercall(H_PROTECT, h_protect);
>> - spapr_register_hypercall(H_READ, h_read);
>> + spapr_register_hypercall(H_ENTER, h_tcg_only);
>> + spapr_register_hypercall(H_REMOVE, h_tcg_only);
>> + spapr_register_hypercall(H_PROTECT, h_tcg_only);
>> + spapr_register_hypercall(H_READ, h_tcg_only);
>>
>> /* hcall-bulk */
>> - spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
>> + spapr_register_hypercall(H_BULK_REMOVE, h_tcg_only);
>> +#endif /* !CONFIG_TCG */
>>
>> /* hcall-hpt-resize */
>> spapr_register_hypercall(H_RESIZE_HPT_PREPARE, h_resize_hpt_prepare);
>> diff --git a/hw/ppc/spapr_hcall_tcg.c b/hw/ppc/spapr_hcall_tcg.c
>> new file mode 100644
>> index 0000000000..92ff24c8dc
>> --- /dev/null
>> +++ b/hw/ppc/spapr_hcall_tcg.c
>> @@ -0,0 +1,343 @@
>> +#include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>> +#include "qapi/error.h"
>> +#include "sysemu/hw_accel.h"
>> +#include "sysemu/runstate.h"
>> +#include "qemu/log.h"
>> +#include "qemu/main-loop.h"
>> +#include "qemu/module.h"
>> +#include "qemu/error-report.h"
>> +#include "cpu.h"
>> +#include "exec/exec-all.h"
>> +#include "helper_regs.h"
>> +#include "hw/ppc/spapr.h"
>> +#include "hw/ppc/spapr_cpu_core.h"
>> +#include "mmu-hash64.h"
>> +#include "mmu-misc.h"
>> +#include "cpu-models.h"
>> +#include "trace.h"
>> +#include "kvm_ppc.h"
>> +#include "hw/ppc/fdt.h"
>> +#include "hw/ppc/spapr_ovec.h"
>> +#include "mmu-book3s-v3.h"
>> +#include "hw/mem/memory-device.h"
>> +
>> +static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
>> +{
>> + /*
>> + * hash value/pteg group index is normalized by HPT mask
>> + */
>> + if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~ppc_hash64_hpt_mask(cpu)) {
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> +static bool is_ram_address(SpaprMachineState *spapr, hwaddr addr)
>> +{
>> + MachineState *machine = MACHINE(spapr);
>> + DeviceMemoryState *dms = machine->device_memory;
>> +
>> + if (addr < machine->ram_size) {
>> + return true;
>> + }
>> + if ((addr >= dms->base)
>> + && ((addr - dms->base) < memory_region_size(&dms->mr))) {
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static target_ulong h_enter(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> + target_ulong flags = args[0];
>> + target_ulong ptex = args[1];
>> + target_ulong pteh = args[2];
>> + target_ulong ptel = args[3];
>> + unsigned apshift;
>> + target_ulong raddr;
>> + target_ulong slot;
>> + const ppc_hash_pte64_t *hptes;
>> +
>> + apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
>> + if (!apshift) {
>> + /* Bad page size encoding */
>> + return H_PARAMETER;
>> + }
>> +
>> + raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
>> +
>> + if (is_ram_address(spapr, raddr)) {
>> + /* Regular RAM - should have WIMG=0010 */
>> + if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
>> + return H_PARAMETER;
>> + }
>> + } else {
>> + target_ulong wimg_flags;
>> + /* Looks like an IO address */
>> + /* FIXME: What WIMG combinations could be sensible for IO?
>> + * For now we allow WIMG=010x, but are there others? */
>> + /* FIXME: Should we check against registered IO addresses? */
>> + wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
>> +
>> + if (wimg_flags != HPTE64_R_I &&
>> + wimg_flags != (HPTE64_R_I | HPTE64_R_M)) {
>> + return H_PARAMETER;
>> + }
>> + }
>> +
>> + pteh &= ~0x60ULL;
>> +
>> + if (!valid_ptex(cpu, ptex)) {
>> + return H_PARAMETER;
>> + }
>> +
>> + slot = ptex & 7ULL;
>> + ptex = ptex & ~7ULL;
>> +
>> + if (likely((flags & H_EXACT) == 0)) {
>> + hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
>> + for (slot = 0; slot < 8; slot++) {
>> + if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
>> + break;
>> + }
>> + }
>> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
>> + if (slot == 8) {
>> + return H_PTEG_FULL;
>> + }
>> + } else {
>> + hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
>> + if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
>> + ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
>> + return H_PTEG_FULL;
>> + }
>> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
>> + }
>> +
>> + spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
>> +
>> + args[0] = ptex + slot;
>> + return H_SUCCESS;
>> +}
>> +
>> +typedef enum {
>> + REMOVE_SUCCESS = 0,
>> + REMOVE_NOT_FOUND = 1,
>> + REMOVE_PARM = 2,
>> + REMOVE_HW = 3,
>> +} RemoveResult;
>> +
>> +static RemoveResult remove_hpte(PowerPCCPU *cpu
>> + , target_ulong ptex,
>> + target_ulong avpn,
>> + target_ulong flags,
>> + target_ulong *vp, target_ulong *rp)
>> +{
>> + const ppc_hash_pte64_t *hptes;
>> + target_ulong v, r;
>> +
>> + if (!valid_ptex(cpu, ptex)) {
>> + return REMOVE_PARM;
>> + }
>> +
>> + hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
>> + v = ppc_hash64_hpte0(cpu, hptes, 0);
>> + r = ppc_hash64_hpte1(cpu, hptes, 0);
>> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
>> +
>> + if ((v & HPTE64_V_VALID) == 0 ||
>> + ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
>> + ((flags & H_ANDCOND) && (v & avpn) != 0)) {
>> + return REMOVE_NOT_FOUND;
>> + }
>> + *vp = v;
>> + *rp = r;
>> + spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
>> + ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
>> + return REMOVE_SUCCESS;
>> +}
>> +
>> +static target_ulong h_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> + CPUPPCState *env = &cpu->env;
>> + target_ulong flags = args[0];
>> + target_ulong ptex = args[1];
>> + target_ulong avpn = args[2];
>> + RemoveResult ret;
>> +
>> + ret = remove_hpte(cpu, ptex, avpn, flags,
>> + &args[0], &args[1]);
>> +
>> + switch (ret) {
>> + case REMOVE_SUCCESS:
>> + check_tlb_flush(env, true);
>> + return H_SUCCESS;
>> +
>> + case REMOVE_NOT_FOUND:
>> + return H_NOT_FOUND;
>> +
>> + case REMOVE_PARM:
>> + return H_PARAMETER;
>> +
>> + case REMOVE_HW:
>> + return H_HARDWARE;
>> + }
>> +
>> + g_assert_not_reached();
>> +}
>> +
>> +#define H_BULK_REMOVE_TYPE 0xc000000000000000ULL
>> +#define H_BULK_REMOVE_REQUEST 0x4000000000000000ULL
>> +#define H_BULK_REMOVE_RESPONSE 0x8000000000000000ULL
>> +#define H_BULK_REMOVE_END 0xc000000000000000ULL
>> +#define H_BULK_REMOVE_CODE 0x3000000000000000ULL
>> +#define H_BULK_REMOVE_SUCCESS 0x0000000000000000ULL
>> +#define H_BULK_REMOVE_NOT_FOUND 0x1000000000000000ULL
>> +#define H_BULK_REMOVE_PARM 0x2000000000000000ULL
>> +#define H_BULK_REMOVE_HW 0x3000000000000000ULL
>> +#define H_BULK_REMOVE_RC 0x0c00000000000000ULL
>> +#define H_BULK_REMOVE_FLAGS 0x0300000000000000ULL
>> +#define H_BULK_REMOVE_ABSOLUTE 0x0000000000000000ULL
>> +#define H_BULK_REMOVE_ANDCOND 0x0100000000000000ULL
>> +#define H_BULK_REMOVE_AVPN 0x0200000000000000ULL
>> +#define H_BULK_REMOVE_PTEX 0x00ffffffffffffffULL
>> +
>> +#define H_BULK_REMOVE_MAX_BATCH 4
>> +
>> +static target_ulong h_bulk_remove(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> + CPUPPCState *env = &cpu->env;
>> + int i;
>> + target_ulong rc = H_SUCCESS;
>> +
>> + for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
>> + target_ulong *tsh = &args[i*2];
>> + target_ulong tsl = args[i*2 + 1];
>> + target_ulong v, r, ret;
>> +
>> + if ((*tsh & H_BULK_REMOVE_TYPE) == H_BULK_REMOVE_END) {
>> + break;
>> + } else if ((*tsh & H_BULK_REMOVE_TYPE) != H_BULK_REMOVE_REQUEST) {
>> + return H_PARAMETER;
>> + }
>> +
>> + *tsh &= H_BULK_REMOVE_PTEX | H_BULK_REMOVE_FLAGS;
>> + *tsh |= H_BULK_REMOVE_RESPONSE;
>> +
>> + if ((*tsh & H_BULK_REMOVE_ANDCOND) && (*tsh & H_BULK_REMOVE_AVPN)) {
>> + *tsh |= H_BULK_REMOVE_PARM;
>> + return H_PARAMETER;
>> + }
>> +
>> + ret = remove_hpte(cpu, *tsh & H_BULK_REMOVE_PTEX, tsl,
>> + (*tsh & H_BULK_REMOVE_FLAGS) >> 26,
>> + &v, &r);
>> +
>> + *tsh |= ret << 60;
>> +
>> + switch (ret) {
>> + case REMOVE_SUCCESS:
>> + *tsh |= (r & (HPTE64_R_C | HPTE64_R_R)) << 43;
>> + break;
>> +
>> + case REMOVE_PARM:
>> + rc = H_PARAMETER;
>> + goto exit;
>> +
>> + case REMOVE_HW:
>> + rc = H_HARDWARE;
>> + goto exit;
>> + }
>> + }
>> + exit:
>> + check_tlb_flush(env, true);
>> +
>> + return rc;
>> +}
>> +
>> +static target_ulong h_protect(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> + CPUPPCState *env = &cpu->env;
>> + target_ulong flags = args[0];
>> + target_ulong ptex = args[1];
>> + target_ulong avpn = args[2];
>> + const ppc_hash_pte64_t *hptes;
>> + target_ulong v, r;
>> +
>> + if (!valid_ptex(cpu, ptex)) {
>> + return H_PARAMETER;
>> + }
>> +
>> + hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
>> + v = ppc_hash64_hpte0(cpu, hptes, 0);
>> + r = ppc_hash64_hpte1(cpu, hptes, 0);
>> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
>> +
>> + if ((v & HPTE64_V_VALID) == 0 ||
>> + ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
>> + return H_NOT_FOUND;
>> + }
>> +
>> + r &= ~(HPTE64_R_PP0 | HPTE64_R_PP | HPTE64_R_N |
>> + HPTE64_R_KEY_HI | HPTE64_R_KEY_LO);
>> + r |= (flags << 55) & HPTE64_R_PP0;
>> + r |= (flags << 48) & HPTE64_R_KEY_HI;
>> + r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
>> + spapr_store_hpte(cpu, ptex,
>> + (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
>> + ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
>> + /* Flush the tlb */
>> + check_tlb_flush(env, true);
>> + /* Don't need a memory barrier, due to qemu's global lock */
>> + spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
>> + return H_SUCCESS;
>> +}
>> +
>> +static target_ulong h_read(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> + target_ulong flags = args[0];
>> + target_ulong ptex = args[1];
>> + int i, ridx, n_entries = 1;
>> + const ppc_hash_pte64_t *hptes;
>> +
>> + if (!valid_ptex(cpu, ptex)) {
>> + return H_PARAMETER;
>> + }
>> +
>> + if (flags & H_READ_4) {
>> + /* Clear the two low order bits */
>> + ptex &= ~(3ULL);
>> + n_entries = 4;
>> + }
>> +
>> + hptes = ppc_hash64_map_hptes(cpu, ptex, n_entries);
>> + for (i = 0, ridx = 0; i < n_entries; i++) {
>> + args[ridx++] = ppc_hash64_hpte0(cpu, hptes, i);
>> + args[ridx++] = ppc_hash64_hpte1(cpu, hptes, i);
>> + }
>> + ppc_hash64_unmap_hptes(cpu, hptes, ptex, n_entries);
>> +
>> + return H_SUCCESS;
>> +}
>> +
>> +
>> +static void hypercall_register_types(void)
>> +{
>> + /* hcall-pft */
>> + spapr_register_hypercall(H_ENTER, h_enter);
>> + spapr_register_hypercall(H_REMOVE, h_remove);
>> + spapr_register_hypercall(H_PROTECT, h_protect);
>> + spapr_register_hypercall(H_READ, h_read);
>> +
>> + /* hcall-bulk */
>> + spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
>> +}
>> +
>> +type_init(hypercall_register_types)
[-- Attachment #2: Type: text/html, Size: 33846 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG
2021-05-03 22:21 ` Fabiano Rosas
2021-05-04 14:43 ` Bruno Piazera Larsen
2021-05-04 15:57 ` Lucas Mateus Martins Araujo e Castro
@ 2021-05-05 4:42 ` David Gibson
2 siblings, 0 replies; 17+ messages in thread
From: David Gibson @ 2021-05-05 4:42 UTC (permalink / raw)
To: Fabiano Rosas
Cc: bruno.larsen, Lucas Mateus Castro (alqotel), qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 6740 bytes --]
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
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg
2021-05-04 18:14 ` Lucas Mateus Martins Araujo e Castro
@ 2021-05-05 4:58 ` David Gibson
0 siblings, 0 replies; 17+ messages in thread
From: David Gibson @ 2021-05-05 4:58 UTC (permalink / raw)
To: Lucas Mateus Martins Araujo e Castro
Cc: bruno.larsen, qemu-ppc, qemu-devel, farosas
[-- Attachment #1: Type: text/plain, Size: 2948 bytes --]
On Tue, May 04, 2021 at 03:14:17PM -0300, Lucas Mateus Martins Araujo e Castro wrote:
>
> On 03/05/2021 01:34, David Gibson wrote:
> > On Fri, Apr 30, 2021 at 03:40:47PM -0300, Lucas Mateus Castro (alqotel) wrote:
> > > Moved h_enter, remove_hpte, h_remove, h_bulk_remove,h_protect and
> > > h_read to spapr_hcall_tcg.c, added h_tcg_only to be used in a !TCG
> > > environment in spapr_hcall.c and changed build options to only build
> > > spapr_hcall_tcg.c when CONFIG_TCG is enabled.
> > This looks good. I'd suggest the name 'spapr_softmmu.c' instead,
> > which more specifically describes what's special about these
> > functions.
> >
> > h_resize_hpt_prepare(), h_resize_hpt_commit() and the functions they
> > depend on belong in the softmmu set as well.
>
> Moved these hcalls to spapr_softmmu.c as well, along with the most
> functions they depend on, but 1 function (push_sregs_to_kvm_pr) is
> also used by hcalls in spapr_hcall.c, so for now I'm just leaving it in
> spapr_hcall.c and exporting it to be used in spapr_softmmu.c.
Right. That one's an ugly workaround for some weird KVM PR behaviour,
and we will need it in the common code.
> On a related note, from what I've seen h_resize_hpt_prepare and
> h_resize_hpt_commit are not implementede in KVM, so they're only
> called when there's softmmu so that's why they can be moved to
> spapr_softmmu.c?
Ah, sorry, I forgot how these worked. The bulk of the logic to
implement these *is* in KVM, but KVM doesn't directly intercept them
as hcalls. Instead the hcall is caught by qemu, but it just forwards
them back to KVM by calling an ioctl(). See the calls to
kvmppc_resize_hpt_prepare() and kvmppc_resize_hpt_commit() in
spapr_hcall.c.
[Aside: the reason for this is that they're not latency critical, and
it's useful for qemu to be aware of and possibly filter HPT resize
requests, but because KVM manages the HPT, qemu can't implement these
directly for the KVM case]
So what we'll need to do for those is keep the first part if
h_resize_hpt_{prepare,commit}() in common code - that's basically just
parameter validation - up to the call to
kvmppc_resize_hpt_{prepare,commit}().
if the kvmppc_() call fails with -ENOSYS then we need to do
if (kvm_enabled()) {
return H_HARDWARE;
}
softmmu_resize_hpt_{prepare,commit}()
Where that softmmu_..() function is the remainder of the logic that's
in the current implementation. That bit can move to the softmmu file
and can be stubbed with an assert-not-reached for the !TCG case.
In practice we should never actually hit that H_HARDWARE - we should
have failed at machine setup if we tried to enable the resize HPT
feature on a KVM that doesn't implement it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64
2021-05-03 4:24 ` David Gibson
@ 2021-05-05 17:30 ` Lucas Mateus Martins Araujo e Castro
2021-05-06 2:03 ` David Gibson
0 siblings, 1 reply; 17+ messages in thread
From: Lucas Mateus Martins Araujo e Castro @ 2021-05-05 17:30 UTC (permalink / raw)
To: David Gibson; +Cc: bruno.larsen, qemu-ppc, qemu-devel, farosas
On 03/05/2021 01:24, David Gibson wrote:
> On Fri, Apr 30, 2021 at 03:40:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
>> The functions ppc_store_lpcr, ppc_hash64_filter_pagesizes and
>> ppc_hash64_unmap_hptes have been moved to mmu-misc.h since they are
>> not needed in a !TCG context and mmu-hash64 should not be compiled
>> in such situation.
>>
>> ppc_store_lpcr and ppc_hash64_filter_pagesizes are used by multiple
>> functions, while ppc_hash64_unmap_hptes is used by rehash_hpt (in
>> spapr_hcall.c).
> Hmm.. looking at it, ppc_store_lpcr() (and helper_store_lpcr()) don't
> really belong in this file at all. The LPCR has some things related
> to the hash MMU, but plenty of others that don't. So, maybe
> misc_helper.c? That might have to be moved again, since misc_helper
> itself should probably mostly not be used for !TCG. But.. one thing
> at a time.
I tested here and compiling misc_helper.c with disable-tcg it's kind of
complicated and it would require many changes in it, so for this patch
just move it there and deal with it in a later patch?
> AFAICT the only user of ppc_hash64_filter_pagesizes() is in
> spapr_caps.c. For now you can just move it next to the caller, it's
> debatable whether it belongs more to PAPR or MMU code.
Also I'm assuming the prototype should also be moved from
"target/ppc/mmu-hash64.h" to "include/hw/ppc/spapr.h" (or some other
spapr_*.h file), or should it be left in the original file?
> ppc_hash64_unmap_hptes() is definitely TCG only and should stay where
> it is. The call from rehash_hpt() can be solved because rehash_hpt()
> itself is TCG only. I've already suggested splitting the TCG (well,
> softmmu) only things out from spapr_hcall.c, so it might simplify
> things to tackle that first.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64
2021-05-05 17:30 ` Lucas Mateus Martins Araujo e Castro
@ 2021-05-06 2:03 ` David Gibson
0 siblings, 0 replies; 17+ messages in thread
From: David Gibson @ 2021-05-06 2:03 UTC (permalink / raw)
To: Lucas Mateus Martins Araujo e Castro
Cc: bruno.larsen, qemu-ppc, qemu-devel, farosas
[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]
On Wed, May 05, 2021 at 02:30:35PM -0300, Lucas Mateus Martins Araujo e Castro wrote:
>
> On 03/05/2021 01:24, David Gibson wrote:
> > On Fri, Apr 30, 2021 at 03:40:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
> > > The functions ppc_store_lpcr, ppc_hash64_filter_pagesizes and
> > > ppc_hash64_unmap_hptes have been moved to mmu-misc.h since they are
> > > not needed in a !TCG context and mmu-hash64 should not be compiled
> > > in such situation.
> > >
> > > ppc_store_lpcr and ppc_hash64_filter_pagesizes are used by multiple
> > > functions, while ppc_hash64_unmap_hptes is used by rehash_hpt (in
> > > spapr_hcall.c).
> > Hmm.. looking at it, ppc_store_lpcr() (and helper_store_lpcr()) don't
> > really belong in this file at all. The LPCR has some things related
> > to the hash MMU, but plenty of others that don't. So, maybe
> > misc_helper.c? That might have to be moved again, since misc_helper
> > itself should probably mostly not be used for !TCG. But.. one thing
> > at a time.
>
> I tested here and compiling misc_helper.c with disable-tcg it's kind of
> complicated and it would require many changes in it, so for this patch
> just move it there and deal with it in a later patch?
Yes, sounds reasonable.
>
> > AFAICT the only user of ppc_hash64_filter_pagesizes() is in
> > spapr_caps.c. For now you can just move it next to the caller, it's
> > debatable whether it belongs more to PAPR or MMU code.
>
> Also I'm assuming the prototype should also be moved from
> "target/ppc/mmu-hash64.h" to "include/hw/ppc/spapr.h" (or some other
> spapr_*.h file), or should it be left in the original file?
Well, if you put it next to the caller you can make it static and
remove the prototype entirely.
>
> > ppc_hash64_unmap_hptes() is definitely TCG only and should stay where
> > it is. The call from rehash_hpt() can be solved because rehash_hpt()
> > itself is TCG only. I've already suggested splitting the TCG (well,
> > softmmu) only things out from spapr_hcall.c, so it might simplify
> > things to tackle that first.
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-05-06 2:21 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 18:40 [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG Lucas Mateus Castro (alqotel)
2021-04-30 18:40 ` [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64 Lucas Mateus Castro (alqotel)
2021-04-30 20:19 ` Fabiano Rosas
2021-05-03 4:24 ` David Gibson
2021-05-05 17:30 ` Lucas Mateus Martins Araujo e Castro
2021-05-06 2:03 ` David Gibson
2021-04-30 18:40 ` [RFC PATCH v2 2/2] hw/ppc: Moved TCG code to spapr_hcall_tcg Lucas Mateus Castro (alqotel)
2021-04-30 23:38 ` Fabiano Rosas
2021-05-03 4:35 ` David Gibson
2021-05-03 4:34 ` David Gibson
2021-05-04 18:14 ` Lucas Mateus Martins Araujo e Castro
2021-05-05 4:58 ` David Gibson
2021-04-30 18:54 ` [RFC PATCH v2 0/2] hw/ppc: code motion to compile without TCG no-reply
2021-05-03 22:21 ` Fabiano Rosas
2021-05-04 14:43 ` Bruno Piazera Larsen
2021-05-04 15:57 ` Lucas Mateus Martins Araujo e Castro
2021-05-05 4:42 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).