Re: [PATCH 06/10] KVM: arm/arm64: Only clean the dcache on translation fault
On 17/10/17 15:36, Christoffer Dall wrote: > On Tue, Oct 17, 2017 at 10:34:15AM +0100, Marc Zyngier wrote: >> On 16/10/17 21:08, Christoffer Dall wrote: >>> On Mon, Oct 09, 2017 at 04:20:28PM +0100, Marc Zyngier wrote: The only case where we actually need to perform a dache maintenance is when we map the page for the first time, and subsequent permission faults do not require cache maintenance. Let's make it conditional on not being a permission fault (and thus a translation fault). >>> >>> Why do we actually need to do any dcache maintenance when faulting in a >>> page? >>> >>> Is this for the case when the stage 1 MMU is disabled, or to support >>> guest mappings using uncached attributes? >> >> These are indeed the two cases that require cleaning the dcache to PoC. >> >>> Can we do better, for example >>> by only flushing the cache if the guest MMU is disabled? >> >> The guest MMU being disabled is easy. But the uncached mapping is much >> trickier, and would involve parsing the guest page tables. Not something >> I'm really eager to implement. >> > > Hmm, if the guest actually maps memory uncached, wouldn't it have to > invalidate caches itself, or is this the annoying thing where disabling > the MMU on hardware that doesn't have stage 2 would in fact always > completely bypass the cache, and therefore we have to do this work? The architecture is massively ambiguous about what is actually required in terms of CMOs when using an uncached mapping (and whether you can hit in the cache or not). But even if the guest had done an invalidate in order not to hit in the cache, the host could have evicted the page to disk, the guest faulted the page back in, and it would still be the host's responsibility to ensure that the guest sees something consistent. > Sorry, I have forgotten all the details here, but wanted to make sure > we're not being overly careful. No worries, I'm always happy to revisit this particular rabbit hole... ;-) Thanks, M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 06/10] KVM: arm/arm64: Only clean the dcache on translation fault
On Tue, Oct 17, 2017 at 10:34:15AM +0100, Marc Zyngier wrote: > On 16/10/17 21:08, Christoffer Dall wrote: > > On Mon, Oct 09, 2017 at 04:20:28PM +0100, Marc Zyngier wrote: > >> The only case where we actually need to perform a dache maintenance > >> is when we map the page for the first time, and subsequent permission > >> faults do not require cache maintenance. Let's make it conditional > >> on not being a permission fault (and thus a translation fault). > > > > Why do we actually need to do any dcache maintenance when faulting in a > > page? > > > > Is this for the case when the stage 1 MMU is disabled, or to support > > guest mappings using uncached attributes? > > These are indeed the two cases that require cleaning the dcache to PoC. > > > Can we do better, for example > > by only flushing the cache if the guest MMU is disabled? > > The guest MMU being disabled is easy. But the uncached mapping is much > trickier, and would involve parsing the guest page tables. Not something > I'm really eager to implement. > Hmm, if the guest actually maps memory uncached, wouldn't it have to invalidate caches itself, or is this the annoying thing where disabling the MMU on hardware that doesn't have stage 2 would in fact always completely bypass the cache, and therefore we have to do this work? Sorry, I have forgotten all the details here, but wanted to make sure we're not being overly careful. Thanks, -Christoffer ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 06/10] KVM: arm/arm64: Only clean the dcache on translation fault
On 16/10/17 21:08, Christoffer Dall wrote: > On Mon, Oct 09, 2017 at 04:20:28PM +0100, Marc Zyngier wrote: >> The only case where we actually need to perform a dache maintenance >> is when we map the page for the first time, and subsequent permission >> faults do not require cache maintenance. Let's make it conditional >> on not being a permission fault (and thus a translation fault). > > Why do we actually need to do any dcache maintenance when faulting in a > page? > > Is this for the case when the stage 1 MMU is disabled, or to support > guest mappings using uncached attributes? These are indeed the two cases that require cleaning the dcache to PoC. > Can we do better, for example > by only flushing the cache if the guest MMU is disabled? The guest MMU being disabled is easy. But the uncached mapping is much trickier, and would involve parsing the guest page tables. Not something I'm really eager to implement. > > Beyond that: > > Reviewed-by: Christoffer Dall Thanks! M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 06/10] KVM: arm/arm64: Only clean the dcache on translation fault
On Mon, Oct 09, 2017 at 04:20:28PM +0100, Marc Zyngier wrote:
> The only case where we actually need to perform a dache maintenance
> is when we map the page for the first time, and subsequent permission
> faults do not require cache maintenance. Let's make it conditional
> on not being a permission fault (and thus a translation fault).
Why do we actually need to do any dcache maintenance when faulting in a
page?
Is this for the case when the stage 1 MMU is disabled, or to support
guest mappings using uncached attributes? Can we do better, for example
by only flushing the cache if the guest MMU is disabled?
Beyond that:
Reviewed-by: Christoffer Dall
>
> Signed-off-by: Marc Zyngier
> ---
> virt/kvm/arm/mmu.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 1d47da22f75c..1911fadde88b 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -1400,7 +1400,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
> phys_addr_t fault_ipa,
> new_pmd = kvm_s2pmd_mkwrite(new_pmd);
> kvm_set_pfn_dirty(pfn);
> }
> - coherent_dcache_guest_page(vcpu, pfn, PMD_SIZE);
> +
> + if (fault_status != FSC_PERM)
> + coherent_dcache_guest_page(vcpu, pfn, PMD_SIZE);
>
> if (exec_fault) {
> new_pmd = kvm_s2pmd_mkexec(new_pmd);
> @@ -1416,7 +1418,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
> phys_addr_t fault_ipa,
> kvm_set_pfn_dirty(pfn);
> mark_page_dirty(kvm, gfn);
> }
> - coherent_dcache_guest_page(vcpu, pfn, PAGE_SIZE);
> +
> + if (fault_status != FSC_PERM)
> + coherent_dcache_guest_page(vcpu, pfn, PAGE_SIZE);
>
> if (exec_fault) {
> new_pte = kvm_s2pte_mkexec(new_pte);
> --
> 2.14.1
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
