Re: [PATCH 08/26] KVM: arm64: Use TTL hint in when invalidating stage-2 translations

Marc Zyngier
2020-05-27T08:59:51Z

On 2020-05-13 10:06, Andrew Scull wrote:
> On Tue, May 12, 2020 at 01:04:31PM +0100, James Morse wrote:
>> Hi Andrew,
>>
>> On 07/05/2020 16:13, Andrew Scull wrote:
>> >> @@ -176,7 +177,7 @@ static void clear_stage2_pud_entry(struct kvm_s2_mmu *mmu, pud_t *pud, phys_addr
>> >> pmd_t *pmd_table __maybe_unused = stage2_pmd_offset(kvm, pud, 0);
>> >> VM_BUG_ON(stage2_pud_huge(kvm, *pud));
>> >> stage2_pud_clear(kvm, pud);
>> >> - kvm_tlb_flush_vmid_ipa(mmu, addr);
>> >> + kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT);
>> >> stage2_pmd_free(kvm, pmd_table);
>> >> put_page(virt_to_page(pud));
>> >> }
>> >> @@ -186,7 +187,7 @@ static void clear_stage2_pmd_entry(struct kvm_s2_mmu *mmu, pmd_t *pmd, phys_addr
>> >> pte_t *pte_table = pte_offset_kernel(pmd, 0);
>> >> VM_BUG_ON(pmd_thp_or_huge(*pmd));
>> >> pmd_clear(pmd);
>> >> - kvm_tlb_flush_vmid_ipa(mmu, addr);
>> >> + kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT);
>> >> free_page((unsigned long)pte_table);
>> >> put_page(virt_to_page(pmd));
>> >> }
>> >
>> > Going by the names, is it possible to give a better level hint for these
>> > cases?
>>
>> There is no leaf entry being invalidated here. After clearing the
>> range, we found we'd
>> emptied (and invalidated) a whole page of mappings:
>> | if (stage2_pmd_table_empty(kvm, start_pmd))
>> | clear_stage2_pud_entry(mmu, pud, start_addr);
>>
>> Now we want to remove the link to the empty page so we can free it. We
>> are changing the
>> structure of the tables, not what gets mapped.
>>
>> I think this is why we need the un-hinted behaviour, to invalidate
>> "any level of the
>> translation table walk required to translate the specified IPA".
>> Otherwise the hardware
>> can look for a leaf at the indicated level, find none, and do nothing.
>>
>>
>> This is sufficiently horrible, its possible I've got it completely
>> wrong! (does it make
>> sense?)
>
> Ok. `addr` is an IPA, that IPA is now omitted from the map so doesn't
> appear in any entry of the table, least of all a leaf entry. That makes
> sense.
>
> Is there a convention to distinguish IPA and PA similar to the
> distinction for VA or does kvmarm just use phys_addr_t all round?
>
> It seems like the TTL patches are failry self contained if it would be
> easier to serparate them out from these larger series?

They are. This whole series is a mix of unrelated patches anyway.
Their only goal is to make my life a bit easier in the distant
future.

I'll repost that anyway, as I have made some cosmetic changes.

Thanks,

M.
--
Jazz is not dead. It just smells funny...