Re: [PATCH v2 2/2] KVM: arm64: Use folio for THP adjustment

[Vincent Donnefort <vdonnefort@google.com>]

[...]

> >  static struct kvm_pgtable_mm_ops kvm_s2_mm_ops = {
> >  	.zalloc_page		= stage2_memcache_zalloc_page,
> >  	.zalloc_pages_exact	= kvm_s2_zalloc_pages_exact,
> > @@ -1274,7 +1229,7 @@ static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot,
> >   *
> >   * Returns the size of the mapping.
> >   */
> > -static long
> > +static unsigned long
> >  transparent_hugepage_adjust(struct kvm *kvm, struct kvm_memory_slot *memslot,
> >  			    unsigned long hva, kvm_pfn_t *pfnp,
> >  			    phys_addr_t *ipap)
> > @@ -1287,10 +1242,7 @@ transparent_hugepage_adjust(struct kvm *kvm, struct kvm_memory_slot *memslot,
> >  	 * block map is contained within the memslot.
> >  	 */
> >  	if (fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
> > -		int sz = get_user_mapping_size(kvm, hva);
> > -
> > -		if (sz < 0)
> > -			return sz;
> > +		size_t sz = folio_size(pfn_folio(pfn));
> 
> Hi,
> 
> Sorry this is an extremely late reply - I just noticed this because Marc
> mentioned it in another thread.
> 
> This doesn't look quite right to me; just because you have a folio of a given
> size, that doesn't mean the whole thing is mapped into this particular address
> space. For example, you could have a (PMD-sized) THP that gets partially
> munmapped - the folio is still PMD-sized but only some of it is mapped and
> should be accessible to the process. Or you could have a large file-backed folio
> (from a filesystem that supports large folios - e.g. XFS) but the application
> only mapped part of the file.

I thought originally this would break the block and the folio with it, but a
quick expriment showed it's not the case.

> 
> Perhaps I've misunderstood and those edge cases can't happen here for some reason?

And fault_supports_stage2_huge_mapping() would probably not be enough. we might
end-up with a portion unmapped at stage-1 but mapped at stage-2. :-( 

> 
> Thanks,
> Ryan
> 
>