Skip to content

Commit

Permalink
target-ppc: Eliminate kvmppc_kern_htab global
Browse files Browse the repository at this point in the history 
fa48b43 "target-ppc: Remove hack for ppc_hash64_load_hpte*() with HV KVM"
purports to remove a hack in the handling of hash page tables (HPTs)
managed by KVM instead of qemu.  However, it actually went in the wrong
direction.

That patch requires anything looking for an external HPT (that is one not
managed by the guest itself) to check both env->external_htab (for a qemu
managed HPT) and kvmppc_kern_htab (for a KVM managed HPT).  That's a
problem because kvmppc_kern_htab is local to mmu-hash64.c, but some places
which need to check for an external HPT are outside that, such as
kvm_arch_get_registers().  The latter was subtly broken by the earlier
patch such that gdbstub can no longer access memory.

Basically a KVM managed HPT is much more like a qemu managed HPT than it is
like a guest managed HPT, so the original "hack" was actually on the right
track.

This partially reverts fa48b43, so we again mark a KVM managed external HPT
by putting a special but non-NULL value in env->external_htab.  It then
goes further, using that marker to eliminate the kvmppc_kern_htab global
entirely.  The ppc_hash64_set_external_hpt() helper function is extended
to set that marker if passed a NULL value (if you're setting an external
HPT, but don't have an actual HPT to set, the assumption is that it must
be a KVM managed HPT).

This also has some flow-on changes to the HPT access helpers, required by
the above changes.

Reported-by: Greg Kurz <[email protected]>
Signed-off-by: David Gibson <[email protected]>
Reviewed-by: Thomas Huth <[email protected]>
Reviewed-by: Greg Kurz <[email protected]>
Tested-by: Greg Kurz <[email protected]>
  • Loading branch information
@dgibson
dgibson committed Mar 15, 2016
1 parent e5c0d3c commit c18ad9a
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 35 deletions.
3 changes: 1 addition & 2 deletions hw/ppc/spapr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ static void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
}

spapr->htab_shift = shift;
kvmppc_kern_htab = true;
spapr->htab = NULL;
} else {
/* kernel-side HPT not needed, allocate in userspace instead */
size_t size = 1ULL << shift;
Expand All @@ -1106,7 +1106,6 @@ static void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,

memset(spapr->htab, 0, size);
spapr->htab_shift = shift;
kvmppc_kern_htab = false;

for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
DIRTY_HPTE(HPTE(spapr->htab, i));
Expand Down
10 changes: 5 additions & 5 deletions hw/ppc/spapr_hcall.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,17 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
break;
}
}
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);
if (index == 8) {
return H_PTEG_FULL;
}
} else {
token = ppc_hash64_start_access(cpu, pte_index);
if (ppc_hash64_load_hpte0(cpu, token, 0) & HPTE64_V_VALID) {
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);
return H_PTEG_FULL;
}
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);
}

ppc_hash64_store_hpte(cpu, pte_index + index,
Expand Down Expand Up @@ -165,7 +165,7 @@ static RemoveResult remove_hpte(PowerPCCPU *cpu, target_ulong ptex,
token = ppc_hash64_start_access(cpu, ptex);
v = ppc_hash64_load_hpte0(cpu, token, 0);
r = ppc_hash64_load_hpte1(cpu, token, 0);
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);

if ((v & HPTE64_V_VALID) == 0 ||
((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
Expand Down Expand Up @@ -288,7 +288,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
token = ppc_hash64_start_access(cpu, pte_index);
v = ppc_hash64_load_hpte0(cpu, token, 0);
r = ppc_hash64_load_hpte1(cpu, token, 0);
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);

if ((v & HPTE64_V_VALID) == 0 ||
((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
Expand Down
40 changes: 18 additions & 22 deletions target-ppc/mmu-hash64.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@
#endif

/*
* Used to indicate whether we have allocated htab in the
* host kernel
* Used to indicate that a CPU has its hash page table (HPT) managed
* within the host kernel
*/
bool kvmppc_kern_htab;
#define MMU_HASH64_KVM_MANAGED_HPT ((void *)-1)

/*
* SLB handling
*/
Expand Down Expand Up @@ -283,7 +284,11 @@ void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,

cpu_synchronize_state(CPU(cpu));

env->external_htab = hpt;
if (hpt) {
env->external_htab = hpt;
} else {
env->external_htab = MMU_HASH64_KVM_MANAGED_HPT;
}
ppc_hash64_set_sdr1(cpu, (target_ulong)(uintptr_t)hpt | (shift - 18),
&local_err);
if (local_err) {
Expand Down Expand Up @@ -396,35 +401,26 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
hwaddr pte_offset;

pte_offset = pte_index * HASH_PTE_SIZE_64;
if (kvmppc_kern_htab) {
if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
/*
* HTAB is controlled by KVM. Fetch the PTEG into a new buffer.
*/
token = kvmppc_hash64_read_pteg(cpu, pte_index);
if (token) {
return token;
}
} else if (cpu->env.external_htab) {
/*
* pteg read failed, even though we have allocated htab via
* kvmppc_reset_htab.
* HTAB is controlled by QEMU. Just point to the internally
* accessible PTEG.
*/
return 0;
}
/*
* HTAB is controlled by QEMU. Just point to the internally
* accessible PTEG.
*/
if (cpu->env.external_htab) {
token = (uint64_t)(uintptr_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)
void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token)
{
if (kvmppc_kern_htab) {
if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
kvmppc_hash64_free_pteg(token);
}
}
Expand Down Expand Up @@ -453,11 +449,11 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
&& HPTE64_V_COMPARE(pte0, ptem)) {
pte->pte0 = pte0;
pte->pte1 = pte1;
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);
return (pte_index + i) * HASH_PTE_SIZE_64;
}
}
ppc_hash64_stop_access(token);
ppc_hash64_stop_access(cpu, token);
/*
* We didn't find a valid entry.
*/
Expand Down Expand Up @@ -772,7 +768,7 @@ void ppc_hash64_store_hpte(PowerPCCPU *cpu,
{
CPUPPCState *env = &cpu->env;

if (kvmppc_kern_htab) {
if (env->external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
kvmppc_hash64_write_pte(env, pte_index, pte0, pte1);
return;
}
Expand Down
9 changes: 3 additions & 6 deletions target-ppc/mmu-hash64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,13 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
#define HPTE64_V_1TB_SEG 0x4000000000000000ULL
#define HPTE64_V_VRMA_MASK 0x4001ffffff000000ULL


extern bool kvmppc_kern_htab;

void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
Error **errp);
void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
Error **errp);

uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index);
void ppc_hash64_stop_access(uint64_t token);
void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token);

static inline target_ulong ppc_hash64_load_hpte0(PowerPCCPU *cpu,
uint64_t token, int index)
Expand All @@ -108,7 +105,7 @@ static inline target_ulong ppc_hash64_load_hpte0(PowerPCCPU *cpu,
uint64_t addr;

addr = token + (index * HASH_PTE_SIZE_64);
if (kvmppc_kern_htab || env->external_htab) {
if (env->external_htab) {
return ldq_p((const void *)(uintptr_t)addr);
} else {
return ldq_phys(CPU(cpu)->as, addr);
Expand All @@ -122,7 +119,7 @@ static inline target_ulong ppc_hash64_load_hpte1(PowerPCCPU *cpu,
uint64_t addr;

addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
if (kvmppc_kern_htab || env->external_htab) {
if (env->external_htab) {
return ldq_p((const void *)(uintptr_t)addr);
} else {
return ldq_phys(CPU(cpu)->as, addr);
Expand Down

0 comments on commit c18ad9a

Please sign in to comment.