On Sat, 09 Jun 2018 10:31:48 +0100,
Christoffer Dall wrote:
>
> On Wed, May 30, 2018 at 01:47:04PM +0100, Marc Zyngier wrote:
> > The arm and arm64 KVM page tables accessors are pointlessly different
> > between the two architectures, and likely both wrong one way or another:
> > arm64 lacks a dsb(), and arm doesn't use WRITE_ONCE.
> >
> > Let's unify them.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > ---
> > arch/arm/include/asm/kvm_mmu.h | 12 -----------
> > arch/arm64/include/asm/kvm_mmu.h | 3 ---
> > virt/kvm/arm/mmu.c | 35 ++++++++++++++++++++++++++++----
> > 3 files changed, 31 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> > index 707a1f06dc5d..468ff945efa0 100644
> > --- a/arch/arm/include/asm/kvm_mmu.h
> > +++ b/arch/arm/include/asm/kvm_mmu.h
> > @@ -75,18 +75,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> > int kvm_mmu_init(void);
> > void kvm_clear_hyp_idmap(void);
> >
> > -static inline void kvm_set_pmd(pmd_t *pmd, pmd_t new_pmd)
> > -{
> > - *pmd = new_pmd;
> > - dsb(ishst);
> > -}
> > -
> > -static inline void kvm_set_pte(pte_t *pte, pte_t new_pte)
> > -{
> > - *pte = new_pte;
> > - dsb(ishst);
> > -}
> > -
> > static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> > {
> > pte_val(pte) |= L_PTE_S2_RDWR;
> > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> > index 9dbca5355029..26c89b63f604 100644
> > --- a/arch/arm64/include/asm/kvm_mmu.h
> > +++ b/arch/arm64/include/asm/kvm_mmu.h
> > @@ -170,9 +170,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> > int kvm_mmu_init(void);
> > void kvm_clear_hyp_idmap(void);
> >
> > -#define kvm_set_pte(ptep, pte) set_pte(ptep, pte)
> > -#define kvm_set_pmd(pmdp, pmd) set_pmd(pmdp, pmd)
> > -
> > static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> > {
> > pte_val(pte) |= PTE_S2_RDWR;
> > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> > index ba66bf7ae299..c9ed239c0840 100644
> > --- a/virt/kvm/arm/mmu.c
> > +++ b/virt/kvm/arm/mmu.c
> > @@ -177,6 +177,33 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr
> > put_page(virt_to_page(pmd));
> > }
> >
> > +static inline void kvm_set_pte(pte_t *ptep, pte_t new_pte)
> > +{
> > + WRITE_ONCE(*ptep, new_pte);
> > + dsb(ishst);
> > +}
> > +
> > +static inline void kvm_set_pmd(pmd_t *pmdp, pmd_t new_pmd)
> > +{
> > + WRITE_ONCE(*pmdp, new_pmd);
> > + dsb(ishst);
> > +}
> > +
>
> arm64 set_pte and set_pmd have an isb() in addition to the dsb(), why
> can we let go of that here?
Good point. There was an offline discussion with Will and Mark a
couple of weeks ago, where we agreed that this ISB wasn't
required. I've of course paged it out. Mark, do you remember the
rational?
Thanks,
M.
--
Jazz is not dead, it just smell funny.
