* [Qemu-devel] [PATCH V9 1/5] target-ppc: Update external_htab even when HTAB is managed by kernel
2014-01-28 7:59 [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Aneesh Kumar K.V
@ 2014-01-28 7:59 ` Aneesh Kumar K.V
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation Aneesh Kumar K.V
` (4 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-01-28 7:59 UTC (permalink / raw)
To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V
We will use this in later patches to make sure we use the right load
functions when copying hpte entries.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1b4e38f50a8d..ac62c8f9294b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -740,6 +740,13 @@ static void spapr_cpu_reset(void *opaque)
env->spr[SPR_HIOR] = 0;
env->external_htab = (uint8_t *)spapr->htab;
+ if (kvm_enabled() && !env->external_htab) {
+ /*
+ * HV KVM, set external_htab to 1 so our ppc_hash64_load_hpte*
+ * functions do the right thing.
+ */
+ env->external_htab = (void *)1;
+ }
env->htab_base = -1;
env->htab_mask = HTAB_SIZE(spapr) - 1;
env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab |
--
1.8.5.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation
2014-01-28 7:59 [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Aneesh Kumar K.V
2014-01-28 7:59 ` [Qemu-devel] [PATCH V9 1/5] target-ppc: Update external_htab even when HTAB is managed by kernel Aneesh Kumar K.V
@ 2014-01-28 8:00 ` Aneesh Kumar K.V
2014-02-11 18:46 ` Aneesh Kumar K.V
2014-02-13 2:59 ` [Qemu-devel] [PATCH V10] " Aneesh Kumar K.V
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 3/5] target-ppc: Fix page table lookup with kvm enabled Aneesh Kumar K.V
` (3 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-01-28 8:00 UTC (permalink / raw)
To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V
Correctly update the htab_mask using the return value of
KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
on GET_SREGS for HV. We check for external htab and if
found true, we don't need to update sdr1
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 8 +++++++-
target-ppc/cpu.h | 1 +
target-ppc/kvm.c | 4 +++-
target-ppc/machine.c | 11 +++++++----
target-ppc/misc_helper.c | 4 +++-
target-ppc/mmu_helper.c | 3 ++-
6 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ac62c8f9294b..009bb0112cc0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -748,7 +748,13 @@ static void spapr_cpu_reset(void *opaque)
env->external_htab = (void *)1;
}
env->htab_base = -1;
- env->htab_mask = HTAB_SIZE(spapr) - 1;
+ /*
+ * htab_mask is the mask used to normalize hash value to PTEG index.
+ * htab_shift is log2 of hash table size.
+ * We have 8 hpte per group, and each hpte is 16 bytes.
+ * ie have 128 bytes per hpte entry.
+ */
+ env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1;
env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab |
(spapr->htab_shift - 18);
}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index bb847676a52e..b0f66e5104dd 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -961,6 +961,7 @@ struct CPUPPCState {
#endif
/* segment registers */
hwaddr htab_base;
+ /* mask used to normalize hash value to PTEG index */
hwaddr htab_mask;
target_ulong sr[32];
/* externally stored hash table */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 781b72f1ea5a..c771ec11ed28 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1029,7 +1029,9 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
- ppc_store_sdr1(env, sregs.u.s.sdr1);
+ if (!env->external_htab) {
+ ppc_store_sdr1(env, sregs.u.s.sdr1);
+ }
/* Sync SLB */
#ifdef TARGET_PPC64
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 12c174f7f3e6..2d46ceccca3a 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -70,7 +70,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
qemu_get_betls(f, &env->pb[i]);
for (i = 0; i < 1024; i++)
qemu_get_betls(f, &env->spr[i]);
- ppc_store_sdr1(env, sdr1);
+ if (!env->external_htab) {
+ ppc_store_sdr1(env, sdr1);
+ }
qemu_get_be32s(f, &env->vscr);
qemu_get_be64s(f, &env->spe_acc);
qemu_get_be32s(f, &env->spe_fscr);
@@ -179,9 +181,10 @@ static int cpu_post_load(void *opaque, int version_id)
env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1];
}
- /* Restore htab_base and htab_mask variables */
- ppc_store_sdr1(env, env->spr[SPR_SDR1]);
-
+ if (!env->external_htab) {
+ /* Restore htab_base and htab_mask variables */
+ ppc_store_sdr1(env, env->spr[SPR_SDR1]);
+ }
hreg_compute_hflags(env);
hreg_compute_mem_idx(env);
diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
index 616aab6fb67d..dc2ebfc4524b 100644
--- a/target-ppc/misc_helper.c
+++ b/target-ppc/misc_helper.c
@@ -38,7 +38,9 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn)
void helper_store_sdr1(CPUPPCState *env, target_ulong val)
{
- ppc_store_sdr1(env, val);
+ if (!env->external_htab) {
+ ppc_store_sdr1(env, val);
+ }
}
void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 04a840b01697..8e2f8e736a12 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2014,6 +2014,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
{
LOG_MMU("%s: " TARGET_FMT_lx "\n", __func__, value);
+ assert(!env->external_htab);
if (env->spr[SPR_SDR1] != value) {
env->spr[SPR_SDR1] = value;
#if defined(TARGET_PPC64)
@@ -2025,7 +2026,7 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
" stored in SDR1\n", htabsize);
htabsize = 28;
}
- env->htab_mask = (1ULL << (htabsize + 18)) - 1;
+ env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
env->htab_base = value & SDR_64_HTABORG;
} else
#endif /* defined(TARGET_PPC64) */
--
1.8.5.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation Aneesh Kumar K.V
@ 2014-02-11 18:46 ` Aneesh Kumar K.V
2014-02-12 10:32 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2014-02-13 2:59 ` [Qemu-devel] [PATCH V10] " Aneesh Kumar K.V
1 sibling, 1 reply; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-02-11 18:46 UTC (permalink / raw)
To: agraf, paulus, Greg Kurz; +Cc: qemu-ppc, qemu-devel
Hi Greg,
can you try the below patch and see if it fix the TCG mode failure ?
-aneesh
commit d98b5098bc04f44ef4e175f689345e92cf469231
Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Date: Tue Feb 11 23:43:12 2014 +0530
tcg fixes
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index e1f778faf3ae..d3aca706fdc9 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -91,7 +91,10 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
pteh &= ~0x60ULL;
- if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ /*
+ * hash value/pteg group index is normalized by htab_mask
+ */
+ if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
return H_PARAMETER;
}
@@ -140,7 +143,10 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
uint64_t token;
target_ulong v, r, rb;
- if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ /*
+ * hash value/pteg group index is normalized by htab_mask
+ */
+ if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
return REMOVE_PARM;
}
@@ -266,7 +272,10 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
uint64_t token;
target_ulong v, r, rb;
- if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ /*
+ * hash value/pteg group index is normalized by htab_mask
+ */
+ if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
return H_PARAMETER;
}
@@ -303,7 +312,10 @@ static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
uint8_t *hpte;
int i, ridx, n_entries = 1;
- if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ /*
+ * hash value/pteg group index is normalized by htab_mask
+ */
+ if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
return H_PARAMETER;
}
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation
2014-02-11 18:46 ` Aneesh Kumar K.V
@ 2014-02-12 10:32 ` Greg Kurz
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kurz @ 2014-02-12 10:32 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: qemu-ppc, paulus, agraf, qemu-devel
On Wed, 12 Feb 2014 00:16:25 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> Hi Greg,
>
> can you try the below patch and see if it fix the TCG mode failure ?
>
> -aneesh
>
Hi Aneesh,
The patche fixes the issue indeed. Maybe a helper could be factored out
since we have 4 times the same magic formula :)
Cheers.
--
Greg
> commit d98b5098bc04f44ef4e175f689345e92cf469231
> Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Date: Tue Feb 11 23:43:12 2014 +0530
>
> tcg fixes
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index e1f778faf3ae..d3aca706fdc9 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -91,7 +91,10 @@ static target_ulong h_enter(PowerPCCPU *cpu,
> sPAPREnvironment *spapr,
>
> pteh &= ~0x60ULL;
>
> - if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + /*
> + * hash value/pteg group index is normalized by htab_mask
> + */
> + if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
> return H_PARAMETER;
> }
>
> @@ -140,7 +143,10 @@ static RemoveResult remove_hpte(CPUPPCState *env,
> target_ulong ptex, uint64_t token;
> target_ulong v, r, rb;
>
> - if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + /*
> + * hash value/pteg group index is normalized by htab_mask
> + */
> + if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
> return REMOVE_PARM;
> }
>
> @@ -266,7 +272,10 @@ static target_ulong h_protect(PowerPCCPU *cpu,
> sPAPREnvironment *spapr, uint64_t token;
> target_ulong v, r, rb;
>
> - if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + /*
> + * hash value/pteg group index is normalized by htab_mask
> + */
> + if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
> return H_PARAMETER;
> }
>
> @@ -303,7 +312,10 @@ static target_ulong h_read(PowerPCCPU *cpu,
> sPAPREnvironment *spapr, uint8_t *hpte;
> int i, ridx, n_entries = 1;
>
> - if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + /*
> + * hash value/pteg group index is normalized by htab_mask
> + */
> + if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
> return H_PARAMETER;
> }
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation Aneesh Kumar K.V
2014-02-11 18:46 ` Aneesh Kumar K.V
@ 2014-02-13 2:59 ` Aneesh Kumar K.V
2014-02-13 10:40 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
1 sibling, 1 reply; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-02-13 2:59 UTC (permalink / raw)
To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Correctly update the htab_mask using the return value of
KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
on GET_SREGS for HV. We check for external htab and if
found true, we don't need to update sdr1
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from V9:
* Fix TCG breakage
hw/ppc/spapr.c | 8 +++++++-
hw/ppc/spapr_hcall.c | 19 +++++++++++++++----
target-ppc/cpu.h | 1 +
target-ppc/kvm.c | 4 +++-
target-ppc/machine.c | 11 +++++++----
target-ppc/misc_helper.c | 4 +++-
target-ppc/mmu_helper.c | 3 ++-
7 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ac62c8f9294b..009bb0112cc0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -748,7 +748,13 @@ static void spapr_cpu_reset(void *opaque)
env->external_htab = (void *)1;
}
env->htab_base = -1;
- env->htab_mask = HTAB_SIZE(spapr) - 1;
+ /*
+ * htab_mask is the mask used to normalize hash value to PTEG index.
+ * htab_shift is log2 of hash table size.
+ * We have 8 hpte per group, and each hpte is 16 bytes.
+ * ie have 128 bytes per hpte entry.
+ */
+ env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1;
env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab |
(spapr->htab_shift - 18);
}
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f755a5392317..c6e123b11ab0 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -40,6 +40,17 @@ static target_ulong compute_tlbie_rb(target_ulong v, target_ulong r,
return rb;
}
+static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index)
+{
+ /*
+ * hash value/pteg group index is normalized by htab_mask
+ */
+ if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
+ return false;
+ }
+ return true;
+}
+
static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong opcode, target_ulong *args)
{
@@ -91,7 +102,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
pteh &= ~0x60ULL;
- if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
if (likely((flags & H_EXACT) == 0)) {
@@ -136,7 +147,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
hwaddr hpte;
target_ulong v, r, rb;
- if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ if (!valid_pte_index(env, ptex)) {
return REMOVE_PARM;
}
@@ -262,7 +273,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
hwaddr hpte;
target_ulong v, r, rb;
- if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
@@ -299,7 +310,7 @@ static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
uint8_t *hpte;
int i, ridx, n_entries = 1;
- if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+ if (!valid_pte_index(env, pte_index)) {
return H_PARAMETER;
}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index bb847676a52e..b0f66e5104dd 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -961,6 +961,7 @@ struct CPUPPCState {
#endif
/* segment registers */
hwaddr htab_base;
+ /* mask used to normalize hash value to PTEG index */
hwaddr htab_mask;
target_ulong sr[32];
/* externally stored hash table */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 781b72f1ea5a..c771ec11ed28 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1029,7 +1029,9 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
- ppc_store_sdr1(env, sregs.u.s.sdr1);
+ if (!env->external_htab) {
+ ppc_store_sdr1(env, sregs.u.s.sdr1);
+ }
/* Sync SLB */
#ifdef TARGET_PPC64
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 12c174f7f3e6..2d46ceccca3a 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -70,7 +70,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
qemu_get_betls(f, &env->pb[i]);
for (i = 0; i < 1024; i++)
qemu_get_betls(f, &env->spr[i]);
- ppc_store_sdr1(env, sdr1);
+ if (!env->external_htab) {
+ ppc_store_sdr1(env, sdr1);
+ }
qemu_get_be32s(f, &env->vscr);
qemu_get_be64s(f, &env->spe_acc);
qemu_get_be32s(f, &env->spe_fscr);
@@ -179,9 +181,10 @@ static int cpu_post_load(void *opaque, int version_id)
env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1];
}
- /* Restore htab_base and htab_mask variables */
- ppc_store_sdr1(env, env->spr[SPR_SDR1]);
-
+ if (!env->external_htab) {
+ /* Restore htab_base and htab_mask variables */
+ ppc_store_sdr1(env, env->spr[SPR_SDR1]);
+ }
hreg_compute_hflags(env);
hreg_compute_mem_idx(env);
diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
index 616aab6fb67d..dc2ebfc4524b 100644
--- a/target-ppc/misc_helper.c
+++ b/target-ppc/misc_helper.c
@@ -38,7 +38,9 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn)
void helper_store_sdr1(CPUPPCState *env, target_ulong val)
{
- ppc_store_sdr1(env, val);
+ if (!env->external_htab) {
+ ppc_store_sdr1(env, val);
+ }
}
void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 04a840b01697..8e2f8e736a12 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2014,6 +2014,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
{
LOG_MMU("%s: " TARGET_FMT_lx "\n", __func__, value);
+ assert(!env->external_htab);
if (env->spr[SPR_SDR1] != value) {
env->spr[SPR_SDR1] = value;
#if defined(TARGET_PPC64)
@@ -2025,7 +2026,7 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
" stored in SDR1\n", htabsize);
htabsize = 28;
}
- env->htab_mask = (1ULL << (htabsize + 18)) - 1;
+ env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
env->htab_base = value & SDR_64_HTABORG;
} else
#endif /* defined(TARGET_PPC64) */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-13 2:59 ` [Qemu-devel] [PATCH V10] " Aneesh Kumar K.V
@ 2014-02-13 10:40 ` Greg Kurz
2014-02-13 14:51 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2014-02-13 10:40 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: qemu-ppc, paulus, agraf, qemu-devel
On Thu, 13 Feb 2014 08:29:28 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> Correctly update the htab_mask using the return value of
> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
> on GET_SREGS for HV. We check for external htab and if
> found true, we don't need to update sdr1
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> Changes from V9:
> * Fix TCG breakage
>
Alex,
This patch is a complete replacement for commit 1df2d6d900a9 in your
ppc-next branch.
Please apply so that TCG works again ! :)
Thanks.
--
Greg
> hw/ppc/spapr.c | 8 +++++++-
> hw/ppc/spapr_hcall.c | 19 +++++++++++++++----
> target-ppc/cpu.h | 1 +
> target-ppc/kvm.c | 4 +++-
> target-ppc/machine.c | 11 +++++++----
> target-ppc/misc_helper.c | 4 +++-
> target-ppc/mmu_helper.c | 3 ++-
> 7 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ac62c8f9294b..009bb0112cc0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -748,7 +748,13 @@ static void spapr_cpu_reset(void *opaque)
> env->external_htab = (void *)1;
> }
> env->htab_base = -1;
> - env->htab_mask = HTAB_SIZE(spapr) - 1;
> + /*
> + * htab_mask is the mask used to normalize hash value to PTEG index.
> + * htab_shift is log2 of hash table size.
> + * We have 8 hpte per group, and each hpte is 16 bytes.
> + * ie have 128 bytes per hpte entry.
> + */
> + env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1;
> env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab |
> (spapr->htab_shift - 18);
> }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index f755a5392317..c6e123b11ab0 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -40,6 +40,17 @@ static target_ulong compute_tlbie_rb(target_ulong v,
> target_ulong r, return rb;
> }
>
> +static inline bool valid_pte_index(CPUPPCState *env, target_ulong
> pte_index) +{
> + /*
> + * hash value/pteg group index is normalized by htab_mask
> + */
> + if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
> + return false;
> + }
> + return true;
> +}
> +
> static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> target_ulong opcode, target_ulong *args)
> {
> @@ -91,7 +102,7 @@ static target_ulong h_enter(PowerPCCPU *cpu,
> sPAPREnvironment *spapr,
>
> pteh &= ~0x60ULL;
>
> - if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + if (!valid_pte_index(env, pte_index)) {
> return H_PARAMETER;
> }
> if (likely((flags & H_EXACT) == 0)) {
> @@ -136,7 +147,7 @@ static RemoveResult remove_hpte(CPUPPCState *env,
> target_ulong ptex, hwaddr hpte;
> target_ulong v, r, rb;
>
> - if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + if (!valid_pte_index(env, ptex)) {
> return REMOVE_PARM;
> }
>
> @@ -262,7 +273,7 @@ static target_ulong h_protect(PowerPCCPU *cpu,
> sPAPREnvironment *spapr, hwaddr hpte;
> target_ulong v, r, rb;
>
> - if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + if (!valid_pte_index(env, pte_index)) {
> return H_PARAMETER;
> }
>
> @@ -299,7 +310,7 @@ static target_ulong h_read(PowerPCCPU *cpu,
> sPAPREnvironment *spapr, uint8_t *hpte;
> int i, ridx, n_entries = 1;
>
> - if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> + if (!valid_pte_index(env, pte_index)) {
> return H_PARAMETER;
> }
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index bb847676a52e..b0f66e5104dd 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -961,6 +961,7 @@ struct CPUPPCState {
> #endif
> /* segment registers */
> hwaddr htab_base;
> + /* mask used to normalize hash value to PTEG index */
> hwaddr htab_mask;
> target_ulong sr[32];
> /* externally stored hash table */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 781b72f1ea5a..c771ec11ed28 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1029,7 +1029,9 @@ int kvm_arch_get_registers(CPUState *cs)
> return ret;
> }
>
> - ppc_store_sdr1(env, sregs.u.s.sdr1);
> + if (!env->external_htab) {
> + ppc_store_sdr1(env, sregs.u.s.sdr1);
> + }
>
> /* Sync SLB */
> #ifdef TARGET_PPC64
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index 12c174f7f3e6..2d46ceccca3a 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -70,7 +70,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int
> version_id) qemu_get_betls(f, &env->pb[i]);
> for (i = 0; i < 1024; i++)
> qemu_get_betls(f, &env->spr[i]);
> - ppc_store_sdr1(env, sdr1);
> + if (!env->external_htab) {
> + ppc_store_sdr1(env, sdr1);
> + }
> qemu_get_be32s(f, &env->vscr);
> qemu_get_be64s(f, &env->spe_acc);
> qemu_get_be32s(f, &env->spe_fscr);
> @@ -179,9 +181,10 @@ static int cpu_post_load(void *opaque, int
> version_id) env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1];
> }
>
> - /* Restore htab_base and htab_mask variables */
> - ppc_store_sdr1(env, env->spr[SPR_SDR1]);
> -
> + if (!env->external_htab) {
> + /* Restore htab_base and htab_mask variables */
> + ppc_store_sdr1(env, env->spr[SPR_SDR1]);
> + }
> hreg_compute_hflags(env);
> hreg_compute_mem_idx(env);
>
> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
> index 616aab6fb67d..dc2ebfc4524b 100644
> --- a/target-ppc/misc_helper.c
> +++ b/target-ppc/misc_helper.c
> @@ -38,7 +38,9 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t
> sprn)
>
> void helper_store_sdr1(CPUPPCState *env, target_ulong val)
> {
> - ppc_store_sdr1(env, val);
> + if (!env->external_htab) {
> + ppc_store_sdr1(env, val);
> + }
> }
>
> void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 04a840b01697..8e2f8e736a12 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -2014,6 +2014,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env,
> target_ulong addr) void ppc_store_sdr1(CPUPPCState *env, target_ulong
> value) {
> LOG_MMU("%s: " TARGET_FMT_lx "\n", __func__, value);
> + assert(!env->external_htab);
> if (env->spr[SPR_SDR1] != value) {
> env->spr[SPR_SDR1] = value;
> #if defined(TARGET_PPC64)
> @@ -2025,7 +2026,7 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong
> value) " stored in SDR1\n", htabsize);
> htabsize = 28;
> }
> - env->htab_mask = (1ULL << (htabsize + 18)) - 1;
> + env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
> env->htab_base = value & SDR_64_HTABORG;
> } else
> #endif /* defined(TARGET_PPC64) */
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-13 10:40 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2014-02-13 14:51 ` Alexander Graf
2014-02-14 13:06 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2014-02-13 14:51 UTC (permalink / raw)
To: Greg Kurz
Cc: list@suse.de:PowerPC, Paul Mackerras, Aneesh Kumar K.V,
QEMU Developers
On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> On Thu, 13 Feb 2014 08:29:28 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> Correctly update the htab_mask using the return value of
>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>> on GET_SREGS for HV. We check for external htab and if
>> found true, we don't need to update sdr1
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>
>> ---
>> Changes from V9:
>> * Fix TCG breakage
>>
>
> Alex,
>
> This patch is a complete replacement for commit 1df2d6d900a9 in your
> ppc-next branch.
>
> Please apply so that TCG works again ! :)
Thanks, applied to ppc-next.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-13 14:51 ` Alexander Graf
@ 2014-02-14 13:06 ` Alexander Graf
2014-02-14 13:54 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2014-02-14 13:06 UTC (permalink / raw)
To: Greg Kurz
Cc: Paul Mackerras, list@suse.de:PowerPC, Aneesh Kumar K.V,
QEMU Developers
On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
>
> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>
>> On Thu, 13 Feb 2014 08:29:28 +0530
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>
>>> Correctly update the htab_mask using the return value of
>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>> on GET_SREGS for HV. We check for external htab and if
>>> found true, we don't need to update sdr1
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>
>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>
>>> ---
>>> Changes from V9:
>>> * Fix TCG breakage
>>>
>>
>> Alex,
>>
>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>> ppc-next branch.
>>
>> Please apply so that TCG works again ! :)
>
> Thanks, applied to ppc-next.
With this patch applied -M pseries fails to boot a kernel for me.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-14 13:06 ` Alexander Graf
@ 2014-02-14 13:54 ` Alexander Graf
2014-02-14 14:28 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2014-02-14 13:54 UTC (permalink / raw)
To: Greg Kurz
Cc: list@suse.de:PowerPC, Paul Mackerras, Aneesh Kumar K.V,
QEMU Developers
On 14.02.2014, at 14:06, Alexander Graf <agraf@suse.de> wrote:
>
> On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
>
>>
>> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>>
>>> On Thu, 13 Feb 2014 08:29:28 +0530
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>
>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>
>>>> Correctly update the htab_mask using the return value of
>>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>>> on GET_SREGS for HV. We check for external htab and if
>>>> found true, we don't need to update sdr1
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>
>>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>>
>>>> ---
>>>> Changes from V9:
>>>> * Fix TCG breakage
>>>>
>>>
>>> Alex,
>>>
>>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>>> ppc-next branch.
>>>
>>> Please apply so that TCG works again ! :)
>>
>> Thanks, applied to ppc-next.
>
> With this patch applied -M pseries fails to boot a kernel for me.
-M mac99 also fails miserably. It almost looks as if the mask cuts off some bits, but please investigate this yourself. I'll remove the patch from the queue meanwhile.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-14 13:54 ` Alexander Graf
@ 2014-02-14 14:28 ` Alexander Graf
2014-02-14 14:42 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2014-02-14 14:28 UTC (permalink / raw)
To: Greg Kurz
Cc: Paul Mackerras, list@suse.de:PowerPC, Aneesh Kumar K.V,
QEMU Developers
On 14.02.2014, at 14:54, Alexander Graf <agraf@suse.de> wrote:
>
> On 14.02.2014, at 14:06, Alexander Graf <agraf@suse.de> wrote:
>
>>
>> On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
>>
>>>
>>> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>>>
>>>> On Thu, 13 Feb 2014 08:29:28 +0530
>>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>>
>>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>>
>>>>> Correctly update the htab_mask using the return value of
>>>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>>>> on GET_SREGS for HV. We check for external htab and if
>>>>> found true, we don't need to update sdr1
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>
>>>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>>>
>>>>> ---
>>>>> Changes from V9:
>>>>> * Fix TCG breakage
>>>>>
>>>>
>>>> Alex,
>>>>
>>>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>>>> ppc-next branch.
>>>>
>>>> Please apply so that TCG works again ! :)
>>>
>>> Thanks, applied to ppc-next.
>>
>> With this patch applied -M pseries fails to boot a kernel for me.
>
> -M mac99 also fails miserably. It almost looks as if the mask cuts off some bits, but please investigate this yourself. I'll remove the patch from the queue meanwhile.
Sigh, this whole series is just way too broken. I'll remove everything as of this patch from my queue. Aneesh, please rework the HTAB load/store patches and make sure that
- TCG works
- -M mac99 works
- 32bit guest works
- 32bit host works (casts!)
Then we can talk about applying them again.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-14 14:28 ` Alexander Graf
@ 2014-02-14 14:42 ` Alexander Graf
2014-02-15 11:02 ` Greg Kurz
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2014-02-14 14:42 UTC (permalink / raw)
To: Greg Kurz
Cc: list@suse.de:PowerPC, Paul Mackerras, Aneesh Kumar K.V,
QEMU Developers
On 14.02.2014, at 15:28, Alexander Graf <agraf@suse.de> wrote:
>
> On 14.02.2014, at 14:54, Alexander Graf <agraf@suse.de> wrote:
>
>>
>> On 14.02.2014, at 14:06, Alexander Graf <agraf@suse.de> wrote:
>>
>>>
>>> On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>>
>>>> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>>>>
>>>>> On Thu, 13 Feb 2014 08:29:28 +0530
>>>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>>>
>>>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>>>
>>>>>> Correctly update the htab_mask using the return value of
>>>>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>>>>> on GET_SREGS for HV. We check for external htab and if
>>>>>> found true, we don't need to update sdr1
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>>
>>>>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>>>>
>>>>>> ---
>>>>>> Changes from V9:
>>>>>> * Fix TCG breakage
>>>>>>
>>>>>
>>>>> Alex,
>>>>>
>>>>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>>>>> ppc-next branch.
>>>>>
>>>>> Please apply so that TCG works again ! :)
>>>>
>>>> Thanks, applied to ppc-next.
>>>
>>> With this patch applied -M pseries fails to boot a kernel for me.
>>
>> -M mac99 also fails miserably. It almost looks as if the mask cuts off some bits, but please investigate this yourself. I'll remove the patch from the queue meanwhile.
>
> Sigh, this whole series is just way too broken. I'll remove everything as of this patch from my queue. Aneesh, please rework the HTAB load/store patches and make sure that
>
> - TCG works
> - -M mac99 works
> - 32bit guest works
> - 32bit host works (casts!)
Oh, and please make sure every patch by itself covers the above, so that they're properly bisectable.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH V10] target-ppc: Fix htab_mask calculation
2014-02-14 14:42 ` Alexander Graf
@ 2014-02-15 11:02 ` Greg Kurz
0 siblings, 0 replies; 27+ messages in thread
From: Greg Kurz @ 2014-02-15 11:02 UTC (permalink / raw)
To: Alexander Graf
Cc: Paul Mackerras, list@suse.de:PowerPC, Aneesh Kumar K.V,
QEMU Developers
On Fri, 14 Feb 2014 15:42:49 +0100
Alexander Graf <agraf@suse.de> wrote:
> [...]
> >> -M mac99 also fails miserably. It almost looks as if the mask cuts off
> >> some bits, but please investigate this yourself. I'll remove the patch
> >> from the queue meanwhile.
> >
> > Sigh, this whole series is just way too broken. I'll remove everything
> > as of this patch from my queue. Aneesh, please rework the HTAB
> > load/store patches and make sure that
> >
> > - TCG works
> > - -M mac99 works
> > - 32bit guest works
> > - 32bit host works (casts!)
>
> Oh, and please make sure every patch by itself covers the above, so that
> they're properly bisectable.
>
>
I have a reworked patchset that fixes TCG and the build issues. I'll repost
as soon as I have done all the testing.
> Alex
>
>
Cheers.
--
Greg
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH V9 3/5] target-ppc: Fix page table lookup with kvm enabled
2014-01-28 7:59 [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Aneesh Kumar K.V
2014-01-28 7:59 ` [Qemu-devel] [PATCH V9 1/5] target-ppc: Update external_htab even when HTAB is managed by kernel Aneesh Kumar K.V
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 2/5] target-ppc: Fix htab_mask calculation Aneesh Kumar K.V
@ 2014-01-28 8:00 ` Aneesh Kumar K.V
2014-02-10 16:27 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 4/5] target-ppc: Change the hpte sore API Aneesh Kumar K.V
` (2 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-01-28 8:00 UTC (permalink / raw)
To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V
With kvm enabled, we store the hash page table information in the hypervisor.
Use ioctl to read the htab contents. Without this we get the below error when
trying to read the guest address
(gdb) x/10 do_fork
0xc000000000098660 <do_fork>: Cannot access memory at address 0xc000000000098660
(gdb)
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 1 +
hw/ppc/spapr_hcall.c | 50 +++++++++++++++++++------------
target-ppc/kvm.c | 53 +++++++++++++++++++++++++++++++++
target-ppc/kvm_ppc.h | 19 ++++++++++++
target-ppc/mmu-hash64.c | 78 ++++++++++++++++++++++++++++++++++++++++---------
target-ppc/mmu-hash64.h | 19 ++++++++----
6 files changed, 181 insertions(+), 39 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 009bb0112cc0..9241cdd2a86e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -685,6 +685,7 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
if (shift > 0) {
/* Kernel handles htab, we don't need to allocate one */
spapr->htab_shift = shift;
+ kvmppc_kern_htab = true;
} else {
if (!spapr->htab) {
/* Allocate an htab if we don't yet have one */
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f755a5392317..01cf6b05fee7 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -50,8 +50,9 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong ptel = args[3];
target_ulong page_shift = 12;
target_ulong raddr;
- target_ulong i;
+ target_ulong index;
hwaddr hpte;
+ uint64_t token;
/* only handle 4k and 16M pages for now */
if (pteh & HPTE64_V_LARGE) {
@@ -94,30 +95,37 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
return H_PARAMETER;
}
+
+ index = 0;
+ hpte = pte_index * HASH_PTE_SIZE_64;
if (likely((flags & H_EXACT) == 0)) {
pte_index &= ~7ULL;
- hpte = pte_index * HASH_PTE_SIZE_64;
- for (i = 0; ; ++i) {
- if (i == 8) {
+ token = ppc_hash64_start_access(cpu, pte_index);
+ do {
+ if (index == 8) {
+ ppc_hash64_stop_access(token);
return H_PTEG_FULL;
}
- if ((ppc_hash64_load_hpte0(env, hpte) & HPTE64_V_VALID) == 0) {
+ if ((ppc_hash64_load_hpte0(env, token, index) & HPTE64_V_VALID) == 0) {
break;
}
- hpte += HASH_PTE_SIZE_64;
- }
+ } while (index++);
+ ppc_hash64_stop_access(token);
} else {
- i = 0;
- hpte = pte_index * HASH_PTE_SIZE_64;
- if (ppc_hash64_load_hpte0(env, hpte) & HPTE64_V_VALID) {
+ token = ppc_hash64_start_access(cpu, pte_index);
+ if (ppc_hash64_load_hpte0(env, token, 0) & HPTE64_V_VALID) {
+ ppc_hash64_stop_access(token);
return H_PTEG_FULL;
}
+ ppc_hash64_stop_access(token);
}
+ hpte += index * HASH_PTE_SIZE_64;
+
ppc_hash64_store_hpte1(env, hpte, ptel);
/* eieio(); FIXME: need some sort of barrier for smp? */
ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY);
- args[0] = pte_index + i;
+ args[0] = pte_index + index;
return H_SUCCESS;
}
@@ -134,16 +142,17 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
target_ulong *vp, target_ulong *rp)
{
hwaddr hpte;
+ uint64_t token;
target_ulong v, r, rb;
if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
return REMOVE_PARM;
}
- hpte = ptex * HASH_PTE_SIZE_64;
-
- v = ppc_hash64_load_hpte0(env, hpte);
- r = ppc_hash64_load_hpte1(env, hpte);
+ token = ppc_hash64_start_access(ppc_env_get_cpu(env), ptex);
+ v = ppc_hash64_load_hpte0(env, token, 0);
+ r = ppc_hash64_load_hpte1(env, token, 0);
+ ppc_hash64_stop_access(token);
if ((v & HPTE64_V_VALID) == 0 ||
((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
@@ -152,6 +161,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
}
*vp = v;
*rp = r;
+ hpte = ptex * HASH_PTE_SIZE_64;
ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY);
rb = compute_tlbie_rb(v, r, ptex);
ppc_tlb_invalidate_one(env, rb);
@@ -260,16 +270,17 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong pte_index = args[1];
target_ulong avpn = args[2];
hwaddr hpte;
+ uint64_t token;
target_ulong v, r, rb;
if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
return H_PARAMETER;
}
- hpte = pte_index * HASH_PTE_SIZE_64;
-
- v = ppc_hash64_load_hpte0(env, hpte);
- r = ppc_hash64_load_hpte1(env, hpte);
+ token = ppc_hash64_start_access(cpu, pte_index);
+ v = ppc_hash64_load_hpte0(env, token, 0);
+ r = ppc_hash64_load_hpte1(env, token, 0);
+ ppc_hash64_stop_access(token);
if ((v & HPTE64_V_VALID) == 0 ||
((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
@@ -282,6 +293,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
r |= (flags << 48) & HPTE64_R_KEY_HI;
r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
rb = compute_tlbie_rb(v, r, pte_index);
+ hpte = pte_index * HASH_PTE_SIZE_64;
ppc_hash64_store_hpte0(env, hpte, (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY);
ppc_tlb_invalidate_one(env, rb);
ppc_hash64_store_hpte1(env, hpte, r);
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index c771ec11ed28..eefd78afc004 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1786,6 +1786,11 @@ bool kvmppc_has_cap_epr(void)
return cap_epr;
}
+bool kvmppc_has_cap_htab_fd(void)
+{
+ return cap_htab_fd;
+}
+
static int kvm_ppc_register_host_cpu_type(void)
{
TypeInfo type_info = {
@@ -1936,3 +1941,51 @@ void kvm_arch_remove_all_hw_breakpoints(void)
void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
{
}
+
+struct kvm_get_htab_buf {
+ struct kvm_get_htab_header header;
+ /*
+ * We require one extra byte for read
+ */
+ target_ulong hpte[(HPTES_PER_GROUP * 2) + 1];
+};
+
+uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
+{
+ int htab_fd;
+ struct kvm_get_htab_fd ghf;
+ struct kvm_get_htab_buf *hpte_buf;
+
+ ghf.flags = 0;
+ ghf.start_index = pte_index;
+ htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
+ if (htab_fd < 0) {
+ goto error_out;
+ }
+
+ hpte_buf = g_malloc0(sizeof(*hpte_buf));
+ /*
+ * Read the hpte group
+ */
+ if (read(htab_fd, hpte_buf, sizeof(*hpte_buf)) < 0) {
+ goto out_close;
+ }
+
+ close(htab_fd);
+ return (uint64_t) hpte_buf->hpte;
+
+out_close:
+ g_free(hpte_buf);
+ close(htab_fd);
+error_out:
+ return 0;
+}
+
+void kvmppc_hash64_free_pteg(uint64_t token)
+{
+ struct kvm_get_htab_buf *htab_buf;
+
+ htab_buf = container_of((void *)token, struct kvm_get_htab_buf, hpte);
+ g_free(htab_buf);
+ return;
+}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 5f78e4be14af..800e1ad0834f 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -39,10 +39,13 @@ uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
int kvmppc_fixup_cpu(PowerPCCPU *cpu);
bool kvmppc_has_cap_epr(void);
int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function);
+bool kvmppc_has_cap_htab_fd(void);
int kvmppc_get_htab_fd(bool write);
int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
uint16_t n_valid, uint16_t n_invalid);
+uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
+void kvmppc_hash64_free_pteg(uint64_t token);
#else
@@ -171,6 +174,11 @@ static inline int kvmppc_define_rtas_kernel_token(uint32_t token,
return -1;
}
+static inline bool kvmppc_has_cap_htab_fd(void)
+{
+ return false;
+}
+
static inline int kvmppc_get_htab_fd(bool write)
{
return -1;
@@ -188,6 +196,17 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
abort();
}
+static inline uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
+ target_ulong pte_index)
+{
+ abort();
+}
+
+static inline void kvmppc_hash64_free_pteg(uint64_t token)
+{
+ abort();
+}
+
#endif
#ifndef CONFIG_KVM
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 67fc1b5dec5e..11f3f6e731bb 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -41,6 +41,11 @@
#endif
/*
+ * Used to indicate whether we have allocated htab in the
+ * host kernel
+ */
+bool kvmppc_kern_htab;
+/*
* SLB handling
*/
@@ -302,29 +307,76 @@ static int ppc_hash64_amr_prot(CPUPPCState *env, ppc_hash_pte64_t pte)
return prot;
}
-static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr pteg_off,
+uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
+{
+ uint64_t token = 0;
+ hwaddr pte_offset;
+
+ pte_offset = pte_index * HASH_PTE_SIZE_64;
+ if (kvmppc_kern_htab) {
+ /*
+ * HTAB is controlled by KVM. Fetch the PTEG into a new buffer.
+ */
+ token = kvmppc_hash64_read_pteg(cpu, pte_index);
+ if (token) {
+ return token;
+ }
+ /*
+ * pteg read failed, even though we have allocated htab via
+ * kvmppc_reset_htab.
+ */
+ return 0;
+ }
+ /*
+ * HTAB is controlled by QEMU. Just point to the internally
+ * accessible PTEG.
+ */
+ if (cpu->env.external_htab) {
+ token = (uint64_t) cpu->env.external_htab + pte_offset;
+ } else if (cpu->env.htab_base) {
+ token = cpu->env.htab_base + pte_offset;
+ }
+ return token;
+}
+
+void ppc_hash64_stop_access(uint64_t token)
+{
+ if (kvmppc_kern_htab) {
+ return kvmppc_hash64_free_pteg(token);
+ }
+}
+
+static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr hash,
bool secondary, target_ulong ptem,
ppc_hash_pte64_t *pte)
{
- hwaddr pte_offset = pteg_off;
- target_ulong pte0, pte1;
int i;
+ uint64_t token;
+ target_ulong pte0, pte1;
+ target_ulong pte_index;
+ pte_index = (hash & env->htab_mask) * HPTES_PER_GROUP;
+ token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index);
+ if (!token) {
+ return -1;
+ }
for (i = 0; i < HPTES_PER_GROUP; i++) {
- pte0 = ppc_hash64_load_hpte0(env, pte_offset);
- pte1 = ppc_hash64_load_hpte1(env, pte_offset);
+ pte0 = ppc_hash64_load_hpte0(env, token, i);
+ pte1 = ppc_hash64_load_hpte1(env, token, i);
if ((pte0 & HPTE64_V_VALID)
&& (secondary == !!(pte0 & HPTE64_V_SECONDARY))
&& HPTE64_V_COMPARE(pte0, ptem)) {
pte->pte0 = pte0;
pte->pte1 = pte1;
- return pte_offset;
+ ppc_hash64_stop_access(token);
+ return (pte_index + i) * HASH_PTE_SIZE_64;
}
-
- pte_offset += HASH_PTE_SIZE_64;
}
-
+ ppc_hash64_stop_access(token);
+ /*
+ * We didn't find a valid entry.
+ */
return -1;
}
@@ -332,7 +384,7 @@ static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env,
ppc_slb_t *slb, target_ulong eaddr,
ppc_hash_pte64_t *pte)
{
- hwaddr pteg_off, pte_offset;
+ hwaddr pte_offset;
hwaddr hash;
uint64_t vsid, epnshift, epnmask, epn, ptem;
@@ -367,8 +419,7 @@ static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env,
" vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
" hash=" TARGET_FMT_plx "\n",
env->htab_base, env->htab_mask, vsid, ptem, hash);
- pteg_off = (hash * HASH_PTEG_SIZE_64) & env->htab_mask;
- pte_offset = ppc_hash64_pteg_search(env, pteg_off, 0, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(env, hash, 0, ptem, pte);
if (pte_offset == -1) {
/* Secondary PTEG lookup */
@@ -377,8 +428,7 @@ static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env,
" hash=" TARGET_FMT_plx "\n", env->htab_base,
env->htab_mask, vsid, ptem, ~hash);
- pteg_off = (~hash * HASH_PTEG_SIZE_64) & env->htab_mask;
- pte_offset = ppc_hash64_pteg_search(env, pteg_off, 1, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(env, ~hash, 1, ptem, pte);
}
return pte_offset;
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 55f5a230fd20..dc027f6d5264 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -75,23 +75,30 @@ int ppc_hash64_handle_mmu_fault(CPUPPCState *env, target_ulong address, int rw,
#define HPTE64_V_1TB_SEG 0x4000000000000000ULL
#define HPTE64_V_VRMA_MASK 0x4001ffffff000000ULL
+
+extern bool kvmppc_kern_htab;
+uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index);
+void ppc_hash64_stop_access(uint64_t token);
+
static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
- hwaddr pte_offset)
+ uint64_t token, int index)
{
+ index *= HASH_PTE_SIZE_64;
if (env->external_htab) {
- return ldq_p(env->external_htab + pte_offset);
+ return ldq_p((const void *)(token + index));
} else {
- return ldq_phys(env->htab_base + pte_offset);
+ return ldq_phys(token + index);
}
}
static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
- hwaddr pte_offset)
+ uint64_t token, int index)
{
+ index *= HASH_PTE_SIZE_64;
if (env->external_htab) {
- return ldq_p(env->external_htab + pte_offset + HASH_PTE_SIZE_64/2);
+ return ldq_p((const void *)(token + index + HASH_PTE_SIZE_64/2));
} else {
- return ldq_phys(env->htab_base + pte_offset + HASH_PTE_SIZE_64/2);
+ return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
}
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH V9 3/5] target-ppc: Fix page table lookup with kvm enabled
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 3/5] target-ppc: Fix page table lookup with kvm enabled Aneesh Kumar K.V
@ 2014-02-10 16:27 ` Greg Kurz
2014-02-12 15:24 ` [Qemu-devel] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code Greg Kurz
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2014-02-10 16:27 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: qemu-ppc, paulus, agraf, qemu-devel
On Tue, 28 Jan 2014 13:30:01 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> With kvm enabled, we store the hash page table information in the
> hypervisor. Use ioctl to read the htab contents. Without this we get the
> below error when trying to read the guest address
>
> (gdb) x/10 do_fork
> 0xc000000000098660 <do_fork>: Cannot access memory at address
> 0xc000000000098660 (gdb)
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr.c | 1 +
> hw/ppc/spapr_hcall.c | 50 +++++++++++++++++++------------
> target-ppc/kvm.c | 53 +++++++++++++++++++++++++++++++++
> target-ppc/kvm_ppc.h | 19 ++++++++++++
> target-ppc/mmu-hash64.c | 78
> ++++++++++++++++++++++++++++++++++++++++--------- target-ppc/mmu-hash64.h
> | 19 ++++++++---- 6 files changed, 181 insertions(+), 39 deletions(-)
>
> [... snip ...]
>
> diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> index 55f5a230fd20..dc027f6d5264 100644
> --- a/target-ppc/mmu-hash64.h
> +++ b/target-ppc/mmu-hash64.h
> @@ -75,23 +75,30 @@ int ppc_hash64_handle_mmu_fault(CPUPPCState *env,
> target_ulong address, int rw, #define HPTE64_V_1TB_SEG
> 0x4000000000000000ULL #define HPTE64_V_VRMA_MASK
> 0x4001ffffff000000ULL
>
> +
> +extern bool kvmppc_kern_htab;
> +uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong
> pte_index); +void ppc_hash64_stop_access(uint64_t token);
> +
> static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
> - hwaddr pte_offset)
> + uint64_t token, int
> index) {
> + index *= HASH_PTE_SIZE_64;
> if (env->external_htab) {
> - return ldq_p(env->external_htab + pte_offset);
> + return ldq_p((const void *)(token + index));
This does not compile on 32 bit:
target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
different size
> } else {
> - return ldq_phys(env->htab_base + pte_offset);
> + return ldq_phys(token + index);
> }
> }
>
> static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
> - hwaddr pte_offset)
> + uint64_t token, int
> index) {
> + index *= HASH_PTE_SIZE_64;
> if (env->external_htab) {
> - return ldq_p(env->external_htab + pte_offset +
> HASH_PTE_SIZE_64/2);
> + return ldq_p((const void *)(token + index +
Same error:
target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of different size
> HASH_PTE_SIZE_64/2)); } else {
> - return ldq_phys(env->htab_base + pte_offset +
> HASH_PTE_SIZE_64/2);
> + return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
> }
> }
>
^
--
Gregory Kurz kurzgreg@fr.ibm.com
gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
^ permalink raw reply [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code
2014-02-10 16:27 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2014-02-12 15:24 ` Greg Kurz
2014-02-13 3:00 ` Aneesh Kumar K.V
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2014-02-12 15:24 UTC (permalink / raw)
To: aneesh.kumar, agraf; +Cc: qemu-ppc, qemu-devel
The 396bb9874 commit reworked page table lookup to support kvm.
Unfortunately this breaks 32 bit build:
target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
different size
target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of
different size
The problem is that we have two cases to handle here:
- the HTAB is external and it is accessed with a pointer
- the HTAB is emulated and it is accessed with a hwaddr
Depending on the way the HTAB is controlled, we should use the appropriate
type:
- controlled by kvm, it is copied to an allocated buffer (pointer)
- controlled by qemu with an allocated buffer (pointer)
- controlled by qemu with soft mmu (hwaddr)
This patch introduces an explicit distinction between the two cases in
the new page table lookup code.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
hw/ppc/spapr_hcall.c | 6 +++---
target-ppc/kvm.c | 10 +++++-----
target-ppc/kvm_ppc.h | 10 +++++-----
target-ppc/mmu-hash64.c | 28 +++++++++++-----------------
target-ppc/mmu-hash64.h | 25 +++++++++++++++++--------
5 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 55d4eef..9653774 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -51,7 +51,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong page_shift = 12;
target_ulong raddr;
target_ulong index;
- uint64_t token;
+ ppc_hash64_access_t token;
/* only handle 4k and 16M pages for now */
if (pteh & HPTE64_V_LARGE) {
@@ -137,7 +137,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
target_ulong flags,
target_ulong *vp, target_ulong *rp)
{
- uint64_t token;
+ ppc_hash64_access_t token;
target_ulong v, r, rb;
if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
@@ -263,7 +263,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong flags = args[0];
target_ulong pte_index = args[1];
target_ulong avpn = args[2];
- uint64_t token;
+ ppc_hash64_access_t token;
target_ulong v, r, rb;
if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index e2b07a4..2eaf956 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1952,7 +1952,7 @@ struct kvm_get_htab_buf {
target_ulong hpte[(HPTES_PER_GROUP * 2) + 1];
};
-uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
+void *kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
{
int htab_fd;
struct kvm_get_htab_fd ghf;
@@ -1974,20 +1974,20 @@ uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
}
close(htab_fd);
- return (uint64_t) hpte_buf->hpte;
+ return hpte_buf->hpte;
out_close:
g_free(hpte_buf);
close(htab_fd);
error_out:
- return 0;
+ return NULL;
}
-void kvmppc_hash64_free_pteg(uint64_t token)
+void kvmppc_hash64_free_pteg(void *pteg)
{
struct kvm_get_htab_buf *htab_buf;
- htab_buf = container_of((void *)token, struct kvm_get_htab_buf, hpte);
+ htab_buf = container_of((void *)pteg, struct kvm_get_htab_buf, hpte);
g_free(htab_buf);
return;
}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index a65d345..bf29e7c 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -44,8 +44,8 @@ int kvmppc_get_htab_fd(bool write);
int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
uint16_t n_valid, uint16_t n_invalid);
-uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
-void kvmppc_hash64_free_pteg(uint64_t token);
+void *kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
+void kvmppc_hash64_free_pteg(void *pteg);
void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
target_ulong pte0, target_ulong pte1);
@@ -199,13 +199,13 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
abort();
}
-static inline uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
- target_ulong pte_index)
+static inline void *kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
+ target_ulong pte_index)
{
abort();
}
-static inline void kvmppc_hash64_free_pteg(uint64_t token)
+static inline void kvmppc_hash64_free_pteg(void *pteg)
{
abort();
}
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 4e574d9..98878ef 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -315,9 +315,10 @@ static int ppc_hash64_amr_prot(CPUPPCState *env, ppc_hash_pte64_t pte)
return prot;
}
-uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
+ppc_hash64_access_t ppc_hash64_start_access(PowerPCCPU *cpu,
+ target_ulong pte_index)
{
- uint64_t token = 0;
+ ppc_hash64_access_t token = { .phys_addr = 0UL };
hwaddr pte_offset;
pte_offset = pte_index * HASH_PTE_SIZE_64;
@@ -325,32 +326,25 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
/*
* HTAB is controlled by KVM. Fetch the PTEG into a new buffer.
*/
- token = kvmppc_hash64_read_pteg(cpu, pte_index);
- if (token) {
- return token;
- }
- /*
- * pteg read failed, even though we have allocated htab via
- * kvmppc_reset_htab.
- */
- return 0;
+ token.ext_ptr = kvmppc_hash64_read_pteg(cpu, pte_index);
+ return token;
}
/*
* HTAB is controlled by QEMU. Just point to the internally
* accessible PTEG.
*/
if (cpu->env.external_htab) {
- token = (uint64_t) cpu->env.external_htab + pte_offset;
+ token.ext_ptr = cpu->env.external_htab + pte_offset;
} else if (cpu->env.htab_base) {
- token = cpu->env.htab_base + pte_offset;
+ token.phys_addr = cpu->env.htab_base + pte_offset;
}
return token;
}
-void ppc_hash64_stop_access(uint64_t token)
+void ppc_hash64_stop_access(ppc_hash64_access_t token)
{
if (kvmppc_kern_htab) {
- return kvmppc_hash64_free_pteg(token);
+ return kvmppc_hash64_free_pteg(token.ext_ptr);
}
}
@@ -359,13 +353,13 @@ static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr hash,
ppc_hash_pte64_t *pte)
{
int i;
- uint64_t token;
+ ppc_hash64_access_t token;
target_ulong pte0, pte1;
target_ulong pte_index;
pte_index = (hash & env->htab_mask) * HPTES_PER_GROUP;
token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index);
- if (!token) {
+ if (token.phys_addr == 0UL) {
return -1;
}
for (i = 0; i < HPTES_PER_GROUP; i++) {
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 9c9ca1d..eb55190 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -79,28 +79,37 @@ void ppc_hash64_store_hpte(CPUPPCState *env, target_ulong index,
extern bool kvmppc_kern_htab;
-uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index);
-void ppc_hash64_stop_access(uint64_t token);
+
+typedef union {
+ void *ext_ptr;
+ hwaddr phys_addr;
+} ppc_hash64_access_t;
+
+ppc_hash64_access_t ppc_hash64_start_access(PowerPCCPU *cpu,
+ target_ulong pte_index);
+void ppc_hash64_stop_access(ppc_hash64_access_t token);
static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
- uint64_t token, int index)
+ ppc_hash64_access_t token,
+ int index)
{
index *= HASH_PTE_SIZE_64;
if (env->external_htab) {
- return ldq_p((const void *)(token + index));
+ return ldq_p(token.ext_ptr + index);
} else {
- return ldq_phys(token + index);
+ return ldq_phys(token.phys_addr + index);
}
}
static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
- uint64_t token, int index)
+ ppc_hash64_access_t token,
+ int index)
{
index *= HASH_PTE_SIZE_64;
if (env->external_htab) {
- return ldq_p((const void *)(token + index + HASH_PTE_SIZE_64/2));
+ return ldq_p(token.ext_ptr + index + HASH_PTE_SIZE_64/2);
} else {
- return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
+ return ldq_phys(token.phys_addr + index + HASH_PTE_SIZE_64/2);
}
}
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code
2014-02-12 15:24 ` [Qemu-devel] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code Greg Kurz
@ 2014-02-13 3:00 ` Aneesh Kumar K.V
2014-02-13 14:53 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-02-13 3:00 UTC (permalink / raw)
To: Greg Kurz, agraf; +Cc: qemu-ppc, qemu-devel
Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> The 396bb9874 commit reworked page table lookup to support kvm.
> Unfortunately this breaks 32 bit build:
>
> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
> target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
> different size
>
> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
> target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of
> different size
>
> The problem is that we have two cases to handle here:
> - the HTAB is external and it is accessed with a pointer
> - the HTAB is emulated and it is accessed with a hwaddr
>
> Depending on the way the HTAB is controlled, we should use the appropriate
> type:
> - controlled by kvm, it is copied to an allocated buffer (pointer)
> - controlled by qemu with an allocated buffer (pointer)
> - controlled by qemu with soft mmu (hwaddr)
>
> This patch introduces an explicit distinction between the two cases in
> the new page table lookup code.
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr_hcall.c | 6 +++---
> target-ppc/kvm.c | 10 +++++-----
> target-ppc/kvm_ppc.h | 10 +++++-----
> target-ppc/mmu-hash64.c | 28 +++++++++++-----------------
> target-ppc/mmu-hash64.h | 25 +++++++++++++++++--------
> 5 files changed, 41 insertions(+), 38 deletions(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 55d4eef..9653774 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -51,7 +51,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> target_ulong page_shift = 12;
> target_ulong raddr;
> target_ulong index;
> - uint64_t token;
> + ppc_hash64_access_t token;
>
> /* only handle 4k and 16M pages for now */
> if (pteh & HPTE64_V_LARGE) {
> @@ -137,7 +137,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
> target_ulong flags,
> target_ulong *vp, target_ulong *rp)
> {
> - uint64_t token;
> + ppc_hash64_access_t token;
> target_ulong v, r, rb;
>
> if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> @@ -263,7 +263,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> target_ulong flags = args[0];
> target_ulong pte_index = args[1];
> target_ulong avpn = args[2];
> - uint64_t token;
> + ppc_hash64_access_t token;
> target_ulong v, r, rb;
>
> if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index e2b07a4..2eaf956 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1952,7 +1952,7 @@ struct kvm_get_htab_buf {
> target_ulong hpte[(HPTES_PER_GROUP * 2) + 1];
> };
>
> -uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
> +void *kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
> {
> int htab_fd;
> struct kvm_get_htab_fd ghf;
> @@ -1974,20 +1974,20 @@ uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
> }
>
> close(htab_fd);
> - return (uint64_t) hpte_buf->hpte;
> + return hpte_buf->hpte;
>
> out_close:
> g_free(hpte_buf);
> close(htab_fd);
> error_out:
> - return 0;
> + return NULL;
> }
>
> -void kvmppc_hash64_free_pteg(uint64_t token)
> +void kvmppc_hash64_free_pteg(void *pteg)
> {
> struct kvm_get_htab_buf *htab_buf;
>
> - htab_buf = container_of((void *)token, struct kvm_get_htab_buf, hpte);
> + htab_buf = container_of((void *)pteg, struct kvm_get_htab_buf, hpte);
> g_free(htab_buf);
> return;
> }
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index a65d345..bf29e7c 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -44,8 +44,8 @@ int kvmppc_get_htab_fd(bool write);
> int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
> int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
> uint16_t n_valid, uint16_t n_invalid);
> -uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
> -void kvmppc_hash64_free_pteg(uint64_t token);
> +void *kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
> +void kvmppc_hash64_free_pteg(void *pteg);
>
> void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
> target_ulong pte0, target_ulong pte1);
> @@ -199,13 +199,13 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
> abort();
> }
>
> -static inline uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
> - target_ulong pte_index)
> +static inline void *kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
> + target_ulong pte_index)
> {
> abort();
> }
>
> -static inline void kvmppc_hash64_free_pteg(uint64_t token)
> +static inline void kvmppc_hash64_free_pteg(void *pteg)
> {
> abort();
> }
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index 4e574d9..98878ef 100644
> --- a/target-ppc/mmu-hash64.c
> +++ b/target-ppc/mmu-hash64.c
> @@ -315,9 +315,10 @@ static int ppc_hash64_amr_prot(CPUPPCState *env, ppc_hash_pte64_t pte)
> return prot;
> }
>
> -uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
> +ppc_hash64_access_t ppc_hash64_start_access(PowerPCCPU *cpu,
> + target_ulong pte_index)
> {
> - uint64_t token = 0;
> + ppc_hash64_access_t token = { .phys_addr = 0UL };
> hwaddr pte_offset;
>
> pte_offset = pte_index * HASH_PTE_SIZE_64;
> @@ -325,32 +326,25 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
> /*
> * HTAB is controlled by KVM. Fetch the PTEG into a new buffer.
> */
> - token = kvmppc_hash64_read_pteg(cpu, pte_index);
> - if (token) {
> - return token;
> - }
> - /*
> - * pteg read failed, even though we have allocated htab via
> - * kvmppc_reset_htab.
> - */
> - return 0;
> + token.ext_ptr = kvmppc_hash64_read_pteg(cpu, pte_index);
> + return token;
> }
> /*
> * HTAB is controlled by QEMU. Just point to the internally
> * accessible PTEG.
> */
> if (cpu->env.external_htab) {
> - token = (uint64_t) cpu->env.external_htab + pte_offset;
> + token.ext_ptr = cpu->env.external_htab + pte_offset;
> } else if (cpu->env.htab_base) {
> - token = cpu->env.htab_base + pte_offset;
> + token.phys_addr = cpu->env.htab_base + pte_offset;
> }
> return token;
> }
>
> -void ppc_hash64_stop_access(uint64_t token)
> +void ppc_hash64_stop_access(ppc_hash64_access_t token)
> {
> if (kvmppc_kern_htab) {
> - return kvmppc_hash64_free_pteg(token);
> + return kvmppc_hash64_free_pteg(token.ext_ptr);
> }
> }
>
> @@ -359,13 +353,13 @@ static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr hash,
> ppc_hash_pte64_t *pte)
> {
> int i;
> - uint64_t token;
> + ppc_hash64_access_t token;
> target_ulong pte0, pte1;
> target_ulong pte_index;
>
> pte_index = (hash & env->htab_mask) * HPTES_PER_GROUP;
> token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index);
> - if (!token) {
> + if (token.phys_addr == 0UL) {
> return -1;
> }
> for (i = 0; i < HPTES_PER_GROUP; i++) {
> diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> index 9c9ca1d..eb55190 100644
> --- a/target-ppc/mmu-hash64.h
> +++ b/target-ppc/mmu-hash64.h
> @@ -79,28 +79,37 @@ void ppc_hash64_store_hpte(CPUPPCState *env, target_ulong index,
>
>
> extern bool kvmppc_kern_htab;
> -uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index);
> -void ppc_hash64_stop_access(uint64_t token);
> +
> +typedef union {
> + void *ext_ptr;
> + hwaddr phys_addr;
> +} ppc_hash64_access_t;
> +
> +ppc_hash64_access_t ppc_hash64_start_access(PowerPCCPU *cpu,
> + target_ulong pte_index);
> +void ppc_hash64_stop_access(ppc_hash64_access_t token);
>
> static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
> - uint64_t token, int index)
> + ppc_hash64_access_t token,
> + int index)
> {
> index *= HASH_PTE_SIZE_64;
> if (env->external_htab) {
> - return ldq_p((const void *)(token + index));
> + return ldq_p(token.ext_ptr + index);
> } else {
> - return ldq_phys(token + index);
> + return ldq_phys(token.phys_addr + index);
> }
> }
>
> static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
> - uint64_t token, int index)
> + ppc_hash64_access_t token,
> + int index)
> {
> index *= HASH_PTE_SIZE_64;
> if (env->external_htab) {
> - return ldq_p((const void *)(token + index + HASH_PTE_SIZE_64/2));
> + return ldq_p(token.ext_ptr + index + HASH_PTE_SIZE_64/2);
> } else {
> - return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
> + return ldq_phys(token.phys_addr + index + HASH_PTE_SIZE_64/2);
> }
> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code
2014-02-13 3:00 ` Aneesh Kumar K.V
@ 2014-02-13 14:53 ` Alexander Graf
2014-02-13 16:54 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2014-02-13 14:53 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: list@suse.de:PowerPC, QEMU Developers, Greg Kurz
On 13.02.2014, at 04:00, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>
>> The 396bb9874 commit reworked page table lookup to support kvm.
>> Unfortunately this breaks 32 bit build:
>>
>> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
>> target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
>> different size
>>
>> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
>> target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of
>> different size
>>
>> The problem is that we have two cases to handle here:
>> - the HTAB is external and it is accessed with a pointer
>> - the HTAB is emulated and it is accessed with a hwaddr
>>
>> Depending on the way the HTAB is controlled, we should use the appropriate
>> type:
>> - controlled by kvm, it is copied to an allocated buffer (pointer)
>> - controlled by qemu with an allocated buffer (pointer)
>> - controlled by qemu with soft mmu (hwaddr)
>>
>> This patch introduces an explicit distinction between the two cases in
>> the new page table lookup code.
>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
I wouldn't mind something slightly lighter weight. How about this one instead? If you guys think it's better to have an actual type for the token I'd pull in this patch as is though.
Alex
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 4e574d9..3240427 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -340,7 +340,7 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
* accessible PTEG.
*/
if (cpu->env.external_htab) {
- token = (uint64_t) cpu->env.external_htab + pte_offset;
+ token = (uint64_t)(uintptr_t) cpu->env.external_htab + pte_offset;
} else if (cpu->env.htab_base) {
token = cpu->env.htab_base + pte_offset;
}
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 9c9ca1d..8fb2ae4 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -85,22 +85,24 @@ void ppc_hash64_stop_access(uint64_t token);
static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
uint64_t token, int index)
{
- index *= HASH_PTE_SIZE_64;
+ uint64_t addr;
+ addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
if (env->external_htab) {
- return ldq_p((const void *)(token + index));
+ return ldq_p((const void *)(uintptr_t)addr);
} else {
- return ldq_phys(token + index);
+ return ldq_phys(addr);
}
}
static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
uint64_t token, int index)
{
- index *= HASH_PTE_SIZE_64;
+ uint64_t addr;
+ addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
if (env->external_htab) {
- return ldq_p((const void *)(token + index + HASH_PTE_SIZE_64/2));
+ return ldq_p((const void *)(uintptr_t)addr);
} else {
- return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
+ return ldq_phys(addr);
}
}
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code
2014-02-13 14:53 ` Alexander Graf
@ 2014-02-13 16:54 ` Greg Kurz
2014-02-14 9:25 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2014-02-13 16:54 UTC (permalink / raw)
To: Alexander Graf; +Cc: list@suse.de:PowerPC, Aneesh Kumar K.V, QEMU Developers
On Thu, 13 Feb 2014 15:53:16 +0100
Alexander Graf <agraf@suse.de> wrote:
>
> On 13.02.2014, at 04:00, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> > Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> >
> >> The 396bb9874 commit reworked page table lookup to support kvm.
> >> Unfortunately this breaks 32 bit build:
> >>
> >> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
> >> target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
> >> different size
> >>
> >> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
> >> target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of
> >> different size
> >>
> >> The problem is that we have two cases to handle here:
> >> - the HTAB is external and it is accessed with a pointer
> >> - the HTAB is emulated and it is accessed with a hwaddr
> >>
> >> Depending on the way the HTAB is controlled, we should use the
> >> appropriate type:
> >> - controlled by kvm, it is copied to an allocated buffer (pointer)
> >> - controlled by qemu with an allocated buffer (pointer)
> >> - controlled by qemu with soft mmu (hwaddr)
> >>
> >> This patch introduces an explicit distinction between the two cases in
> >> the new page table lookup code.
> >
> > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> I wouldn't mind something slightly lighter weight. How about this one
> instead? If you guys think it's better to have an actual type for the
> token I'd pull in this patch as is though.
>
>
> Alex
>
I confess... I consider castings evil and favor explicit typing. :)
On the other hand, I have no strong opinions against your patch. The "token"
code is quite simple and risks of confusion are low, we can live with it.
Please add:
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Thanks for your time.
--
Greg
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index 4e574d9..3240427 100644
> --- a/target-ppc/mmu-hash64.c
> +++ b/target-ppc/mmu-hash64.c
> @@ -340,7 +340,7 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu,
> target_ulong pte_index)
> * accessible PTEG.
> */
> if (cpu->env.external_htab) {
> - token = (uint64_t) cpu->env.external_htab + pte_offset;
> + token = (uint64_t)(uintptr_t) cpu->env.external_htab +
> pte_offset; } else if (cpu->env.htab_base) {
> token = cpu->env.htab_base + pte_offset;
> }
> diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> index 9c9ca1d..8fb2ae4 100644
> --- a/target-ppc/mmu-hash64.h
> +++ b/target-ppc/mmu-hash64.h
> @@ -85,22 +85,24 @@ void ppc_hash64_stop_access(uint64_t token);
> static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env,
> uint64_t token, int
> index) {
> - index *= HASH_PTE_SIZE_64;
> + uint64_t addr;
> + addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
> if (env->external_htab) {
> - return ldq_p((const void *)(token + index));
> + return ldq_p((const void *)(uintptr_t)addr);
> } else {
> - return ldq_phys(token + index);
> + return ldq_phys(addr);
> }
> }
>
> static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
> uint64_t token, int
> index) {
> - index *= HASH_PTE_SIZE_64;
> + uint64_t addr;
> + addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
> if (env->external_htab) {
> - return ldq_p((const void *)(token + index +
> HASH_PTE_SIZE_64/2));
> + return ldq_p((const void *)(uintptr_t)addr);
> } else {
> - return ldq_phys(token + index + HASH_PTE_SIZE_64/2);
> + return ldq_phys(addr);
> }
> }
>
>
--
Gregory Kurz kurzgreg@fr.ibm.com
gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: fix 32 bit build break in the page table lookup code
2014-02-13 16:54 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2014-02-14 9:25 ` Alexander Graf
0 siblings, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2014-02-14 9:25 UTC (permalink / raw)
To: Greg Kurz; +Cc: list@suse.de:PowerPC, Aneesh Kumar K.V, QEMU Developers
On 13.02.2014, at 17:54, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> On Thu, 13 Feb 2014 15:53:16 +0100
> Alexander Graf <agraf@suse.de> wrote:
>>
>> On 13.02.2014, at 04:00, Aneesh Kumar K.V
>> <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>
>>> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>>>
>>>> The 396bb9874 commit reworked page table lookup to support kvm.
>>>> Unfortunately this breaks 32 bit build:
>>>>
>>>> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte0’:
>>>> target-ppc/mmu-hash64.h:90:23: error: cast to pointer from integer of
>>>> different size
>>>>
>>>> target-ppc/mmu-hash64.h: In function ‘ppc_hash64_load_hpte1’:
>>>> target-ppc/mmu-hash64.h:101:23: error: cast to pointer from integer of
>>>> different size
>>>>
>>>> The problem is that we have two cases to handle here:
>>>> - the HTAB is external and it is accessed with a pointer
>>>> - the HTAB is emulated and it is accessed with a hwaddr
>>>>
>>>> Depending on the way the HTAB is controlled, we should use the
>>>> appropriate type:
>>>> - controlled by kvm, it is copied to an allocated buffer (pointer)
>>>> - controlled by qemu with an allocated buffer (pointer)
>>>> - controlled by qemu with soft mmu (hwaddr)
>>>>
>>>> This patch introduces an explicit distinction between the two cases in
>>>> the new page table lookup code.
>>>
>>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>
>> I wouldn't mind something slightly lighter weight. How about this one
>> instead? If you guys think it's better to have an actual type for the
>> token I'd pull in this patch as is though.
>>
>>
>> Alex
>>
>
> I confess... I consider castings evil and favor explicit typing. :)
Heh - I dislike unions :).
> On the other hand, I have no strong opinions against your patch. The "token"
> code is quite simple and risks of confusion are low, we can live with it.
>
> Please add:
>
> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>
> Thanks for your time.
I've folded my patch into the offending patch in my queue. That way we ensure bisectability of the tree.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH V9 4/5] target-ppc: Change the hpte sore API
2014-01-28 7:59 [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Aneesh Kumar K.V
` (2 preceding siblings ...)
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 3/5] target-ppc: Fix page table lookup with kvm enabled Aneesh Kumar K.V
@ 2014-01-28 8:00 ` Aneesh Kumar K.V
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 5/5] target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab Aneesh Kumar K.V
2014-02-06 14:58 ` [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Alexander Graf
5 siblings, 0 replies; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-01-28 8:00 UTC (permalink / raw)
To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V
For updating in kernel htab we need to provide both pte0 and pte1, hence update
the interface to take pte0 and pte1 together
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/ppc/spapr_hcall.c | 20 ++++++--------------
target-ppc/mmu-hash64.c | 3 ++-
target-ppc/mmu-hash64.h | 22 ++++++++--------------
3 files changed, 16 insertions(+), 29 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 01cf6b05fee7..55d4eef1d960 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -51,7 +51,6 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong page_shift = 12;
target_ulong raddr;
target_ulong index;
- hwaddr hpte;
uint64_t token;
/* only handle 4k and 16M pages for now */
@@ -97,7 +96,6 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
}
index = 0;
- hpte = pte_index * HASH_PTE_SIZE_64;
if (likely((flags & H_EXACT) == 0)) {
pte_index &= ~7ULL;
token = ppc_hash64_start_access(cpu, pte_index);
@@ -119,11 +117,9 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
}
ppc_hash64_stop_access(token);
}
- hpte += index * HASH_PTE_SIZE_64;
- ppc_hash64_store_hpte1(env, hpte, ptel);
- /* eieio(); FIXME: need some sort of barrier for smp? */
- ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY);
+ ppc_hash64_store_hpte(env, pte_index + index,
+ pteh | HPTE64_V_HPTE_DIRTY, ptel);
args[0] = pte_index + index;
return H_SUCCESS;
@@ -141,7 +137,6 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
target_ulong flags,
target_ulong *vp, target_ulong *rp)
{
- hwaddr hpte;
uint64_t token;
target_ulong v, r, rb;
@@ -161,8 +156,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
}
*vp = v;
*rp = r;
- hpte = ptex * HASH_PTE_SIZE_64;
- ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY);
+ ppc_hash64_store_hpte(env, ptex, HPTE64_V_HPTE_DIRTY, 0);
rb = compute_tlbie_rb(v, r, ptex);
ppc_tlb_invalidate_one(env, rb);
return REMOVE_SUCCESS;
@@ -269,7 +263,6 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
target_ulong flags = args[0];
target_ulong pte_index = args[1];
target_ulong avpn = args[2];
- hwaddr hpte;
uint64_t token;
target_ulong v, r, rb;
@@ -293,12 +286,11 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
r |= (flags << 48) & HPTE64_R_KEY_HI;
r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
rb = compute_tlbie_rb(v, r, pte_index);
- hpte = pte_index * HASH_PTE_SIZE_64;
- ppc_hash64_store_hpte0(env, hpte, (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY);
+ ppc_hash64_store_hpte(env, pte_index,
+ (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
ppc_tlb_invalidate_one(env, rb);
- ppc_hash64_store_hpte1(env, hpte, r);
/* Don't need a memory barrier, due to qemu's global lock */
- ppc_hash64_store_hpte0(env, hpte, v | HPTE64_V_HPTE_DIRTY);
+ ppc_hash64_store_hpte(env, pte_index, v | HPTE64_V_HPTE_DIRTY, r);
return H_SUCCESS;
}
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 11f3f6e731bb..fb297d62e3a6 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -558,7 +558,8 @@ int ppc_hash64_handle_mmu_fault(CPUPPCState *env, target_ulong eaddr,
}
if (new_pte1 != pte.pte1) {
- ppc_hash64_store_hpte1(env, pte_offset, new_pte1);
+ ppc_hash64_store_hpte(env, pte_offset / HASH_PTE_SIZE_64,
+ pte.pte0, new_pte1);
}
/* 7. Determine the real address from the PTE */
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index dc027f6d5264..3b6769ad130b 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -102,23 +102,17 @@ static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
}
}
-static inline void ppc_hash64_store_hpte0(CPUPPCState *env,
- hwaddr pte_offset, target_ulong pte0)
+static inline void ppc_hash64_store_hpte(CPUPPCState *env,
+ target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
{
+ pte_index *= HASH_PTE_SIZE_64;
if (env->external_htab) {
- stq_p(env->external_htab + pte_offset, pte0);
+ stq_p(env->external_htab + pte_index, pte0);
+ stq_p(env->external_htab + pte_index + HASH_PTE_SIZE_64/2, pte1);
} else {
- stq_phys(env->htab_base + pte_offset, pte0);
- }
-}
-
-static inline void ppc_hash64_store_hpte1(CPUPPCState *env,
- hwaddr pte_offset, target_ulong pte1)
-{
- if (env->external_htab) {
- stq_p(env->external_htab + pte_offset + HASH_PTE_SIZE_64/2, pte1);
- } else {
- stq_phys(env->htab_base + pte_offset + HASH_PTE_SIZE_64/2, pte1);
+ stq_phys(env->htab_base + pte_index, pte0);
+ stq_phys(env->htab_base + pte_index + HASH_PTE_SIZE_64/2, pte1);
}
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH V9 5/5] target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab
2014-01-28 7:59 [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Aneesh Kumar K.V
` (3 preceding siblings ...)
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 4/5] target-ppc: Change the hpte sore API Aneesh Kumar K.V
@ 2014-01-28 8:00 ` Aneesh Kumar K.V
2014-02-10 15:25 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2014-02-06 14:58 ` [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Alexander Graf
5 siblings, 1 reply; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-01-28 8:00 UTC (permalink / raw)
To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V
This support updating htab managed by the hypervisor. Currently we don't have
any user for this feature. This actually bring the store_hpte interface
in-line with the load_hpte one. We may want to use this when we want to
emulate henter hcall in qemu for HV kvm.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
target-ppc/kvm.c | 30 ++++++++++++++++++++++++++++++
target-ppc/kvm_ppc.h | 10 ++++++++++
target-ppc/mmu-hash64.c | 18 ++++++++++++++++++
target-ppc/mmu-hash64.h | 16 ++--------------
4 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index eefd78afc004..893b59f99fa3 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1989,3 +1989,33 @@ void kvmppc_hash64_free_pteg(uint64_t token)
g_free(htab_buf);
return;
}
+
+void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
+{
+ int htab_fd;
+ struct kvm_get_htab_fd ghf;
+ struct kvm_get_htab_buf hpte_buf;
+
+ ghf.flags = 0;
+ ghf.start_index = 0; /* Ignored */
+ htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
+ if (htab_fd < 0) {
+ goto error_out;
+ }
+
+ hpte_buf.header.n_valid = 1;
+ hpte_buf.header.n_invalid = 0;
+ hpte_buf.header.index = pte_index;
+ hpte_buf.hpte[0] = pte0;
+ hpte_buf.hpte[1] = pte1;
+ /*
+ * Write the hpte entry
+ */
+ write(htab_fd, &hpte_buf, sizeof(hpte_buf));
+ close(htab_fd);
+ return;
+
+error_out:
+ return;
+}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 800e1ad0834f..a65d34571914 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -47,6 +47,9 @@ int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
void kvmppc_hash64_free_pteg(uint64_t token);
+void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1);
+
#else
static inline uint32_t kvmppc_get_tbfreq(void)
@@ -207,6 +210,13 @@ static inline void kvmppc_hash64_free_pteg(uint64_t token)
abort();
}
+static inline void kvmppc_hash64_write_pte(CPUPPCState *env,
+ target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
+{
+ abort();
+}
+
#endif
#ifndef CONFIG_KVM
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index fb297d62e3a6..9f5db1b3d9b5 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -595,3 +595,21 @@ hwaddr ppc_hash64_get_phys_page_debug(CPUPPCState *env, target_ulong addr)
return ppc_hash64_pte_raddr(slb, pte, addr) & TARGET_PAGE_MASK;
}
+
+void ppc_hash64_store_hpte(CPUPPCState *env,
+ target_ulong pte_index,
+ target_ulong pte0, target_ulong pte1)
+{
+ if (kvmppc_kern_htab) {
+ return kvmppc_hash64_write_pte(env, pte_index, pte0, pte1);
+ }
+
+ pte_index *= HASH_PTE_SIZE_64;
+ if (env->external_htab) {
+ stq_p(env->external_htab + pte_index, pte0);
+ stq_p(env->external_htab + pte_index + HASH_PTE_SIZE_64/2, pte1);
+ } else {
+ stq_phys(env->htab_base + pte_index, pte0);
+ stq_phys(env->htab_base + pte_index + HASH_PTE_SIZE_64/2, pte1);
+ }
+}
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 3b6769ad130b..9c9ca1dfe2b5 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -9,6 +9,8 @@ int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
hwaddr ppc_hash64_get_phys_page_debug(CPUPPCState *env, target_ulong addr);
int ppc_hash64_handle_mmu_fault(CPUPPCState *env, target_ulong address, int rw,
int mmu_idx);
+void ppc_hash64_store_hpte(CPUPPCState *env, target_ulong index,
+ target_ulong pte0, target_ulong pte1);
#endif
/*
@@ -102,20 +104,6 @@ static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env,
}
}
-static inline void ppc_hash64_store_hpte(CPUPPCState *env,
- target_ulong pte_index,
- target_ulong pte0, target_ulong pte1)
-{
- pte_index *= HASH_PTE_SIZE_64;
- if (env->external_htab) {
- stq_p(env->external_htab + pte_index, pte0);
- stq_p(env->external_htab + pte_index + HASH_PTE_SIZE_64/2, pte1);
- } else {
- stq_phys(env->htab_base + pte_index, pte0);
- stq_phys(env->htab_base + pte_index + HASH_PTE_SIZE_64/2, pte1);
- }
-}
-
typedef struct {
uint64_t pte0, pte1;
} ppc_hash_pte64_t;
--
1.8.5.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [Qemu-ppc] [PATCH V9 5/5] target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 5/5] target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab Aneesh Kumar K.V
@ 2014-02-10 15:25 ` Greg Kurz
2014-02-12 15:40 ` [Qemu-devel] [PATCH] target-ppc: fix warn_unused_result build break with in-kernel HTAB support Greg Kurz
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2014-02-10 15:25 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: qemu-ppc, paulus, agraf, qemu-devel
On Tue, 28 Jan 2014 13:30:03 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> This support updating htab managed by the hypervisor. Currently we don't
> have any user for this feature. This actually bring the store_hpte
> interface in-line with the load_hpte one. We may want to use this when we
> want to emulate henter hcall in qemu for HV kvm.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> target-ppc/kvm.c | 30 ++++++++++++++++++++++++++++++
> target-ppc/kvm_ppc.h | 10 ++++++++++
> target-ppc/mmu-hash64.c | 18 ++++++++++++++++++
> target-ppc/mmu-hash64.h | 16 ++--------------
> 4 files changed, 60 insertions(+), 14 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index eefd78afc004..893b59f99fa3 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1989,3 +1989,33 @@ void kvmppc_hash64_free_pteg(uint64_t token)
> g_free(htab_buf);
> return;
> }
> +
> +void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
> + target_ulong pte0, target_ulong pte1)
> +{
> + int htab_fd;
> + struct kvm_get_htab_fd ghf;
> + struct kvm_get_htab_buf hpte_buf;
> +
> + ghf.flags = 0;
> + ghf.start_index = 0; /* Ignored */
> + htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
> + if (htab_fd < 0) {
> + goto error_out;
> + }
> +
> + hpte_buf.header.n_valid = 1;
> + hpte_buf.header.n_invalid = 0;
> + hpte_buf.header.index = pte_index;
> + hpte_buf.hpte[0] = pte0;
> + hpte_buf.hpte[1] = pte1;
> + /*
> + * Write the hpte entry
> + */
> + write(htab_fd, &hpte_buf, sizeof(hpte_buf));
Hmmm... this does not compile for me because of the warn_unused_result
attribute. And BTW, why not checking the return value ?
> + close(htab_fd);
> + return;
> +
> +error_out:
> + return;
> +}
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 800e1ad0834f..a65d34571914 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -47,6 +47,9 @@ int kvmppc_load_htab_chunk(QEMUFile *f, int fd,
> uint32_t index, uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
> target_ulong pte_index); void kvmppc_hash64_free_pteg(uint64_t token);
>
> +void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
> + target_ulong pte0, target_ulong pte1);
> +
> #else
>
> static inline uint32_t kvmppc_get_tbfreq(void)
> @@ -207,6 +210,13 @@ static inline void kvmppc_hash64_free_pteg(uint64_t
> token) abort();
> }
>
> +static inline void kvmppc_hash64_write_pte(CPUPPCState *env,
> + target_ulong pte_index,
> + target_ulong pte0,
> target_ulong pte1) +{
> + abort();
> +}
> +
> #endif
>
> #ifndef CONFIG_KVM
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index fb297d62e3a6..9f5db1b3d9b5 100644
> --- a/target-ppc/mmu-hash64.c
> +++ b/target-ppc/mmu-hash64.c
> @@ -595,3 +595,21 @@ hwaddr ppc_hash64_get_phys_page_debug(CPUPPCState
> *env, target_ulong addr)
>
> return ppc_hash64_pte_raddr(slb, pte, addr) & TARGET_PAGE_MASK;
> }
> +
> +void ppc_hash64_store_hpte(CPUPPCState *env,
> + target_ulong pte_index,
> + target_ulong pte0, target_ulong pte1)
> +{
> + if (kvmppc_kern_htab) {
> + return kvmppc_hash64_write_pte(env, pte_index, pte0, pte1);
> + }
> +
> + pte_index *= HASH_PTE_SIZE_64;
> + if (env->external_htab) {
> + stq_p(env->external_htab + pte_index, pte0);
> + stq_p(env->external_htab + pte_index + HASH_PTE_SIZE_64/2, pte1);
> + } else {
> + stq_phys(env->htab_base + pte_index, pte0);
> + stq_phys(env->htab_base + pte_index + HASH_PTE_SIZE_64/2, pte1);
> + }
> +}
> diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> index 3b6769ad130b..9c9ca1dfe2b5 100644
> --- a/target-ppc/mmu-hash64.h
> +++ b/target-ppc/mmu-hash64.h
> @@ -9,6 +9,8 @@ int ppc_store_slb (CPUPPCState *env, target_ulong rb,
> target_ulong rs); hwaddr ppc_hash64_get_phys_page_debug(CPUPPCState *env,
> target_ulong addr); int ppc_hash64_handle_mmu_fault(CPUPPCState *env,
> target_ulong address, int rw, int mmu_idx);
> +void ppc_hash64_store_hpte(CPUPPCState *env, target_ulong index,
> + target_ulong pte0, target_ulong pte1);
> #endif
>
> /*
> @@ -102,20 +104,6 @@ static inline target_ulong
> ppc_hash64_load_hpte1(CPUPPCState *env, }
> }
>
> -static inline void ppc_hash64_store_hpte(CPUPPCState *env,
> - target_ulong pte_index,
> - target_ulong pte0, target_ulong
> pte1) -{
> - pte_index *= HASH_PTE_SIZE_64;
> - if (env->external_htab) {
> - stq_p(env->external_htab + pte_index, pte0);
> - stq_p(env->external_htab + pte_index + HASH_PTE_SIZE_64/2, pte1);
> - } else {
> - stq_phys(env->htab_base + pte_index, pte0);
> - stq_phys(env->htab_base + pte_index + HASH_PTE_SIZE_64/2, pte1);
> - }
> -}
> -
> typedef struct {
> uint64_t pte0, pte1;
> } ppc_hash_pte64_t;
--
Gregory Kurz kurzgreg@fr.ibm.com
gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
^ permalink raw reply [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH] target-ppc: fix warn_unused_result build break with in-kernel HTAB support
2014-02-10 15:25 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2014-02-12 15:40 ` Greg Kurz
2014-02-13 3:00 ` Aneesh Kumar K.V
0 siblings, 1 reply; 27+ messages in thread
From: Greg Kurz @ 2014-02-12 15:40 UTC (permalink / raw)
To: aneesh.kumar, agraf; +Cc: qemu-ppc, qemu-devel
The 7029677e4 commit introduced the following build break:
target-ppc/kvm.c: In function ‘kvmppc_hash64_write_pte’:
target-ppc/kvm.c:2017:10: error: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Werror=unused-result]
write(htab_fd, &hpte_buf, sizeof(hpte_buf));
^
Even though nothing is done for the moment if kvm_htab_write() fails, we
obviously need to be explicit. Let's add an *empty* return path to please
gcc.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
target-ppc/kvm.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 2eaf956..343376c 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2012,9 +2012,15 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
hpte_buf.hpte[0] = pte0;
hpte_buf.hpte[1] = pte1;
/*
- * Write the hpte entry
+ * Write the hpte entry.
+ * CAUTION: write() has the warn_unused_result attribute. Hence we
+ * need to check the return value, even though we do nothing.
*/
- write(htab_fd, &hpte_buf, sizeof(hpte_buf));
+ if (write(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) {
+ goto out_close;
+ }
+
+out_close:
close(htab_fd);
return;
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix warn_unused_result build break with in-kernel HTAB support
2014-02-12 15:40 ` [Qemu-devel] [PATCH] target-ppc: fix warn_unused_result build break with in-kernel HTAB support Greg Kurz
@ 2014-02-13 3:00 ` Aneesh Kumar K.V
2014-02-13 14:51 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Aneesh Kumar K.V @ 2014-02-13 3:00 UTC (permalink / raw)
To: Greg Kurz, agraf; +Cc: qemu-ppc, qemu-devel
Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> The 7029677e4 commit introduced the following build break:
>
> target-ppc/kvm.c: In function ‘kvmppc_hash64_write_pte’:
> target-ppc/kvm.c:2017:10: error: ignoring return value of ‘write’, declared
> with attribute warn_unused_result [-Werror=unused-result]
> write(htab_fd, &hpte_buf, sizeof(hpte_buf));
> ^
>
> Even though nothing is done for the moment if kvm_htab_write() fails, we
> obviously need to be explicit. Let's add an *empty* return path to please
> gcc.
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> target-ppc/kvm.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2eaf956..343376c 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -2012,9 +2012,15 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
> hpte_buf.hpte[0] = pte0;
> hpte_buf.hpte[1] = pte1;
> /*
> - * Write the hpte entry
> + * Write the hpte entry.
> + * CAUTION: write() has the warn_unused_result attribute. Hence we
> + * need to check the return value, even though we do nothing.
> */
> - write(htab_fd, &hpte_buf, sizeof(hpte_buf));
> + if (write(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) {
> + goto out_close;
> + }
> +
> +out_close:
> close(htab_fd);
> return;
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH] target-ppc: fix warn_unused_result build break with in-kernel HTAB support
2014-02-13 3:00 ` Aneesh Kumar K.V
@ 2014-02-13 14:51 ` Alexander Graf
0 siblings, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2014-02-13 14:51 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: list@suse.de:PowerPC, QEMU Developers, Greg Kurz
On 13.02.2014, at 04:00, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>
>> The 7029677e4 commit introduced the following build break:
>>
>> target-ppc/kvm.c: In function ‘kvmppc_hash64_write_pte’:
>> target-ppc/kvm.c:2017:10: error: ignoring return value of ‘write’, declared
>> with attribute warn_unused_result [-Werror=unused-result]
>> write(htab_fd, &hpte_buf, sizeof(hpte_buf));
>> ^
>>
>> Even though nothing is done for the moment if kvm_htab_write() fails, we
>> obviously need to be explicit. Let's add an *empty* return path to please
>> gcc.
>>
>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Thanks, applied to ppc-next.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server
2014-01-28 7:59 [Qemu-devel] [PATCH V9 0/5] target-ppc: Add support for dumping guest memory using qemu gdb server Aneesh Kumar K.V
` (4 preceding siblings ...)
2014-01-28 8:00 ` [Qemu-devel] [PATCH V9 5/5] target-ppc: Update ppc_hash64_store_hpte to support updating in-kernel htab Aneesh Kumar K.V
@ 2014-02-06 14:58 ` Alexander Graf
5 siblings, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2014-02-06 14:58 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: list@suse.de:PowerPC, Paul Mackerras, QEMU Developers
On 28.01.2014, at 08:59, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Hi,
>
>
> This patch series implement support for dumping guest memory using qemu gdb server. The last patch also enable qemu monitor command dump-guest-memory
>
> With this patch series we can now do
>
> (gdb) x/4i htab_call_hpte_insert1
> 0xc0000000000470d8 <.htab_call_hpte_insert1>: bl 0xc0000000000470d8 <.htab_call_hpte_insert1>
> 0xc0000000000470dc <.htab_call_hpte_insert1+4>: cmpdi r3,0
> 0xc0000000000470e0 <.htab_call_hpte_insert1+8>: bge 0xc000000000047190 <htab_pte_insert_ok>
> 0xc0000000000470e4 <.htab_call_hpte_insert1+12>: cmpdi r3,-2
> (gdb) target remote localhost:1234
> Remote debugging using localhost:1234
> .plpar_hcall_norets () at arch/powerpc/platforms/pseries/hvCall.S:119
> 119 HCALL_INST_POSTCALL_NORETS
> (gdb) x/4i htab_call_hpte_insert1
> 0xc0000000000470d8 <.htab_call_hpte_insert1>: bl 0xc00000000005f8f0 <pSeries_lpar_hpte_insert>
> 0xc0000000000470dc <.htab_call_hpte_insert1+4>: cmpdi r3,0
> 0xc0000000000470e0 <.htab_call_hpte_insert1+8>: bge 0xc000000000047190 <htab_pte_insert_ok>
> 0xc0000000000470e4 <.htab_call_hpte_insert1+12>: cmpdi r3,-2
> (gdb)
>
> NOTE: We still don't support inserting breakpoints.
>
> Before Fix:
> (qemu) memsave 0xc0000000000470d8 10 memdump
> Invalid parameter 'addr'
> (qemu)
>
> After fix:
>
> (qemu) memsave 0xc0000000000470d8 10 memdump
> (qemu)
>
> Changes from V8:
> * Add hpte store support. Currently we don't have any user for this.
Thanks, applied all to ppc-next.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread