[PATCH v3 09/21] KVM: arm64: Convert unmap_stage2_range() to generic page-table API

[Alexandru Elisei]

Hi Will,

I think I have answered my own question (again).

On 9/2/20 5:23 PM, Alexandru Elisei wrote:
> Hello,
>
> On 8/25/20 10:39 AM, Will Deacon wrote:
>> Convert unmap_stage2_range() to use kvm_pgtable_stage2_unmap() instead
>> of walking the page-table directly.
>>
>> 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/kvm/mmu.c | 57 +++++++++++++++++++++++++-------------------
>>  1 file changed, 32 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index 704b471a48ce..751ce2462765 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -39,6 +39,33 @@ static bool is_iomap(unsigned long flags)
>>  	return flags & KVM_S2PTE_FLAG_IS_IOMAP;
>>  }
>>  
>> +/*
>> + * Release kvm_mmu_lock periodically if the memory region is large. Otherwise,
>> + * we may see kernel panics with CONFIG_DETECT_HUNG_TASK,
>> + * CONFIG_LOCKUP_DETECTOR, CONFIG_LOCKDEP. Additionally, holding the lock too
>> + * long will also starve other vCPUs. We have to also make sure that the page
>> + * tables are not freed while we released the lock.
>> + */
>> +#define stage2_apply_range(kvm, addr, end, fn, resched)			\
>> +({									\
>> +	int ret;							\
>> +	struct kvm *__kvm = (kvm);					\
>> +	bool __resched = (resched);					\
>> +	u64 next, __addr = (addr), __end = (end);			\
>> +	do {								\
>> +		struct kvm_pgtable *pgt = __kvm->arch.mmu.pgt;		\
>> +		if (!pgt)						\
>> +			break;						\
> I'm 100% sure there's a reason why we've dropped the READ_ONCE, but it still looks
> to me like the compiler might decide to optimize by reading pgt once at the start
> of the loop and stashing it in a register. Would you mind explaining what I am
> missing?

I think the reason is that kvm_pgtable_stage2_unmap() has access to pgt via the
back pointer to mmu, and the function is in a different compilation unit now, so
the compiler cannot infer anything about pgt staying the same between function
calls. Is that correct?

Thanks,

Alex