On Wed, Sep 11, 2013 at 01:35:56PM +0100, Marc Zyngier wrote:
> On 11/09/13 13:21, Anup Patel wrote:
> > On Tue, Sep 10, 2013 at 3:21 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> >> Anup Patel recently brought up the fact that when we run a guest
> >> with cache disabled, we don't flush the cache to the Point of
> >> Coherency, hence possibly missing bits of data that have been
> >> written in the cache, but have not reached memory.
> >>
> >> There are several approaches to this issue:
> >> - Using the DC bit when caches are off: this breaks guests assuming
> >> caches off while doing DMA operations. Bootloaders, for example.
> >> - Fetch the memory attributes on translation fault, and flush the
> >> cache while handling the fault. This relies on using the PAR_EL1
> >> register to obtain the Stage-1 memory attributes.
> >>
> >> While this second solution is clearly not ideal (it duplicates what
> >> the HW already does to generate HPFAR_EL2), it is more correct than
> >> not doing anything.
> >>
> >> Cc: Anup Patel <anup@xxxxxxxxxxxxxx>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> >> ---
> >>
> >> arch/arm/include/asm/kvm_mmu.h | 4 ++--
> >> arch/arm/kvm/mmu.c | 2 +-
> >> arch/arm64/include/asm/kvm_host.h | 1 +
> >> arch/arm64/include/asm/kvm_mmu.h | 9 +++++++--
> >> arch/arm64/kernel/asm-offsets.c | 1 +
> >> arch/arm64/kvm/hyp.S | 28 ++++++++++++----------------
> >> 6 files changed, 24 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> >> index 472ac70..faffdf0 100644
> >> --- a/arch/arm/include/asm/kvm_mmu.h
> >> +++ b/arch/arm/include/asm/kvm_mmu.h
> >> @@ -105,7 +105,7 @@ static inline void kvm_set_s2pte_writable(pte_t *pte)
> >>
> >> struct kvm;
> >>
> >> -static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
> >> +static inline void coherent_icache_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn)
> >> {
> >> /*
> >> * If we are going to insert an instruction page and the icache is
> >> @@ -120,7 +120,7 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
> >> * need any kind of flushing (DDI 0406C.b - Page B3-1392).
> >> */
> >> if (icache_is_pipt()) {
> >> - unsigned long hva = gfn_to_hva(kvm, gfn);
> >> + unsigned long hva = gfn_to_hva(vcpu->kvm, gfn);
> >> __cpuc_coherent_user_range(hva, hva + PAGE_SIZE);
> >> } else if (!icache_is_vivt_asid_tagged()) {
> >> /* any kind of VIPT cache */
> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> >> index 0988d9e..ffb3cc4 100644
> >> --- a/arch/arm/kvm/mmu.c
> >> +++ b/arch/arm/kvm/mmu.c
> >> @@ -547,7 +547,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> >> return -EFAULT;
> >>
> >> new_pte = pfn_pte(pfn, PAGE_S2);
> >> - coherent_icache_guest_page(vcpu->kvm, gfn);
> >> + coherent_icache_guest_page(vcpu, gfn);
> >>
> >> spin_lock(&vcpu->kvm->mmu_lock);
> >> if (mmu_notifier_retry(vcpu->kvm, mmu_seq))
> >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >> index 331a852..2530f92 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -79,6 +79,7 @@ struct kvm_mmu_memory_cache {
> >> struct kvm_vcpu_fault_info {
> >> u32 esr_el2; /* Hyp Syndrom Register */
> >> u64 far_el2; /* Hyp Fault Address Register */
> >> + u64 par_el2; /* Result of FAR_EL2 translation */
> >
> > Please rename this to par_el1 because we are saving result of Stage1
> > translation only.
>
> That'd be very confusing, as we deal with PAR_EL1 separately, as part of
> the guest context.
>
> If anything, I may rename it to "what_I_d_love_HPFAR_to_return" instead.
>
while extremely clear, it may be a bit verbose ;)
I honestly think that 'making up' a register name is a bit confusing as
well. I can live with it, but perhaps fault_ipa, or s1_fault_attr or
something like that would work. Eh.
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
