On 08/04/16 16:09, Marc Zyngier wrote:
On 08/04/16 14:15, Christoffer Dall wrote:
On Mon, Apr 04, 2016 at 05:26:12PM +0100, Suzuki K Poulose wrote:
We have common routines to modify hyp and stage2 page tables
based on the 'kvm' parameter. For a smoother transition to
using separate routines for each, duplicate the routines
and modify the copy to work on hyp.
Marks the forked routines with _hyp_ and gets rid of the
kvm parameter which is no longer needed and is NULL for hyp.
Also, gets rid of calls to kvm_tlb_flush_by_vmid_ipa() calls
from the hyp versions. Uses explicit host page table accessors
instead of the kvm_* page table helpers.
Suggested-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
+static void unmap_hyp_ptes(pmd_t *pmd, phys_addr_t addr, phys_addr_t end)
+{
+ pte_t *pte, *start_pte;
+
+ start_pte = pte = pte_offset_kernel(pmd, addr);
+ do {
+ if (!pte_none(*pte)) {
+ pte_t old_pte = *pte;
+
+ kvm_set_pte(pte, __pte(0));
+
+ /* XXX: Do we need to invalidate the cache for device mappings ? */
no, we will not be swapping out any pages mapped in Hyp mode so you can
get rid of both of the following two lines.
OK, will remove this hunk.
+ if (!kvm_is_device_pfn(pte_pfn(old_pte)))
+ kvm_flush_dcache_pte(old_pte);
+
+ put_page(virt_to_page(pte));
+ }
+ } while (pte++, addr += PAGE_SIZE, addr != end);
+
+ if (hyp_pte_table_empty(start_pte))
+ clear_hyp_pmd_entry(pmd);
+}
+
+static void unmap_hyp_pmds(pud_t *pud, phys_addr_t addr, phys_addr_t end)
+{
+ phys_addr_t next;
+ pmd_t *pmd, *start_pmd;
+
+ start_pmd = pmd = pmd_offset(pud, addr);
+ do {
+ next = pmd_addr_end(addr, end);
+ if (!pmd_none(*pmd)) {
+ if (pmd_thp_or_huge(*pmd)) {
do we ever actually map anything with section mappings in the Hyp
mappings?
No, this is purely a page mapping so far. On my system, the HYP text is
just over 4 pages big (4k pages), so the incentive is pretty low, unless
we can demonstrate some big gains due to the reduced TLB impact.
+static void unmap_hyp_puds(pgd_t *pgd, phys_addr_t addr, phys_addr_t end)
+{
+ phys_addr_t next;
+ pud_t *pud, *start_pud;
+
+ start_pud = pud = pud_offset(pgd, addr);
+ do {
+ next = pud_addr_end(addr, end);
+ if (!pud_none(*pud)) {
+ if (pud_huge(*pud)) {
do we ever actually map anything with huge pud
mappings for the Hyp space?
Same thing. Looks like there is some potential simplification here.
Right, we don't map anything with section mapping. I can clean these up.
+static void unmap_hyp_range(pgd_t *pgdp, phys_addr_t start, u64 size)
+{
+ pgd_t *pgd;
+ phys_addr_t addr = start, end = start + size;
+ phys_addr_t next;
+
+ pgd = pgdp + pgd_index(addr);
+ do {
+ next = pgd_addr_end(addr, end);
+ if (!pgd_none(*pgd))
+ unmap_hyp_puds(pgd, addr, next);
+ } while (pgd++, addr = next, addr != end);
shouldn't we flush the EL2 (hyp) TLB here, strictly speaking?
Or do we rely on all mappings ever created/torn down here to always have
the same VA/PA relationship? Since we didn't flush the EL2 TLB in the
existing code, that indeed does seem to be the case.
Actually, we never unmap anything from HYP.
Except for the kvm tearing down where we clean up all the hyp table.
Once a structure (kvm, vcpu)is mapped there, it stays forever, whatever happens
to the VM (that's because we'd otherwise have to refcount the number of objects in a page,
and I'm lazy...).
Thats one of my TODO list if there is sufficient interest in getting that done.
Thanks
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
