Message ID | 20250109133817.314401-3-pbonzini@redhat.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | KVM: e500: map readonly host pages for read, and cleanup | expand |
On Thu, Jan 09, 2025, Paolo Bonzini wrote: > kvmppc_e500_ref_setup is returning whether the guest TLB entry is writable, > which is than passed to kvm_release_faultin_page. This makes little sense s/than/then > for two reasons: first, because the function sets up the private data for > the page and the return value feels like it has been bolted on the side; > second, because what really matters is whether the _shadow_ TLB entry is > writable. If it is not writable, the page can be released as non-dirty. > Shift from using tlbe_is_writable(gtlbe) to doing the same check on > the shadow TLB entry. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/powerpc/kvm/e500_mmu_host.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c > index 732335444d68..06e23c625be0 100644 > --- a/arch/powerpc/kvm/e500_mmu_host.c > +++ b/arch/powerpc/kvm/e500_mmu_host.c > @@ -242,7 +242,7 @@ static inline int tlbe_is_writable(struct kvm_book3e_206_tlb_entry *tlbe) > return tlbe->mas7_3 & (MAS3_SW|MAS3_UW); > } > > -static inline bool kvmppc_e500_ref_setup(struct tlbe_ref *ref, > +static inline void kvmppc_e500_ref_setup(struct tlbe_ref *ref, > struct kvm_book3e_206_tlb_entry *gtlbe, > kvm_pfn_t pfn, unsigned int wimg) > { > @@ -251,8 +251,6 @@ static inline bool kvmppc_e500_ref_setup(struct tlbe_ref *ref, > > /* Use guest supplied MAS2_G and MAS2_E */ > ref->flags |= (gtlbe->mas2 & MAS2_ATTRIB_MASK) | wimg; > - > - return tlbe_is_writable(gtlbe); > } > > static inline void kvmppc_e500_ref_release(struct tlbe_ref *ref) > @@ -493,10 +491,11 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500, > goto out; > } > } > - writable = kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg); > > + kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg); > kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize, > ref, gvaddr, stlbe); > + writable = tlbe_is_writable(stlbe); > > /* Clear i-cache for new pages */ > kvmppc_mmu_flush_icache(pfn); > -- > 2.47.1 >
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c index 732335444d68..06e23c625be0 100644 --- a/arch/powerpc/kvm/e500_mmu_host.c +++ b/arch/powerpc/kvm/e500_mmu_host.c @@ -242,7 +242,7 @@ static inline int tlbe_is_writable(struct kvm_book3e_206_tlb_entry *tlbe) return tlbe->mas7_3 & (MAS3_SW|MAS3_UW); } -static inline bool kvmppc_e500_ref_setup(struct tlbe_ref *ref, +static inline void kvmppc_e500_ref_setup(struct tlbe_ref *ref, struct kvm_book3e_206_tlb_entry *gtlbe, kvm_pfn_t pfn, unsigned int wimg) { @@ -251,8 +251,6 @@ static inline bool kvmppc_e500_ref_setup(struct tlbe_ref *ref, /* Use guest supplied MAS2_G and MAS2_E */ ref->flags |= (gtlbe->mas2 & MAS2_ATTRIB_MASK) | wimg; - - return tlbe_is_writable(gtlbe); } static inline void kvmppc_e500_ref_release(struct tlbe_ref *ref) @@ -493,10 +491,11 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500, goto out; } } - writable = kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg); + kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg); kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize, ref, gvaddr, stlbe); + writable = tlbe_is_writable(stlbe); /* Clear i-cache for new pages */ kvmppc_mmu_flush_icache(pfn);
kvmppc_e500_ref_setup is returning whether the guest TLB entry is writable, which is than passed to kvm_release_faultin_page. This makes little sense for two reasons: first, because the function sets up the private data for the page and the return value feels like it has been bolted on the side; second, because what really matters is whether the _shadow_ TLB entry is writable. If it is not writable, the page can be released as non-dirty. Shift from using tlbe_is_writable(gtlbe) to doing the same check on the shadow TLB entry. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/powerpc/kvm/e500_mmu_host.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)