* [PATCH v3 17/22] powerpc/kvm/book3s: use find_kvm_host_pte in kvmppc_book3s_instantiate_page
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_64_mmu_radix.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 7b18b298f513..8ce3191cc801 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -813,20 +813,20 @@ int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
* Read the PTE from the process' radix tree and use that
* so we get the shift and attribute bits.
*/
- local_irq_disable();
- ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL, &shift);
+ spin_lock(&kvm->mmu_lock);
+ ptep = find_kvm_host_pte(kvm, mmu_seq, hva, &shift);
/*
* If the PTE disappeared temporarily due to a THP
* collapse, just return and let the guest try again.
*/
if (!ptep) {
- local_irq_enable();
+ spin_unlock(&kvm->mmu_lock);
if (page)
put_page(page);
return RESUME_GUEST;
}
- pte = *ptep;
- local_irq_enable();
+ pte = READ_ONCE(*ptep);
+ spin_unlock(&kvm->mmu_lock);
/* If we're logging dirty pages, always map single pages */
large_enable = !(memslot->flags & KVM_MEM_LOG_DIRTY_PAGES);
--
2.25.3
^ permalink raw reply related
* [PATCH v3 15/22] powerpc/kvm/book3s: use find_kvm_host_pte in pute_tce functions
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: Alexey Kardashevskiy, Aneesh Kumar K.V, npiggin, kirill, leonardo
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Current code just hold rmap lock to ensure parallel page table update is
prevented. That is not sufficient. The kernel should also check whether
a mmu_notifer callback was running in parallel.
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_64_vio_hv.c | 30 +++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index 6fcaf1fa8e02..acc3ce570be7 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -437,8 +437,8 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
return H_SUCCESS;
}
-static long kvmppc_rm_ua_to_hpa(struct kvm_vcpu *vcpu,
- unsigned long ua, unsigned long *phpa)
+static long kvmppc_rm_ua_to_hpa(struct kvm_vcpu *vcpu, unsigned long mmu_seq,
+ unsigned long ua, unsigned long *phpa)
{
pte_t *ptep, pte;
unsigned shift = 0;
@@ -452,10 +452,17 @@ static long kvmppc_rm_ua_to_hpa(struct kvm_vcpu *vcpu,
* to exit which will agains result in the below page table walk
* to finish.
*/
- ptep = __find_linux_pte(vcpu->arch.pgdir, ua, NULL, &shift);
- if (!ptep || !pte_present(*ptep))
+ /* an rmap lock won't make it safe. because that just ensure hash
+ * page table entries are removed with rmap lock held. After that
+ * mmu notifier returns and we go ahead and removing ptes from Qemu page table.
+ */
+ ptep = find_kvm_host_pte(vcpu->kvm, mmu_seq, ua, &shift);
+ if (!ptep)
+ return -ENXIO;
+
+ pte = READ_ONCE(*ptep);
+ if (!pte_present(pte))
return -ENXIO;
- pte = *ptep;
if (!shift)
shift = PAGE_SHIFT;
@@ -477,10 +484,12 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
unsigned long liobn, unsigned long ioba,
unsigned long tce_list, unsigned long npages)
{
+ struct kvm *kvm = vcpu->kvm;
struct kvmppc_spapr_tce_table *stt;
long i, ret = H_SUCCESS;
unsigned long tces, entry, ua = 0;
unsigned long *rmap = NULL;
+ unsigned long mmu_seq;
bool prereg = false;
struct kvmppc_spapr_tce_iommu_table *stit;
@@ -488,6 +497,12 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
if (kvm_is_radix(vcpu->kvm))
return H_TOO_HARD;
+ /*
+ * used to check for invalidations in progress
+ */
+ mmu_seq = kvm->mmu_notifier_seq;
+ smp_rmb();
+
stt = kvmppc_find_table(vcpu->kvm, liobn);
if (!stt)
return H_TOO_HARD;
@@ -547,7 +562,9 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
* real page.
*/
lock_rmap(rmap);
- if (kvmppc_rm_ua_to_hpa(vcpu, ua, &tces)) {
+
+ arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
+ if (kvmppc_rm_ua_to_hpa(vcpu, mmu_seq, ua, &tces)) {
ret = H_TOO_HARD;
goto unlock_exit;
}
@@ -593,6 +610,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
if (rmap)
unlock_rmap(rmap);
+ arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
return ret;
}
--
2.25.3
^ permalink raw reply related
* [PATCH v3 14/22] powerpc/kvm/book3s: Use find_kvm_host_pte in h_enter
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Since kvmppc_do_h_enter can get called in realmode use low level
arch_spin_lock which is safe to be called in realmode.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 5 ++---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 22 ++++++----------------
2 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 121146f5a331..9d48173a8b77 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -281,11 +281,10 @@ static long kvmppc_virtmode_do_h_enter(struct kvm *kvm, unsigned long flags,
{
long ret;
- /* Protect linux PTE lookup from page table destruction */
- rcu_read_lock_sched(); /* this disables preemption too */
+ preempt_disable();
ret = kvmppc_do_h_enter(kvm, flags, pte_index, pteh, ptel,
kvm->mm->pgd, false, pte_idx_ret);
- rcu_read_unlock_sched();
+ preempt_enable();
if (ret == H_TOO_HARD) {
/* this can't happen */
pr_err("KVM: Oops, kvmppc_h_enter returned too hard!\n");
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 03f8347de48b..83e987fecf97 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -210,7 +210,7 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
pte_t *ptep;
unsigned int writing;
unsigned long mmu_seq;
- unsigned long rcbits, irq_flags = 0;
+ unsigned long rcbits;
if (kvm_is_radix(kvm))
return H_FUNCTION;
@@ -248,17 +248,9 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
/* Translate to host virtual address */
hva = __gfn_to_hva_memslot(memslot, gfn);
- /*
- * If we had a page table table change after lookup, we would
- * retry via mmu_notifier_retry.
- */
- if (!realmode)
- local_irq_save(irq_flags);
- /*
- * If called in real mode we have MSR_EE = 0. Otherwise
- * we disable irq above.
- */
- ptep = __find_linux_pte(pgdir, hva, NULL, &hpage_shift);
+
+ arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
+ ptep = find_kvm_host_pte(kvm, mmu_seq, hva, &hpage_shift);
if (ptep) {
pte_t pte;
unsigned int host_pte_size;
@@ -272,8 +264,7 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
* to <= host page size, if host is using hugepage
*/
if (host_pte_size < psize) {
- if (!realmode)
- local_irq_restore(flags);
+ arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
return H_PARAMETER;
}
pte = kvmppc_read_update_linux_pte(ptep, writing);
@@ -287,8 +278,7 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
pa |= gpa & ~PAGE_MASK;
}
}
- if (!realmode)
- local_irq_restore(irq_flags);
+ arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
ptel &= HPTE_R_KEY | HPTE_R_PP0 | (psize-1);
ptel |= pa;
--
2.25.3
^ permalink raw reply related
* [PATCH v3 16/22] powerpc/kvm/book3s: Avoid using rmap to protect parallel page table update.
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: Alexey Kardashevskiy, Aneesh Kumar K.V, npiggin, kirill, leonardo
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
We now depend on kvm->mmu_lock
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_64_vio_hv.c | 38 +++++++----------------------
1 file changed, 9 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index acc3ce570be7..167029e57c8f 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -74,8 +74,8 @@ struct kvmppc_spapr_tce_table *kvmppc_find_table(struct kvm *kvm,
EXPORT_SYMBOL_GPL(kvmppc_find_table);
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
-static long kvmppc_rm_tce_to_ua(struct kvm *kvm, unsigned long tce,
- unsigned long *ua, unsigned long **prmap)
+static long kvmppc_rm_tce_to_ua(struct kvm *kvm,
+ unsigned long tce, unsigned long *ua)
{
unsigned long gfn = tce >> PAGE_SHIFT;
struct kvm_memory_slot *memslot;
@@ -87,9 +87,6 @@ static long kvmppc_rm_tce_to_ua(struct kvm *kvm, unsigned long tce,
*ua = __gfn_to_hva_memslot(memslot, gfn) |
(tce & ~(PAGE_MASK | TCE_PCI_READ | TCE_PCI_WRITE));
- if (prmap)
- *prmap = &memslot->arch.rmap[gfn - memslot->base_gfn];
-
return 0;
}
@@ -116,7 +113,7 @@ static long kvmppc_rm_tce_validate(struct kvmppc_spapr_tce_table *stt,
if (iommu_tce_check_gpa(stt->page_shift, gpa))
return H_PARAMETER;
- if (kvmppc_rm_tce_to_ua(stt->kvm, tce, &ua, NULL))
+ if (kvmppc_rm_tce_to_ua(stt->kvm, tce, &ua))
return H_TOO_HARD;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
@@ -411,7 +408,7 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
return ret;
dir = iommu_tce_direction(tce);
- if ((dir != DMA_NONE) && kvmppc_rm_tce_to_ua(vcpu->kvm, tce, &ua, NULL))
+ if ((dir != DMA_NONE) && kvmppc_rm_tce_to_ua(vcpu->kvm, tce, &ua))
return H_PARAMETER;
entry = ioba >> stt->page_shift;
@@ -488,7 +485,6 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
struct kvmppc_spapr_tce_table *stt;
long i, ret = H_SUCCESS;
unsigned long tces, entry, ua = 0;
- unsigned long *rmap = NULL;
unsigned long mmu_seq;
bool prereg = false;
struct kvmppc_spapr_tce_iommu_table *stit;
@@ -530,7 +526,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
*/
struct mm_iommu_table_group_mem_t *mem;
- if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce_list, &ua, NULL))
+ if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce_list, &ua))
return H_TOO_HARD;
mem = mm_iommu_lookup_rm(vcpu->kvm->mm, ua, IOMMU_PAGE_SIZE_4K);
@@ -546,23 +542,9 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
* We do not require memory to be preregistered in this case
* so lock rmap and do __find_linux_pte_or_hugepte().
*/
- if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce_list, &ua, &rmap))
- return H_TOO_HARD;
-
- rmap = (void *) vmalloc_to_phys(rmap);
- if (WARN_ON_ONCE_RM(!rmap))
+ if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce_list, &ua))
return H_TOO_HARD;
- /*
- * Synchronize with the MMU notifier callbacks in
- * book3s_64_mmu_hv.c (kvm_unmap_hva_range_hv etc.).
- * While we have the rmap lock, code running on other CPUs
- * cannot finish unmapping the host real page that backs
- * this guest real page, so we are OK to access the host
- * real page.
- */
- lock_rmap(rmap);
-
arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
if (kvmppc_rm_ua_to_hpa(vcpu, mmu_seq, ua, &tces)) {
ret = H_TOO_HARD;
@@ -582,7 +564,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
unsigned long tce = be64_to_cpu(((u64 *)tces)[i]);
ua = 0;
- if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce, &ua, NULL)) {
+ if (kvmppc_rm_tce_to_ua(vcpu->kvm, tce, &ua)) {
ret = H_PARAMETER;
goto invalidate_exit;
}
@@ -607,10 +589,8 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
iommu_tce_kill_rm(stit->tbl, entry, npages);
unlock_exit:
- if (rmap)
- unlock_rmap(rmap);
-
- arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
+ if (!prereg)
+ arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
return ret;
}
--
2.25.3
^ permalink raw reply related
* [PATCH v3 13/22] powerpc/kvm/book3s: Use find_kvm_host_pte in page fault handler
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 6404df613ea3..121146f5a331 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -602,20 +602,20 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
* Read the PTE from the process' radix tree and use that
* so we get the shift and attribute bits.
*/
- local_irq_disable();
- ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL, &shift);
+ spin_lock(&kvm->mmu_lock);
+ ptep = find_kvm_host_pte(kvm, mmu_seq, hva, &shift);
/*
* If the PTE disappeared temporarily due to a THP
* collapse, just return and let the guest try again.
*/
if (!ptep) {
- local_irq_enable();
+ spin_unlock(&kvm->mmu_lock);
if (page)
put_page(page);
return RESUME_GUEST;
}
- pte = *ptep;
- local_irq_enable();
+ pte = READ_ONCE(*ptep);
+ spin_unlock(&kvm->mmu_lock);
hpa = pte_pfn(pte) << PAGE_SHIFT;
pte_size = PAGE_SIZE;
if (shift)
--
2.25.3
^ permalink raw reply related
* [PATCH v3 11/22] powerpc/kvm/book3s: Use kvm helpers to walk shadow or secondary table
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
update kvmppc_hv_handle_set_rc to use find_kvm_nested_guest_pte and
find_kvm_secondary_pte
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_book3s.h | 2 +-
arch/powerpc/include/asm/kvm_book3s_64.h | 3 +++
arch/powerpc/kvm/book3s_64_mmu_radix.c | 18 +++++++++---------
arch/powerpc/kvm/book3s_hv_nested.c | 13 ++++++-------
4 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 506e4df2d730..37c8b50cb505 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -198,7 +198,7 @@ extern void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte, unsigned long gpa,
unsigned int shift,
const struct kvm_memory_slot *memslot,
unsigned int lpid);
-extern bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable,
+extern bool kvmppc_hv_handle_set_rc(struct kvm *kvm, bool nested,
bool writing, unsigned long gpa,
unsigned int lpid);
extern int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 2c2635967d6e..2860521992b6 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -647,6 +647,9 @@ static inline pte_t *find_kvm_secondary_pte(struct kvm *kvm, unsigned long ea,
return pte;
}
+extern pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
+ unsigned long ea, unsigned *hshift);
+
#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
#endif /* __ASM_KVM_BOOK3S_64_H__ */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index bf8b72a274fa..7b18b298f513 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -735,7 +735,7 @@ int kvmppc_create_pte(struct kvm *kvm, pgd_t *pgtable, pte_t pte,
return ret;
}
-bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable, bool writing,
+bool kvmppc_hv_handle_set_rc(struct kvm *kvm, bool nested, bool writing,
unsigned long gpa, unsigned int lpid)
{
unsigned long pgflags;
@@ -750,12 +750,12 @@ bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable, bool writing,
pgflags = _PAGE_ACCESSED;
if (writing)
pgflags |= _PAGE_DIRTY;
- /*
- * We are walking the secondary (partition-scoped) page table here.
- * We can do this without disabling irq because the Linux MM
- * subsystem doesn't do THP splits and collapses on this tree.
- */
- ptep = __find_linux_pte(pgtable, gpa, NULL, &shift);
+
+ if (nested)
+ ptep = find_kvm_nested_guest_pte(kvm, lpid, gpa, &shift);
+ else
+ ptep = find_kvm_secondary_pte(kvm, gpa, &shift);
+
if (ptep && pte_present(*ptep) && (!writing || pte_write(*ptep))) {
kvmppc_radix_update_pte(kvm, ptep, 0, pgflags, gpa, shift);
return true;
@@ -948,8 +948,8 @@ int kvmppc_book3s_radix_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
/* Failed to set the reference/change bits */
if (dsisr & DSISR_SET_RC) {
spin_lock(&kvm->mmu_lock);
- if (kvmppc_hv_handle_set_rc(kvm, kvm->arch.pgtable,
- writing, gpa, kvm->arch.lpid))
+ if (kvmppc_hv_handle_set_rc(kvm, false, writing,
+ gpa, kvm->arch.lpid))
dsisr &= ~DSISR_SET_RC;
spin_unlock(&kvm->mmu_lock);
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index b2cc3eaec618..99011f1b772a 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -750,8 +750,8 @@ static struct kvm_nested_guest *kvmhv_find_nested(struct kvm *kvm, int lpid)
return kvm->arch.nested_guests[lpid];
}
-static pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
- unsigned long ea, unsigned *hshift)
+pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
+ unsigned long ea, unsigned *hshift)
{
struct kvm_nested_guest *gp;
pte_t *pte;
@@ -767,7 +767,6 @@ static pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
return pte;
}
-
static inline bool kvmhv_n_rmap_is_equal(u64 rmap_1, u64 rmap_2)
{
return !((rmap_1 ^ rmap_2) & (RMAP_NESTED_LPID_MASK |
@@ -1226,16 +1225,16 @@ static long kvmhv_handle_nested_set_rc(struct kvm_vcpu *vcpu,
spin_lock(&kvm->mmu_lock);
/* Set the rc bit in the pte of our (L0) pgtable for the L1 guest */
- ret = kvmppc_hv_handle_set_rc(kvm, kvm->arch.pgtable, writing,
- gpte.raddr, kvm->arch.lpid);
+ ret = kvmppc_hv_handle_set_rc(kvm, false, writing,
+ gpte.raddr, kvm->arch.lpid);
if (!ret) {
ret = -EINVAL;
goto out_unlock;
}
/* Set the rc bit in the pte of the shadow_pgtable for the nest guest */
- ret = kvmppc_hv_handle_set_rc(kvm, gp->shadow_pgtable, writing, n_gpa,
- gp->shadow_lpid);
+ ret = kvmppc_hv_handle_set_rc(kvm, true, writing,
+ n_gpa, gp->shadow_lpid);
if (!ret)
ret = -EINVAL;
else
--
2.25.3
^ permalink raw reply related
* [PATCH v3 12/22] powerpc/kvm/book3s: Add helper for host page table walk
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_book3s_64.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 2860521992b6..1ca1f6495012 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -647,6 +647,22 @@ static inline pte_t *find_kvm_secondary_pte(struct kvm *kvm, unsigned long ea,
return pte;
}
+static inline pte_t *find_kvm_host_pte(struct kvm *kvm, unsigned long mmu_seq,
+ unsigned long ea, unsigned *hshift)
+{
+ pte_t *pte;
+
+ VM_WARN(!spin_is_locked(&kvm->mmu_lock),
+ "%s called with kvm mmu_lock not held \n", __func__);
+
+ if (mmu_notifier_retry(kvm, mmu_seq))
+ return NULL;
+
+ pte = __find_linux_pte(kvm->mm->pgd, ea, NULL, hshift);
+
+ return pte;
+}
+
extern pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
unsigned long ea, unsigned *hshift);
--
2.25.3
^ permalink raw reply related
* [PATCH v3 10/22] powerpc/kvm/nested: Add helper to walk nested shadow linux page table.
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
The locking rules for walking nested shadow linux page table is different from process
scoped table. Hence add a helper for nested page table walk and also
add check whether we are holding the right locks.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 7f1fc5db13ea..b2cc3eaec618 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -750,6 +750,24 @@ static struct kvm_nested_guest *kvmhv_find_nested(struct kvm *kvm, int lpid)
return kvm->arch.nested_guests[lpid];
}
+static pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
+ unsigned long ea, unsigned *hshift)
+{
+ struct kvm_nested_guest *gp;
+ pte_t *pte;
+
+ gp = kvmhv_find_nested(kvm, lpid);
+ if (!gp)
+ return NULL;
+
+ VM_WARN(!spin_is_locked(&kvm->mmu_lock),
+ "%s called with kvm mmu_lock not held \n", __func__);
+ pte = __find_linux_pte(gp->shadow_pgtable, ea, NULL, hshift);
+
+ return pte;
+}
+
+
static inline bool kvmhv_n_rmap_is_equal(u64 rmap_1, u64 rmap_2)
{
return !((rmap_1 ^ rmap_2) & (RMAP_NESTED_LPID_MASK |
@@ -792,19 +810,15 @@ static void kvmhv_update_nest_rmap_rc(struct kvm *kvm, u64 n_rmap,
unsigned long clr, unsigned long set,
unsigned long hpa, unsigned long mask)
{
- struct kvm_nested_guest *gp;
unsigned long gpa;
unsigned int shift, lpid;
pte_t *ptep;
gpa = n_rmap & RMAP_NESTED_GPA_MASK;
lpid = (n_rmap & RMAP_NESTED_LPID_MASK) >> RMAP_NESTED_LPID_SHIFT;
- gp = kvmhv_find_nested(kvm, lpid);
- if (!gp)
- return;
/* Find the pte */
- ptep = __find_linux_pte(gp->shadow_pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_nested_guest_pte(kvm, lpid, gpa, &shift);
/*
* If the pte is present and the pfn is still the same, update the pte.
* If the pfn has changed then this is a stale rmap entry, the nested
@@ -854,7 +868,7 @@ static void kvmhv_remove_nest_rmap(struct kvm *kvm, u64 n_rmap,
return;
/* Find and invalidate the pte */
- ptep = __find_linux_pte(gp->shadow_pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_nested_guest_pte(kvm, lpid, gpa, &shift);
/* Don't spuriously invalidate ptes if the pfn has changed */
if (ptep && pte_present(*ptep) && ((pte_val(*ptep) & mask) == hpa))
kvmppc_unmap_pte(kvm, ptep, gpa, shift, NULL, gp->shadow_lpid);
@@ -921,7 +935,7 @@ static bool kvmhv_invalidate_shadow_pte(struct kvm_vcpu *vcpu,
int shift;
spin_lock(&kvm->mmu_lock);
- ptep = __find_linux_pte(gp->shadow_pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_nested_guest_pte(kvm, gp->l1_lpid, gpa, &shift);
if (!shift)
shift = PAGE_SHIFT;
if (ptep && pte_present(*ptep)) {
--
2.25.3
^ permalink raw reply related
* [PATCH v3 09/22] powerpc/kvm/book3s: Add helper to walk partition scoped linux page table.
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
The locking rules for walking partition scoped table is different from process
scoped table. Hence add a helper for secondary linux page table walk and also
add check whether we are holding the right locks.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_book3s_64.h | 13 +++++++++++++
arch/powerpc/kvm/book3s_64_mmu_radix.c | 12 ++++++------
arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 04b2b927bb5a..2c2635967d6e 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -14,6 +14,7 @@
#include <asm/book3s/64/mmu-hash.h>
#include <asm/cpu_has_feature.h>
#include <asm/ppc-opcode.h>
+#include <asm/pte-walk.h>
#ifdef CONFIG_PPC_PSERIES
static inline bool kvmhv_on_pseries(void)
@@ -634,6 +635,18 @@ extern void kvmhv_remove_nest_rmap_range(struct kvm *kvm,
unsigned long gpa, unsigned long hpa,
unsigned long nbytes);
+static inline pte_t *find_kvm_secondary_pte(struct kvm *kvm, unsigned long ea,
+ unsigned *hshift)
+{
+ pte_t *pte;
+
+ VM_WARN(!spin_is_locked(&kvm->mmu_lock),
+ "%s called with kvm mmu_lock not held \n", __func__);
+ pte = __find_linux_pte(kvm->arch.pgtable, ea, NULL, hshift);
+
+ return pte;
+}
+
#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
#endif /* __ASM_KVM_BOOK3S_64_H__ */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 9f050064d2a2..bf8b72a274fa 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -980,11 +980,11 @@ int kvm_unmap_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
return 0;
}
- ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_secondary_pte(kvm, gpa, &shift);
if (ptep && pte_present(*ptep))
kvmppc_unmap_pte(kvm, ptep, gpa, shift, memslot,
kvm->arch.lpid);
- return 0;
+ return 0;
}
/* Called with kvm->mmu_lock held */
@@ -1000,7 +1000,7 @@ int kvm_age_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_DONE)
return ref;
- ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_secondary_pte(kvm, gpa, &shift);
if (ptep && pte_present(*ptep) && pte_young(*ptep)) {
old = kvmppc_radix_update_pte(kvm, ptep, _PAGE_ACCESSED, 0,
gpa, shift);
@@ -1027,7 +1027,7 @@ int kvm_test_age_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_DONE)
return ref;
- ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_secondary_pte(kvm, gpa, &shift);
if (ptep && pte_present(*ptep) && pte_young(*ptep))
ref = 1;
return ref;
@@ -1047,7 +1047,7 @@ static int kvm_radix_test_clear_dirty(struct kvm *kvm,
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_DONE)
return ret;
- ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_secondary_pte(kvm, gpa, &shift);
if (ptep && pte_present(*ptep) && pte_dirty(*ptep)) {
ret = 1;
if (shift)
@@ -1108,7 +1108,7 @@ void kvmppc_radix_flush_memslot(struct kvm *kvm,
gpa = memslot->base_gfn << PAGE_SHIFT;
spin_lock(&kvm->mmu_lock);
for (n = memslot->npages; n; --n) {
- ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
+ ptep = find_kvm_secondary_pte(kvm, gpa, &shift);
if (ptep && pte_present(*ptep))
kvmppc_unmap_pte(kvm, ptep, gpa, shift, memslot,
kvm->arch.lpid);
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index dc97e5be76f6..7f1fc5db13ea 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -1362,7 +1362,7 @@ static long int __kvmhv_nested_page_fault(struct kvm_run *run,
/* See if can find translation in our partition scoped tables for L1 */
pte = __pte(0);
spin_lock(&kvm->mmu_lock);
- pte_p = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
+ pte_p = find_kvm_secondary_pte(kvm, gpa, &shift);
if (!shift)
shift = PAGE_SHIFT;
if (pte_p)
--
2.25.3
^ permalink raw reply related
* [PATCH v3 08/22] powerpc/kvm/book3s: switch from raw_spin_*lock to arch_spin_lock.
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: Aneesh Kumar K.V, npiggin, Suraj Jitindar Singh, kirill, leonardo
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
These functions can get called in realmode. Hence use low level
arch_spin_lock which is safe to be called in realmode.
Cc: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 220305454c23..03f8347de48b 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -948,7 +948,7 @@ static long kvmppc_do_h_page_init_zero(struct kvm_vcpu *vcpu,
return ret;
/* Check if we've been invalidated */
- raw_spin_lock(&kvm->mmu_lock.rlock);
+ arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
if (mmu_notifier_retry(kvm, mmu_seq)) {
ret = H_TOO_HARD;
goto out_unlock;
@@ -960,7 +960,7 @@ static long kvmppc_do_h_page_init_zero(struct kvm_vcpu *vcpu,
kvmppc_update_dirty_map(memslot, dest >> PAGE_SHIFT, PAGE_SIZE);
out_unlock:
- raw_spin_unlock(&kvm->mmu_lock.rlock);
+ arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
return ret;
}
@@ -984,7 +984,7 @@ static long kvmppc_do_h_page_init_copy(struct kvm_vcpu *vcpu,
return ret;
/* Check if we've been invalidated */
- raw_spin_lock(&kvm->mmu_lock.rlock);
+ arch_spin_lock(&kvm->mmu_lock.rlock.raw_lock);
if (mmu_notifier_retry(kvm, mmu_seq)) {
ret = H_TOO_HARD;
goto out_unlock;
@@ -996,7 +996,7 @@ static long kvmppc_do_h_page_init_copy(struct kvm_vcpu *vcpu,
kvmppc_update_dirty_map(dest_memslot, dest >> PAGE_SHIFT, PAGE_SIZE);
out_unlock:
- raw_spin_unlock(&kvm->mmu_lock.rlock);
+ arch_spin_unlock(&kvm->mmu_lock.rlock.raw_lock);
return ret;
}
--
2.25.3
^ permalink raw reply related
* [PATCH v3 07/22] powerpc/perf/callchain: Use __get_user_pages_fast in read_user_stack_slow
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
read_user_stack_slow is called with interrupts soft disabled and it copies contents
from the page which we find mapped to a specific address. To convert
userspace address to pfn, the kernel now uses lockless page table walk.
The kernel needs to make sure the pfn value read remains stable and is not released
and reused for another process while the contents are read from the page. This
can only be achieved by holding a page reference.
One of the first approaches I tried was to check the pte value after the kernel
copies the contents from the page. But as shown below we can still get it wrong
CPU0 CPU1
pte = READ_ONCE(*ptep);
pte_clear(pte);
put_page(page);
page = alloc_page();
memcpy(page_address(page), "secret password", nr);
memcpy(buf, kaddr + offset, nb);
put_page(page);
handle_mm_fault()
page = alloc_page();
set_pte(pte, page);
if (pte_val(pte) != pte_val(*ptep))
Hence switch to __get_user_pages_fast.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/perf/callchain_64.c | 46 ++++++++++----------------------
1 file changed, 14 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
index df1ffd8b20f2..b63086b663ef 100644
--- a/arch/powerpc/perf/callchain_64.c
+++ b/arch/powerpc/perf/callchain_64.c
@@ -26,43 +26,25 @@
*/
int read_user_stack_slow(void __user *ptr, void *buf, int nb)
{
- int ret = -EFAULT;
- pgd_t *pgdir;
- pte_t *ptep, pte;
- unsigned int shift;
+
unsigned long addr = (unsigned long) ptr;
unsigned long offset;
- unsigned long pfn, flags;
+ struct page *page;
+ int nrpages;
void *kaddr;
- pgdir = current->mm->pgd;
- if (!pgdir)
- return -EFAULT;
+ nrpages = __get_user_pages_fast(addr, 1, 1, &page);
+ if (nrpages == 1) {
+ kaddr = page_address(page);
+
+ /* align address to page boundary */
+ offset = addr & ~PAGE_MASK;
- local_irq_save(flags);
- ptep = find_current_mm_pte(pgdir, addr, NULL, &shift);
- if (!ptep)
- goto err_out;
- if (!shift)
- shift = PAGE_SHIFT;
-
- /* align address to page boundary */
- offset = addr & ((1UL << shift) - 1);
-
- pte = READ_ONCE(*ptep);
- if (!pte_present(pte) || !pte_user(pte))
- goto err_out;
- pfn = pte_pfn(pte);
- if (!page_is_ram(pfn))
- goto err_out;
-
- /* no highmem to worry about here */
- kaddr = pfn_to_kaddr(pfn);
- memcpy(buf, kaddr + offset, nb);
- ret = 0;
-err_out:
- local_irq_restore(flags);
- return ret;
+ memcpy(buf, kaddr + offset, nb);
+ put_page(page);
+ return 0;
+ }
+ return -EFAULT;
}
static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
--
2.25.3
^ permalink raw reply related
* [PATCH v3 06/22] powerpc/mce: Don't reload pte val in addr_to_pfn
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: Mahesh Salgaonkar, kirill, npiggin, Aneesh Kumar K.V, leonardo
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
A lockless page table walk should be safe against parallel THP collapse, THP
split and madvise(MADV_DONTNEED)/parallel fault. This patch makes sure kernel
won't reload the pteval when checking for different conditions. The patch also added
a check for pte_present to make sure the kernel is indeed operating
on a PTE and not a pointer to level 0 table page.
The pfn value we find here can be different from the actual pfn on which
machine check happened. This can happen if we raced with a parallel update
of the page table. In such a scenario we end up isolating a wrong pfn. But that
doesn't have any other side effect.
Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/kernel/mce_power.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 067b094bfeff..1d18991f3854 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -27,7 +27,7 @@
*/
unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
{
- pte_t *ptep;
+ pte_t *ptep, pte;
unsigned int shift;
unsigned long pfn, flags;
struct mm_struct *mm;
@@ -39,19 +39,23 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
local_irq_save(flags);
ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
+ if (!ptep) {
+ pfn = ULONG_MAX;
+ goto out;
+ }
+ pte = READ_ONCE(*ptep);
- if (!ptep || pte_special(*ptep)) {
+ if (!pte_present(pte) || pte_special(pte)) {
pfn = ULONG_MAX;
goto out;
}
if (shift <= PAGE_SHIFT)
- pfn = pte_pfn(*ptep);
+ pfn = pte_pfn(pte);
else {
unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
- pfn = pte_pfn(__pte(pte_val(*ptep) | (addr & rpnmask)));
+ pfn = pte_pfn(__pte(pte_val(pte) | (addr & rpnmask)));
}
-
out:
local_irq_restore(flags);
return pfn;
--
2.25.3
^ permalink raw reply related
* [PATCH v3 05/22] powerpc/book3s64/hash: Use the pte_t address from the caller
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Don't fetch the pte value using lockless page table walk. Instead use the value from the
caller. hash_preload is called with ptl lock held. So it is safe to use the
pte_t address directly.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/book3s64/hash_utils.c | 27 +++++----------------------
1 file changed, 5 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 525eac4ee2c2..3d727f73a8db 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1546,14 +1546,11 @@ static bool should_hash_preload(struct mm_struct *mm, unsigned long ea)
}
#endif
-static void hash_preload(struct mm_struct *mm, unsigned long ea,
+static void hash_preload(struct mm_struct *mm, pte_t *ptep, unsigned long ea,
bool is_exec, unsigned long trap)
{
- int hugepage_shift;
unsigned long vsid;
pgd_t *pgdir;
- pte_t *ptep;
- unsigned long flags;
int rc, ssize, update_flags = 0;
unsigned long access = _PAGE_PRESENT | _PAGE_READ | (is_exec ? _PAGE_EXEC : 0);
@@ -1575,30 +1572,18 @@ static void hash_preload(struct mm_struct *mm, unsigned long ea,
vsid = get_user_vsid(&mm->context, ea, ssize);
if (!vsid)
return;
- /*
- * Hash doesn't like irqs. Walking linux page table with irq disabled
- * saves us from holding multiple locks.
- */
- local_irq_save(flags);
- /*
- * THP pages use update_mmu_cache_pmd. We don't do
- * hash preload there. Hence can ignore THP here
- */
- ptep = find_current_mm_pte(pgdir, ea, NULL, &hugepage_shift);
- if (!ptep)
- goto out_exit;
-
- WARN_ON(hugepage_shift);
#ifdef CONFIG_PPC_64K_PAGES
/* If either H_PAGE_4K_PFN or cache inhibited is set (and we are on
* a 64K kernel), then we don't preload, hash_page() will take
* care of it once we actually try to access the page.
* That way we don't have to duplicate all of the logic for segment
* page size demotion here
+ * Called with PTL held, hence can be sure the value won't change in
+ * between.
*/
if ((pte_val(*ptep) & H_PAGE_4K_PFN) || pte_ci(*ptep))
- goto out_exit;
+ return;
#endif /* CONFIG_PPC_64K_PAGES */
/* Is that local to this CPU ? */
@@ -1623,8 +1608,6 @@ static void hash_preload(struct mm_struct *mm, unsigned long ea,
mm_ctx_user_psize(&mm->context),
mm_ctx_user_psize(&mm->context),
pte_val(*ptep));
-out_exit:
- local_irq_restore(flags);
}
/*
@@ -1675,7 +1658,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
return;
}
- hash_preload(vma->vm_mm, address, is_exec, trap);
+ hash_preload(vma->vm_mm, ptep, address, is_exec, trap);
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
--
2.25.3
^ permalink raw reply related
* [PATCH v3 04/22] powerpc/hash64: Restrict page table lookup using init_mm with __flush_hash_table_range
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
This is only used with init_mm currently. Walking init_mm is much simpler
because we don't need to handle concurrent page table like other mm_context
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
.../include/asm/book3s/64/tlbflush-hash.h | 3 +--
arch/powerpc/kernel/pci_64.c | 2 +-
arch/powerpc/mm/book3s64/hash_tlb.c | 16 +++-------------
3 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
index 64d02a704bcb..3b95769739c7 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush-hash.h
@@ -113,8 +113,7 @@ static inline void hash__flush_tlb_kernel_range(unsigned long start,
struct mmu_gather;
extern void hash__tlb_flush(struct mmu_gather *tlb);
/* Private function for use by PCI IO mapping code */
-extern void __flush_hash_table_range(struct mm_struct *mm, unsigned long start,
- unsigned long end);
+extern void __flush_hash_table_range(unsigned long start, unsigned long end);
extern void flush_tlb_pmd_range(struct mm_struct *mm, pmd_t *pmd,
unsigned long addr);
#endif /* _ASM_POWERPC_BOOK3S_64_TLBFLUSH_HASH_H */
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index f83d1f69b1dd..30d07fc79dd1 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -100,7 +100,7 @@ int pcibios_unmap_io_space(struct pci_bus *bus)
pci_name(bus->self));
#ifdef CONFIG_PPC_BOOK3S_64
- __flush_hash_table_range(&init_mm, res->start + _IO_BASE,
+ __flush_hash_table_range(res->start + _IO_BASE,
res->end + _IO_BASE + 1);
#endif
return 0;
diff --git a/arch/powerpc/mm/book3s64/hash_tlb.c b/arch/powerpc/mm/book3s64/hash_tlb.c
index 4a70d8dd39cd..1fa2173413b5 100644
--- a/arch/powerpc/mm/book3s64/hash_tlb.c
+++ b/arch/powerpc/mm/book3s64/hash_tlb.c
@@ -176,7 +176,6 @@ void hash__tlb_flush(struct mmu_gather *tlb)
* from the hash table (and the TLB). But keeps
* the linux PTEs intact.
*
- * @mm : mm_struct of the target address space (generally init_mm)
* @start : starting address
* @end : ending address (not included in the flush)
*
@@ -189,17 +188,14 @@ void hash__tlb_flush(struct mmu_gather *tlb)
* Because of that usage pattern, it is implemented for small size rather
* than speed.
*/
-void __flush_hash_table_range(struct mm_struct *mm, unsigned long start,
- unsigned long end)
+void __flush_hash_table_range(unsigned long start, unsigned long end)
{
- bool is_thp;
int hugepage_shift;
unsigned long flags;
start = _ALIGN_DOWN(start, PAGE_SIZE);
end = _ALIGN_UP(end, PAGE_SIZE);
- BUG_ON(!mm->pgd);
/*
* Note: Normally, we should only ever use a batch within a
@@ -212,21 +208,15 @@ void __flush_hash_table_range(struct mm_struct *mm, unsigned long start,
local_irq_save(flags);
arch_enter_lazy_mmu_mode();
for (; start < end; start += PAGE_SIZE) {
- pte_t *ptep = find_current_mm_pte(mm->pgd, start, &is_thp,
- &hugepage_shift);
+ pte_t *ptep = find_init_mm_pte(start, &hugepage_shift);
unsigned long pte;
if (ptep == NULL)
continue;
pte = pte_val(*ptep);
- if (is_thp)
- trace_hugepage_invalidate(start, pte);
if (!(pte & H_PAGE_HASHPTE))
continue;
- if (unlikely(is_thp))
- hpte_do_hugepage_flush(mm, start, (pmd_t *)ptep, pte);
- else
- hpte_need_flush(mm, start, ptep, pte, hugepage_shift);
+ hpte_need_flush(&init_mm, start, ptep, pte, hugepage_shift);
}
arch_leave_lazy_mmu_mode();
local_irq_restore(flags);
--
2.25.3
^ permalink raw reply related
* [PATCH v3 03/22] powerpc/mm/hash64: use _PAGE_PTE when checking for pte_present
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
This makes the pte_present check stricter by checking for additional _PAGE_PTE
bit. A level 1 pte pointer (THP pte) can be switched to a pointer to level 0 pte
page table page by following two operations.
1) THP split.
2) madvise(MADV_DONTNEED) in parallel to page fault.
A lockless page table walk need to make sure we can handle such changes
gracefully.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 15 ++++++++++-----
arch/powerpc/mm/book3s64/hash_utils.c | 11 +++++++++--
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 368b136517e0..03521a8b0292 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -553,6 +553,12 @@ static inline pte_t pte_clear_savedwrite(pte_t pte)
}
#endif /* CONFIG_NUMA_BALANCING */
+static inline bool pte_hw_valid(pte_t pte)
+{
+ return (pte_raw(pte) & cpu_to_be64(_PAGE_PRESENT | _PAGE_PTE)) ==
+ cpu_to_be64(_PAGE_PRESENT | _PAGE_PTE);
+}
+
static inline int pte_present(pte_t pte)
{
/*
@@ -561,12 +567,11 @@ static inline int pte_present(pte_t pte)
* invalid during ptep_set_access_flags. Hence we look for _PAGE_INVALID
* if we find _PAGE_PRESENT cleared.
*/
- return !!(pte_raw(pte) & cpu_to_be64(_PAGE_PRESENT | _PAGE_INVALID));
-}
-static inline bool pte_hw_valid(pte_t pte)
-{
- return !!(pte_raw(pte) & cpu_to_be64(_PAGE_PRESENT));
+ if (pte_hw_valid(pte))
+ return true;
+ return (pte_raw(pte) & cpu_to_be64(_PAGE_INVALID | _PAGE_PTE)) ==
+ cpu_to_be64(_PAGE_INVALID | _PAGE_PTE);
}
#ifdef CONFIG_PPC_MEM_KEYS
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index e951e87a974d..525eac4ee2c2 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1350,8 +1350,15 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,
goto bail;
}
- /* Add _PAGE_PRESENT to the required access perm */
- access |= _PAGE_PRESENT;
+ /*
+ * Add _PAGE_PRESENT to the required access perm. If there are parallel
+ * updates to the pte that can possibly clear _PAGE_PTE, catch that too.
+ *
+ * We can safely use the return pte address in rest of the function
+ * because we do set H_PAGE_BUSY which prevents further updates to pte
+ * from generic code.
+ */
+ access |= _PAGE_PRESENT | _PAGE_PTE;
/*
* Pre-check access permissions (will be re-checked atomically
--
2.25.3
^ permalink raw reply related
* [PATCH v3 02/22] powerpc/pkeys: Check vma before returning key fault error to the user
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: Ram Pai, Aneesh Kumar K.V, npiggin, Sandipan Das, kirill,
leonardo
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
If multiple threads in userspace keep changing the protection keys
mapping a range, there can be a scenario where kernel takes a key fault
but the pkey value found in the siginfo struct is a permissive one.
This can confuse the userspace as shown in the below test case.
/* use this to control the number of test iterations */
static void pkeyreg_set(int pkey, unsigned long rights)
{
unsigned long reg, shift;
shift = (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
asm volatile("mfspr %0, 0xd" : "=r"(reg));
reg &= ~(((unsigned long) PKEY_BITS_MASK) << shift);
reg |= (rights & PKEY_BITS_MASK) << shift;
asm volatile("mtspr 0xd, %0" : : "r"(reg));
}
static unsigned long pkeyreg_get(void)
{
unsigned long reg;
asm volatile("mfspr %0, 0xd" : "=r"(reg));
return reg;
}
static int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
{
return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
}
static int sys_pkey_alloc(unsigned long flags, unsigned long access_rights)
{
return syscall(SYS_pkey_alloc, flags, access_rights);
}
static int sys_pkey_free(int pkey)
{
return syscall(SYS_pkey_free, pkey);
}
static int faulting_pkey;
static int permissive_pkey;
static pthread_barrier_t pkey_set_barrier;
static pthread_barrier_t mprotect_barrier;
static void pkey_handle_fault(int signum, siginfo_t *sinfo, void *ctx)
{
unsigned long pkeyreg;
/* FIXME: printf is not signal-safe but for the current purpose,
it gets the job done. */
printf("pkey: exp = %d, got = %d\n", faulting_pkey, sinfo->si_pkey);
fflush(stdout);
assert(sinfo->si_code == SEGV_PKUERR);
assert(sinfo->si_pkey == faulting_pkey);
/* clear pkey permissions to let the faulting instruction continue */
pkeyreg_set(faulting_pkey, 0x0);
}
static void *do_mprotect_fault(void *p)
{
unsigned long rights, pkeyreg, pgsize;
unsigned int i;
void *region;
int pkey;
srand(time(NULL));
pgsize = sysconf(_SC_PAGESIZE);
rights = PKEY_DISABLE_WRITE;
region = p;
/* allocate key, no permissions */
assert((pkey = sys_pkey_alloc(0, PKEY_DISABLE_ACCESS)) > 0);
pkeyreg_set(4, 0x0);
/* cache the pkey here as the faulting pkey for future reference
in the signal handler */
faulting_pkey = pkey;
printf("%s: faulting pkey = %d\n", __func__, faulting_pkey);
/* try to allocate, mprotect and free pkeys repeatedly */
for (i = 0; i < NUM_ITERATIONS; i++) {
/* sync up with the other thread here */
pthread_barrier_wait(&pkey_set_barrier);
/* make sure that the pkey used by the non-faulting thread
is made permissive for this thread's context too so that
no faults are triggered because it still might have been
set to a restrictive value */
// pkeyreg_set(permissive_pkey, 0x0);
/* sync up with the other thread here */
pthread_barrier_wait(&mprotect_barrier);
/* perform mprotect */
assert(!sys_pkey_mprotect(region, pgsize, PROT_READ | PROT_WRITE, pkey));
/* choose a random byte from the protected region and
attempt to write to it, this will generate a fault */
*((char *) region + (rand() % pgsize)) = rand();
/* restore pkey permissions as the signal handler may have
cleared the bit out for the sake of continuing */
pkeyreg_set(pkey, PKEY_DISABLE_WRITE);
}
/* free pkey */
sys_pkey_free(pkey);
return NULL;
}
static void *do_mprotect_nofault(void *p)
{
unsigned long pgsize;
unsigned int i, j;
void *region;
int pkey;
pgsize = sysconf(_SC_PAGESIZE);
region = p;
/* try to allocate, mprotect and free pkeys repeatedly */
for (i = 0; i < NUM_ITERATIONS; i++) {
/* allocate pkey, all permissions */
assert((pkey = sys_pkey_alloc(0, 0)) > 0);
permissive_pkey = pkey;
/* sync up with the other thread here */
pthread_barrier_wait(&pkey_set_barrier);
pthread_barrier_wait(&mprotect_barrier);
/* perform mprotect on the common page, no faults will
be triggered as this is most permissive */
assert(!sys_pkey_mprotect(region, pgsize, PROT_READ | PROT_WRITE, pkey));
/* free pkey */
assert(!sys_pkey_free(pkey));
}
return NULL;
}
int main(int argc, char **argv)
{
pthread_t fault_thread, nofault_thread;
unsigned long pgsize;
struct sigaction act;
pthread_attr_t attr;
cpu_set_t fault_cpuset, nofault_cpuset;
unsigned int i;
void *region;
/* allocate memory region to protect */
pgsize = sysconf(_SC_PAGESIZE);
assert(region = memalign(pgsize, pgsize));
CPU_ZERO(&fault_cpuset);
CPU_SET(0, &fault_cpuset);
CPU_ZERO(&nofault_cpuset);
CPU_SET(8, &nofault_cpuset);
assert(!pthread_attr_init(&attr));
/* setup sigsegv signal handler */
act.sa_handler = 0;
act.sa_sigaction = pkey_handle_fault;
assert(!sigprocmask(SIG_SETMASK, 0, &act.sa_mask));
act.sa_flags = SA_SIGINFO;
act.sa_restorer = 0;
assert(!sigaction(SIGSEGV, &act, NULL));
/* setup barrier for the two threads */
pthread_barrier_init(&pkey_set_barrier, NULL, 2);
pthread_barrier_init(&mprotect_barrier, NULL, 2);
/* setup and start threads */
assert(!pthread_create(&fault_thread, &attr, &do_mprotect_fault, region));
assert(!pthread_setaffinity_np(fault_thread, sizeof(cpu_set_t), &fault_cpuset));
assert(!pthread_create(&nofault_thread, &attr, &do_mprotect_nofault, region));
assert(!pthread_setaffinity_np(nofault_thread, sizeof(cpu_set_t), &nofault_cpuset));
/* cleanup */
assert(!pthread_attr_destroy(&attr));
assert(!pthread_join(fault_thread, NULL));
assert(!pthread_join(nofault_thread, NULL));
assert(!pthread_barrier_destroy(&pkey_set_barrier));
assert(!pthread_barrier_destroy(&mprotect_barrier));
free(region);
puts("PASS");
return EXIT_SUCCESS;
}
The above test can result the below failure without this patch.
pkey: exp = 3, got = 3
pkey: exp = 3, got = 4
a.out: pkey-siginfo-race.c:100: pkey_handle_fault: Assertion `sinfo->si_pkey == faulting_pkey' failed.
Aborted
Check for vma access before considering this a key fault. If vma pkey allow
access retry the acess again.
Test case is written by Sandipan Das <sandipan@linux.ibm.com> hence added SOB
from him.
Cc: Ram Pai <linuxram@linux.ibm.com>
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/fault.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 8e529e4708e1..44457bae77a0 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -319,14 +319,6 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
static bool access_pkey_error(bool is_write, bool is_exec, bool is_pkey,
struct vm_area_struct *vma)
{
- /*
- * Read or write was blocked by protection keys. This is
- * always an unconditional error and can never result in
- * a follow-up action to resolve the fault, like a COW.
- */
- if (is_pkey)
- return true;
-
/*
* Make sure to check the VMA so that we do not perform
* faults just to hit a pkey fault as soon as we fill in a
--
2.25.3
^ permalink raw reply related
* [PATCH v3 01/22] powerpc/pkeys: Avoid using lockless page table walk
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: Ram Pai, Aneesh Kumar K.V, npiggin, kirill, leonardo
In-Reply-To: <20200420124434.47330-1-aneesh.kumar@linux.ibm.com>
Fetch pkey from vma instead of linux page table. Also document the fact that in
some cases the pkey returned in siginfo won't be the same as the one we took
keyfault on. Even with linux page table walk, we can end up in a similar scenario.
Cc: Ram Pai <linuxram@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/mmu.h | 9 ---
arch/powerpc/mm/book3s64/hash_utils.c | 24 --------
arch/powerpc/mm/fault.c | 83 +++++++++++++++++++--------
3 files changed, 60 insertions(+), 56 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 0699cfeeb8c9..cf2a08bfd5cd 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -291,15 +291,6 @@ static inline bool early_radix_enabled(void)
}
#endif
-#ifdef CONFIG_PPC_MEM_KEYS
-extern u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address);
-#else
-static inline u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address)
-{
- return 0;
-}
-#endif /* CONFIG_PPC_MEM_KEYS */
-
#ifdef CONFIG_STRICT_KERNEL_RWX
static inline bool strict_kernel_rwx_enabled(void)
{
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 8ed2411c3f39..e951e87a974d 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1671,30 +1671,6 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
hash_preload(vma->vm_mm, address, is_exec, trap);
}
-#ifdef CONFIG_PPC_MEM_KEYS
-/*
- * Return the protection key associated with the given address and the
- * mm_struct.
- */
-u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address)
-{
- pte_t *ptep;
- u16 pkey = 0;
- unsigned long flags;
-
- if (!mm || !mm->pgd)
- return 0;
-
- local_irq_save(flags);
- ptep = find_linux_pte(mm->pgd, address, NULL, NULL);
- if (ptep)
- pkey = pte_to_pkey_bits(pte_val(READ_ONCE(*ptep)));
- local_irq_restore(flags);
-
- return pkey;
-}
-#endif /* CONFIG_PPC_MEM_KEYS */
-
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
static inline void tm_flush_hash_page(int local)
{
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 84af6c8eecf7..8e529e4708e1 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -118,9 +118,34 @@ static noinline int bad_area(struct pt_regs *regs, unsigned long address)
return __bad_area(regs, address, SEGV_MAPERR);
}
-static int bad_key_fault_exception(struct pt_regs *regs, unsigned long address,
- int pkey)
+#ifdef CONFIG_PPC_MEM_KEYS
+static noinline int bad_access_pkey(struct pt_regs *regs, unsigned long address,
+ struct vm_area_struct *vma)
{
+ struct mm_struct *mm = current->mm;
+ int pkey;
+
+ /*
+ * We don't try to fetch the pkey from page table because reading
+ * page table without locking doesn't guarantee stable pte value.
+ * Hence the pkey value that we return to userspace can be different
+ * from the pkey that actually caused access error.
+ *
+ * It does *not* guarantee that the VMA we find here
+ * was the one that we faulted on.
+ *
+ * 1. T1 : mprotect_key(foo, PAGE_SIZE, pkey=4);
+ * 2. T1 : set AMR to deny access to pkey=4, touches, page
+ * 3. T1 : faults...
+ * 4. T2: mprotect_key(foo, PAGE_SIZE, pkey=5);
+ * 5. T1 : enters fault handler, takes mmap_sem, etc...
+ * 6. T1 : reaches here, sees vma_pkey(vma)=5, when we really
+ * faulted on a pte with its pkey=4.
+ */
+ pkey = vma_pkey(vma);
+
+ up_read(&mm->mmap_sem);
+
/*
* If we are in kernel mode, bail out with a SEGV, this will
* be caught by the assembly which will restore the non-volatile
@@ -133,6 +158,7 @@ static int bad_key_fault_exception(struct pt_regs *regs, unsigned long address,
return 0;
}
+#endif
static noinline int bad_access(struct pt_regs *regs, unsigned long address)
{
@@ -289,8 +315,31 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
return false;
}
-static bool access_error(bool is_write, bool is_exec,
- struct vm_area_struct *vma)
+#ifdef CONFIG_PPC_MEM_KEYS
+static bool access_pkey_error(bool is_write, bool is_exec, bool is_pkey,
+ struct vm_area_struct *vma)
+{
+ /*
+ * Read or write was blocked by protection keys. This is
+ * always an unconditional error and can never result in
+ * a follow-up action to resolve the fault, like a COW.
+ */
+ if (is_pkey)
+ return true;
+
+ /*
+ * Make sure to check the VMA so that we do not perform
+ * faults just to hit a pkey fault as soon as we fill in a
+ * page. Only called for current mm, hence foreign == 0
+ */
+ if (!arch_vma_access_permitted(vma, is_write, is_exec, 0))
+ return true;
+
+ return false;
+}
+#endif
+
+static bool access_error(bool is_write, bool is_exec, struct vm_area_struct *vma)
{
/*
* Allow execution from readable areas if the MMU does not
@@ -483,10 +532,6 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
- if (error_code & DSISR_KEYFAULT)
- return bad_key_fault_exception(regs, address,
- get_mm_addr_key(mm, address));
-
/*
* We want to do this outside mmap_sem, because reading code around nip
* can result in fault, which will cause a deadlock when called with
@@ -555,6 +600,13 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
return bad_area(regs, address);
good_area:
+
+#ifdef CONFIG_PPC_MEM_KEYS
+ if (unlikely(access_pkey_error(is_write, is_exec,
+ (error_code & DSISR_KEYFAULT), vma)))
+ return bad_access_pkey(regs, address, vma);
+#endif /* CONFIG_PPC_MEM_KEYS */
+
if (unlikely(access_error(is_write, is_exec, vma)))
return bad_access(regs, address);
@@ -565,21 +617,6 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
*/
fault = handle_mm_fault(vma, address, flags);
-#ifdef CONFIG_PPC_MEM_KEYS
- /*
- * we skipped checking for access error due to key earlier.
- * Check that using handle_mm_fault error return.
- */
- if (unlikely(fault & VM_FAULT_SIGSEGV) &&
- !arch_vma_access_permitted(vma, is_write, is_exec, 0)) {
-
- int pkey = vma_pkey(vma);
-
- up_read(&mm->mmap_sem);
- return bad_key_fault_exception(regs, address, pkey);
- }
-#endif /* CONFIG_PPC_MEM_KEYS */
-
major |= fault & VM_FAULT_MAJOR;
if (fault_signal_pending(fault, regs))
--
2.25.3
^ permalink raw reply related
* [PATCH v3 00/22] Avoid IPI while updating page table entries.
From: Aneesh Kumar K.V @ 2020-04-20 12:44 UTC (permalink / raw)
To: linuxppc-dev, mpe, linux-mm, linux-kernel, kvm-ppc
Cc: leonardo, Aneesh Kumar K.V, npiggin, kirill
Problem Summary:
Slow termination of KVM guest with large guest RAM config due to a large number
of IPIs that were caused by clearing level 1 PTE entries (THP) entries.
This is shown in the stack trace below.
- qemu-system-ppc [kernel.vmlinux] [k] smp_call_function_many
- smp_call_function_many
- 36.09% smp_call_function_many
serialize_against_pte_lookup
radix__pmdp_huge_get_and_clear
zap_huge_pmd
unmap_page_range
unmap_vmas
unmap_region
__do_munmap
__vm_munmap
sys_munmap
system_call
__munmap
qemu_ram_munmap
qemu_anon_ram_free
reclaim_ramblock
call_rcu_thread
qemu_thread_start
start_thread
__clone
Why we need to do IPI when clearing PMD entries:
This was added as part of commit: 13bd817bb884 ("powerpc/thp: Serialize pmd clear against a linux page table walk")
serialize_against_pte_lookup makes sure that all parallel lockless page table
walk completes before we convert a PMD pte entry to regular pmd entry.
We end up doing that conversion in the below scenarios
1) __split_huge_zero_page_pmd
2) do_huge_pmd_wp_page_fallback
3) MADV_DONTNEED running parallel to page faults.
local_irq_disable and lockless page table walk:
The lockless page table walk work with the assumption that we can dereference
the page table contents without holding a lock. For this to work, we need to
make sure we read the page table contents atomically and page table pages are
not going to be freed/released while we are walking the
table pages. We can achieve by using a rcu based freeing for page table pages or
if the architecture implements broadcast tlbie, we can block the IPI as we walk the
page table pages.
To support both the above framework, lockless page table walk is done with
irq disabled instead of rcu_read_lock()
We do have two interface for lockless page table walk, gup fast and __find_linux_pte.
This patch series makes __find_linux_pte table walk safe against the conversion of PMD PTE
to regular PMD.
gup fast:
gup fast is already safe against THP split because kernel now differentiate between a pmd
split and a compound page split. gup fast can run parallel to a pmd split and we prevent
a parallel gup fast to a hugepage split, by freezing the page refcount and failing the
speculative page ref increment.
Similar to how gup is safe against parallel pmd split, this patch series updates the
__find_linux_pte callers to be safe against a parallel pmd split. We do that by enforcing
the following rules.
1) Don't reload the pte value, because that can be updated in parallel.
2) Code should be able to work with a stale PTE value and not the recent one. ie,
the pte value that we are looking at may not be the latest value in the page table.
3) Before looking at pte value check for _PAGE_PTE bit. We now do this as part of pte_present()
check.
Performance:
This speeds up Qemu guest RAM del/unplug time as below
128 core, 496GB guest:
Without patch:
munmap start: timer = 13162 ms, PID=7684
munmap finish: timer = 95312 ms, PID=7684 - delta = 82150 ms
With patch (upto removing IPI)
munmap start: timer = 196449 ms, PID=6681
munmap finish: timer = 196488 ms, PID=6681 - delta = 39ms
With patch (with adding the tlb invalidate in pmdp_huge_get_and_clear_full)
munmap start: timer = 196345 ms, PID=6879
munmap finish: timer = 196714 ms, PID=6879 - delta = 369ms
Changes from V2:
* Rebase to lastest kernel
Changes from V1:
* Update commit messages
* Qemu Performance numbers
Aneesh Kumar K.V (22):
powerpc/pkeys: Avoid using lockless page table walk
powerpc/pkeys: Check vma before returning key fault error to the user
powerpc/mm/hash64: use _PAGE_PTE when checking for pte_present
powerpc/hash64: Restrict page table lookup using init_mm with
__flush_hash_table_range
powerpc/book3s64/hash: Use the pte_t address from the caller
powerpc/mce: Don't reload pte val in addr_to_pfn
powerpc/perf/callchain: Use __get_user_pages_fast in
read_user_stack_slow
powerpc/kvm/book3s: switch from raw_spin_*lock to arch_spin_lock.
powerpc/kvm/book3s: Add helper to walk partition scoped linux page
table.
powerpc/kvm/nested: Add helper to walk nested shadow linux page table.
powerpc/kvm/book3s: Use kvm helpers to walk shadow or secondary table
powerpc/kvm/book3s: Add helper for host page table walk
powerpc/kvm/book3s: Use find_kvm_host_pte in page fault handler
powerpc/kvm/book3s: Use find_kvm_host_pte in h_enter
powerpc/kvm/book3s: use find_kvm_host_pte in pute_tce functions
powerpc/kvm/book3s: Avoid using rmap to protect parallel page table
update.
powerpc/kvm/book3s: use find_kvm_host_pte in
kvmppc_book3s_instantiate_page
powerpc/kvm/book3s: Use find_kvm_host_pte in kvmppc_get_hpa
powerpc/kvm/book3s: Use pte_present instead of opencoding
_PAGE_PRESENT check
powerpc/mm/book3s64: Avoid sending IPI on clearing PMD
mm: change pmdp_huge_get_and_clear_full take vm_area_struct as arg
powerpc/mm/book3s64: Fix MADV_DONTNEED and parallel page fault race
arch/powerpc/include/asm/book3s/64/pgtable.h | 20 +++--
.../include/asm/book3s/64/tlbflush-hash.h | 3 +-
arch/powerpc/include/asm/kvm_book3s.h | 2 +-
arch/powerpc/include/asm/kvm_book3s_64.h | 34 ++++++++-
arch/powerpc/include/asm/mmu.h | 9 ---
arch/powerpc/kernel/mce_power.c | 14 ++--
arch/powerpc/kernel/pci_64.c | 2 +-
arch/powerpc/kvm/book3s_64_mmu_hv.c | 15 ++--
arch/powerpc/kvm/book3s_64_mmu_radix.c | 40 +++++-----
arch/powerpc/kvm/book3s_64_vio_hv.c | 64 ++++++++--------
arch/powerpc/kvm/book3s_hv_nested.c | 37 ++++++---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 58 +++++---------
arch/powerpc/mm/book3s64/hash_pgtable.c | 11 ---
arch/powerpc/mm/book3s64/hash_tlb.c | 16 +---
arch/powerpc/mm/book3s64/hash_utils.c | 62 ++++-----------
arch/powerpc/mm/book3s64/pgtable.c | 24 ++++--
arch/powerpc/mm/book3s64/radix_pgtable.c | 19 ++---
arch/powerpc/mm/fault.c | 75 +++++++++++++------
arch/powerpc/perf/callchain_64.c | 46 ++++--------
arch/s390/include/asm/pgtable.h | 4 +-
include/asm-generic/pgtable.h | 4 +-
mm/huge_memory.c | 4 +-
22 files changed, 275 insertions(+), 288 deletions(-)
--
2.25.3
^ permalink raw reply
* [PATCH 23/60] perf metrictroup: Split the metricgroup__add_metric function
From: Arnaldo Carvalho de Melo @ 2020-04-20 11:52 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Mark Rutland, Madhavan Srinivasan, Peter Zijlstra, Jin Yao,
Jiri Olsa, Kan Liang, Andi Kleen, Clark Williams, Anju T Sudhakar,
Mamatha Inamdar, Sukadev Bhattiprolu, Ravi Bangoria, Kajol Jain,
Arnaldo Carvalho de Melo, Joe Mario, Namhyung Kim, Michael Petlan,
Greg Kroah-Hartman, linux-kernel, Alexander Shishkin,
linux-perf-users, Jiri Olsa, linuxppc-dev
In-Reply-To: <20200420115316.18781-1-acme@kernel.org>
From: Kajol Jain <kjain@linux.ibm.com>
This patch refactors metricgroup__add_metric function where some part of
it move to function metricgroup__add_metric_param. No logic change.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Mamatha Inamdar <mamatha4@linux.vnet.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lore.kernel.org/lkml/20200401203340.31402-4-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/metricgroup.c | 60 ++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 926449a7cdbf..7ad81c8177ea 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -485,6 +485,39 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+static int __metricgroup__add_metric(struct strbuf *events,
+ struct list_head *group_list, struct pmu_event *pe)
+{
+
+ const char **ids;
+ int idnum;
+ struct egroup *eg;
+
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+ return -EINVAL;
+
+ if (events->len > 0)
+ strbuf_addf(events, ",");
+
+ if (metricgroup__has_constraint(pe))
+ metricgroup__add_metric_non_group(events, ids, idnum);
+ else
+ metricgroup__add_metric_weak_group(events, ids, idnum);
+
+ eg = malloc(sizeof(*eg));
+ if (!eg)
+ return -ENOMEM;
+
+ eg->ids = ids;
+ eg->idnum = idnum;
+ eg->metric_name = pe->metric_name;
+ eg->metric_expr = pe->metric_expr;
+ eg->metric_unit = pe->unit;
+ list_add_tail(&eg->nd, group_list);
+
+ return 0;
+}
+
static int metricgroup__add_metric(const char *metric, struct strbuf *events,
struct list_head *group_list)
{
@@ -504,35 +537,12 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
continue;
if (match_metric(pe->metric_group, metric) ||
match_metric(pe->metric_name, metric)) {
- const char **ids;
- int idnum;
- struct egroup *eg;
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- if (expr__find_other(pe->metric_expr,
- NULL, &ids, &idnum) < 0)
- continue;
- if (events->len > 0)
- strbuf_addf(events, ",");
-
- if (metricgroup__has_constraint(pe))
- metricgroup__add_metric_non_group(events, ids, idnum);
- else
- metricgroup__add_metric_weak_group(events, ids, idnum);
-
- eg = malloc(sizeof(struct egroup));
- if (!eg) {
- ret = -ENOMEM;
+ ret = __metricgroup__add_metric(events, group_list, pe);
+ if (ret == -ENOMEM)
break;
- }
- eg->ids = ids;
- eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
- eg->metric_expr = pe->metric_expr;
- eg->metric_unit = pe->unit;
- list_add_tail(&eg->nd, group_list);
- ret = 0;
}
}
return ret;
--
2.21.1
^ permalink raw reply related
* [PATCH 22/60] perf expr: Add expr_scanner_ctx object
From: Arnaldo Carvalho de Melo @ 2020-04-20 11:52 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Mark Rutland, Madhavan Srinivasan, Peter Zijlstra, Jin Yao,
Kan Liang, Andi Kleen, Clark Williams, Anju T Sudhakar,
Mamatha Inamdar, Sukadev Bhattiprolu, Ravi Bangoria, Kajol Jain,
Arnaldo Carvalho de Melo, Joe Mario, Namhyung Kim, Michael Petlan,
Greg Kroah-Hartman, linux-kernel, Alexander Shishkin,
linux-perf-users, Jiri Olsa, linuxppc-dev
In-Reply-To: <20200420115316.18781-1-acme@kernel.org>
From: Jiri Olsa <jolsa@kernel.org>
Add the expr_scanner_ctx object to hold user data for the expr scanner.
Currently it holds only start_token, Kajol Jain will use it to hold 24x7
runtime param.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Mamatha Inamdar <mamatha4@linux.vnet.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lore.kernel.org/lkml/20200401203340.31402-3-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/expr.c | 6 ++++--
tools/perf/util/expr.h | 4 ++++
tools/perf/util/expr.l | 10 +++++-----
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c8ccc548a585..c3382d58cf40 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -3,7 +3,6 @@
#include <assert.h>
#include "expr.h"
#include "expr-bison.h"
-#define YY_EXTRA_TYPE int
#include "expr-flex.h"
#ifdef PARSER_DEBUG
@@ -30,11 +29,14 @@ static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
+ struct expr_scanner_ctx scanner_ctx = {
+ .start_token = start,
+ };
YY_BUFFER_STATE buffer;
void *scanner;
int ret;
- ret = expr_lex_init_extra(start, &scanner);
+ ret = expr_lex_init_extra(&scanner_ctx, &scanner);
if (ret)
return ret;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index b9e53f2b5844..0938ad166ece 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -15,6 +15,10 @@ struct expr_parse_ctx {
struct expr_parse_id ids[MAX_PARSE_ID];
};
+struct expr_scanner_ctx {
+ int start_token;
+};
+
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index eaad29243c23..2582c2464938 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -76,13 +76,13 @@ sym [0-9a-zA-Z_\.:@]+
symbol {spec}*{sym}*{spec}*{sym}*
%%
- {
- int start_token;
+ struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
- start_token = expr_get_extra(yyscanner);
+ {
+ int start_token = sctx->start_token;
- if (start_token) {
- expr_set_extra(NULL, yyscanner);
+ if (sctx->start_token) {
+ sctx->start_token = 0;
return start_token;
}
}
--
2.21.1
^ permalink raw reply related
* [PATCH 21/60] perf expr: Add expr_ prefix for parse_ctx and parse_id
From: Arnaldo Carvalho de Melo @ 2020-04-20 11:52 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner
Cc: Mark Rutland, Madhavan Srinivasan, Peter Zijlstra, Jin Yao,
Kan Liang, Andi Kleen, Clark Williams, Anju T Sudhakar,
Mamatha Inamdar, Sukadev Bhattiprolu, Ravi Bangoria, Kajol Jain,
Arnaldo Carvalho de Melo, Joe Mario, Namhyung Kim, Michael Petlan,
Greg Kroah-Hartman, linux-kernel, Alexander Shishkin,
linux-perf-users, Jiri Olsa, linuxppc-dev
In-Reply-To: <20200420115316.18781-1-acme@kernel.org>
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_ prefix for parse_ctx and parse_id, to straighten out the
expr* namespace.
There's no functional change.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Mamatha Inamdar <mamatha4@linux.vnet.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lore.kernel.org/lkml/20200401203340.31402-2-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/tests/expr.c | 4 ++--
tools/perf/util/expr.c | 10 +++++-----
tools/perf/util/expr.h | 12 ++++++------
tools/perf/util/expr.y | 6 +++---
tools/perf/util/stat-shadow.c | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 28313e59d6f6..ea10fc4412c4 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -6,7 +6,7 @@
#include <string.h>
#include <linux/zalloc.h>
-static int test(struct parse_ctx *ctx, const char *e, double val2)
+static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
@@ -22,7 +22,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
const char **other;
double val;
int i, ret;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
int num_other;
expr__ctx_init(&ctx);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index fd192ddf93c1..c8ccc548a585 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -11,7 +11,7 @@ extern int expr_debug;
#endif
/* Caller must make sure id is allocated */
-void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
+void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
{
int idx;
@@ -21,13 +21,13 @@ void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
ctx->ids[idx].val = val;
}
-void expr__ctx_init(struct parse_ctx *ctx)
+void expr__ctx_init(struct expr_parse_ctx *ctx)
{
ctx->num_ids = 0;
}
static int
-__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
+__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
YY_BUFFER_STATE buffer;
@@ -52,7 +52,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
{
return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
}
@@ -75,7 +75,7 @@ int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other)
{
int err, i = 0, j = 0;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9377538f4097..b9e53f2b5844 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -5,19 +5,19 @@
#define EXPR_MAX_OTHER 20
#define MAX_PARSE_ID EXPR_MAX_OTHER
-struct parse_id {
+struct expr_parse_id {
const char *name;
double val;
};
-struct parse_ctx {
+struct expr_parse_ctx {
int num_ids;
- struct parse_id ids[MAX_PARSE_ID];
+ struct expr_parse_id ids[MAX_PARSE_ID];
};
-void expr__ctx_init(struct parse_ctx *ctx);
-void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
+void expr__ctx_init(struct expr_parse_ctx *ctx);
+void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other);
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 4720cbe79357..cd17486c1c5d 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -15,7 +15,7 @@
%define api.pure full
%parse-param { double *final_val }
-%parse-param { struct parse_ctx *ctx }
+%parse-param { struct expr_parse_ctx *ctx }
%parse-param {void *scanner}
%lex-param {void* scanner}
@@ -39,14 +39,14 @@
%{
static void expr_error(double *final_val __maybe_unused,
- struct parse_ctx *ctx __maybe_unused,
+ struct expr_parse_ctx *ctx __maybe_unused,
void *scanner,
const char *s)
{
pr_debug("%s\n", s);
}
-static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
+static int lookup_id(struct expr_parse_ctx *ctx, char *id, double *val)
{
int i;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 03ecb8cd0eec..1ad5c5be7e97 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -729,7 +729,7 @@ static void generic_metric(struct perf_stat_config *config,
struct runtime_stat *st)
{
print_metric_t print_metric = out->print_metric;
- struct parse_ctx pctx;
+ struct expr_parse_ctx pctx;
double ratio, scale;
int i;
void *ctxp = out->ctx;
--
2.21.1
^ permalink raw reply related
* Re: [PATCH 7/7] KVM: MIPS: clean up redundant kvm_run parameters in assembly
From: maobibo @ 2020-04-20 10:32 UTC (permalink / raw)
To: Tianjia Zhang, pbonzini, tsbogend, paulus, mpe, benh, borntraeger,
frankja, david, cohuck, heiko.carstens, gor,
sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx,
mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev,
suzuki.poulose, christoffer.dall, peterx, thuth
Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev,
kvmarm, linux-arm-kernel
In-Reply-To: <20200419075106.16248-8-tianjia.zhang@linux.alibaba.com>
On 04/19/2020 03:51 PM, Tianjia Zhang wrote:
> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
> structure. Earlier than historical reasons, many kvm-related function
> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
> This patch does a unified cleanup of these remaining redundant parameters.
>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
> arch/mips/include/asm/kvm_host.h | 4 ++--
> arch/mips/kvm/entry.c | 15 +++++----------
> arch/mips/kvm/mips.c | 3 ++-
> arch/mips/kvm/trap_emul.c | 2 +-
> arch/mips/kvm/vz.c | 2 +-
> 5 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index 971439297cea..db915c55166d 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -310,7 +310,7 @@ struct kvm_mmu_memory_cache {
> #define KVM_MIPS_GUEST_TLB_SIZE 64
> struct kvm_vcpu_arch {
> void *guest_ebase;
> - int (*vcpu_run)(struct kvm_run *run, struct kvm_vcpu *vcpu);
> + int (*vcpu_run)(struct kvm_vcpu *vcpu);
>
> /* Host registers preserved across guest mode execution */
> unsigned long host_stack;
> @@ -821,7 +821,7 @@ int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks);
> /* Debug: dump vcpu state */
> int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu);
>
> -extern int kvm_mips_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu);
> +extern int kvm_mips_handle_exit(struct kvm_vcpu *vcpu);
>
> /* Building of entry/exception code */
> int kvm_mips_entry_setup(void);
> diff --git a/arch/mips/kvm/entry.c b/arch/mips/kvm/entry.c
> index 16e1c93b484f..e3f29af3b6cd 100644
> --- a/arch/mips/kvm/entry.c
> +++ b/arch/mips/kvm/entry.c
> @@ -204,7 +204,7 @@ static inline void build_set_exc_base(u32 **p, unsigned int reg)
> * Assemble the start of the vcpu_run function to run a guest VCPU. The function
> * conforms to the following prototype:
> *
> - * int vcpu_run(struct kvm_run *run, struct kvm_vcpu *vcpu);
> + * int vcpu_run(struct kvm_vcpu *vcpu);
> *
> * The exit from the guest and return to the caller is handled by the code
> * generated by kvm_mips_build_ret_to_host().
> @@ -217,8 +217,7 @@ void *kvm_mips_build_vcpu_run(void *addr)
> unsigned int i;
>
> /*
> - * A0: run
> - * A1: vcpu
> + * A0: vcpu
> */
>
> /* k0/k1 not being used in host kernel context */
> @@ -237,10 +236,10 @@ void *kvm_mips_build_vcpu_run(void *addr)
> kvm_mips_build_save_scratch(&p, V1, K1);
>
> /* VCPU scratch register has pointer to vcpu */
> - UASM_i_MTC0(&p, A1, scratch_vcpu[0], scratch_vcpu[1]);
> + UASM_i_MTC0(&p, A0, scratch_vcpu[0], scratch_vcpu[1]);
>
> /* Offset into vcpu->arch */
> - UASM_i_ADDIU(&p, K1, A1, offsetof(struct kvm_vcpu, arch));
> + UASM_i_ADDIU(&p, K1, A0, offsetof(struct kvm_vcpu, arch));
>
> /*
> * Save the host stack to VCPU, used for exception processing
> @@ -628,10 +627,7 @@ void *kvm_mips_build_exit(void *addr)
> /* Now that context has been saved, we can use other registers */
>
> /* Restore vcpu */
> - UASM_i_MFC0(&p, S1, scratch_vcpu[0], scratch_vcpu[1]);
> -
> - /* Restore run (vcpu->run) */
> - UASM_i_LW(&p, S0, offsetof(struct kvm_vcpu, run), S1);
> + UASM_i_MFC0(&p, S0, scratch_vcpu[0], scratch_vcpu[1]);
>
> /*
> * Save Host level EPC, BadVaddr and Cause to VCPU, useful to process
> @@ -793,7 +789,6 @@ void *kvm_mips_build_exit(void *addr)
> * with this in the kernel
> */
> uasm_i_move(&p, A0, S0);
> - uasm_i_move(&p, A1, S1);
> UASM_i_LA(&p, T9, (unsigned long)kvm_mips_handle_exit);
> uasm_i_jalr(&p, RA, T9);
> UASM_i_ADDIU(&p, SP, SP, -CALLFRAME_SIZ);
I suggest keeping asm code untouched, the change for c code is much easier to understand, however I do not see obvious advantage to remove one redundant function parameter :)
regards
bibo,mao
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 9710477a9827..32850470c037 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -1186,8 +1186,9 @@ static void kvm_mips_set_c0_status(void)
> /*
> * Return value is in the form (errcode<<2 | RESUME_FLAG_HOST | RESUME_FLAG_NV)
> */
> -int kvm_mips_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
> +int kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
> {
> + struct kvm_run *run = vcpu->run;
> u32 cause = vcpu->arch.host_cp0_cause;
> u32 exccode = (cause >> CAUSEB_EXCCODE) & 0x1f;
> u32 __user *opc = (u32 __user *) vcpu->arch.pc;
> diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
> index d822f3aee3dc..04c864cc356a 100644
> --- a/arch/mips/kvm/trap_emul.c
> +++ b/arch/mips/kvm/trap_emul.c
> @@ -1238,7 +1238,7 @@ static int kvm_trap_emul_vcpu_run(struct kvm_vcpu *vcpu)
> */
> kvm_mips_suspend_mm(cpu);
>
> - r = vcpu->arch.vcpu_run(vcpu->run, vcpu);
> + r = vcpu->arch.vcpu_run(vcpu);
>
> /* We may have migrated while handling guest exits */
> cpu = smp_processor_id();
> diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
> index 94f1d23828e3..c5878fa0636d 100644
> --- a/arch/mips/kvm/vz.c
> +++ b/arch/mips/kvm/vz.c
> @@ -3152,7 +3152,7 @@ static int kvm_vz_vcpu_run(struct kvm_vcpu *vcpu)
> kvm_vz_vcpu_load_tlb(vcpu, cpu);
> kvm_vz_vcpu_load_wired(vcpu);
>
> - r = vcpu->arch.vcpu_run(vcpu->run, vcpu);
> + r = vcpu->arch.vcpu_run(vcpu);
>
> kvm_vz_vcpu_save_wired(vcpu);
>
>
^ permalink raw reply
* Re: [PATCH 7/7] KVM: MIPS: clean up redundant kvm_run parameters in assembly
From: Tianjia Zhang @ 2020-04-20 10:42 UTC (permalink / raw)
To: maobibo, pbonzini, tsbogend, paulus, mpe, benh, borntraeger,
frankja, david, cohuck, heiko.carstens, gor,
sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx,
mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev,
suzuki.poulose, christoffer.dall, peterx, thuth
Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev,
kvmarm, linux-arm-kernel
In-Reply-To: <0b110e7f-9d08-496e-158e-8c3ff7307423@loongson.cn>
On 2020/4/20 18:32, maobibo wrote:
>
>
> On 04/19/2020 03:51 PM, Tianjia Zhang wrote:
>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>> structure. Earlier than historical reasons, many kvm-related function
>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>> This patch does a unified cleanup of these remaining redundant parameters.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>> ---
>> arch/mips/include/asm/kvm_host.h | 4 ++--
>> arch/mips/kvm/entry.c | 15 +++++----------
>> arch/mips/kvm/mips.c | 3 ++-
>> arch/mips/kvm/trap_emul.c | 2 +-
>> arch/mips/kvm/vz.c | 2 +-
>> 5 files changed, 11 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
>> index 971439297cea..db915c55166d 100644
>> --- a/arch/mips/include/asm/kvm_host.h
>> +++ b/arch/mips/include/asm/kvm_host.h
>> @@ -310,7 +310,7 @@ struct kvm_mmu_memory_cache {
>> #define KVM_MIPS_GUEST_TLB_SIZE 64
>> struct kvm_vcpu_arch {
>> void *guest_ebase;
>> - int (*vcpu_run)(struct kvm_run *run, struct kvm_vcpu *vcpu);
>> + int (*vcpu_run)(struct kvm_vcpu *vcpu);
>>
>> /* Host registers preserved across guest mode execution */
>> unsigned long host_stack;
>> @@ -821,7 +821,7 @@ int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks);
>> /* Debug: dump vcpu state */
>> int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu);
>>
>> -extern int kvm_mips_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu);
>> +extern int kvm_mips_handle_exit(struct kvm_vcpu *vcpu);
>>
>> /* Building of entry/exception code */
>> int kvm_mips_entry_setup(void);
>> diff --git a/arch/mips/kvm/entry.c b/arch/mips/kvm/entry.c
>> index 16e1c93b484f..e3f29af3b6cd 100644
>> --- a/arch/mips/kvm/entry.c
>> +++ b/arch/mips/kvm/entry.c
>> @@ -204,7 +204,7 @@ static inline void build_set_exc_base(u32 **p, unsigned int reg)
>> * Assemble the start of the vcpu_run function to run a guest VCPU. The function
>> * conforms to the following prototype:
>> *
>> - * int vcpu_run(struct kvm_run *run, struct kvm_vcpu *vcpu);
>> + * int vcpu_run(struct kvm_vcpu *vcpu);
>> *
>> * The exit from the guest and return to the caller is handled by the code
>> * generated by kvm_mips_build_ret_to_host().
>> @@ -217,8 +217,7 @@ void *kvm_mips_build_vcpu_run(void *addr)
>> unsigned int i;
>>
>> /*
>> - * A0: run
>> - * A1: vcpu
>> + * A0: vcpu
>> */
>>
>> /* k0/k1 not being used in host kernel context */
>> @@ -237,10 +236,10 @@ void *kvm_mips_build_vcpu_run(void *addr)
>> kvm_mips_build_save_scratch(&p, V1, K1);
>>
>> /* VCPU scratch register has pointer to vcpu */
>> - UASM_i_MTC0(&p, A1, scratch_vcpu[0], scratch_vcpu[1]);
>> + UASM_i_MTC0(&p, A0, scratch_vcpu[0], scratch_vcpu[1]);
>>
>> /* Offset into vcpu->arch */
>> - UASM_i_ADDIU(&p, K1, A1, offsetof(struct kvm_vcpu, arch));
>> + UASM_i_ADDIU(&p, K1, A0, offsetof(struct kvm_vcpu, arch));
>>
>> /*
>> * Save the host stack to VCPU, used for exception processing
>> @@ -628,10 +627,7 @@ void *kvm_mips_build_exit(void *addr)
>> /* Now that context has been saved, we can use other registers */
>>
>> /* Restore vcpu */
>> - UASM_i_MFC0(&p, S1, scratch_vcpu[0], scratch_vcpu[1]);
>> -
>> - /* Restore run (vcpu->run) */
>> - UASM_i_LW(&p, S0, offsetof(struct kvm_vcpu, run), S1);
>> + UASM_i_MFC0(&p, S0, scratch_vcpu[0], scratch_vcpu[1]);
>>
>> /*
>> * Save Host level EPC, BadVaddr and Cause to VCPU, useful to process
>> @@ -793,7 +789,6 @@ void *kvm_mips_build_exit(void *addr)
>> * with this in the kernel
>> */
>> uasm_i_move(&p, A0, S0);
>> - uasm_i_move(&p, A1, S1);
>> UASM_i_LA(&p, T9, (unsigned long)kvm_mips_handle_exit);
>> uasm_i_jalr(&p, RA, T9);
>> UASM_i_ADDIU(&p, SP, SP, -CALLFRAME_SIZ);
>
> I suggest keeping asm code untouched, the change for c code is much easier to understand, however I do not see obvious advantage to remove one redundant function parameter :)
>
>
> regards
> bibo,mao
>
This is acceptable, asm code is an independent patch, is to provide
convenience for this operation.
>
>> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
>> index 9710477a9827..32850470c037 100644
>> --- a/arch/mips/kvm/mips.c
>> +++ b/arch/mips/kvm/mips.c
>> @@ -1186,8 +1186,9 @@ static void kvm_mips_set_c0_status(void)
>> /*
>> * Return value is in the form (errcode<<2 | RESUME_FLAG_HOST | RESUME_FLAG_NV)
>> */
>> -int kvm_mips_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
>> +int kvm_mips_handle_exit(struct kvm_vcpu *vcpu)
>> {
>> + struct kvm_run *run = vcpu->run;
>> u32 cause = vcpu->arch.host_cp0_cause;
>> u32 exccode = (cause >> CAUSEB_EXCCODE) & 0x1f;
>> u32 __user *opc = (u32 __user *) vcpu->arch.pc;
>> diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
>> index d822f3aee3dc..04c864cc356a 100644
>> --- a/arch/mips/kvm/trap_emul.c
>> +++ b/arch/mips/kvm/trap_emul.c
>> @@ -1238,7 +1238,7 @@ static int kvm_trap_emul_vcpu_run(struct kvm_vcpu *vcpu)
>> */
>> kvm_mips_suspend_mm(cpu);
>>
>> - r = vcpu->arch.vcpu_run(vcpu->run, vcpu);
>> + r = vcpu->arch.vcpu_run(vcpu);
>>
>> /* We may have migrated while handling guest exits */
>> cpu = smp_processor_id();
>> diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
>> index 94f1d23828e3..c5878fa0636d 100644
>> --- a/arch/mips/kvm/vz.c
>> +++ b/arch/mips/kvm/vz.c
>> @@ -3152,7 +3152,7 @@ static int kvm_vz_vcpu_run(struct kvm_vcpu *vcpu)
>> kvm_vz_vcpu_load_tlb(vcpu, cpu);
>> kvm_vz_vcpu_load_wired(vcpu);
>>
>> - r = vcpu->arch.vcpu_run(vcpu->run, vcpu);
>> + r = vcpu->arch.vcpu_run(vcpu);
>>
>> kvm_vz_vcpu_save_wired(vcpu);
>>
>>
^ permalink raw reply
* Re: [PATCH 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Janosch Frank @ 2020-04-20 8:25 UTC (permalink / raw)
To: Christian Borntraeger, Tianjia Zhang, pbonzini, tsbogend, paulus,
mpe, benh, david, cohuck, heiko.carstens, gor,
sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx,
mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev,
suzuki.poulose, christoffer.dall, peterx, thuth
Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev,
kvmarm, linux-arm-kernel
In-Reply-To: <7a783487-2f9b-08a6-0ff6-f57bb90495a1@de.ibm.com>
[-- Attachment #1.1: Type: text/plain, Size: 11587 bytes --]
On 4/20/20 9:07 AM, Christian Borntraeger wrote:
>
>
> On 19.04.20 09:51, Tianjia Zhang wrote:
>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>> structure. Earlier than historical reasons, many kvm-related function
>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>> This patch does a unified cleanup of these remaining redundant parameters.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>> ---
>> arch/s390/kvm/kvm-s390.c | 127 +++++++++++++++++++++------------------
>> 1 file changed, 67 insertions(+), 60 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 443af3ead739..cf420d013ba3 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -4173,24 +4173,25 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>> return rc;
>> }
>>
>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
>> {
>> + struct kvm_run *run = vcpu->run;
>
> Please use kvm_run as variable name. This makes all of the changes below go away.
Definitely
@Tianjia
This patch produces way to much churn to be picked up as is.
I do not want patches that touch so much for small cosmetic changes.
>
>
>> struct runtime_instr_cb *riccb;
>> struct gs_cb *gscb;
>>
>> - riccb = (struct runtime_instr_cb *) &kvm_run->s.regs.riccb;
>> - gscb = (struct gs_cb *) &kvm_run->s.regs.gscb;
>> - vcpu->arch.sie_block->gpsw.mask = kvm_run->psw_mask;
>> - vcpu->arch.sie_block->gpsw.addr = kvm_run->psw_addr;
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> - vcpu->arch.sie_block->todpr = kvm_run->s.regs.todpr;
>> - vcpu->arch.sie_block->pp = kvm_run->s.regs.pp;
>> - vcpu->arch.sie_block->gbea = kvm_run->s.regs.gbea;
>> - }
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_PFAULT) {
>> - vcpu->arch.pfault_token = kvm_run->s.regs.pft;
>> - vcpu->arch.pfault_select = kvm_run->s.regs.pfs;
>> - vcpu->arch.pfault_compare = kvm_run->s.regs.pfc;
>> + riccb = (struct runtime_instr_cb *) &run->s.regs.riccb;
>> + gscb = (struct gs_cb *) &run->s.regs.gscb;
>> + vcpu->arch.sie_block->gpsw.mask = run->psw_mask;
>> + vcpu->arch.sie_block->gpsw.addr = run->psw_addr;
>> + if (run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> + vcpu->arch.sie_block->todpr = run->s.regs.todpr;
>> + vcpu->arch.sie_block->pp = run->s.regs.pp;
>> + vcpu->arch.sie_block->gbea = run->s.regs.gbea;
>> + }
>> + if (run->kvm_dirty_regs & KVM_SYNC_PFAULT) {
>> + vcpu->arch.pfault_token = run->s.regs.pft;
>> + vcpu->arch.pfault_select = run->s.regs.pfs;
>> + vcpu->arch.pfault_compare = run->s.regs.pfc;
>> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
>> kvm_clear_async_pf_completion_queue(vcpu);
>> }
>> @@ -4198,7 +4199,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> * If userspace sets the riccb (e.g. after migration) to a valid state,
>> * we should enable RI here instead of doing the lazy enablement.
>> */
>> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_RICCB) &&
>> + if ((run->kvm_dirty_regs & KVM_SYNC_RICCB) &&
>> test_kvm_facility(vcpu->kvm, 64) &&
>> riccb->v &&
>> !(vcpu->arch.sie_block->ecb3 & ECB3_RI)) {
>> @@ -4209,7 +4210,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> * If userspace sets the gscb (e.g. after migration) to non-zero,
>> * we should enable GS here instead of doing the lazy enablement.
>> */
>> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_GSCB) &&
>> + if ((run->kvm_dirty_regs & KVM_SYNC_GSCB) &&
>> test_kvm_facility(vcpu->kvm, 133) &&
>> gscb->gssm &&
>> !vcpu->arch.gs_enabled) {
>> @@ -4218,10 +4219,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
>> vcpu->arch.gs_enabled = 1;
>> }
>> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_BPBC) &&
>> + if ((run->kvm_dirty_regs & KVM_SYNC_BPBC) &&
>> test_kvm_facility(vcpu->kvm, 82)) {
>> vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
>> - vcpu->arch.sie_block->fpf |= kvm_run->s.regs.bpbc ? FPF_BPBC : 0;
>> + vcpu->arch.sie_block->fpf |= run->s.regs.bpbc ? FPF_BPBC : 0;
>> }
>> if (MACHINE_HAS_GS) {
>> preempt_disable();
>> @@ -4232,45 +4233,47 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> }
>> if (vcpu->arch.gs_enabled) {
>> current->thread.gs_cb = (struct gs_cb *)
>> - &vcpu->run->s.regs.gscb;
>> + &run->s.regs.gscb;
>> restore_gs_cb(current->thread.gs_cb);
>> }
>> preempt_enable();
>> }
>> - /* SIE will load etoken directly from SDNX and therefore kvm_run */
>> + /* SIE will load etoken directly from SDNX and therefore run */
>> }
>>
>> -static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void sync_regs(struct kvm_vcpu *vcpu)
>> {
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_PREFIX)
>> - kvm_s390_set_prefix(vcpu, kvm_run->s.regs.prefix);
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_CRS) {
>> - memcpy(&vcpu->arch.sie_block->gcr, &kvm_run->s.regs.crs, 128);
>> + struct kvm_run *run = vcpu->run;
>> +
>> + if (run->kvm_dirty_regs & KVM_SYNC_PREFIX)
>> + kvm_s390_set_prefix(vcpu, run->s.regs.prefix);
>> + if (run->kvm_dirty_regs & KVM_SYNC_CRS) {
>> + memcpy(&vcpu->arch.sie_block->gcr, &run->s.regs.crs, 128);
>> /* some control register changes require a tlb flush */
>> kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
>> }
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> - kvm_s390_set_cpu_timer(vcpu, kvm_run->s.regs.cputm);
>> - vcpu->arch.sie_block->ckc = kvm_run->s.regs.ckc;
>> + if (run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> + kvm_s390_set_cpu_timer(vcpu, run->s.regs.cputm);
>> + vcpu->arch.sie_block->ckc = run->s.regs.ckc;
>> }
>> save_access_regs(vcpu->arch.host_acrs);
>> - restore_access_regs(vcpu->run->s.regs.acrs);
>> + restore_access_regs(run->s.regs.acrs);
>> /* save host (userspace) fprs/vrs */
>> save_fpu_regs();
>> vcpu->arch.host_fpregs.fpc = current->thread.fpu.fpc;
>> vcpu->arch.host_fpregs.regs = current->thread.fpu.regs;
>> if (MACHINE_HAS_VX)
>> - current->thread.fpu.regs = vcpu->run->s.regs.vrs;
>> + current->thread.fpu.regs = run->s.regs.vrs;
>> else
>> - current->thread.fpu.regs = vcpu->run->s.regs.fprs;
>> - current->thread.fpu.fpc = vcpu->run->s.regs.fpc;
>> + current->thread.fpu.regs = run->s.regs.fprs;
>> + current->thread.fpu.fpc = run->s.regs.fpc;
>> if (test_fp_ctl(current->thread.fpu.fpc))
>> /* User space provided an invalid FPC, let's clear it */
>> current->thread.fpu.fpc = 0;
>>
>> /* Sync fmt2 only data */
>> if (likely(!kvm_s390_pv_cpu_is_protected(vcpu))) {
>> - sync_regs_fmt2(vcpu, kvm_run);
>> + sync_regs_fmt2(vcpu);
>> } else {
>> /*
>> * In several places we have to modify our internal view to
>> @@ -4282,19 +4285,21 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> * do only accept the condition code from userspace.
>> */
>> vcpu->arch.sie_block->gpsw.mask &= ~PSW_MASK_CC;
>> - vcpu->arch.sie_block->gpsw.mask |= kvm_run->psw_mask &
>> + vcpu->arch.sie_block->gpsw.mask |= run->psw_mask &
>> PSW_MASK_CC;
>> }
>>
>> - kvm_run->kvm_dirty_regs = 0;
>> + run->kvm_dirty_regs = 0;
>> }
>>
>> -static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void store_regs_fmt2(struct kvm_vcpu *vcpu)
>> {
>> - kvm_run->s.regs.todpr = vcpu->arch.sie_block->todpr;
>> - kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
>> - kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
>> - kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
>> + struct kvm_run *run = vcpu->run;
>> +
>> + run->s.regs.todpr = vcpu->arch.sie_block->todpr;
>> + run->s.regs.pp = vcpu->arch.sie_block->pp;
>> + run->s.regs.gbea = vcpu->arch.sie_block->gbea;
>> + run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
>> if (MACHINE_HAS_GS) {
>> __ctl_set_bit(2, 4);
>> if (vcpu->arch.gs_enabled)
>> @@ -4310,39 +4315,41 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> /* SIE will save etoken directly into SDNX and therefore kvm_run */
>> }
>>
>> -static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void store_regs(struct kvm_vcpu *vcpu)
>> {
>> - kvm_run->psw_mask = vcpu->arch.sie_block->gpsw.mask;
>> - kvm_run->psw_addr = vcpu->arch.sie_block->gpsw.addr;
>> - kvm_run->s.regs.prefix = kvm_s390_get_prefix(vcpu);
>> - memcpy(&kvm_run->s.regs.crs, &vcpu->arch.sie_block->gcr, 128);
>> - kvm_run->s.regs.cputm = kvm_s390_get_cpu_timer(vcpu);
>> - kvm_run->s.regs.ckc = vcpu->arch.sie_block->ckc;
>> - kvm_run->s.regs.pft = vcpu->arch.pfault_token;
>> - kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
>> - kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
>> - save_access_regs(vcpu->run->s.regs.acrs);
>> + struct kvm_run *run = vcpu->run;
>> +
>> + run->psw_mask = vcpu->arch.sie_block->gpsw.mask;
>> + run->psw_addr = vcpu->arch.sie_block->gpsw.addr;
>> + run->s.regs.prefix = kvm_s390_get_prefix(vcpu);
>> + memcpy(&run->s.regs.crs, &vcpu->arch.sie_block->gcr, 128);
>> + run->s.regs.cputm = kvm_s390_get_cpu_timer(vcpu);
>> + run->s.regs.ckc = vcpu->arch.sie_block->ckc;
>> + run->s.regs.pft = vcpu->arch.pfault_token;
>> + run->s.regs.pfs = vcpu->arch.pfault_select;
>> + run->s.regs.pfc = vcpu->arch.pfault_compare;
>> + save_access_regs(run->s.regs.acrs);
>> restore_access_regs(vcpu->arch.host_acrs);
>> /* Save guest register state */
>> save_fpu_regs();
>> - vcpu->run->s.regs.fpc = current->thread.fpu.fpc;
>> + run->s.regs.fpc = current->thread.fpu.fpc;
>> /* Restore will be done lazily at return */
>> current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc;
>> current->thread.fpu.regs = vcpu->arch.host_fpregs.regs;
>> if (likely(!kvm_s390_pv_cpu_is_protected(vcpu)))
>> - store_regs_fmt2(vcpu, kvm_run);
>> + store_regs_fmt2(vcpu);
>> }
>>
>> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> {
>> - struct kvm_run *kvm_run = vcpu->run;
>> + struct kvm_run *run = vcpu->run;
>> int rc;
>>
>> - if (kvm_run->immediate_exit)
>> + if (run->immediate_exit)
>> return -EINTR;
>>
>> - if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
>> - kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
>> + if (run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
>> + run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
>> return -EINVAL;
>>
>> vcpu_load(vcpu);
>> @@ -4368,14 +4375,14 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> goto out;
>> }
>>
>> - sync_regs(vcpu, kvm_run);
>> + sync_regs(vcpu);
>> enable_cpu_timer_accounting(vcpu);
>>
>> might_fault();
>> rc = __vcpu_run(vcpu);
>>
>> if (signal_pending(current) && !rc) {
>> - kvm_run->exit_reason = KVM_EXIT_INTR;
>> + run->exit_reason = KVM_EXIT_INTR;
>> rc = -EINTR;
>> }
>>
>> @@ -4390,7 +4397,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> }
>>
>> disable_cpu_timer_accounting(vcpu);
>> - store_regs(vcpu, kvm_run);
>> + store_regs(vcpu);
>>
>> kvm_sigset_deactivate(vcpu);
>>
>>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 24/29] mm: remove __vmalloc_node_flags_caller
From: Geert Uytterhoeven @ 2020-04-20 9:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-hyperv, David Airlie, DRI Development, Linux MM,
K. Y. Srinivasan, Sumit Semwal, Linux-Arch, linux-s390, Wei Liu,
Stephen Hemminger, the arch/x86 maintainers, Peter Zijlstra,
Laura Abbott, Nitin Gupta, Daniel Vetter, Haiyang Zhang,
linaro-mm-sig, bpf, Linux ARM, Robin Murphy,
Linux Kernel Mailing List, Minchan Kim, Linux IOMMU, Sakari Ailus,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200414131348.444715-25-hch@lst.de>
Hi Christoph,
On Tue, Apr 14, 2020 at 3:21 PM Christoph Hellwig <hch@lst.de> wrote:
> Just use __vmalloc_node instead which gets and extra argument. To be
> able to to use __vmalloc_node in all caller make it available outside
> of vmalloc and implement it in nommu.c.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
One more nommu failure below...
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -150,8 +150,8 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask)
> }
> EXPORT_SYMBOL(__vmalloc);
>
> -void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags,
> - void *caller)
> +void *__vmalloc_node(unsigned long size, unsigned long align, gfp_t gfp_mask,
> + int node, const void *caller)
> {
> return __vmalloc(size, flags);
On Mon, Apr 20, 2020 at 10:39 AM <noreply@ellerman.id.au> wrote:
> FAILED linux-next/m5272c3_defconfig/m68k-gcc8 Mon Apr 20, 18:38
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/14213623/
>
> mm/nommu.c:164:25: error: 'flags' undeclared (first use in this function); did you mean 'class'?
"return __vmalloc(size, gfp_mask);"
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox