[PATCH v3 06/21] KVM: arm64: Add support for stage-2 map()/unmap() in generic page-table

[Will Deacon]

On Thu, Sep 03, 2020 at 09:18:27PM +1000, Gavin Shan wrote:
> On 8/25/20 7:39 PM, Will Deacon wrote:
> > Add stage-2 map() and unmap() operations to the generic page-table code.
> > 
> > Cc: Marc Zyngier <maz at kernel.org>
> > Cc: Quentin Perret <qperret at google.com>
> > Signed-off-by: Will Deacon <will at kernel.org>
> > ---
> >   arch/arm64/include/asm/kvm_pgtable.h |  39 ++++
> >   arch/arm64/kvm/hyp/pgtable.c         | 262 +++++++++++++++++++++++++++
> >   2 files changed, 301 insertions(+)

[...]

> > +static int stage2_map_walk_table_post(u64 addr, u64 end, u32 level,
> > +				      kvm_pte_t *ptep,
> > +				      struct stage2_map_data *data)
> > +{
> > +	int ret = 0;
> > +
> > +	if (!data->anchor)
> > +		return 0;
> > +
> > +	free_page((unsigned long)kvm_pte_follow(*ptep));
> > +	put_page(virt_to_page(ptep));
> > +
> > +	if (data->anchor == ptep) {
> > +		data->anchor = NULL;
> > +		ret = stage2_map_walk_leaf(addr, end, level, ptep, data);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> 
> As discussed in another thread, *ptep has been invalidated in stage2_map_walk_table_pre().
> It means *ptep has value of zero. The following call to free_page() is going to release
> the page frame corresponding to physical address 0x0. It's not correct. We might cache
> the original value of this page table entry so that it can be used here.

Ah, yes, I see what you mean. But it's odd that I haven't run into this
myself, so let me try to reproduce the issue first. Another solution is
to invalidate the table entry only by clearing the valid bit of the pte,
rather than zapping the entire thing to 0, which can be done later when we
clear the anchor.

Cheers,

Will