On Fri, Feb 15, 2019 at 06:00:23PM +0100, Cédric Le Goater wrote: > From: Benjamin Herrenschmidt <b...@kernel.crashing.org> > > With mttcg, we can have MMU lookups happening at the same time > as the guest modifying the page tables. > > Since the HPTEs of the hash table MMU contains two words (or > double worlds on 64-bit), we need to make sure we read them > in the right order, with the correct memory barrier. > > Additionally, when using emulated SPAPR mode, the hypercalls > writing to the hash table must also perform the udpates in > the right order. > > Note: This part is still not entirely correct > > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > Signed-off-by: Cédric Le Goater <c...@kaod.org>
Yeah, this stuff was written under the assumption it was protected by the BQL, which is getting less true all the time. Applied. > --- > hw/ppc/spapr.c | 21 +++++++++++++++++++-- > target/ppc/mmu-hash32.c | 6 ++++++ > target/ppc/mmu-hash64.c | 6 ++++++ > 3 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 60572eb59275..1afe31ee6163 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1507,8 +1507,25 @@ static void spapr_store_hpte(PPCVirtualHypervisor > *vhyp, hwaddr ptex, > if (!spapr->htab) { > kvmppc_write_hpte(ptex, pte0, pte1); > } else { > - stq_p(spapr->htab + offset, pte0); > - stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1); > + if (pte0 & HPTE64_V_VALID) { > + stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1); > + /* > + * When setting valid, we write PTE1 first. This ensures > + * proper synchronization with the reading code in > + * ppc_hash64_pteg_search() > + */ > + smp_wmb(); > + stq_p(spapr->htab + offset, pte0); > + } else { > + stq_p(spapr->htab + offset, pte0); > + /* > + * When clearing it we set PTE0 first. This ensures proper > + * synchronization with the reading code in > + * ppc_hash64_pteg_search() > + */ > + smp_wmb(); > + stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1); > + } > } > } > > diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c > index 03ae3c127985..e8562a7c8780 100644 > --- a/target/ppc/mmu-hash32.c > +++ b/target/ppc/mmu-hash32.c > @@ -319,6 +319,12 @@ static hwaddr ppc_hash32_pteg_search(PowerPCCPU *cpu, > hwaddr pteg_off, > > for (i = 0; i < HPTES_PER_GROUP; i++) { > pte0 = ppc_hash32_load_hpte0(cpu, pte_offset); > + /* > + * pte0 contains the valid bit and must be read before pte1, > + * otherwise we might see an old pte1 with a new valid bit and > + * thus an inconsistent hpte value > + */ > + smp_rmb(); > pte1 = ppc_hash32_load_hpte1(cpu, pte_offset); > > if ((pte0 & HPTE32_V_VALID) > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c > index f6c822ef917b..b3c4d33faa55 100644 > --- a/target/ppc/mmu-hash64.c > +++ b/target/ppc/mmu-hash64.c > @@ -506,6 +506,12 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, > hwaddr hash, > } > for (i = 0; i < HPTES_PER_GROUP; i++) { > pte0 = ppc_hash64_hpte0(cpu, pteg, i); > + /* > + * pte0 contains the valid bit and must be read before pte1, > + * otherwise we might see an old pte1 with a new valid bit and > + * thus an inconsistent hpte value > + */ > + smp_rmb(); > pte1 = ppc_hash64_hpte1(cpu, pteg, i); > > /* This compares V, B, H (secondary) and the AVPN */ -- 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