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 <[email protected]> > --- > 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 >
