[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/6] ppc/hash64: Rework R and C bit updates
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH 3/6] ppc/hash64: Rework R and C bit updates |
Date: |
Fri, 12 Apr 2019 16:20:07 +1000 |
User-agent: |
Mutt/1.11.3 (2019-02-01) |
On Thu, Apr 11, 2019 at 10:00:01AM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <address@hidden>
>
> With MT-TCG, we are now running translation in a racy way, thus
> we need to mimic hardware when it comes to updating the R and
> C bits, by doing byte stores.
>
> The current "store_hpte" abstraction is ill suited for this, we
> replace it with two separate callbacks for setting R and C.
>
> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> Signed-off-by: Cédric Le Goater <address@hidden>
Applied, thanks.
> ---
> include/hw/ppc/spapr.h | 2 ++
> target/ppc/cpu.h | 4 +--
> target/ppc/mmu-hash64.h | 2 --
> hw/ppc/spapr.c | 41 +++++++++++++++++++---
> hw/ppc/spapr_hcall.c | 13 +++----
> target/ppc/mmu-hash64.c | 76 ++++++++++++++++++++++++-----------------
> 6 files changed, 93 insertions(+), 45 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9331f5e0b955..7e32f309c2be 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -784,6 +784,8 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int
> shift,
> Error **errp);
> void spapr_clear_pending_events(SpaprMachineState *spapr);
> int spapr_max_server_number(SpaprMachineState *spapr);
> +void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> + uint64_t pte0, uint64_t pte1);
>
> /* DRC callbacks. */
> void spapr_core_release(DeviceState *dev);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 07cea7268c0c..a4420636dde0 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1268,8 +1268,8 @@ struct PPCVirtualHypervisorClass {
> void (*unmap_hptes)(PPCVirtualHypervisor *vhyp,
> const ppc_hash_pte64_t *hptes,
> hwaddr ptex, int n);
> - void (*store_hpte)(PPCVirtualHypervisor *vhyp, hwaddr ptex,
> - uint64_t pte0, uint64_t pte1);
> + void (*hpte_set_c)(PPCVirtualHypervisor *vhyp, hwaddr ptex, uint64_t
> pte1);
> + void (*hpte_set_r)(PPCVirtualHypervisor *vhyp, hwaddr ptex, uint64_t
> pte1);
> void (*get_pate)(PPCVirtualHypervisor *vhyp, ppc_v3_pate_t *entry);
> target_ulong (*encode_hpt_for_kvm_pr)(PPCVirtualHypervisor *vhyp);
> };
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 6b555b72200f..bc362964122d 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -10,8 +10,6 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
> hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
> int mmu_idx);
> -void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> - uint64_t pte0, uint64_t pte1);
> void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
> target_ulong pte_index,
> target_ulong pte0, target_ulong pte1);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b81e237635cd..c56939a43b64 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1520,10 +1520,10 @@ static void spapr_unmap_hptes(PPCVirtualHypervisor
> *vhyp,
> /* Nothing to do for qemu managed HPT */
> }
>
> -static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr ptex,
> - uint64_t pte0, uint64_t pte1)
> +void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> + uint64_t pte0, uint64_t pte1)
> {
> - SpaprMachineState *spapr = SPAPR_MACHINE(vhyp);
> + SpaprMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);
> hwaddr offset = ptex * HASH_PTE_SIZE_64;
>
> if (!spapr->htab) {
> @@ -1551,6 +1551,38 @@ static void spapr_store_hpte(PPCVirtualHypervisor
> *vhyp, hwaddr ptex,
> }
> }
>
> +static void spapr_hpte_set_c(PPCVirtualHypervisor *vhyp, hwaddr ptex,
> + uint64_t pte1)
> +{
> + hwaddr offset = ptex * HASH_PTE_SIZE_64 + 15;
> + SpaprMachineState *spapr = SPAPR_MACHINE(vhyp);
> +
> + if (!spapr->htab) {
> + /* There should always be a hash table when this is called */
> + error_report("spapr_hpte_set_c called with no hash table !");
> + return;
> + }
> +
> + /* The HW performs a non-atomic byte update */
> + stb_p(spapr->htab + offset, (pte1 & 0xff) | 0x80);
> +}
> +
> +static void spapr_hpte_set_r(PPCVirtualHypervisor *vhyp, hwaddr ptex,
> + uint64_t pte1)
> +{
> + hwaddr offset = ptex * HASH_PTE_SIZE_64 + 14;
> + SpaprMachineState *spapr = SPAPR_MACHINE(vhyp);
> +
> + if (!spapr->htab) {
> + /* There should always be a hash table when this is called */
> + error_report("spapr_hpte_set_r called with no hash table !");
> + return;
> + }
> +
> + /* The HW performs a non-atomic byte update */
> + stb_p(spapr->htab + offset, ((pte1 >> 8) & 0xff) | 0x01);
> +}
> +
> int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
> {
> int shift;
> @@ -4291,7 +4323,8 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> vhc->hpt_mask = spapr_hpt_mask;
> vhc->map_hptes = spapr_map_hptes;
> vhc->unmap_hptes = spapr_unmap_hptes;
> - vhc->store_hpte = spapr_store_hpte;
> + vhc->hpte_set_c = spapr_hpte_set_c;
> + vhc->hpte_set_r = spapr_hpte_set_r;
> vhc->get_pate = spapr_get_pate;
> vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
> xic->ics_get = spapr_ics_get;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7761d4a841af..a27c9a01aa97 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -118,7 +118,7 @@ static target_ulong h_enter(PowerPCCPU *cpu,
> SpaprMachineState *spapr,
> ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
> }
>
> - ppc_hash64_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY,
> ptel);
> + spapr_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
>
> args[0] = ptex + slot;
> return H_SUCCESS;
> @@ -131,7 +131,8 @@ typedef enum {
> REMOVE_HW = 3,
> } RemoveResult;
>
> -static RemoveResult remove_hpte(PowerPCCPU *cpu, target_ulong ptex,
> +static RemoveResult remove_hpte(PowerPCCPU *cpu
> + , target_ulong ptex,
> target_ulong avpn,
> target_ulong flags,
> target_ulong *vp, target_ulong *rp)
> @@ -155,7 +156,7 @@ static RemoveResult remove_hpte(PowerPCCPU *cpu,
> target_ulong ptex,
> }
> *vp = v;
> *rp = r;
> - ppc_hash64_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
> + spapr_store_hpte(cpu, ptex, HPTE64_V_HPTE_DIRTY, 0);
> ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
> return REMOVE_SUCCESS;
> }
> @@ -289,13 +290,13 @@ static target_ulong h_protect(PowerPCCPU *cpu,
> SpaprMachineState *spapr,
> r |= (flags << 55) & HPTE64_R_PP0;
> r |= (flags << 48) & HPTE64_R_KEY_HI;
> r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
> - ppc_hash64_store_hpte(cpu, ptex,
> - (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
> + spapr_store_hpte(cpu, ptex,
> + (v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
> ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
> /* Flush the tlb */
> check_tlb_flush(env, true);
> /* Don't need a memory barrier, due to qemu's global lock */
> - ppc_hash64_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
> + spapr_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
> return H_SUCCESS;
> }
>
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 8956ad55d96f..b4a27d46e382 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -724,6 +724,39 @@ static void ppc_hash64_set_dsi(CPUState *cs, uint64_t
> dar, uint64_t dsisr)
> }
>
>
> +static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
> +{
> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
> +
> + if (cpu->vhyp) {
> + PPCVirtualHypervisorClass *vhc =
> + PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> + vhc->hpte_set_r(cpu->vhyp, ptex, pte1);
> + return;
> + }
> + base = ppc_hash64_hpt_base(cpu);
> +
> +
> + /* The HW performs a non-atomic byte update */
> + stb_phys(CPU(cpu)->as, base + offset, ((pte1 >> 8) & 0xff) | 0x01);
> +}
> +
> +static void ppc_hash64_set_c(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
> +{
> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 15;
> +
> + if (cpu->vhyp) {
> + PPCVirtualHypervisorClass *vhc =
> + PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> + vhc->hpte_set_c(cpu->vhyp, ptex, pte1);
> + return;
> + }
> + base = ppc_hash64_hpt_base(cpu);
> +
> + /* The HW performs a non-atomic byte update */
> + stb_phys(CPU(cpu)->as, base + offset, (pte1 & 0xff) | 0x80);
> +}
> +
> int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> int rwx, int mmu_idx)
> {
> @@ -734,7 +767,6 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr
> eaddr,
> hwaddr ptex;
> ppc_hash_pte64_t pte;
> int exec_prot, pp_prot, amr_prot, prot;
> - uint64_t new_pte1;
> const int need_prot[] = {PAGE_READ, PAGE_WRITE, PAGE_EXEC};
> hwaddr raddr;
>
> @@ -882,19 +914,19 @@ skip_slb_search:
>
> /* 6. Update PTE referenced and changed bits if necessary */
>
> - new_pte1 = pte.pte1 | HPTE64_R_R; /* set referenced bit */
> - if (rwx == 1) {
> - new_pte1 |= HPTE64_R_C; /* set changed (dirty) bit */
> - } else {
> - /*
> - * Treat the page as read-only for now, so that a later write
> - * will pass through this function again to set the C bit
> - */
> - prot &= ~PAGE_WRITE;
> + if (!(pte.pte1 & HPTE64_R_R)) {
> + ppc_hash64_set_r(cpu, ptex, pte.pte1);
> }
> -
> - if (new_pte1 != pte.pte1) {
> - ppc_hash64_store_hpte(cpu, ptex, pte.pte0, new_pte1);
> + if (!(pte.pte1 & HPTE64_R_C)) {
> + if (rwx == 1) {
> + ppc_hash64_set_c(cpu, ptex, pte.pte1);
> + } else {
> + /*
> + * Treat the page as read-only for now, so that a later write
> + * will pass through this function again to set the C bit
> + */
> + prot &= ~PAGE_WRITE;
> + }
> }
>
> /* 7. Determine the real address from the PTE */
> @@ -953,24 +985,6 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu,
> target_ulong addr)
> & TARGET_PAGE_MASK;
> }
>
> -void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
> - uint64_t pte0, uint64_t pte1)
> -{
> - hwaddr base;
> - hwaddr offset = ptex * HASH_PTE_SIZE_64;
> -
> - if (cpu->vhyp) {
> - PPCVirtualHypervisorClass *vhc =
> - PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> - vhc->store_hpte(cpu->vhyp, ptex, pte0, pte1);
> - return;
> - }
> - base = ppc_hash64_hpt_base(cpu);
> -
> - stq_phys(CPU(cpu)->as, base + offset, pte0);
> - stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2, pte1);
> -}
> -
> void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
> target_ulong pte0, target_ulong pte1)
> {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH 0/6] ppc: misc fixes for Radix and Hash, Cédric Le Goater, 2019/04/11
- [Qemu-devel] [PATCH 2/6] ppc/spapr: Use proper HPTE accessors for H_READ, Cédric Le Goater, 2019/04/11
- [Qemu-devel] [PATCH 1/6] target/ppc: Don't check UPRT in radix mode when in HV real mode, Cédric Le Goater, 2019/04/11
- [Qemu-devel] [PATCH 3/6] ppc/hash64: Rework R and C bit updates, Cédric Le Goater, 2019/04/11
- Re: [Qemu-devel] [PATCH 3/6] ppc/hash64: Rework R and C bit updates,
David Gibson <=
- [Qemu-devel] [PATCH 4/6] ppc/hash32: Rework R and C bit updates, Cédric Le Goater, 2019/04/11
- [Qemu-devel] [PATCH 5/6] memory_ldst: Add atomic ops for PTE updates, Cédric Le Goater, 2019/04/11
- [Qemu-devel] [PATCH 6/6] ppc: Fix radix RC updates, Cédric Le Goater, 2019/04/11