On 19 February 2015 at 15:19, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > On 19/02/15 13:44, Ard Biesheuvel wrote: >> On 19 February 2015 at 13:40, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: >>> On 19/02/15 10:54, Ard Biesheuvel wrote: >>>> --- >>>> arch/arm/kvm/mmu.c | 2 +- >>>> arch/arm64/include/asm/kvm_arm.h | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >>>> index 136662547ca6..fa8ec55220ea 100644 >>>> --- a/arch/arm/kvm/mmu.c >>>> +++ b/arch/arm/kvm/mmu.c >>>> @@ -1530,7 +1530,7 @@ void kvm_toggle_cache(struct kvm_vcpu *vcpu, bool was_enabled) >>>> stage2_flush_vm(vcpu->kvm); >>>> >>>> /* Caches are now on, stop trapping VM ops (until a S/W op) */ >>>> - if (now_enabled) >>>> + if (0)//now_enabled) >>>> vcpu_set_hcr(vcpu, vcpu_get_hcr(vcpu) & ~HCR_TVM); >>>> >>>> trace_kvm_toggle_cache(*vcpu_pc(vcpu), was_enabled, now_enabled); >>>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h >>>> index 8afb863f5a9e..437e1ec17539 100644 >>>> --- a/arch/arm64/include/asm/kvm_arm.h >>>> +++ b/arch/arm64/include/asm/kvm_arm.h >>>> @@ -75,7 +75,7 @@ >>>> * FMO: Override CPSR.F and enable signaling with VF >>>> * SWIO: Turn set/way invalidates into set/way clean+invalidate >>>> */ >>>> -#define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \ >>>> +#define HCR_GUEST_FLAGS (HCR_TSC | /* HCR_TSW | */ HCR_TWE | HCR_TWI | HCR_VM | \ >>> >>> Why do we stop to trap S/W ops here? We can't let the guest issue those >>> without doing anything, as this will break anything that expects the >>> data to make it to memory. Think of the 32bit kernel decompressor, for >>> example. >>> >> >> TBH patch #3 is just a q'n'd hack to ensure that the TVM bit remains >> set in HCR. I was assuming that cleaning the entire cache on mmu >> enable/disable would be sufficient to quantify the performance impact >> and check whether patch #2 works as advertised. > > OK. > >> I was wondering: isn't calling stage2_flush_vm() for each set of each >> way very costly? > > It's only called once, when TVM is not set. We then set TVM to make sure > that this doesn't happen anymore, until we stop trapping. > Ah, right, of course. > Of course, with your new approach, this doesn't work anymore and we'd > need to find another approach. > Well, *if* this approach is feasible in the first place, I'm sure we can find another bit to set to keep track of whether to perform the s/w operations. For now, I just ripped it out because I was afraid it might skew performance measurements done with the TVM kept set. -- Ard. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
