[PATCH v4 02/21] KVM: arm64: Add stand-alone page-table walker infrastructure

[Will Deacon]

On Tue, Sep 08, 2020 at 10:03:30AM +1000, Gavin Shan wrote:
> On 9/8/20 1:23 AM, Will Deacon wrote:
> > The KVM page-table code is intricately tied into the kernel page-table
> > code and re-uses the pte/pmd/pud/p4d/pgd macros directly in an attempt
> > to reduce code duplication. Unfortunately, the reality is that there is
> > an awful lot of code required to make this work, and at the end of the
> > day you're limited to creating page-tables with the same configuration
> > as the host kernel. Furthermore, lifting the page-table code to run
> > directly at EL2 on a non-VHE system (as we plan to to do in future
> > patches) is practically impossible due to the number of dependencies it
> > has on the core kernel.
> > 
> > Introduce a framework for walking Armv8 page-tables configured
> > independently from the host kernel.
> > 
> > Cc: Marc Zyngier <maz at kernel.org>
> > Cc: Quentin Perret <qperret at google.com>
> > Signed-off-by: Will Deacon <will at kernel.org>
> > ---
> 
> It looks good to me. However, If you get a chance to respin, I guess
> it would be nice to add some comments about kvm_block_mapping_supported().
> More details are provoided below.
> 
> Reviewed-by: Gavin Shan <gshan at redhat.com>

[...]

> > +static bool kvm_block_mapping_supported(u64 addr, u64 end, u64 phys, u32 level)
> > +{
> > +	u64 granule = kvm_granule_size(level);
> > +
> > +	/*
> > +	 * Reject invalid block mappings and don't bother with 4TB mappings for
> > +	 * 52-bit PAs.
> > +	 */
> > +	if (level == 0 || (PAGE_SIZE != SZ_4K && level == 1))
> > +		return false;
> > +
> > +	if (granule > (end - addr))
> > +		return false;
> > +
> > +	return IS_ALIGNED(addr, granule) && IS_ALIGNED(phys, granule);
> > +}
> > +
> 
> If you get a chance to respin, it would be nice to have more comments to
> explain why 4TB block mapping is rejected here. I guess it depends on the
> fact: hugeTLB/THP doesn't support such kind of huge block mapping yet?

It's just because I'm lazy, really :) We don't know if we're using 52-bit
PAs here, and I couldn't be bothered to propagate that information given
that this doesn't seem like something anybody will be using at the moment.

Will