Alexander Graf <ag...@suse.de> writes:
> On 23.12.2013, at 18:23, Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
> wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com>
>>
>> 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.ku...@linux.vnet.ibm.com>
>> ---
>> Changes from V7:
>> * used uint64_t token
>>
>> 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 3c0f29c99820..05244244301a 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -676,6 +676,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);
>
> Didn't I mention that stores should be converted as well?
For the specific use case of x/10i we didn't needed store. And I though
you agreed that just getting read alone now is useful if we can be
sure store follow later. I was hoping to get to that later once this
gets upstream
>
>> /* 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 b77ce5e94cbc..8f5c6b39eb38 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1783,6 +1783,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 = {
>> @@ -1902,3 +1907,51 @@ int kvm_arch_on_sigbus(int code, void *addr)
>> void kvm_arch_init_irq_routing(KVMState *s)
>> {
>> }
>> +
>> +struct kvm_get_htab_buf {
>> + struct kvm_get_htab_header header;
>> + /*
>> + * We required one extra byte for read
>
> Why past tense?
Wrong english, nothing else :). Will fix
-aneesh
