From: Christoffer Dall <c.dall@virtualopensystems.com> To: Min-gyu Kim <mingyu84.kim@samsung.com> Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, "Marc Zyngier" <marc.zyngier@arm.com>, 김창환 <changhwan.m.kim@samsung.com> Subject: Re: [PATCH v2 06/14] KVM: ARM: Memory virtualization setup Date: Sat, 6 Oct 2012 17:33:43 -0400 [thread overview] Message-ID: <CANM98qKAFQgfwRuFP2WOP4Af+1Un-wuQPC8-TiGi4B+s3XB2hQ@mail.gmail.com> (raw) In-Reply-To: <000401cda2a0$69670a40$3c351ec0$@samsung.com> On Thu, Oct 4, 2012 at 10:23 PM, Min-gyu Kim <mingyu84.kim@samsung.com> wrote: > > >> -----Original Message----- >> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On >> Behalf Of Christoffer Dall >> Sent: Monday, October 01, 2012 6:11 PM >> To: kvm@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> kvmarm@lists.cs.columbia.edu >> Cc: Marc Zyngier >> Subject: [PATCH v2 06/14] KVM: ARM: Memory virtualization setup >> >> +static void stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache >> *cache, >> + phys_addr_t addr, const pte_t *new_pte) { >> + pgd_t *pgd; >> + pud_t *pud; >> + pmd_t *pmd; >> + pte_t *pte, old_pte; >> + >> + /* Create 2nd stage page table mapping - Level 1 */ >> + pgd = kvm->arch.pgd + pgd_index(addr); >> + pud = pud_offset(pgd, addr); >> + if (pud_none(*pud)) { >> + if (!cache) >> + return; /* ignore calls from kvm_set_spte_hva */ >> + pmd = mmu_memory_cache_alloc(cache); >> + pud_populate(NULL, pud, pmd); >> + pmd += pmd_index(addr); >> + get_page(virt_to_page(pud)); >> + } else >> + pmd = pmd_offset(pud, addr); >> + >> + /* Create 2nd stage page table mapping - Level 2 */ >> + if (pmd_none(*pmd)) { >> + if (!cache) >> + return; /* ignore calls from kvm_set_spte_hva */ >> + pte = mmu_memory_cache_alloc(cache); >> + clean_pte_table(pte); >> + pmd_populate_kernel(NULL, pmd, pte); >> + pte += pte_index(addr); >> + get_page(virt_to_page(pmd)); >> + } else >> + pte = pte_offset_kernel(pmd, addr); >> + >> + /* Create 2nd stage page table mapping - Level 3 */ >> + old_pte = *pte; >> + set_pte_ext(pte, *new_pte, 0); >> + if (pte_present(old_pte)) >> + __kvm_tlb_flush_vmid(kvm); >> + else >> + get_page(virt_to_page(pte)); >> +} > > > I'm not sure about the 3-level page table, but isn't it necessary to > clean the page table for 2nd level? > There are two mmu_memory_cache_alloc calls. One has following clean_pte_table > and the other doesn't have. hmm, it probably is - I couldn't really find the common case where this is done in the kernel normally (except for some custom loop in ioremap and idmap), but I added this fix: diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 5394a52..f11ba27f 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -430,6 +430,7 @@ static void stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, if (!cache) return; /* ignore calls from kvm_set_spte_hva */ pmd = mmu_memory_cache_alloc(cache); + clean_dcache_area(pmd, PTRS_PER_PMD * sizeof(pmd_t)); pud_populate(NULL, pud, pmd); pmd += pmd_index(addr); get_page(virt_to_page(pud)); > > And why do you ignore calls from kvm_set_spte_hva? It is supposed to happen when > host moves the page, right? Then you ignore the case because it can be handled > later when fault actually happens? Is there any other reason that I miss? > kvm_set_spte_hva tells us that a page at some IPA is going to be backed by another physical page, which means we must adjust the stage 2 mapping. However, if we don't have that page mapped in the stage 2 page table, we don't need to do anything, and certainly don't want to start allocating unnecessary level2 and level3 page tables. Thanks! -Christoffer
WARNING: multiple messages have this Message-ID (diff)
From: c.dall@virtualopensystems.com (Christoffer Dall) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 06/14] KVM: ARM: Memory virtualization setup Date: Sat, 6 Oct 2012 17:33:43 -0400 [thread overview] Message-ID: <CANM98qKAFQgfwRuFP2WOP4Af+1Un-wuQPC8-TiGi4B+s3XB2hQ@mail.gmail.com> (raw) In-Reply-To: <000401cda2a0$69670a40$3c351ec0$@samsung.com> On Thu, Oct 4, 2012 at 10:23 PM, Min-gyu Kim <mingyu84.kim@samsung.com> wrote: > > >> -----Original Message----- >> From: kvm-owner at vger.kernel.org [mailto:kvm-owner at vger.kernel.org] On >> Behalf Of Christoffer Dall >> Sent: Monday, October 01, 2012 6:11 PM >> To: kvm at vger.kernel.org; linux-arm-kernel at lists.infradead.org; >> kvmarm at lists.cs.columbia.edu >> Cc: Marc Zyngier >> Subject: [PATCH v2 06/14] KVM: ARM: Memory virtualization setup >> >> +static void stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache >> *cache, >> + phys_addr_t addr, const pte_t *new_pte) { >> + pgd_t *pgd; >> + pud_t *pud; >> + pmd_t *pmd; >> + pte_t *pte, old_pte; >> + >> + /* Create 2nd stage page table mapping - Level 1 */ >> + pgd = kvm->arch.pgd + pgd_index(addr); >> + pud = pud_offset(pgd, addr); >> + if (pud_none(*pud)) { >> + if (!cache) >> + return; /* ignore calls from kvm_set_spte_hva */ >> + pmd = mmu_memory_cache_alloc(cache); >> + pud_populate(NULL, pud, pmd); >> + pmd += pmd_index(addr); >> + get_page(virt_to_page(pud)); >> + } else >> + pmd = pmd_offset(pud, addr); >> + >> + /* Create 2nd stage page table mapping - Level 2 */ >> + if (pmd_none(*pmd)) { >> + if (!cache) >> + return; /* ignore calls from kvm_set_spte_hva */ >> + pte = mmu_memory_cache_alloc(cache); >> + clean_pte_table(pte); >> + pmd_populate_kernel(NULL, pmd, pte); >> + pte += pte_index(addr); >> + get_page(virt_to_page(pmd)); >> + } else >> + pte = pte_offset_kernel(pmd, addr); >> + >> + /* Create 2nd stage page table mapping - Level 3 */ >> + old_pte = *pte; >> + set_pte_ext(pte, *new_pte, 0); >> + if (pte_present(old_pte)) >> + __kvm_tlb_flush_vmid(kvm); >> + else >> + get_page(virt_to_page(pte)); >> +} > > > I'm not sure about the 3-level page table, but isn't it necessary to > clean the page table for 2nd level? > There are two mmu_memory_cache_alloc calls. One has following clean_pte_table > and the other doesn't have. hmm, it probably is - I couldn't really find the common case where this is done in the kernel normally (except for some custom loop in ioremap and idmap), but I added this fix: diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 5394a52..f11ba27f 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -430,6 +430,7 @@ static void stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, if (!cache) return; /* ignore calls from kvm_set_spte_hva */ pmd = mmu_memory_cache_alloc(cache); + clean_dcache_area(pmd, PTRS_PER_PMD * sizeof(pmd_t)); pud_populate(NULL, pud, pmd); pmd += pmd_index(addr); get_page(virt_to_page(pud)); > > And why do you ignore calls from kvm_set_spte_hva? It is supposed to happen when > host moves the page, right? Then you ignore the case because it can be handled > later when fault actually happens? Is there any other reason that I miss? > kvm_set_spte_hva tells us that a page at some IPA is going to be backed by another physical page, which means we must adjust the stage 2 mapping. However, if we don't have that page mapped in the stage 2 page table, we don't need to do anything, and certainly don't want to start allocating unnecessary level2 and level3 page tables. Thanks! -Christoffer
next prev parent reply other threads:[~2012-10-06 21:33 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-10-01 9:09 [PATCH v2 00/14] KVM/ARM Implementation Christoffer Dall 2012-10-01 9:09 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 01/14] ARM: Add page table and page defines needed by KVM Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 02/14] ARM: Section based HYP idmap Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 03/14] ARM: Factor out cpuid implementor and part number Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 04/14] KVM: ARM: Initial skeleton to compile KVM support Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 05/14] KVM: ARM: Hypervisor inititalization Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 06/14] KVM: ARM: Memory virtualization setup Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-05 2:23 ` Min-gyu Kim 2012-10-05 2:23 ` Min-gyu Kim 2012-10-06 21:33 ` Christoffer Dall [this message] 2012-10-06 21:33 ` Christoffer Dall 2012-10-09 12:56 ` [kvmarm] " Marc Zyngier 2012-10-09 12:56 ` Marc Zyngier 2012-10-10 1:02 ` Min-gyu Kim 2012-10-10 1:02 ` Min-gyu Kim 2012-10-01 9:10 ` [PATCH v2 07/14] KVM: ARM: Inject IRQs and FIQs from userspace Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:10 ` [PATCH v2 08/14] KVM: ARM: World-switch implementation Christoffer Dall 2012-10-01 9:10 ` Christoffer Dall 2012-10-01 9:11 ` [PATCH v2 09/14] KVM: ARM: Emulation framework and CP15 emulation Christoffer Dall 2012-10-01 9:11 ` Christoffer Dall 2012-10-01 9:11 ` [PATCH v2 10/14] KVM: ARM: User space API for getting/setting co-proc registers Christoffer Dall 2012-10-01 9:11 ` Christoffer Dall 2012-10-01 9:11 ` [PATCH v2 11/14] KVM: ARM: Demux CCSIDR in the userspace API Christoffer Dall 2012-10-01 9:11 ` Christoffer Dall 2012-10-01 9:11 ` [PATCH v2 12/14] KVM: ARM: VFP userspace interface Christoffer Dall 2012-10-01 9:11 ` Christoffer Dall 2012-10-09 18:11 ` [kvmarm] " Peter Maydell 2012-10-09 18:11 ` Peter Maydell 2012-10-09 18:13 ` Christoffer Dall 2012-10-09 18:13 ` Christoffer Dall 2012-10-11 1:42 ` Rusty Russell 2012-10-11 1:42 ` Rusty Russell 2012-10-01 9:11 ` [PATCH v2 13/14] KVM: ARM: Handle guest faults in KVM Christoffer Dall 2012-10-01 9:11 ` Christoffer Dall 2012-10-01 9:11 ` [PATCH v2 14/14] KVM: ARM: Handle I/O aborts Christoffer Dall 2012-10-01 9:11 ` Christoffer Dall 2012-10-10 18:47 ` [PATCH v2 00/14] KVM/ARM Implementation Marcelo Tosatti 2012-10-10 18:47 ` Marcelo Tosatti
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CANM98qKAFQgfwRuFP2WOP4Af+1Un-wuQPC8-TiGi4B+s3XB2hQ@mail.gmail.com \ --to=c.dall@virtualopensystems.com \ --cc=changhwan.m.kim@samsung.com \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=marc.zyngier@arm.com \ --cc=mingyu84.kim@samsung.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.