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...
