* [RESEND PATCH 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
From: ira.weiny @ 2019-02-20 5:30 UTC (permalink / raw)
To: John Hubbard, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
Peter Zijlstra, Jason Gunthorpe, Benjamin Herrenschmidt,
Paul Mackerras, David S. Miller, Martin Schwidefsky,
Heiko Carstens, Rich Felker, Yoshinori Sato, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Ralf Baechle, Paul Burton,
James Hogan
Cc: linux-fbdev, kvm, linux-sh, linux-fpga, dri-devel, linux-mips,
linux-mm, sparclinux, Ira Weiny, devel, linux-s390, rds-devel,
linux-scsi, linux-rdma, xen-devel, devel, linux-media, kvm-ppc,
ceph-devel, virtualization, netdev, linux-kernel, linuxppc-dev
In-Reply-To: <20190220053040.10831-1-ira.weiny@intel.com>
From: Ira Weiny <ira.weiny@intel.com>
To facilitate additional options to get_user_pages_fast() change the
singular write parameter to be gup_flags.
This patch does not change any functionality. New functionality will
follow in subsequent patches.
Some of the get_user_pages_fast() call sites were unchanged because they
already passed FOLL_WRITE or 0 for the write parameter.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
arch/mips/mm/gup.c | 11 ++++++-----
arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
arch/powerpc/kvm/e500_mmu.c | 2 +-
arch/powerpc/mm/mmu_context_iommu.c | 4 ++--
arch/s390/kvm/interrupt.c | 2 +-
arch/s390/mm/gup.c | 12 ++++++------
arch/sh/mm/gup.c | 11 ++++++-----
arch/sparc/mm/gup.c | 9 +++++----
arch/x86/kvm/paging_tmpl.h | 2 +-
arch/x86/kvm/svm.c | 2 +-
drivers/fpga/dfl-afu-dma-region.c | 2 +-
drivers/gpu/drm/via/via_dmablit.c | 3 ++-
drivers/infiniband/hw/hfi1/user_pages.c | 3 ++-
drivers/misc/genwqe/card_utils.c | 2 +-
drivers/misc/vmw_vmci/vmci_host.c | 2 +-
drivers/misc/vmw_vmci/vmci_queue_pair.c | 6 ++++--
drivers/platform/goldfish/goldfish_pipe.c | 3 ++-
drivers/rapidio/devices/rio_mport_cdev.c | 4 +++-
drivers/sbus/char/oradax.c | 2 +-
drivers/scsi/st.c | 3 ++-
drivers/staging/gasket/gasket_page_table.c | 4 ++--
drivers/tee/tee_shm.c | 2 +-
drivers/vfio/vfio_iommu_spapr_tce.c | 3 ++-
drivers/vhost/vhost.c | 2 +-
drivers/video/fbdev/pvr2fb.c | 2 +-
drivers/virt/fsl_hypervisor.c | 2 +-
drivers/xen/gntdev.c | 2 +-
fs/orangefs/orangefs-bufmap.c | 2 +-
include/linux/mm.h | 4 ++--
kernel/futex.c | 2 +-
lib/iov_iter.c | 7 +++++--
mm/gup.c | 10 +++++-----
mm/util.c | 8 ++++----
net/ceph/pagevec.c | 2 +-
net/rds/info.c | 2 +-
net/rds/rdma.c | 3 ++-
36 files changed, 81 insertions(+), 65 deletions(-)
diff --git a/arch/mips/mm/gup.c b/arch/mips/mm/gup.c
index 0d14e0d8eacf..4c2b4483683c 100644
--- a/arch/mips/mm/gup.c
+++ b/arch/mips/mm/gup.c
@@ -235,7 +235,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* get_user_pages_fast() - pin user pages in memory
* @start: starting user address
* @nr_pages: number of pages from start to pin
- * @write: whether pages will be written to
+ * @gup_flags: flags modifying pin behaviour
* @pages: array that receives pointers to the pages pinned.
* Should be at least nr_pages long.
*
@@ -247,8 +247,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* requested. If nr_pages is 0 or negative, returns 0. If no pages
* were pinned, returns -errno.
*/
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
+int get_user_pages_fast(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
{
struct mm_struct *mm = current->mm;
unsigned long addr, len, end;
@@ -273,7 +273,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
next = pgd_addr_end(addr, end);
if (pgd_none(pgd))
goto slow;
- if (!gup_pud_range(pgd, addr, next, write, pages, &nr))
+ if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
+ pages, &nr))
goto slow;
} while (pgdp++, addr = next, addr != end);
local_irq_enable();
@@ -289,7 +290,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
pages += nr;
ret = get_user_pages_unlocked(start, (end - start) >> PAGE_SHIFT,
- pages, write ? FOLL_WRITE : 0);
+ pages, gup_flags);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index bd2dcfbf00cd..8fcb0a921e46 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -582,7 +582,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
/* If writing != 0, then the HPTE must allow writing, if we get here */
write_ok = writing;
hva = gfn_to_hva_memslot(memslot, gfn);
- npages = get_user_pages_fast(hva, 1, writing, pages);
+ npages = get_user_pages_fast(hva, 1, writing ? FOLL_WRITE : 0, pages);
if (npages < 1) {
/* Check if it's an I/O mapping */
down_read(¤t->mm->mmap_sem);
@@ -1175,7 +1175,7 @@ void *kvmppc_pin_guest_page(struct kvm *kvm, unsigned long gpa,
if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID))
goto err;
hva = gfn_to_hva_memslot(memslot, gfn);
- npages = get_user_pages_fast(hva, 1, 1, pages);
+ npages = get_user_pages_fast(hva, 1, FOLL_WRITE, pages);
if (npages < 1)
goto err;
page = pages[0];
diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 24296f4cadc6..e0af53fd78c5 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -783,7 +783,7 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
if (!pages)
return -ENOMEM;
- ret = get_user_pages_fast(cfg->array, num_pages, 1, pages);
+ ret = get_user_pages_fast(cfg->array, num_pages, FOLL_WRITE, pages);
if (ret < 0)
goto free_pages;
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index a712a650a8b6..acb0990c8364 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -190,7 +190,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
for (i = 0; i < entries; ++i) {
cur_ua = ua + (i << PAGE_SHIFT);
if (1 != get_user_pages_fast(cur_ua,
- 1/* pages */, 1/* iswrite */, &page)) {
+ 1/* pages */, FOLL_WRITE, &page)) {
ret = -EFAULT;
for (j = 0; j < i; ++j)
put_page(pfn_to_page(mem->hpas[j] >>
@@ -209,7 +209,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
if (mm_iommu_move_page_from_cma(page))
goto populate;
if (1 != get_user_pages_fast(cur_ua,
- 1/* pages */, 1/* iswrite */,
+ 1/* pages */, FOLL_WRITE,
&page)) {
ret = -EFAULT;
for (j = 0; j < i; ++j)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index fcb55b02990e..69d9366b966c 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2278,7 +2278,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
ret = -EFAULT;
goto out;
}
- ret = get_user_pages_fast(map->addr, 1, 1, &map->page);
+ ret = get_user_pages_fast(map->addr, 1, FOLL_WRITE, &map->page);
if (ret < 0)
goto out;
BUG_ON(ret != 1);
diff --git a/arch/s390/mm/gup.c b/arch/s390/mm/gup.c
index 2809d11c7a28..0a6faf3d9960 100644
--- a/arch/s390/mm/gup.c
+++ b/arch/s390/mm/gup.c
@@ -265,7 +265,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* get_user_pages_fast() - pin user pages in memory
* @start: starting user address
* @nr_pages: number of pages from start to pin
- * @write: whether pages will be written to
+ * @gup_flags: flags modifying pin behaviour
* @pages: array that receives pointers to the pages pinned.
* Should be at least nr_pages long.
*
@@ -277,22 +277,22 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* requested. If nr_pages is 0 or negative, returns 0. If no pages
* were pinned, returns -errno.
*/
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
+int get_user_pages_fast(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
{
int nr, ret;
might_sleep();
start &= PAGE_MASK;
- nr = __get_user_pages_fast(start, nr_pages, write, pages);
+ nr = __get_user_pages_fast(start, nr_pages, gup_flags & FOLL_WRITE,
+ pages);
if (nr == nr_pages)
return nr;
/* Try to get the remaining pages with get_user_pages */
start += nr << PAGE_SHIFT;
pages += nr;
- ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
- write ? FOLL_WRITE : 0);
+ ret = get_user_pages_unlocked(start, nr_pages - nr, pages, gup_flags);
/* Have to be a bit careful with return values */
if (nr > 0)
ret = (ret < 0) ? nr : ret + nr;
diff --git a/arch/sh/mm/gup.c b/arch/sh/mm/gup.c
index 3e27f6d1f1ec..277c882f7489 100644
--- a/arch/sh/mm/gup.c
+++ b/arch/sh/mm/gup.c
@@ -204,7 +204,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* get_user_pages_fast() - pin user pages in memory
* @start: starting user address
* @nr_pages: number of pages from start to pin
- * @write: whether pages will be written to
+ * @gup_flags: flags modifying pin behaviour
* @pages: array that receives pointers to the pages pinned.
* Should be at least nr_pages long.
*
@@ -216,8 +216,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* requested. If nr_pages is 0 or negative, returns 0. If no pages
* were pinned, returns -errno.
*/
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
+int get_user_pages_fast(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
{
struct mm_struct *mm = current->mm;
unsigned long addr, len, end;
@@ -241,7 +241,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
next = pgd_addr_end(addr, end);
if (pgd_none(pgd))
goto slow;
- if (!gup_pud_range(pgd, addr, next, write, pages, &nr))
+ if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
+ pages, &nr))
goto slow;
} while (pgdp++, addr = next, addr != end);
local_irq_enable();
@@ -261,7 +262,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
ret = get_user_pages_unlocked(start,
(end - start) >> PAGE_SHIFT, pages,
- write ? FOLL_WRITE : 0);
+ gup_flags);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c
index aee6dba83d0e..1e770a517d4a 100644
--- a/arch/sparc/mm/gup.c
+++ b/arch/sparc/mm/gup.c
@@ -245,8 +245,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
return nr;
}
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
+int get_user_pages_fast(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
{
struct mm_struct *mm = current->mm;
unsigned long addr, len, end;
@@ -303,7 +303,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
next = pgd_addr_end(addr, end);
if (pgd_none(pgd))
goto slow;
- if (!gup_pud_range(pgd, addr, next, write, pages, &nr))
+ if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
+ pages, &nr))
goto slow;
} while (pgdp++, addr = next, addr != end);
@@ -324,7 +325,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
ret = get_user_pages_unlocked(start,
(end - start) >> PAGE_SHIFT, pages,
- write ? FOLL_WRITE : 0);
+ gup_flags);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 6bdca39829bc..08715034e315 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -140,7 +140,7 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
pt_element_t *table;
struct page *page;
- npages = get_user_pages_fast((unsigned long)ptep_user, 1, 1, &page);
+ npages = get_user_pages_fast((unsigned long)ptep_user, 1, FOLL_WRITE, &page);
/* Check if the user is doing something meaningless. */
if (unlikely(npages != 1))
return -EFAULT;
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index f13a3a24d360..173596a020cb 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1803,7 +1803,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
return NULL;
/* Pin the user virtual address. */
- npinned = get_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages);
+ npinned = get_user_pages_fast(uaddr, npages, FOLL_WRITE, pages);
if (npinned != npages) {
pr_err("SEV: Failure locking %lu pages.\n", npages);
goto err;
diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
index e18a786fc943..c438722bf4e1 100644
--- a/drivers/fpga/dfl-afu-dma-region.c
+++ b/drivers/fpga/dfl-afu-dma-region.c
@@ -102,7 +102,7 @@ static int afu_dma_pin_pages(struct dfl_feature_platform_data *pdata,
goto unlock_vm;
}
- pinned = get_user_pages_fast(region->user_addr, npages, 1,
+ pinned = get_user_pages_fast(region->user_addr, npages, FOLL_WRITE,
region->pages);
if (pinned < 0) {
ret = pinned;
diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
index 345bda4494e1..0c8b09602910 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -239,7 +239,8 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg, drm_via_dmablit_t *xfer)
if (NULL == vsg->pages)
return -ENOMEM;
ret = get_user_pages_fast((unsigned long)xfer->mem_addr,
- vsg->num_pages, vsg->direction == DMA_FROM_DEVICE,
+ vsg->num_pages,
+ vsg->direction == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
vsg->pages);
if (ret != vsg->num_pages) {
if (ret < 0)
diff --git a/drivers/infiniband/hw/hfi1/user_pages.c b/drivers/infiniband/hw/hfi1/user_pages.c
index 24b592c6522e..78ccacaf97d0 100644
--- a/drivers/infiniband/hw/hfi1/user_pages.c
+++ b/drivers/infiniband/hw/hfi1/user_pages.c
@@ -105,7 +105,8 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np
{
int ret;
- ret = get_user_pages_fast(vaddr, npages, writable, pages);
+ ret = get_user_pages_fast(vaddr, npages, writable ? FOLL_WRITE : 0,
+ pages);
if (ret < 0)
return ret;
diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 25265fd0fd6e..89cff9d1012b 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -603,7 +603,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
/* pin user pages in memory */
rc = get_user_pages_fast(data & PAGE_MASK, /* page aligned addr */
m->nr_pages,
- m->write, /* readable/writable */
+ m->write ? FOLL_WRITE : 0, /* readable/writable */
m->page_list); /* ptrs to pages */
if (rc < 0)
goto fail_get_user_pages;
diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
index 997f92543dd4..422d08da3244 100644
--- a/drivers/misc/vmw_vmci/vmci_host.c
+++ b/drivers/misc/vmw_vmci/vmci_host.c
@@ -242,7 +242,7 @@ static int vmci_host_setup_notify(struct vmci_ctx *context,
/*
* Lock physical page backing a given user VA.
*/
- retval = get_user_pages_fast(uva, 1, 1, &context->notify_page);
+ retval = get_user_pages_fast(uva, 1, FOLL_WRITE, &context->notify_page);
if (retval != 1) {
context->notify_page = NULL;
return VMCI_ERROR_GENERIC;
diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index 264f4ed8eef2..c5396ee32e51 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -666,7 +666,8 @@ static int qp_host_get_user_memory(u64 produce_uva,
int err = VMCI_SUCCESS;
retval = get_user_pages_fast((uintptr_t) produce_uva,
- produce_q->kernel_if->num_pages, 1,
+ produce_q->kernel_if->num_pages,
+ FOLL_WRITE,
produce_q->kernel_if->u.h.header_page);
if (retval < (int)produce_q->kernel_if->num_pages) {
pr_debug("get_user_pages_fast(produce) failed (retval=%d)",
@@ -678,7 +679,8 @@ static int qp_host_get_user_memory(u64 produce_uva,
}
retval = get_user_pages_fast((uintptr_t) consume_uva,
- consume_q->kernel_if->num_pages, 1,
+ consume_q->kernel_if->num_pages,
+ FOLL_WRITE,
consume_q->kernel_if->u.h.header_page);
if (retval < (int)consume_q->kernel_if->num_pages) {
pr_debug("get_user_pages_fast(consume) failed (retval=%d)",
diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 321bc673c417..cef0133aa47a 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -274,7 +274,8 @@ static int pin_user_pages(unsigned long first_page,
*iter_last_page_size = last_page_size;
}
- ret = get_user_pages_fast(first_page, requested_pages, !is_write,
+ ret = get_user_pages_fast(first_page, requested_pages,
+ !is_write ? FOLL_WRITE : 0,
pages);
if (ret <= 0)
return -EFAULT;
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
index cbe467ff1aba..f681b3e9e970 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -868,7 +868,9 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
pinned = get_user_pages_fast(
(unsigned long)xfer->loc_addr & PAGE_MASK,
- nr_pages, dir == DMA_FROM_DEVICE, page_list);
+ nr_pages,
+ dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
+ page_list);
if (pinned != nr_pages) {
if (pinned < 0) {
diff --git a/drivers/sbus/char/oradax.c b/drivers/sbus/char/oradax.c
index 6516bc3cb58b..790aa148670d 100644
--- a/drivers/sbus/char/oradax.c
+++ b/drivers/sbus/char/oradax.c
@@ -437,7 +437,7 @@ static int dax_lock_page(void *va, struct page **p)
dax_dbg("uva %p", va);
- ret = get_user_pages_fast((unsigned long)va, 1, 1, p);
+ ret = get_user_pages_fast((unsigned long)va, 1, FOLL_WRITE, p);
if (ret == 1) {
dax_dbg("locked page %p, for VA %p", *p, va);
return 0;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 7ff22d3f03e3..871b25914c07 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4918,7 +4918,8 @@ static int sgl_map_user_pages(struct st_buffer *STbp,
/* Try to fault in all of the necessary pages */
/* rw==READ means read from drive, write into memory area */
- res = get_user_pages_fast(uaddr, nr_pages, rw == READ, pages);
+ res = get_user_pages_fast(uaddr, nr_pages, rw == READ ? FOLL_WRITE : 0,
+ pages);
/* Errors and no page mapped should return here */
if (res < nr_pages)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 26755d9ca41d..f67fdf1d3817 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -486,8 +486,8 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl,
ptes[i].dma_addr = pg_tbl->coherent_pages[0].paddr +
off + i * PAGE_SIZE;
} else {
- ret = get_user_pages_fast(page_addr - offset, 1, 1,
- &page);
+ ret = get_user_pages_fast(page_addr - offset, 1,
+ FOLL_WRITE, &page);
if (ret <= 0) {
dev_err(pg_tbl->device,
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 0b9ab1d0dd45..49fd7312e2aa 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -273,7 +273,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
goto err;
}
- rc = get_user_pages_fast(start, num_pages, 1, shm->pages);
+ rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages);
if (rc > 0)
shm->num_pages = rc;
if (rc != num_pages) {
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index c424913324e3..a4b10bb4086b 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -532,7 +532,8 @@ static int tce_iommu_use_page(unsigned long tce, unsigned long *hpa)
enum dma_data_direction direction = iommu_tce_direction(tce);
if (get_user_pages_fast(tce & PAGE_MASK, 1,
- direction != DMA_TO_DEVICE, &page) != 1)
+ direction != DMA_TO_DEVICE ? FOLL_WRITE : 0,
+ &page) != 1)
return -EFAULT;
*hpa = __pa((unsigned long) page_address(page));
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 24a129fcdd61..72685b1659ff 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1700,7 +1700,7 @@ static int set_bit_to_user(int nr, void __user *addr)
int bit = nr + (log % PAGE_SIZE) * 8;
int r;
- r = get_user_pages_fast(log, 1, 1, &page);
+ r = get_user_pages_fast(log, 1, FOLL_WRITE, &page);
if (r < 0)
return r;
BUG_ON(r != 1);
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 8a53d1de611d..41390c8e0f67 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -686,7 +686,7 @@ static ssize_t pvr2fb_write(struct fb_info *info, const char *buf,
if (!pages)
return -ENOMEM;
- ret = get_user_pages_fast((unsigned long)buf, nr_pages, true, pages);
+ ret = get_user_pages_fast((unsigned long)buf, nr_pages, FOLL_WRITE, pages);
if (ret < nr_pages) {
nr_pages = ret;
ret = -EINVAL;
diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 8ba726e600e9..6446bcab4185 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -244,7 +244,7 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
/* Get the physical addresses of the source buffer */
num_pinned = get_user_pages_fast(param.local_vaddr - lb_offset,
- num_pages, param.source != -1, pages);
+ num_pages, param.source != -1 ? FOLL_WRITE : 0, pages);
if (num_pinned != num_pages) {
/* get_user_pages() failed */
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 5efc5eee9544..7b47f1e6aab4 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -852,7 +852,7 @@ static int gntdev_get_page(struct gntdev_copy_batch *batch, void __user *virt,
unsigned long xen_pfn;
int ret;
- ret = get_user_pages_fast(addr, 1, writeable, &page);
+ ret = get_user_pages_fast(addr, 1, writeable ? FOLL_WRITE : 0, &page);
if (ret < 0)
return ret;
diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c
index 443bcd8c3c19..5a7c4fda682f 100644
--- a/fs/orangefs/orangefs-bufmap.c
+++ b/fs/orangefs/orangefs-bufmap.c
@@ -269,7 +269,7 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap,
/* map the pages */
ret = get_user_pages_fast((unsigned long)user_desc->ptr,
- bufmap->page_count, 1, bufmap->page_array);
+ bufmap->page_count, FOLL_WRITE, bufmap->page_array);
if (ret < 0)
return ret;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 05a105d9d4c3..8e1f3cd7482a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1537,8 +1537,8 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages);
+int get_user_pages_fast(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages);
/* Container for pinned pfns / pages */
struct frame_vector {
diff --git a/kernel/futex.c b/kernel/futex.c
index fdd312da0992..e10209946f8b 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -546,7 +546,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, enum futex_a
if (unlikely(should_fail_futex(fshared)))
return -EFAULT;
- err = get_user_pages_fast(address, 1, 1, &page);
+ err = get_user_pages_fast(address, 1, FOLL_WRITE, &page);
/*
* If write access is not required (eg. FUTEX_WAIT), try
* and get read-only access.
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index be4bd627caf0..6dbae0692719 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1280,7 +1280,9 @@ ssize_t iov_iter_get_pages(struct iov_iter *i,
len = maxpages * PAGE_SIZE;
addr &= ~(PAGE_SIZE - 1);
n = DIV_ROUND_UP(len, PAGE_SIZE);
- res = get_user_pages_fast(addr, n, iov_iter_rw(i) != WRITE, pages);
+ res = get_user_pages_fast(addr, n,
+ iov_iter_rw(i) != WRITE ? FOLL_WRITE : 0,
+ pages);
if (unlikely(res < 0))
return res;
return (res == n ? len : res * PAGE_SIZE) - *start;
@@ -1361,7 +1363,8 @@ ssize_t iov_iter_get_pages_alloc(struct iov_iter *i,
p = get_pages_array(n);
if (!p)
return -ENOMEM;
- res = get_user_pages_fast(addr, n, iov_iter_rw(i) != WRITE, p);
+ res = get_user_pages_fast(addr, n,
+ iov_iter_rw(i) != WRITE ? FOLL_WRITE : 0, p);
if (unlikely(res < 0)) {
kvfree(p);
return res;
diff --git a/mm/gup.c b/mm/gup.c
index 681388236106..6f32d36b3c5b 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1863,7 +1863,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* get_user_pages_fast() - pin user pages in memory
* @start: starting user address
* @nr_pages: number of pages from start to pin
- * @write: whether pages will be written to
+ * @gup_flags: flags modifying pin behaviour
* @pages: array that receives pointers to the pages pinned.
* Should be at least nr_pages long.
*
@@ -1875,8 +1875,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* requested. If nr_pages is 0 or negative, returns 0. If no pages
* were pinned, returns -errno.
*/
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
+int get_user_pages_fast(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
{
unsigned long addr, len, end;
int nr = 0, ret = 0;
@@ -1894,7 +1894,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
if (gup_fast_permitted(start, nr_pages)) {
local_irq_disable();
- gup_pgd_range(addr, end, write ? FOLL_WRITE : 0, pages, &nr);
+ gup_pgd_range(addr, end, gup_flags, pages, &nr);
local_irq_enable();
ret = nr;
}
@@ -1905,7 +1905,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
pages += nr;
ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
- write ? FOLL_WRITE : 0);
+ gup_flags);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/mm/util.c b/mm/util.c
index 1ea055138043..01ffe145c62b 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -306,7 +306,7 @@ EXPORT_SYMBOL_GPL(__get_user_pages_fast);
* get_user_pages_fast() - pin user pages in memory
* @start: starting user address
* @nr_pages: number of pages from start to pin
- * @write: whether pages will be written to
+ * @gup_flags: flags modifying pin behaviour
* @pages: array that receives pointers to the pages pinned.
* Should be at least nr_pages long.
*
@@ -327,10 +327,10 @@ EXPORT_SYMBOL_GPL(__get_user_pages_fast);
* get_user_pages_fast simply falls back to get_user_pages.
*/
int __weak get_user_pages_fast(unsigned long start,
- int nr_pages, int write, struct page **pages)
+ int nr_pages, unsigned int gup_flags,
+ struct page **pages)
{
- return get_user_pages_unlocked(start, nr_pages, pages,
- write ? FOLL_WRITE : 0);
+ return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
}
EXPORT_SYMBOL_GPL(get_user_pages_fast);
diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
index d3736f5bffec..74cafc0142ea 100644
--- a/net/ceph/pagevec.c
+++ b/net/ceph/pagevec.c
@@ -27,7 +27,7 @@ struct page **ceph_get_direct_page_vector(const void __user *data,
while (got < num_pages) {
rc = get_user_pages_fast(
(unsigned long)data + ((unsigned long)got * PAGE_SIZE),
- num_pages - got, write_page, pages + got);
+ num_pages - got, write_page ? FOLL_WRITE : 0, pages + got);
if (rc < 0)
break;
BUG_ON(rc == 0);
diff --git a/net/rds/info.c b/net/rds/info.c
index e367a97a18c8..03f6fd56d237 100644
--- a/net/rds/info.c
+++ b/net/rds/info.c
@@ -193,7 +193,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, char __user *optval,
ret = -ENOMEM;
goto out;
}
- ret = get_user_pages_fast(start, nr_pages, 1, pages);
+ ret = get_user_pages_fast(start, nr_pages, FOLL_WRITE, pages);
if (ret != nr_pages) {
if (ret > 0)
nr_pages = ret;
diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 182ab8430594..b340ed4fc43a 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c
@@ -158,7 +158,8 @@ static int rds_pin_pages(unsigned long user_addr, unsigned int nr_pages,
{
int ret;
- ret = get_user_pages_fast(user_addr, nr_pages, write, pages);
+ ret = get_user_pages_fast(user_addr, nr_pages, write ? FOLL_WRITE : 0,
+ pages);
if (ret >= 0 && ret < nr_pages) {
while (ret--)
--
2.20.1
^ permalink raw reply related
* [PATCH v2] powerpc/prom_init: add __init markers to all functions
From: Masahiro Yamada @ 2019-02-20 5:53 UTC (permalink / raw)
To: linux-kbuild, linuxppc-dev, Michael Ellerman
Cc: Mathieu Malaterre, Aneesh Kumar K.V, linux-kernel,
Masahiro Yamada, Paul Mackerras
It is fragile to rely on the compiler's optimization to avoid the
section mismatch. Some functions may not be necessarily inlined
when the compiler's inlining heuristic changes.
Add __init markers consistently.
As for prom_getprop() and prom_getproplen(), they are marked as
'inline', so inlining is guaranteed because PowerPC never enables
CONFIG_OPTIMIZE_INLINING. However, it would be better to leave the
inlining decision to the compiler. I replaced 'inline' with __init.
I added __maybe_unused to prom_getproplen() because it is currently
relying on the side-effect of 'inline'; GCC does not report
-Wunused-function warnings for functions with 'inline' marker.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v2:
- Add __maybe_unsed to prom_getproplen()
- Add __init to enter_prom() as well
arch/powerpc/kernel/prom_init.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f33ff41..1bad0ac 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -138,9 +138,9 @@ extern void __start(unsigned long r3, unsigned long r4, unsigned long r5,
unsigned long r9);
#ifdef CONFIG_PPC64
-extern int enter_prom(struct prom_args *args, unsigned long entry);
+extern int __init enter_prom(struct prom_args *args, unsigned long entry);
#else
-static inline int enter_prom(struct prom_args *args, unsigned long entry)
+static int __init enter_prom(struct prom_args *args, unsigned long entry)
{
return ((int (*)(struct prom_args *))entry)(args);
}
@@ -501,19 +501,20 @@ static int __init prom_next_node(phandle *nodep)
}
}
-static inline int prom_getprop(phandle node, const char *pname,
+static int __init prom_getprop(phandle node, const char *pname,
void *value, size_t valuelen)
{
return call_prom("getprop", 4, 1, node, ADDR(pname),
(u32)(unsigned long) value, (u32) valuelen);
}
-static inline int prom_getproplen(phandle node, const char *pname)
+static int __init __maybe_unused prom_getproplen(phandle node,
+ const char *pname)
{
return call_prom("getproplen", 2, 1, node, ADDR(pname));
}
-static void add_string(char **str, const char *q)
+static void __init add_string(char **str, const char *q)
{
char *p = *str;
@@ -523,7 +524,7 @@ static void add_string(char **str, const char *q)
*str = p;
}
-static char *tohex(unsigned int x)
+static char __init *tohex(unsigned int x)
{
static const char digits[] __initconst = "0123456789abcdef";
static char result[9] __prombss;
@@ -570,7 +571,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
#define islower(c) ('a' <= (c) && (c) <= 'z')
#define toupper(c) (islower(c) ? ((c) - 'a' + 'A') : (c))
-static unsigned long prom_strtoul(const char *cp, const char **endp)
+static unsigned long __init prom_strtoul(const char *cp, const char **endp)
{
unsigned long result = 0, base = 10, value;
@@ -595,7 +596,7 @@ static unsigned long prom_strtoul(const char *cp, const char **endp)
return result;
}
-static unsigned long prom_memparse(const char *ptr, const char **retptr)
+static unsigned long __init prom_memparse(const char *ptr, const char **retptr)
{
unsigned long ret = prom_strtoul(ptr, retptr);
int shift = 0;
@@ -2924,7 +2925,7 @@ static void __init fixup_device_tree_pasemi(void)
prom_setprop(iob, name, "device_type", "isa", sizeof("isa"));
}
#else /* !CONFIG_PPC_PASEMI_NEMO */
-static inline void fixup_device_tree_pasemi(void) { }
+static void __init fixup_device_tree_pasemi(void) { }
#endif
static void __init fixup_device_tree(void)
@@ -2986,15 +2987,15 @@ static void __init prom_check_initrd(unsigned long r3, unsigned long r4)
#ifdef CONFIG_PPC64
#ifdef CONFIG_RELOCATABLE
-static void reloc_toc(void)
+static void __init reloc_toc(void)
{
}
-static void unreloc_toc(void)
+static void __init unreloc_toc(void)
{
}
#else
-static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
+static void __init __reloc_toc(unsigned long offset, unsigned long nr_entries)
{
unsigned long i;
unsigned long *toc_entry;
@@ -3008,7 +3009,7 @@ static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
}
}
-static void reloc_toc(void)
+static void __init reloc_toc(void)
{
unsigned long offset = reloc_offset();
unsigned long nr_entries =
@@ -3019,7 +3020,7 @@ static void reloc_toc(void)
mb();
}
-static void unreloc_toc(void)
+static void __init unreloc_toc(void)
{
unsigned long offset = reloc_offset();
unsigned long nr_entries =
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Akshay Adiga @ 2019-02-20 6:04 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <1550549702.xfczazszdw.astroid@bobo.none>
On Tue, Feb 19, 2019 at 02:21:04PM +1000, Nicholas Piggin wrote:
> Michael Ellerman's on February 8, 2019 11:04 am:
> > Nicholas Piggin <npiggin@gmail.com> writes:
> >> Russell Currey's on February 6, 2019 4:28 pm:
> >>> Without restoring the IAMR after idle, execution prevention on POWER9
> >>> with Radix MMU is overwritten and the kernel can freely execute userspace without
> >>> faulting.
> >>>
> >>> This is necessary when returning from any stop state that modifies user
> >>> state, as well as hypervisor state.
> >>>
> >>> To test how this fails without this patch, load the lkdtm driver and
> >>> do the following:
> >>>
> >>> echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
> >>>
> >>> which won't fault, then boot the kernel with powersave=off, where it
> >>> will fault. Applying this patch will fix this.
> >>>
> >>> Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
> >>> space")
> >>> Cc: <stable@vger.kernel.org>
> >>> Signed-off-by: Russell Currey <ruscur@russell.cc>
> >>
> >> Good catch and debugging. This really should be a quirk, we don't want
> >> to have to restore this thing on a thread switch.
> >
> > I'm not sure I follow. We don't context switch it on Radix, but we do
> > on hash if pkeys are enabled.
>
> Badly worded, I mean a hardware quirk. It should follow thread
> switches. Still, avoiding it for the no-loss case is better than
> nothing. We can just revisit it as an optimization if future
> hardware does not require the restore.
Apparently, the POWER9 Processor User’s Manual v2.0 documents that
IAMR can be lost, and that is not just the end.
Pasting excerpt from "Section 23.5.9.2 State Loss and Restoration,Page 309"
On the POWER9 core, the only state that can be lost for
Stop levels less than four, when PSSCR[ESL] = ‘1’ are the
following SPRs: CR, FPSCR, VSCR, XER, DSCR, AMR, IAMR, UAMOR,
AMOR, DAWR, DAWRX.
My observation is that AMOR is being used in kernel as of today
and AMOR is also lost (recreated in similar scenarios where
IAMR is lost).
^ permalink raw reply
* Re: [PATCH] powerpc: Make PPC_64K_PAGES depend on only 44x or PPC_BOOK3S_64
From: Scott Wood @ 2019-02-20 6:29 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <87h8czc1by.fsf@concordia.ellerman.id.au>
On Wed, 2019-02-20 at 01:14 +1100, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
> > On 02/08/2019 12:34 PM, Michael Ellerman wrote:
> > > In commit 7820856a4fcd ("powerpc/mm/book3e/64: Remove unsupported
> > > 64Kpage size from 64bit booke") we dropped the 64K page size support
> > > from the 64-bit nohash (Book3E) code.
> > >
> > > But we didn't update the dependencies of the PPC_64K_PAGES option,
> > > meaning a randconfig can still trigger this code and cause a build
> > > breakage, eg:
> > > arch/powerpc/include/asm/nohash/64/pgtable.h:14:2: error: #error
> > > "Page size not supported"
> > > arch/powerpc/include/asm/nohash/mmu-book3e.h:275:2: error: #error
> > > Unsupported page size
> > >
> > > So remove PPC_BOOK3E_64 from the dependencies. This also means we
> > > don't need to worry about PPC_FSL_BOOK3E, because that was just trying
> > > to prevent the PPC_BOOK3E_64=y && PPC_FSL_BOOK3E=y case.
> >
> > Does it means some cleanup could be done, for instance:
> >
> > arch/powerpc/include/asm/nohash/64/pgalloc.h:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/include/asm/nohash/64/pgalloc.h:#endif /*
> > CONFIG_PPC_64K_PAGES */
> > arch/powerpc/include/asm/nohash/64/pgtable.h:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/include/asm/nohash/64/slice.h:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/include/asm/nohash/64/slice.h:#else /* CONFIG_PPC_64K_PAGES
> > */
> > arch/powerpc/include/asm/nohash/64/slice.h:#endif /*
> > !CONFIG_PPC_64K_PAGES */
> > arch/powerpc/include/asm/nohash/pte-book3e.h:#ifdef CONFIG_PPC_64K_PAGES
> >
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
>
> Probably.
>
> Some of the FSL chips do support 64K pages at least according to some
> datasheets. I don't know what would be required to get it working, or if
> it even works in practice.
>
> So it would be nice to get 64K working on those chips, but probably no
> one has time or motivation to do it. In which case yeah all that code
> should be removed.
The primary TLB (TLB0) on these chips only supports 4K pages. TLB1 supports
many different sizes but is much smaller, hardware tablewalk only loads into
TLB0, etc.
-Scott
^ permalink raw reply
* Re: [PATCH v2] powerpc/prom_init: add __init markers to all functions
From: Christophe Leroy @ 2019-02-20 6:54 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild, linuxppc-dev, Michael Ellerman
Cc: Mathieu Malaterre, Aneesh Kumar K.V, Paul Mackerras, linux-kernel
In-Reply-To: <1550641996-21937-1-git-send-email-yamada.masahiro@socionext.com>
Le 20/02/2019 à 06:53, Masahiro Yamada a écrit :
> It is fragile to rely on the compiler's optimization to avoid the
> section mismatch. Some functions may not be necessarily inlined
> when the compiler's inlining heuristic changes.
>
> Add __init markers consistently.
>
> As for prom_getprop() and prom_getproplen(), they are marked as
> 'inline', so inlining is guaranteed because PowerPC never enables
> CONFIG_OPTIMIZE_INLINING. However, it would be better to leave the
> inlining decision to the compiler. I replaced 'inline' with __init.
> I added __maybe_unused to prom_getproplen() because it is currently
> relying on the side-effect of 'inline'; GCC does not report
> -Wunused-function warnings for functions with 'inline' marker.
__maybe_unused is really a bad trick that should be avoided, as it hides
unused functions.
Why is it a problem to keep prom_getproplen() as 'static inline' ? Most
small helpers are defined that way. Usually they are in an included
header file, but what's really the problem with having it here directly ?
Christophe
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v2:
> - Add __maybe_unsed to prom_getproplen()
> - Add __init to enter_prom() as well
>
> arch/powerpc/kernel/prom_init.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index f33ff41..1bad0ac 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -138,9 +138,9 @@ extern void __start(unsigned long r3, unsigned long r4, unsigned long r5,
> unsigned long r9);
>
> #ifdef CONFIG_PPC64
> -extern int enter_prom(struct prom_args *args, unsigned long entry);
> +extern int __init enter_prom(struct prom_args *args, unsigned long entry);
> #else
> -static inline int enter_prom(struct prom_args *args, unsigned long entry)
> +static int __init enter_prom(struct prom_args *args, unsigned long entry)
> {
> return ((int (*)(struct prom_args *))entry)(args);
> }
> @@ -501,19 +501,20 @@ static int __init prom_next_node(phandle *nodep)
> }
> }
>
> -static inline int prom_getprop(phandle node, const char *pname,
> +static int __init prom_getprop(phandle node, const char *pname,
> void *value, size_t valuelen)
> {
> return call_prom("getprop", 4, 1, node, ADDR(pname),
> (u32)(unsigned long) value, (u32) valuelen);
> }
>
> -static inline int prom_getproplen(phandle node, const char *pname)
> +static int __init __maybe_unused prom_getproplen(phandle node,
> + const char *pname)
> {
> return call_prom("getproplen", 2, 1, node, ADDR(pname));
> }
>
> -static void add_string(char **str, const char *q)
> +static void __init add_string(char **str, const char *q)
> {
> char *p = *str;
>
> @@ -523,7 +524,7 @@ static void add_string(char **str, const char *q)
> *str = p;
> }
>
> -static char *tohex(unsigned int x)
> +static char __init *tohex(unsigned int x)
> {
> static const char digits[] __initconst = "0123456789abcdef";
> static char result[9] __prombss;
> @@ -570,7 +571,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
> #define islower(c) ('a' <= (c) && (c) <= 'z')
> #define toupper(c) (islower(c) ? ((c) - 'a' + 'A') : (c))
>
> -static unsigned long prom_strtoul(const char *cp, const char **endp)
> +static unsigned long __init prom_strtoul(const char *cp, const char **endp)
> {
> unsigned long result = 0, base = 10, value;
>
> @@ -595,7 +596,7 @@ static unsigned long prom_strtoul(const char *cp, const char **endp)
> return result;
> }
>
> -static unsigned long prom_memparse(const char *ptr, const char **retptr)
> +static unsigned long __init prom_memparse(const char *ptr, const char **retptr)
> {
> unsigned long ret = prom_strtoul(ptr, retptr);
> int shift = 0;
> @@ -2924,7 +2925,7 @@ static void __init fixup_device_tree_pasemi(void)
> prom_setprop(iob, name, "device_type", "isa", sizeof("isa"));
> }
> #else /* !CONFIG_PPC_PASEMI_NEMO */
> -static inline void fixup_device_tree_pasemi(void) { }
> +static void __init fixup_device_tree_pasemi(void) { }
> #endif
>
> static void __init fixup_device_tree(void)
> @@ -2986,15 +2987,15 @@ static void __init prom_check_initrd(unsigned long r3, unsigned long r4)
>
> #ifdef CONFIG_PPC64
> #ifdef CONFIG_RELOCATABLE
> -static void reloc_toc(void)
> +static void __init reloc_toc(void)
> {
> }
>
> -static void unreloc_toc(void)
> +static void __init unreloc_toc(void)
> {
> }
> #else
> -static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
> +static void __init __reloc_toc(unsigned long offset, unsigned long nr_entries)
> {
> unsigned long i;
> unsigned long *toc_entry;
> @@ -3008,7 +3009,7 @@ static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
> }
> }
>
> -static void reloc_toc(void)
> +static void __init reloc_toc(void)
> {
> unsigned long offset = reloc_offset();
> unsigned long nr_entries =
> @@ -3019,7 +3020,7 @@ static void reloc_toc(void)
> mb();
> }
>
> -static void unreloc_toc(void)
> +static void __init unreloc_toc(void)
> {
> unsigned long offset = reloc_offset();
> unsigned long nr_entries =
>
^ permalink raw reply
* [PATCH 0/3] powerpc: sstep: Emulation test infrastructure
From: Sandipan Das @ 2019-02-20 6:56 UTC (permalink / raw)
To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria, dja
This aims to extend the current test infrastructure for in-kernel
instruction emulation by adding support for validating basic integer
operations and will verify the GPRs, LR, XER and CR.
There can be multiple test cases for each instruction. Each test case
has to be provided with the initial register state (in the form of a
pt_regs) and the 32-bit instruction to test.
Apart from verifying the end result, problems with the behaviour of
certain instructions for things like setting certain bits in CR or
XER (which can also be processor dependent) can be identified.
For example, the newly introduced CA32 bit in XER, exclusive to P9
CPUs as of now, was not being set when expected for some of the
arithmetic and shift instructions. With this infrastructure, it will
be easier to identify such problems and rectify them. The test cases
for the addc[.] instruction demonstrate this for different scenarios
where the CA and CA32 bits of XER should be set.
Changelog:
RFC -> v1:
- Integrate with current test infrastructure that already tests
some load and store instructions.
- Remove first two patches that introduce new instructions fields
in favour of extending the macros in the current infrastructure.
- Add a message to indicate that the tests are being run based on
suggestions from Daniel.
Sandipan Das (3):
powerpc: sstep: Add tests for compute type instructions
powerpc: sstep: Add tests for add[.] instruction
powerpc: sstep: Add tests for addc[.] instruction
arch/powerpc/include/asm/ppc-opcode.h | 1 +
arch/powerpc/lib/Makefile | 3 +-
arch/powerpc/lib/test_emulate_step.c | 535 +++++++++++++++++-
.../lib/test_emulate_step_exec_instr.S | 150 +++++
4 files changed, 684 insertions(+), 5 deletions(-)
create mode 100644 arch/powerpc/lib/test_emulate_step_exec_instr.S
--
2.19.2
^ permalink raw reply
* [PATCH 1/3] powerpc: sstep: Add tests for compute type instructions
From: Sandipan Das @ 2019-02-20 6:56 UTC (permalink / raw)
To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria, dja
In-Reply-To: <cover.1550645549.git.sandipan@linux.ibm.com>
This enhances the current selftest framework for validating
the in-kernel instruction emulation infrastructure by adding
support for compute type instructions i.e. integer ALU-based
instructions. Originally, this framework was limited to only
testing load and store instructions.
While most of the GPRs can be validated, support for SPRs is
limited to LR, CR and XER for now.
When writing the test cases, one must ensure that the Stack
Pointer (GPR1) or the Thread Pointer (GPR13) are not touched
by any means as these are vital non-volatile registers.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
arch/powerpc/lib/Makefile | 3 +-
arch/powerpc/lib/test_emulate_step.c | 167 +++++++++++++++++-
.../lib/test_emulate_step_exec_instr.S | 150 ++++++++++++++++
3 files changed, 315 insertions(+), 5 deletions(-)
create mode 100644 arch/powerpc/lib/test_emulate_step_exec_instr.S
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3bf9fc6fd36c..79396e184bca 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -30,7 +30,8 @@ obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
obj64-$(CONFIG_SMP) += locks.o
obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
-obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o
+obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o \
+ test_emulate_step_exec_instr.o
obj-y += checksum_$(BITS).o checksum_wrappers.o \
string_$(BITS).o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 6c47daa61614..3d7f7bae51cc 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -1,5 +1,5 @@
/*
- * Simple sanity test for emulate_step load/store instructions.
+ * Simple sanity tests for instruction emulation infrastructure.
*
* Copyright IBM Corp. 2016
*
@@ -14,6 +14,7 @@
#include <linux/ptrace.h>
#include <asm/sstep.h>
#include <asm/ppc-opcode.h>
+#include <asm/code-patching.h>
#define IMM_L(i) ((uintptr_t)(i) & 0xffff)
@@ -49,6 +50,11 @@
#define TEST_LXVD2X(s, a, b) (PPC_INST_LXVD2X | VSX_XX1((s), R##a, R##b))
#define TEST_STXVD2X(s, a, b) (PPC_INST_STXVD2X | VSX_XX1((s), R##a, R##b))
+#define MAX_SUBTESTS 16
+
+#define IGNORE_GPR(n) (0x1UL << (n))
+#define IGNORE_XER (0x1UL << 32)
+#define IGNORE_CCR (0x1UL << 33)
static void __init init_pt_regs(struct pt_regs *regs)
{
@@ -72,9 +78,15 @@ static void __init init_pt_regs(struct pt_regs *regs)
msr_cached = true;
}
-static void __init show_result(char *ins, char *result)
+static void __init show_result(char *mnemonic, char *result)
{
- pr_info("%-14s : %s\n", ins, result);
+ pr_info("%-14s : %s\n", mnemonic, result);
+}
+
+static void __init show_result_with_descr(char *mnemonic, char *descr,
+ char *result)
+{
+ pr_info("%-14s : %-50s %s\n", mnemonic, descr, result);
}
static void __init test_ld(void)
@@ -426,7 +438,7 @@ static void __init test_lxvd2x_stxvd2x(void)
}
#endif /* CONFIG_VSX */
-static int __init test_emulate_step(void)
+static void __init run_tests_load_store(void)
{
test_ld();
test_lwz();
@@ -437,6 +449,153 @@ static int __init test_emulate_step(void)
test_lfdx_stfdx();
test_lvx_stvx();
test_lxvd2x_stxvd2x();
+}
+
+struct compute_test {
+ char *mnemonic;
+ struct {
+ char *descr;
+ unsigned long flags;
+ unsigned int instr;
+ struct pt_regs regs;
+ } subtests[MAX_SUBTESTS + 1];
+};
+
+static struct compute_test compute_tests[] = {
+ {
+ .mnemonic = "nop",
+ .subtests = {
+ {
+ .descr = "R0 = LONG_MAX",
+ .instr = PPC_INST_NOP,
+ .regs = {
+ .gpr[0] = LONG_MAX,
+ }
+ }
+ }
+ }
+};
+
+static int __init emulate_compute_instr(struct pt_regs *regs,
+ unsigned int instr)
+{
+ struct instruction_op op;
+
+ if (!regs || !instr)
+ return -EINVAL;
+
+ if (analyse_instr(&op, regs, instr) != 1 ||
+ GETTYPE(op.type) != COMPUTE) {
+ pr_info("emulation failed, instruction = 0x%08x\n", instr);
+ return -EFAULT;
+ }
+
+ emulate_update_regs(regs, &op);
+ return 0;
+}
+
+static int __init execute_compute_instr(struct pt_regs *regs,
+ unsigned int instr)
+{
+ extern unsigned int exec_instr_execute[];
+ extern int exec_instr(struct pt_regs *regs);
+
+ if (!regs || !instr)
+ return -EINVAL;
+
+ /* Patch the NOP with the actual instruction */
+ patch_instruction(&exec_instr_execute[0], instr);
+ if (exec_instr(regs)) {
+ pr_info("execution failed, instruction = 0x%08x\n", instr);
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+#define gpr_mismatch(gprn, exp, got) \
+ pr_info("GPR%u mismatch, exp = 0x%016lx, got = 0x%016lx\n", \
+ gprn, exp, got)
+
+#define reg_mismatch(name, exp, got) \
+ pr_info("%s mismatch, exp = 0x%016lx, got = 0x%016lx\n", \
+ name, exp, got)
+
+static void __init run_tests_compute(void)
+{
+ unsigned long flags;
+ struct compute_test *test;
+ struct pt_regs *regs, exp, got;
+ unsigned int i, j, k, instr;
+ bool ignore_gpr, ignore_xer, ignore_ccr, passed;
+
+ for (i = 0; i < ARRAY_SIZE(compute_tests); i++) {
+ test = &compute_tests[i];
+
+ for (j = 0; j < MAX_SUBTESTS && test->subtests[j].descr; j++) {
+ instr = test->subtests[j].instr;
+ flags = test->subtests[j].flags;
+ regs = &test->subtests[j].regs;
+ ignore_xer = flags & IGNORE_XER;
+ ignore_ccr = flags & IGNORE_CCR;
+ passed = true;
+
+ memcpy(&exp, regs, sizeof(struct pt_regs));
+ memcpy(&got, regs, sizeof(struct pt_regs));
+
+ /*
+ * Set a compatible MSR value explicitly to ensure
+ * that XER and CR bits are updated appropriately
+ */
+ exp.msr = MSR_KERNEL;
+ got.msr = MSR_KERNEL;
+
+ if (emulate_compute_instr(&got, instr) ||
+ execute_compute_instr(&exp, instr)) {
+ passed = false;
+ goto print;
+ }
+
+ /* Verify GPR values */
+ for (k = 0; k < 32; k++) {
+ ignore_gpr = flags & IGNORE_GPR(k);
+ if (!ignore_gpr && exp.gpr[k] != got.gpr[k]) {
+ passed = false;
+ gpr_mismatch(k, exp.gpr[k], got.gpr[k]);
+ }
+ }
+
+ /* Verify LR value */
+ if (exp.link != got.link) {
+ passed = false;
+ reg_mismatch("LR", exp.link, got.link);
+ }
+
+ /* Verify XER value */
+ if (!ignore_xer && exp.xer != got.xer) {
+ passed = false;
+ reg_mismatch("XER", exp.xer, got.xer);
+ }
+
+ /* Verify CR value */
+ if (!ignore_ccr && exp.ccr != got.ccr) {
+ passed = false;
+ reg_mismatch("CR", exp.ccr, got.ccr);
+ }
+
+print:
+ show_result_with_descr(test->mnemonic,
+ test->subtests[j].descr,
+ passed ? "PASS" : "FAIL");
+ }
+ }
+}
+
+static int __init test_emulate_step(void)
+{
+ printk(KERN_INFO "Running instruction emulation self-tests ...\n");
+ run_tests_load_store();
+ run_tests_compute();
return 0;
}
diff --git a/arch/powerpc/lib/test_emulate_step_exec_instr.S b/arch/powerpc/lib/test_emulate_step_exec_instr.S
new file mode 100644
index 000000000000..84cef7d78d9d
--- /dev/null
+++ b/arch/powerpc/lib/test_emulate_step_exec_instr.S
@@ -0,0 +1,150 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Non-emulated single-stepping support (currently limited to basic integer
+ * computations) used to validate the instruction emulation infrastructure.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ */
+
+#include <asm/asm-offsets.h>
+#include <asm/ppc_asm.h>
+#include <linux/errno.h>
+
+/* int exec_instr(struct pt_regs *regs) */
+_GLOBAL(exec_instr)
+
+ /*
+ * Stack frame layout (INT_FRAME_SIZE bytes)
+ * In-memory pt_regs (SP + STACK_FRAME_OVERHEAD)
+ * Scratch space (SP + 8)
+ * Back chain (SP + 0)
+ */
+
+ /*
+ * Allocate a new stack frame with enough space to hold the register
+ * states in an in-memory pt_regs and also create the back chain to
+ * the caller's stack frame.
+ */
+ stdu r1, -INT_FRAME_SIZE(r1)
+
+ /*
+ * Save non-volatile GPRs on stack. This includes TOC pointer (GPR2)
+ * and local variables (GPR14 to GPR31). The register for the pt_regs
+ * parameter (GPR3) is saved additionally to ensure that the resulting
+ * register state can still be saved even if GPR3 gets overwritten
+ * when loading the initial register state for the test instruction.
+ * The stack pointer (GPR1) and the thread pointer (GPR13) are not
+ * saved as these should not be modified anyway.
+ */
+ SAVE_2GPRS(2, r1)
+ SAVE_NVGPRS(r1)
+
+ /*
+ * Save LR on stack to ensure that the return address is available
+ * even if it gets overwritten by the test instruction.
+ */
+ mflr r0
+ std r0, _LINK(r1)
+
+ /*
+ * Save CR on stack. For simplicity, the entire register is saved
+ * even though only fields 2 to 4 are non-volatile.
+ */
+ mfcr r0
+ std r0, _CCR(r1)
+
+ /*
+ * Load register state for the test instruction without touching the
+ * critical non-volatile registers. The register state is passed as a
+ * pointer to a pt_regs instance.
+ */
+ subi r31, r3, GPR0
+
+ /* Load LR from pt_regs */
+ ld r0, _LINK(r31)
+ mtlr r0
+
+ /* Load CR from pt_regs */
+ ld r0, _CCR(r31)
+ mtcr r0
+
+ /* Load XER from pt_regs */
+ ld r0, _XER(r31)
+ mtxer r0
+
+ /* Load GPRs from pt_regs */
+ REST_GPR(0, r31)
+ REST_10GPRS(2, r31)
+ REST_GPR(12, r31)
+ REST_NVGPRS(r31)
+
+ .global exec_instr_execute
+exec_instr_execute:
+ /* Placeholder for the test instruction */
+1: nop
+
+ /*
+ * Since GPR3 is overwritten, temporarily restore it back to its
+ * original state, i.e. the pointer to pt_regs, to ensure that the
+ * resulting register state can be saved. Before doing this, a copy
+ * of it is created in the scratch space which is used later on to
+ * save it to pt_regs.
+ */
+ std r3, 8(r1)
+ REST_GPR(3, r1)
+
+ /* Save resulting GPR state to pt_regs */
+ subi r3, r3, GPR0
+ SAVE_GPR(0, r3)
+ SAVE_GPR(2, r3)
+ SAVE_8GPRS(4, r3)
+ SAVE_GPR(12, r3)
+ SAVE_NVGPRS(r3)
+
+ /* Save resulting LR to pt_regs */
+ mflr r0
+ std r0, _LINK(r3)
+
+ /* Save resulting CR to pt_regs */
+ mfcr r0
+ std r0, _CCR(r3)
+
+ /* Save resulting XER to pt_regs */
+ mfxer r0
+ std r0, _XER(r3)
+
+ /* Restore resulting GPR3 from scratch space and save it to pt_regs */
+ ld r0, 8(r1)
+ std r0, GPR3(r3)
+
+ /* Set return value to denote execution success */
+ li r3, 0
+
+ /* Continue */
+ b 3f
+
+ /* Set return value to denote execution failure */
+2: li r3, -EFAULT
+
+ /* Restore the non-volatile GPRs from stack */
+3: REST_GPR(2, r1)
+ REST_NVGPRS(r1)
+
+ /* Restore LR from stack to be able to return */
+ ld r0, _LINK(r1)
+ mtlr r0
+
+ /* Restore CR from stack */
+ ld r0, _CCR(r1)
+ mtcr r0
+
+ /* Tear down stack frame */
+ addi r1, r1, INT_FRAME_SIZE
+
+ /* Return */
+ blr
+
+ /* Setup exception table */
+ EX_TABLE(1b, 2b)
+
+_ASM_NOKPROBE_SYMBOL(exec_instr)
--
2.19.2
^ permalink raw reply related
* [PATCH 2/3] powerpc: sstep: Add tests for add[.] instruction
From: Sandipan Das @ 2019-02-20 6:56 UTC (permalink / raw)
To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria, dja
In-Reply-To: <cover.1550645549.git.sandipan@linux.ibm.com>
This adds test cases for the add[.] instruction.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
arch/powerpc/lib/test_emulate_step.c | 176 +++++++++++++++++++++++++++
1 file changed, 176 insertions(+)
diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 3d7f7bae51cc..bf88b20e53d7 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -49,6 +49,10 @@
___PPC_RA(a) | ___PPC_RB(b))
#define TEST_LXVD2X(s, a, b) (PPC_INST_LXVD2X | VSX_XX1((s), R##a, R##b))
#define TEST_STXVD2X(s, a, b) (PPC_INST_STXVD2X | VSX_XX1((s), R##a, R##b))
+#define TEST_ADD(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) | \
+ ___PPC_RA(a) | ___PPC_RB(b))
+#define TEST_ADD_DOT(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) | \
+ ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
#define MAX_SUBTESTS 16
@@ -473,6 +477,178 @@ static struct compute_test compute_tests[] = {
}
}
}
+ },
+ {
+ .mnemonic = "add",
+ .subtests = {
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MIN",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MIN,
+ }
+ },
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MAX",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = LONG_MAX, RB = LONG_MAX",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MAX,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = ULONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = 0x1",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = 0x1,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MIN",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MIN,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MAX",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = INT_MAX, RB = INT_MAX",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MAX,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = UINT_MAX",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = UINT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = 0x1",
+ .instr = TEST_ADD(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = 0x1,
+ }
+ }
+ }
+ },
+ {
+ .mnemonic = "add.",
+ .subtests = {
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MIN",
+ .flags = IGNORE_CCR,
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MIN,
+ }
+ },
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MAX",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = LONG_MAX, RB = LONG_MAX",
+ .flags = IGNORE_CCR,
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MAX,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = ULONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = 0x1",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = 0x1,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MIN",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MIN,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MAX",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = INT_MAX, RB = INT_MAX",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MAX,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = UINT_MAX",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = UINT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = 0x1",
+ .instr = TEST_ADD_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = 0x1,
+ }
+ }
+ }
}
};
--
2.19.2
^ permalink raw reply related
* [PATCH 3/3] powerpc: sstep: Add tests for addc[.] instruction
From: Sandipan Das @ 2019-02-20 6:57 UTC (permalink / raw)
To: mpe; +Cc: naveen.n.rao, paulus, linuxppc-dev, ravi.bangoria, dja
In-Reply-To: <cover.1550645549.git.sandipan@linux.ibm.com>
This adds test cases for the addc[.] instruction.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
arch/powerpc/include/asm/ppc-opcode.h | 1 +
arch/powerpc/lib/test_emulate_step.c | 192 ++++++++++++++++++++++++++
2 files changed, 193 insertions(+)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 19a8834e0398..87b73aa56b53 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -326,6 +326,7 @@
#define PPC_INST_ADDI 0x38000000
#define PPC_INST_ADDIS 0x3c000000
#define PPC_INST_ADD 0x7c000214
+#define PPC_INST_ADDC 0x7c000014
#define PPC_INST_SUB 0x7c000050
#define PPC_INST_BLR 0x4e800020
#define PPC_INST_BLRL 0x4e800021
diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index bf88b20e53d7..1c13b3bebeca 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -53,6 +53,10 @@
___PPC_RA(a) | ___PPC_RB(b))
#define TEST_ADD_DOT(t, a, b) (PPC_INST_ADD | ___PPC_RT(t) | \
___PPC_RA(a) | ___PPC_RB(b) | 0x1)
+#define TEST_ADDC(t, a, b) (PPC_INST_ADDC | ___PPC_RT(t) | \
+ ___PPC_RA(a) | ___PPC_RB(b))
+#define TEST_ADDC_DOT(t, a, b) (PPC_INST_ADDC | ___PPC_RT(t) | \
+ ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
#define MAX_SUBTESTS 16
@@ -649,6 +653,194 @@ static struct compute_test compute_tests[] = {
}
}
}
+ },
+ {
+ .mnemonic = "addc",
+ .subtests = {
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MIN",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MIN,
+ }
+ },
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MAX",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = LONG_MAX, RB = LONG_MAX",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MAX,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = ULONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = 0x1",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = 0x1,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MIN",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MIN,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MAX",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = INT_MAX, RB = INT_MAX",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MAX,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = UINT_MAX",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = UINT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = 0x1",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = 0x1,
+ }
+ },
+ {
+ .descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
+ .instr = TEST_ADDC(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN | (uint)INT_MIN,
+ .gpr[22] = LONG_MIN | (uint)INT_MIN,
+ }
+ }
+ }
+ },
+ {
+ .mnemonic = "addc.",
+ .subtests = {
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MIN",
+ .flags = IGNORE_CCR,
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MIN,
+ }
+ },
+ {
+ .descr = "RA = LONG_MIN, RB = LONG_MAX",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = LONG_MAX, RB = LONG_MAX",
+ .flags = IGNORE_CCR,
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MAX,
+ .gpr[22] = LONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = ULONG_MAX,
+ }
+ },
+ {
+ .descr = "RA = ULONG_MAX, RB = 0x1",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = ULONG_MAX,
+ .gpr[22] = 0x1,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MIN",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MIN,
+ }
+ },
+ {
+ .descr = "RA = INT_MIN, RB = INT_MAX",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MIN,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = INT_MAX, RB = INT_MAX",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = INT_MAX,
+ .gpr[22] = INT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = UINT_MAX",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = UINT_MAX,
+ }
+ },
+ {
+ .descr = "RA = UINT_MAX, RB = 0x1",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = UINT_MAX,
+ .gpr[22] = 0x1,
+ }
+ },
+ {
+ .descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
+ .instr = TEST_ADDC_DOT(20, 21, 22),
+ .regs = {
+ .gpr[21] = LONG_MIN | (uint)INT_MIN,
+ .gpr[22] = LONG_MIN | (uint)INT_MIN,
+ }
+ }
+ }
}
};
--
2.19.2
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Akshay Adiga @ 2019-02-20 7:15 UTC (permalink / raw)
To: Russell Currey; +Cc: linuxppc-dev
In-Reply-To: <20190206062837.26917-1-ruscur@russell.cc>
On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
> Without restoring the IAMR after idle, execution prevention on POWER9
> with Radix MMU is overwritten and the kernel can freely execute userspace without
> faulting.
>
> This is necessary when returning from any stop state that modifies user
> state, as well as hypervisor state.
>
> To test how this fails without this patch, load the lkdtm driver and
> do the following:
>
> echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
>
> which won't fault, then boot the kernel with powersave=off, where it
> will fault. Applying this patch will fix this.
>
> Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
> space")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> arch/powerpc/include/asm/cpuidle.h | 1 +
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 43e5f31fe64d..ad67dbe59498 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -77,6 +77,7 @@ struct stop_sprs {
> u64 mmcr1;
> u64 mmcr2;
> u64 mmcra;
> + u64 iamr;
> };
>
> #define PNV_IDLE_NAME_LEN 16
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 9ffc72ded73a..10e0314c2b0d 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -774,6 +774,7 @@ int main(void)
> STOP_SPR(STOP_MMCR1, mmcr1);
> STOP_SPR(STOP_MMCR2, mmcr2);
> STOP_SPR(STOP_MMCRA, mmcra);
> + STOP_SPR(STOP_IAMR, iamr);
> #endif
>
> DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 7f5ac2e8581b..bb4f552f6c7e 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -200,6 +200,12 @@ pnv_powersave_common:
> /* Continue saving state */
> SAVE_GPR(2, r1)
> SAVE_NVGPRS(r1)
> +
> +BEGIN_FTR_SECTION
> + mfspr r5, SPRN_IAMR
> + std r5, STOP_IAMR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> +
Are we trying to add for both power8 and power9 ?
power9 would be CPU_FTR_ARCH_300.
^ permalink raw reply
* [PATCH] KVM: PPC: Book3S HV: Context switch IAMR on Power9
From: Michael Ellerman @ 2019-02-20 8:55 UTC (permalink / raw)
To: linuxppc-dev; +Cc: paulus, kvm-ppc
kvmhv_p9_guest_entry() implements a fast-path guest entry for Power9
when guest and host are both running with the Radix MMU.
Currently in that path we don't save the host AMR (Authority Mask
Register) value, and we always restore 0 on return to the host. That
is OK at the moment because the AMR is not used for storage keys with
the Radix MMU.
However we plan to start using the AMR on Radix to prevent the kernel
from reading/writing to userspace outside of copy_to/from_user(). In
order to make that work we need to save/restore the AMR value.
We only restore the value if it is different from the guest value,
which is already in the register when we exit to the host. This should
mean we rarely need to actually restore the value when running a
modern Linux as a guest, because it will be using the same value as
us.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kvm/book3s_hv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 5a066fc299e1..105a3f78a760 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3455,6 +3455,7 @@ int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
unsigned long host_dscr = mfspr(SPRN_DSCR);
unsigned long host_tidr = mfspr(SPRN_TIDR);
unsigned long host_iamr = mfspr(SPRN_IAMR);
+ unsigned long host_amr = mfspr(SPRN_AMR);
s64 dec;
u64 tb;
int trap, save_pmu;
@@ -3571,13 +3572,15 @@ int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
mtspr(SPRN_PSPB, 0);
mtspr(SPRN_WORT, 0);
- mtspr(SPRN_AMR, 0);
mtspr(SPRN_UAMOR, 0);
mtspr(SPRN_DSCR, host_dscr);
mtspr(SPRN_TIDR, host_tidr);
mtspr(SPRN_IAMR, host_iamr);
mtspr(SPRN_PSPB, 0);
+ if (host_amr != vcpu->arch.amr)
+ mtspr(SPRN_AMR, host_amr);
+
msr_check_and_set(MSR_FP | MSR_VEC | MSR_VSX);
store_fp_state(&vcpu->arch.fp);
#ifdef CONFIG_ALTIVEC
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Akshay Adiga @ 2019-02-20 8:58 UTC (permalink / raw)
To: Russell Currey; +Cc: linuxppc-dev
In-Reply-To: <20190206062837.26917-1-ruscur@russell.cc>
On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
> Without restoring the IAMR after idle, execution prevention on POWER9
> with Radix MMU is overwritten and the kernel can freely execute userspace without
> faulting.
>
> This is necessary when returning from any stop state that modifies user
> state, as well as hypervisor state.
>
> To test how this fails without this patch, load the lkdtm driver and
> do the following:
>
> echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
>
> which won't fault, then boot the kernel with powersave=off, where it
> will fault. Applying this patch will fix this.
>
> Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
> space")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> arch/powerpc/include/asm/cpuidle.h | 1 +
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 43e5f31fe64d..ad67dbe59498 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -77,6 +77,7 @@ struct stop_sprs {
> u64 mmcr1;
> u64 mmcr2;
> u64 mmcra;
> + u64 iamr;
> };
>
> #define PNV_IDLE_NAME_LEN 16
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 9ffc72ded73a..10e0314c2b0d 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -774,6 +774,7 @@ int main(void)
> STOP_SPR(STOP_MMCR1, mmcr1);
> STOP_SPR(STOP_MMCR2, mmcr2);
> STOP_SPR(STOP_MMCRA, mmcra);
> + STOP_SPR(STOP_IAMR, iamr);
> #endif
>
> DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 7f5ac2e8581b..bb4f552f6c7e 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -200,6 +200,12 @@ pnv_powersave_common:
> /* Continue saving state */
> SAVE_GPR(2, r1)
> SAVE_NVGPRS(r1)
> +
> +BEGIN_FTR_SECTION
> + mfspr r5, SPRN_IAMR
> + std r5, STOP_IAMR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> +
> mfcr r5
> std r5,_CCR(r1)
> std r1,PACAR1(r13)
> @@ -924,6 +930,13 @@ BEGIN_FTR_SECTION
> END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> REST_NVGPRS(r1)
> REST_GPR(2, r1)
> +
> +BEGIN_FTR_SECTION
> + ld r4, STOP_IAMR(r13)
> + mtspr SPRN_IAMR, r4
> + isync
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> +
> ld r4,PACAKMSR(r13)
> ld r5,_LINK(r1)
> ld r6,_CCR(r1)
> @@ -946,6 +959,13 @@ pnv_wakeup_noloss:
> BEGIN_FTR_SECTION
> CHECK_HMI_INTERRUPT
> END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> +
> +BEGIN_FTR_SECTION
> + ld r4, STOP_IAMR(r13)
> + mtspr SPRN_IAMR, r4
> + isync
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> +
pnv_wakeup_noloss gets called from two paths:
1) cpu wakes up at 0x100 (ESL=EC=1)
2) cpu wakes up at next instruction (ESL=EC=0)
We know for the fact that its not lost with ESL=EC=0 , so we
should put it somewhere else.
I would like to put the restore code in pnv_restore_hyp_resource_arch300
it already has some work arounds we can add another.
By this time we cr3 has comparison of SRR1[46:47] with 2
542 pnv_restore_hyp_resource_arch300:
543 /*
544 * Workaround for POWER9, if we lost resources, the ERAT
545 * might have been mixed up and needs flushing. We also need
546 * to reload MMCR0 (see comment above). We also need to set
547 * then clear bit 60 in MMCRA to ensure the PMU starts running.
548 */
549 blt cr3,1f
550 BEGIN_FTR_SECTION
551 PPC_INVALIDATE_ERAT
552 ld r1,PACAR1(r13)
553 ld r4,_MMCR0(r1)
554 mtspr SPRN_MMCR0,r4
555 END_FTR_SECTION_IFCLR(CPU_FTR_POWER9_DD2_1)
556 mfspr r4,SPRN_MMCRA
557 ori r4,r4,(1 << (63-60))
558 mtspr SPRN_MMCRA,r4
559 xori r4,r4,(1 << (63-60))
560 mtspr SPRN_MMCRA,r4
+ 561 BEGIN_FTR_SECTION
+ 562 ld r4,STOP_IAMR(r13)
+ 563 mtspr SPRN_IAMR,r4
+ 564 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
565 1:
566 /*
I have a patchset to handle both AMOR and IAMR,
need to test it on power8 before posting.
> ld r4,PACAKMSR(r13)
> ld r5,_NIP(r1)
> ld r6,_CCR(r1)
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: Simplify machine check handling
From: Aravinda Prasad @ 2019-02-20 9:20 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20190220010550.GD5353@blackberry>
On Wednesday 20 February 2019 06:35 AM, Paul Mackerras wrote:
> This makes the handling of machine check interrupts that occur inside
> a guest simpler and more robust, with less done in assembler code and
> in real mode.
>
> Now, when a machine check occurs inside a guest, we always get the
> machine check event struct and put a copy in the vcpu struct for the
> vcpu where the machine check occurred. We no longer call
> machine_check_queue_event() from kvmppc_realmode_mc_power7(), because
> on POWER8, when a vcpu is running on an offline secondary thread and
> we call machine_check_queue_event(), that calls irq_work_queue(),
> which doesn't work because the CPU is offline, but instead triggers
> the WARN_ON(lazy_irq_pending()) in pnv_smp_cpu_kill_self() (which
> fires again and again because nothing clears the condition).
>
> All that machine_check_queue_event() actually does is to cause the
> event to be printed to the console. For a machine check occurring in
> the guest, we now print the event in kvmppc_handle_exit_hv()
> instead.
>
> The assembly code at label machine_check_realmode now just calls C
> code and then continues exiting the guest. We no longer either
> synthesize a machine check for the guest in assembly code or return
> to the guest without a machine check.
>
> The code in kvmppc_handle_exit_hv() is extended to handle the case
> where the guest is not FWNMI-capable. In that case we now always
> synthesize a machine check interrupt for the guest. Previously, if
> the host thinks it has recovered the machine check fully, it would
> return to the guest without any notification that the machine check
> had occurred. If the machine check was caused by some action of the
> guest (such as creating duplicate SLB entries), it is much better to
> tell the guest that it has caused a problem. Therefore we now always
> generate a machine check interrupt for guests that are not
> FWNMI-capable.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 3 +-
> arch/powerpc/kvm/book3s.c | 7 +++++
> arch/powerpc/kvm/book3s_hv.c | 18 +++++++++--
> arch/powerpc/kvm/book3s_hv_ras.c | 56 +++++++++------------------------
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 40 ++---------------------
> 5 files changed, 42 insertions(+), 82 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index b3bf4f6..d283d31 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -143,6 +143,7 @@ extern void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
>
> extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
> extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);
> +extern void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags);
> extern void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);
> extern void kvmppc_core_queue_fpunavail(struct kvm_vcpu *vcpu);
> extern void kvmppc_core_queue_vec_unavail(struct kvm_vcpu *vcpu);
> @@ -646,7 +647,7 @@ long int kvmppc_rm_h_confer(struct kvm_vcpu *vcpu, int target,
> unsigned int yield_count);
> long kvmppc_h_random(struct kvm_vcpu *vcpu);
> void kvmhv_commence_exit(int trap);
> -long kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu);
> +void kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu);
> void kvmppc_subcore_enter_guest(void);
> void kvmppc_subcore_exit_guest(void);
> long kvmppc_realmode_hmi_handler(void);
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 22a46c6..10c5579 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -195,6 +195,13 @@ void kvmppc_book3s_queue_irqprio(struct kvm_vcpu *vcpu, unsigned int vec)
> }
> EXPORT_SYMBOL_GPL(kvmppc_book3s_queue_irqprio);
>
> +void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags)
> +{
> + /* might as well deliver this straight away */
> + kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_MACHINE_CHECK, flags);
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_core_queue_machine_check);
> +
> void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
> {
> /* might as well deliver this straight away */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 1860c0b..d8bf05a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1215,6 +1215,22 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> r = RESUME_GUEST;
> break;
> case BOOK3S_INTERRUPT_MACHINE_CHECK:
> + /* Print the MCE event to host console. */
> + machine_check_print_event_info(&vcpu->arch.mce_evt, false);
> +
> + /*
> + * If the guest can do FWNMI, exit to userspace so it can
> + * deliver a FWNMI to the guest.
> + * Otherwise we synthesize a machine check for the guest
> + * so that it knows that the machine check occurred.
> + */
> + if (!vcpu->kvm->arch.fwnmi_enabled) {
> + ulong flags = vcpu->arch.shregs.msr & 0x083c0000;
> + kvmppc_core_queue_machine_check(vcpu, flags);
> + r = RESUME_GUEST;
> + break;
> + }
> +
> /* Exit to guest with KVM_EXIT_NMI as exit reason */
> run->exit_reason = KVM_EXIT_NMI;
> run->hw.hardware_exit_reason = vcpu->arch.trap;
> @@ -1227,8 +1243,6 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> run->flags |= KVM_RUN_PPC_NMI_DISP_NOT_RECOV;
>
> r = RESUME_HOST;
> - /* Print the MCE event to host console. */
> - machine_check_print_event_info(&vcpu->arch.mce_evt, false);
> break;
> case BOOK3S_INTERRUPT_PROGRAM:
> {
> diff --git a/arch/powerpc/kvm/book3s_hv_ras.c b/arch/powerpc/kvm/book3s_hv_ras.c
> index 0787f12..9aa10b1 100644
> --- a/arch/powerpc/kvm/book3s_hv_ras.c
> +++ b/arch/powerpc/kvm/book3s_hv_ras.c
> @@ -69,7 +69,7 @@ static void reload_slb(struct kvm_vcpu *vcpu)
> *
> * Returns: 0 => exit guest, 1 => deliver machine check to guest
You have to remove the above comment as the function now returns nothing.
Reviewed-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
Regards,
Aravinda
> */
> -static long kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
> +static void kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
> {
> unsigned long srr1 = vcpu->arch.shregs.msr;
> struct machine_check_event mce_evt;
> @@ -111,52 +111,24 @@ static long kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
> }
>
> /*
> - * See if we have already handled the condition in the linux host.
> - * We assume that if the condition is recovered then linux host
> - * will have generated an error log event that we will pick
> - * up and log later.
> - * Don't release mce event now. We will queue up the event so that
> - * we can log the MCE event info on host console.
> + * Now get the event and stash it in the vcpu struct so it can
> + * be handled by the primary thread in virtual mode. We can't
> + * call machine_check_queue_event() here if we are running on
> + * an offline secondary thread.
> */
> - if (!get_mce_event(&mce_evt, MCE_EVENT_DONTRELEASE))
> - goto out;
> -
> - if (mce_evt.version == MCE_V1 &&
> - (mce_evt.severity == MCE_SEV_NO_ERROR ||
> - mce_evt.disposition == MCE_DISPOSITION_RECOVERED))
> - handled = 1;
> -
> -out:
> - /*
> - * For guest that supports FWNMI capability, hook the MCE event into
> - * vcpu structure. We are going to exit the guest with KVM_EXIT_NMI
> - * exit reason. On our way to exit we will pull this event from vcpu
> - * structure and print it from thread 0 of the core/subcore.
> - *
> - * For guest that does not support FWNMI capability (old QEMU):
> - * We are now going enter guest either through machine check
> - * interrupt (for unhandled errors) or will continue from
> - * current HSRR0 (for handled errors) in guest. Hence
> - * queue up the event so that we can log it from host console later.
> - */
> - if (vcpu->kvm->arch.fwnmi_enabled) {
> - /*
> - * Hook up the mce event on to vcpu structure.
> - * First clear the old event.
> - */
> - memset(&vcpu->arch.mce_evt, 0, sizeof(vcpu->arch.mce_evt));
> - if (get_mce_event(&mce_evt, MCE_EVENT_RELEASE)) {
> - vcpu->arch.mce_evt = mce_evt;
> - }
> - } else
> - machine_check_queue_event();
> + if (get_mce_event(&mce_evt, MCE_EVENT_RELEASE)) {
> + if (handled && mce_evt.version == MCE_V1)
> + mce_evt.disposition = MCE_DISPOSITION_RECOVERED;
> + } else {
> + memset(&mce_evt, 0, sizeof(mce_evt));
> + }
>
> - return handled;
> + vcpu->arch.mce_evt = mce_evt;
> }
>
> -long kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu)
> +void kvmppc_realmode_machine_check(struct kvm_vcpu *vcpu)
> {
> - return kvmppc_realmode_mc_power7(vcpu);
> + kvmppc_realmode_mc_power7(vcpu);
> }
>
> /* Check if dynamic split is in force and return subcore size accordingly. */
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 9b8d50a..f24f6a2 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -2826,49 +2826,15 @@ kvm_cede_exit:
> #endif /* CONFIG_KVM_XICS */
> 3: b guest_exit_cont
>
> - /* Try to handle a machine check in real mode */
> + /* Try to do machine check recovery in real mode */
> machine_check_realmode:
> mr r3, r9 /* get vcpu pointer */
> bl kvmppc_realmode_machine_check
> nop
> + /* all machine checks go to virtual mode for further handling */
> ld r9, HSTATE_KVM_VCPU(r13)
> li r12, BOOK3S_INTERRUPT_MACHINE_CHECK
> - /*
> - * For the guest that is FWNMI capable, deliver all the MCE errors
> - * (handled/unhandled) by exiting the guest with KVM_EXIT_NMI exit
> - * reason. This new approach injects machine check errors in guest
> - * address space to guest with additional information in the form
> - * of RTAS event, thus enabling guest kernel to suitably handle
> - * such errors.
> - *
> - * For the guest that is not FWNMI capable (old QEMU) fallback
> - * to old behaviour for backward compatibility:
> - * Deliver unhandled/fatal (e.g. UE) MCE errors to guest either
> - * through machine check interrupt (set HSRR0 to 0x200).
> - * For handled errors (no-fatal), just go back to guest execution
> - * with current HSRR0.
> - * if we receive machine check with MSR(RI=0) then deliver it to
> - * guest as machine check causing guest to crash.
> - */
> - ld r11, VCPU_MSR(r9)
> - rldicl. r0, r11, 64-MSR_HV_LG, 63 /* check if it happened in HV mode */
> - bne guest_exit_cont /* if so, exit to host */
> - /* Check if guest is capable of handling NMI exit */
> - ld r10, VCPU_KVM(r9)
> - lbz r10, KVM_FWNMI(r10)
> - cmpdi r10, 1 /* FWNMI capable? */
> - beq guest_exit_cont /* if so, exit with KVM_EXIT_NMI. */
> -
> - /* if not, fall through for backward compatibility. */
> - andi. r10, r11, MSR_RI /* check for unrecoverable exception */
> - beq 1f /* Deliver a machine check to guest */
> - ld r10, VCPU_PC(r9)
> - cmpdi r3, 0 /* Did we handle MCE ? */
> - bne 2f /* Continue guest execution. */
> - /* If not, deliver a machine check. SRR0/1 are already set */
> -1: li r10, BOOK3S_INTERRUPT_MACHINE_CHECK
> - bl kvmppc_msr_interrupt
> -2: b fast_interrupt_c_return
> + b guest_exit_cont
>
> /*
> * Call C code to handle a HMI in real mode.
>
--
Regards,
Aravinda
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/64s: Better printing of machine check info for guest MCEs
From: Aravinda Prasad @ 2019-02-20 9:21 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20190220010623.GE5353@blackberry>
On Wednesday 20 February 2019 06:36 AM, Paul Mackerras wrote:
> This adds an "in_guest" parameter to machine_check_print_event_info()
> so that we can avoid trying to translate guest NIP values into
> symbolic form using the host kernel's symbol table.
Reviewed-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
Regards,
Aravinda
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
> arch/powerpc/include/asm/mce.h | 2 +-
> arch/powerpc/kernel/mce.c | 8 +++++---
> arch/powerpc/kvm/book3s_hv.c | 4 ++--
> arch/powerpc/platforms/powernv/opal.c | 2 +-
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index a8b8903..17996bc 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -209,7 +209,7 @@ extern int get_mce_event(struct machine_check_event *mce, bool release);
> extern void release_mce_event(void);
> extern void machine_check_queue_event(void);
> extern void machine_check_print_event_info(struct machine_check_event *evt,
> - bool user_mode);
> + bool user_mode, bool in_guest);
> #ifdef CONFIG_PPC_BOOK3S_64
> void flush_and_reload_slb(void);
> #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index bd933a7..d01b690 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -301,13 +301,13 @@ static void machine_check_process_queued_event(struct irq_work *work)
> while (__this_cpu_read(mce_queue_count) > 0) {
> index = __this_cpu_read(mce_queue_count) - 1;
> evt = this_cpu_ptr(&mce_event_queue[index]);
> - machine_check_print_event_info(evt, false);
> + machine_check_print_event_info(evt, false, false);
> __this_cpu_dec(mce_queue_count);
> }
> }
>
> void machine_check_print_event_info(struct machine_check_event *evt,
> - bool user_mode)
> + bool user_mode, bool in_guest)
> {
> const char *level, *sevstr, *subtype;
> static const char *mc_ue_types[] = {
> @@ -387,7 +387,9 @@ void machine_check_print_event_info(struct machine_check_event *evt,
> evt->disposition == MCE_DISPOSITION_RECOVERED ?
> "Recovered" : "Not recovered");
>
> - if (user_mode) {
> + if (in_guest) {
> + printk("%s Guest NIP: %016llx\n", evt->srr0);
> + } else if (user_mode) {
> printk("%s NIP: [%016llx] PID: %d Comm: %s\n", level,
> evt->srr0, current->pid, current->comm);
> } else {
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index d8bf05a..81cba4b 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1216,7 +1216,7 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
> case BOOK3S_INTERRUPT_MACHINE_CHECK:
> /* Print the MCE event to host console. */
> - machine_check_print_event_info(&vcpu->arch.mce_evt, false);
> + machine_check_print_event_info(&vcpu->arch.mce_evt, false, true);
>
> /*
> * If the guest can do FWNMI, exit to userspace so it can
> @@ -1406,7 +1406,7 @@ static int kvmppc_handle_nested_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
> /* Pass the machine check to the L1 guest */
> r = RESUME_HOST;
> /* Print the MCE event to host console. */
> - machine_check_print_event_info(&vcpu->arch.mce_evt, false);
> + machine_check_print_event_info(&vcpu->arch.mce_evt, false, true);
> break;
> /*
> * We get these next two if the guest accesses a page which it thinks
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 79586f1..05c85be 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -587,7 +587,7 @@ int opal_machine_check(struct pt_regs *regs)
> evt.version);
> return 0;
> }
> - machine_check_print_event_info(&evt, user_mode(regs));
> + machine_check_print_event_info(&evt, user_mode(regs), false);
>
> if (opal_recover_mce(regs, &evt))
> return 1;
>
--
Regards,
Aravinda
^ permalink raw reply
* Re: [PATCH] powerpc: Make PPC_64K_PAGES depend on only 44x or PPC_BOOK3S_64
From: Michael Ellerman @ 2019-02-20 9:23 UTC (permalink / raw)
To: Scott Wood, Christophe Leroy, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <5e552f3d0c69890d5beb88475c8f9ba7a76bf64b.camel@buserror.net>
Scott Wood <oss@buserror.net> writes:
> On Wed, 2019-02-20 at 01:14 +1100, Michael Ellerman wrote:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>> > On 02/08/2019 12:34 PM, Michael Ellerman wrote:
>> > > In commit 7820856a4fcd ("powerpc/mm/book3e/64: Remove unsupported
>> > > 64Kpage size from 64bit booke") we dropped the 64K page size support
>> > > from the 64-bit nohash (Book3E) code.
>> > >
>> > > But we didn't update the dependencies of the PPC_64K_PAGES option,
>> > > meaning a randconfig can still trigger this code and cause a build
>> > > breakage, eg:
>> > > arch/powerpc/include/asm/nohash/64/pgtable.h:14:2: error: #error
>> > > "Page size not supported"
>> > > arch/powerpc/include/asm/nohash/mmu-book3e.h:275:2: error: #error
>> > > Unsupported page size
>> > >
>> > > So remove PPC_BOOK3E_64 from the dependencies. This also means we
>> > > don't need to worry about PPC_FSL_BOOK3E, because that was just trying
>> > > to prevent the PPC_BOOK3E_64=y && PPC_FSL_BOOK3E=y case.
>> >
>> > Does it means some cleanup could be done, for instance:
>> >
>> > arch/powerpc/include/asm/nohash/64/pgalloc.h:#ifndef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/include/asm/nohash/64/pgalloc.h:#endif /*
>> > CONFIG_PPC_64K_PAGES */
>> > arch/powerpc/include/asm/nohash/64/pgtable.h:#ifdef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/include/asm/nohash/64/slice.h:#ifdef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/include/asm/nohash/64/slice.h:#else /* CONFIG_PPC_64K_PAGES
>> > */
>> > arch/powerpc/include/asm/nohash/64/slice.h:#endif /*
>> > !CONFIG_PPC_64K_PAGES */
>> > arch/powerpc/include/asm/nohash/pte-book3e.h:#ifdef CONFIG_PPC_64K_PAGES
>> >
>> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
>> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
>> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
>> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
>> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
>>
>> Probably.
>>
>> Some of the FSL chips do support 64K pages at least according to some
>> datasheets. I don't know what would be required to get it working, or if
>> it even works in practice.
>>
>> So it would be nice to get 64K working on those chips, but probably no
>> one has time or motivation to do it. In which case yeah all that code
>> should be removed.
>
> The primary TLB (TLB0) on these chips only supports 4K pages. TLB1 supports
> many different sizes but is much smaller, hardware tablewalk only loads into
> TLB0, etc.
Aha thanks. I wondered if there was some reason for it.
So that makes it sound much less interesting, meaning all that 64K page
code should go.
cheers
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/64s: Better printing of machine check info for guest MCEs
From: Mahesh J Salgaonkar @ 2019-02-20 9:35 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <20190220010623.GE5353@blackberry>
On 2019-02-20 12:06:23 Wed, Paul Mackerras wrote:
> This adds an "in_guest" parameter to machine_check_print_event_info()
> so that we can avoid trying to translate guest NIP values into
> symbolic form using the host kernel's symbol table.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
> arch/powerpc/include/asm/mce.h | 2 +-
> arch/powerpc/kernel/mce.c | 8 +++++---
> arch/powerpc/kvm/book3s_hv.c | 4 ++--
> arch/powerpc/platforms/powernv/opal.c | 2 +-
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index a8b8903..17996bc 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -209,7 +209,7 @@ extern int get_mce_event(struct machine_check_event *mce, bool release);
> extern void release_mce_event(void);
> extern void machine_check_queue_event(void);
> extern void machine_check_print_event_info(struct machine_check_event *evt,
> - bool user_mode);
> + bool user_mode, bool in_guest);
> #ifdef CONFIG_PPC_BOOK3S_64
> void flush_and_reload_slb(void);
> #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index bd933a7..d01b690 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -301,13 +301,13 @@ static void machine_check_process_queued_event(struct irq_work *work)
> while (__this_cpu_read(mce_queue_count) > 0) {
> index = __this_cpu_read(mce_queue_count) - 1;
> evt = this_cpu_ptr(&mce_event_queue[index]);
> - machine_check_print_event_info(evt, false);
> + machine_check_print_event_info(evt, false, false);
> __this_cpu_dec(mce_queue_count);
> }
> }
>
> void machine_check_print_event_info(struct machine_check_event *evt,
> - bool user_mode)
> + bool user_mode, bool in_guest)
> {
> const char *level, *sevstr, *subtype;
> static const char *mc_ue_types[] = {
> @@ -387,7 +387,9 @@ void machine_check_print_event_info(struct machine_check_event *evt,
> evt->disposition == MCE_DISPOSITION_RECOVERED ?
> "Recovered" : "Not recovered");
>
> - if (user_mode) {
> + if (in_guest) {
> + printk("%s Guest NIP: %016llx\n", evt->srr0);
Missing 'level' argument. This should be:
printk("%s Guest NIP: %016llx\n", level, evt->srr0);
Rest looks fine to me.
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: Simplify machine check handling
From: Mahesh J Salgaonkar @ 2019-02-20 9:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <20190220010550.GD5353@blackberry>
On 2019-02-20 12:05:50 Wed, Paul Mackerras wrote:
> This makes the handling of machine check interrupts that occur inside
> a guest simpler and more robust, with less done in assembler code and
> in real mode.
>
> Now, when a machine check occurs inside a guest, we always get the
> machine check event struct and put a copy in the vcpu struct for the
> vcpu where the machine check occurred. We no longer call
> machine_check_queue_event() from kvmppc_realmode_mc_power7(), because
> on POWER8, when a vcpu is running on an offline secondary thread and
> we call machine_check_queue_event(), that calls irq_work_queue(),
> which doesn't work because the CPU is offline, but instead triggers
> the WARN_ON(lazy_irq_pending()) in pnv_smp_cpu_kill_self() (which
> fires again and again because nothing clears the condition).
>
> All that machine_check_queue_event() actually does is to cause the
> event to be printed to the console. For a machine check occurring in
> the guest, we now print the event in kvmppc_handle_exit_hv()
> instead.
>
> The assembly code at label machine_check_realmode now just calls C
> code and then continues exiting the guest. We no longer either
> synthesize a machine check for the guest in assembly code or return
> to the guest without a machine check.
>
> The code in kvmppc_handle_exit_hv() is extended to handle the case
> where the guest is not FWNMI-capable. In that case we now always
> synthesize a machine check interrupt for the guest. Previously, if
> the host thinks it has recovered the machine check fully, it would
> return to the guest without any notification that the machine check
> had occurred. If the machine check was caused by some action of the
> guest (such as creating duplicate SLB entries), it is much better to
> tell the guest that it has caused a problem. Therefore we now always
> generate a machine check interrupt for guests that are not
> FWNMI-capable.
Looks good to me.
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [PATCH v3 1/7] dump_stack: Support adding to the dump stack arch description
From: Michael Ellerman @ 2019-02-20 9:47 UTC (permalink / raw)
To: Andrea Parri, Petr Mladek
Cc: linux-arch, sergey.senozhatsky, linux-kernel, Steven Rostedt,
linuxppc-dev, tj, akpm, dyoung
In-Reply-To: <20190219233925.GA5648@andrea>
Andrea Parri <andrea.parri@amarulasolutions.com> writes:
> On Mon, Feb 11, 2019 at 03:38:59PM +0100, Petr Mladek wrote:
>> On Mon 2019-02-11 13:50:35, Andrea Parri wrote:
>> > On Thu, Feb 07, 2019 at 11:46:29PM +1100, Michael Ellerman wrote:
>> > > Arch code can set a "dump stack arch description string" which is
>> > > displayed with oops output to describe the hardware platform.
>> > >
>> > > It is useful to initialise this as early as possible, so that an early
>> > > oops will have the hardware description.
>> > >
>> > > However in practice we discover the hardware platform in stages, so it
>> > > would be useful to be able to incrementally fill in the hardware
>> > > description as we discover it.
>> > >
>> > > This patch adds that ability, by creating dump_stack_add_arch_desc().
>> > >
>> > > If there is no existing string it behaves exactly like
>> > > dump_stack_set_arch_desc(). However if there is an existing string it
>> > > appends to it, with a leading space.
>> > >
>> > > This makes it easy to call it multiple times from different parts of the
>> > > code and get a reasonable looking result.
>> > >
>> > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> > > ---
>> > > include/linux/printk.h | 5 ++++
>> > > lib/dump_stack.c | 58 ++++++++++++++++++++++++++++++++++++++++++
>> > > 2 files changed, 63 insertions(+)
>> > >
>> > > v3: No change, just widened Cc list.
>> > >
>> > > v2: Add a smp_wmb() and comment.
>> > >
>> > > v1 is here for reference https://lore.kernel.org/lkml/1430824337-15339-1-git-send-email-mpe@ellerman.id.au/
>> > >
>> > > I'll take this series via the powerpc tree if no one minds?
>> > >
>> > >
>> > > diff --git a/include/linux/printk.h b/include/linux/printk.h
>> > > index 77740a506ebb..d5fb4f960271 100644
>> > > --- a/include/linux/printk.h
>> > > +++ b/include/linux/printk.h
>> > > @@ -198,6 +198,7 @@ u32 log_buf_len_get(void);
>> > > void log_buf_vmcoreinfo_setup(void);
>> > > void __init setup_log_buf(int early);
>> > > __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
>> > > +__printf(1, 2) void dump_stack_add_arch_desc(const char *fmt, ...);
>> > > void dump_stack_print_info(const char *log_lvl);
>> > > void show_regs_print_info(const char *log_lvl);
>> > > extern asmlinkage void dump_stack(void) __cold;
>> > > @@ -256,6 +257,10 @@ static inline __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...)
>> > > {
>> > > }
>> > >
>> > > +static inline __printf(1, 2) void dump_stack_add_arch_desc(const char *fmt, ...)
>> > > +{
>> > > +}
>> > > +
>> > > static inline void dump_stack_print_info(const char *log_lvl)
>> > > {
>> > > }
>> > > diff --git a/lib/dump_stack.c b/lib/dump_stack.c
>> > > index 5cff72f18c4a..69b710ff92b5 100644
>> > > --- a/lib/dump_stack.c
>> > > +++ b/lib/dump_stack.c
>> > > @@ -35,6 +35,64 @@ void __init dump_stack_set_arch_desc(const char *fmt, ...)
>> > > va_end(args);
>> > > }
>> > >
>> > > +/**
>> > > + * dump_stack_add_arch_desc - add arch-specific info to show with task dumps
>> > > + * @fmt: printf-style format string
>> > > + * @...: arguments for the format string
>> > > + *
>> > > + * See dump_stack_set_arch_desc() for why you'd want to use this.
>> > > + *
>> > > + * This version adds to any existing string already created with either
>> > > + * dump_stack_set_arch_desc() or dump_stack_add_arch_desc(). If there is an
>> > > + * existing string a space will be prepended to the passed string.
>> > > + */
>> > > +void __init dump_stack_add_arch_desc(const char *fmt, ...)
>> > > +{
>> > > + va_list args;
>> > > + int pos, len;
>> > > + char *p;
>> > > +
>> > > + /*
>> > > + * If there's an existing string we snprintf() past the end of it, and
>> > > + * then turn the terminating NULL of the existing string into a space
>> > > + * to create one string separated by a space.
>> > > + *
>> > > + * If there's no existing string we just snprintf() to the buffer, like
>> > > + * dump_stack_set_arch_desc(), but without calling it because we'd need
>> > > + * a varargs version.
>> > > + */
>> > > + len = strnlen(dump_stack_arch_desc_str, sizeof(dump_stack_arch_desc_str));
>> > > + pos = len;
>> > > +
>> > > + if (len)
>> > > + pos++;
>> > > +
>> > > + if (pos >= sizeof(dump_stack_arch_desc_str))
>> > > + return; /* Ran out of space */
>> > > +
>> > > + p = &dump_stack_arch_desc_str[pos];
>> > > +
>> > > + va_start(args, fmt);
>> > > + vsnprintf(p, sizeof(dump_stack_arch_desc_str) - pos, fmt, args);
>> > > + va_end(args);
>> > > +
>> > > + if (len) {
>> > > + /*
>> > > + * Order the stores above in vsnprintf() vs the store of the
>> > > + * space below which joins the two strings. Note this doesn't
>> > > + * make the code truly race free because there is no barrier on
>> > > + * the read side. ie. Another CPU might load the uninitialised
>> > > + * tail of the buffer first and then the space below (rather
>> > > + * than the NULL that was there previously), and so print the
>> > > + * uninitialised tail. But the whole string lives in BSS so in
>> > > + * practice it should just see NULLs.
>> >
>> > The comment doesn't say _why_ we need to order these stores: IOW, what
>> > will or can go wrong without this order? This isn't clear to me.
>> >
>> > Another good practice when adding smp_*-constructs (as discussed, e.g.,
>> > at KS'18) is to indicate the matching construct/synch. mechanism.
>>
>> Yes, one barrier without a counter-part is suspicious.
>
> As is this silence...,
>
> Michael, what happened to this patch? did you submit a new version?
No, I'm just busy, it's the merge window next week :)
I thought the comment was pretty clear, if the stores are observed out
of order we might print the uninitialised tail.
And the barrier on the read side would need to be in printk somewhere,
which is obviously unpleasant.
>> If the parallel access is really needed then we could define the
>> current length as atomic_t and use:
>>
>> + atomic_cmpxchg() to reserve the space for the string
>> + %*s to limit the printed length
>>
>> In the worst case, we would print an incomplete string.
>> See below for a sample code.
>
> Seems worth exploring, IMO; but I'd like to first hear _clear about
> the _intended semantics (before digging into alternatives)...
It is not my intention to support concurrent updates of the string. The
idea is you setup the string early in boot.
The concern with a concurrent reader is simply that the string is dumped
in the panic path, and you never really know when you're going to panic.
Even if you only write to the string before doing SMP bringup you might
still have another CPU go rogue and panic before then.
But I probably should have just not added the barrier, it's over
paranoid and will almost certainly never matter in practice.
cheers
^ permalink raw reply
* Re: [PATCH 2/6] powerpc sstep: Add darn instruction emulation
From: Sandipan Das @ 2019-02-20 9:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: paulus, naveen.n.rao, linuxppc-dev, anton, ravi.bangoria
In-Reply-To: <874l8z8eqz.fsf@concordia.ellerman.id.au>
Hi Michael,
On 20/02/19 6:20 AM, Michael Ellerman wrote:
> Sandipan Das <sandipan@linux.ibm.com> writes:
>
>> This adds emulation support for the following integer instructions:
>> * Deliver A Random Number (darn)
>
> This doesn't build with old binutils. We need to support old binutils.
>
> {standard input}:4343: Error: Unrecognized opcode: `darn'
>
>
> You need to use PPC_DARN().
>
> cheers
>
> [...]
Thanks for pointing these out. Will post v2 with the required changes.
- Sandipan
^ permalink raw reply
* Re: [PATCHv6 3/4] pci: layerscape: Add the EP mode support.
From: Lorenzo Pieralisi @ 2019-02-20 10:06 UTC (permalink / raw)
To: Xiaowei Bao
Cc: mark.rutland@arm.com, Roy Zang, arnd@arndb.de,
devicetree@vger.kernel.org, gregkh@linuxfoundation.org,
kstewart@linuxfoundation.org, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
kishon@ti.com, M.h. Lian, robh+dt@kernel.org,
cyrille.pitchen@free-electrons.com,
linux-arm-kernel@lists.infradead.org, pombredanne@nexb.com,
bhelgaas@google.com, Leo Li, shawnguo@kernel.org,
shawn.lin@rock-chips.com, Mingkai Hu
In-Reply-To: <HE1PR04MB3306890D5A1656EC818521A2F57D0@HE1PR04MB3306.eurprd04.prod.outlook.com>
On Wed, Feb 20, 2019 at 03:09:01AM +0000, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Sent: 2019年2月19日 19:27
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; arnd@arndb.de; gregkh@linuxfoundation.org; M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>; kstewart@linuxfoundation.org; cyrille.pitchen@free-electrons.com; pombredanne@nexb.com; shawn.lin@rock-chips.com; linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCHv6 3/4] pci: layerscape: Add the EP mode support.
>
> On Tue, Jan 22, 2019 at 02:33:27PM +0800, Xiaowei Bao wrote:
> > Add the PCIe EP mode support for layerscape platform.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > Reviewed-by: Minghuan Lian <minghuan.lian@nxp.com>
> > Reviewed-by: Zhiqiang Hou <zhiqiang.hou@nxp.com>
> > Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com>
> > ---
> > depends on:
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> > chwork.kernel.org%2Fproject%2Flinux-pci%2Flist%2F%3Fseries%3D66177&
> > ;data=02%7C01%7Cxiaowei.bao%40nxp.com%7C6f8772ba47c74d8ee0aa08d6965d3e
> > b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636861724572611193&
> > ;sdata=b3Acj0fu7c9vSoHe9VzeAkEMbMkpyfYPsXtf6fA8Flk%3D&reserved=0
> >
> > v2:
> > - remove the EP mode check function.
> > v3:
> > - modif the return value when enter default case.
> > v4:
> > - no change.
> > v5:
> > - no change.
> > v6:
> > - modify the code base on the submit patch of the EP framework.
>
> Can I apply this series to my pci/endpoint branch (where I queued Kishon's EP features rework patches) ? Can you check please ?
> [Xiaowei Bao] of course, in my patch, I found a compile warning, but
> this series patch have approved by you, I don't know how to do, the
> compile warning: " struct pci_epc *epc = ep->epc;" in
> "ls_pcie_ep_init" function, I am so sorry, could you help me remove
> this code, thanks a lot.
If you want me to apply your patches you need to rebase them against
my pci/endpoint branch and make sure the code is correct, I have
applied your previous series but as you know it failed because it
depends on Kishon's clean-up series.
So rebase your code against my pci/endpoint branch, make sure it
compiles with no warnings, test it and send a v7.
Thanks,
Lorenzo
> Thanks,
> Lorenzo
>
> > drivers/pci/controller/dwc/Makefile | 2 +-
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 157
> > ++++++++++++++++++++++++
> > 2 files changed, 158 insertions(+), 1 deletions(-) create mode
> > 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
> >
> > diff --git a/drivers/pci/controller/dwc/Makefile
> > b/drivers/pci/controller/dwc/Makefile
> > index 7bcdcdf..b5f3b83 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> > obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> > obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> > obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> > -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> > +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
> > a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > new file mode 100644
> > index 0000000..ddc2dbb
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCIe controller EP driver for Freescale Layerscape SoCs
> > + *
> > + * Copyright (C) 2018 NXP Semiconductor.
> > + *
> > + * Author: Xiaowei Bao <xiaowei.bao@nxp.com> */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/of_pci.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_address.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/resource.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
> > +
> > +struct ls_pcie_ep {
> > + struct dw_pcie *pci;
> > +};
> > +
> > +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > +
> > +static int ls_pcie_establish_link(struct dw_pcie *pci) {
> > + return 0;
> > +}
> > +
> > +static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > + .start_link = ls_pcie_establish_link, };
> > +
> > +static const struct of_device_id ls_pcie_ep_of_match[] = {
> > + { .compatible = "fsl,ls-pcie-ep",},
> > + { },
> > +};
> > +
> > +static const struct pci_epc_features ls_pcie_epc_features = {
> > + .linkup_notifier = false,
> > + .msi_capable = true,
> > + .msix_capable = false,
> > +};
> > +
> > +static const struct pci_epc_features* ls_pcie_ep_get_features(struct
> > +dw_pcie_ep *ep) {
> > + return &ls_pcie_epc_features;
> > +}
> > +
> > +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + struct pci_epc *epc = ep->epc;
> > + enum pci_barno bar;
> > +
> > + for (bar = BAR_0; bar <= BAR_5; bar++)
> > + dw_pcie_ep_reset_bar(pci, bar);
> > +}
> > +
> > +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > + enum pci_epc_irq_type type, u16 interrupt_num) {
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + switch (type) {
> > + case PCI_EPC_IRQ_LEGACY:
> > + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> > + case PCI_EPC_IRQ_MSI:
> > + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> > + case PCI_EPC_IRQ_MSIX:
> > + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> > + default:
> > + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static struct dw_pcie_ep_ops pcie_ep_ops = {
> > + .ep_init = ls_pcie_ep_init,
> > + .raise_irq = ls_pcie_ep_raise_irq,
> > + .get_features = ls_pcie_ep_get_features, };
> > +
> > +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> > + struct platform_device *pdev)
> > +{
> > + struct dw_pcie *pci = pcie->pci;
> > + struct device *dev = pci->dev;
> > + struct dw_pcie_ep *ep;
> > + struct resource *res;
> > + int ret;
> > +
> > + ep = &pci->ep;
> > + ep->ops = &pcie_ep_ops;
> > +
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> > + if (!res)
> > + return -EINVAL;
> > +
> > + ep->phys_base = res->start;
> > + ep->addr_size = resource_size(res);
> > +
> > + ret = dw_pcie_ep_init(ep);
> > + if (ret) {
> > + dev_err(dev, "failed to initialize endpoint\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct dw_pcie *pci;
> > + struct ls_pcie_ep *pcie;
> > + struct resource *dbi_base;
> > + int ret;
> > +
> > + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> > + if (!pcie)
> > + return -ENOMEM;
> > +
> > + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> > + if (!pci)
> > + return -ENOMEM;
> > +
> > + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > + if (IS_ERR(pci->dbi_base))
> > + return PTR_ERR(pci->dbi_base);
> > +
> > + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> > + pci->dev = dev;
> > + pci->ops = &ls_pcie_ep_ops;
> > + pcie->pci = pci;
> > +
> > + platform_set_drvdata(pdev, pcie);
> > +
> > + ret = ls_add_pcie_ep(pcie, pdev);
> > +
> > + return ret;
> > +}
> > +
> > +static struct platform_driver ls_pcie_ep_driver = {
> > + .driver = {
> > + .name = "layerscape-pcie-ep",
> > + .of_match_table = ls_pcie_ep_of_match,
> > + .suppress_bind_attrs = true,
> > + },
> > +};
> > +builtin_platform_driver_probe(ls_pcie_ep_driver, ls_pcie_ep_probe);
> > --
> > 1.7.1
> >
^ permalink raw reply
* Re: [PATCH] powerpc/64s: Fix possible corruption on big endian due to pgd/pud_present()
From: Michael Ellerman @ 2019-02-20 11:18 UTC (permalink / raw)
To: Segher Boessenkool
Cc: erhard_f, jack, linuxppc-dev, linux-kernel, linux-mm,
aneesh.kumar
In-Reply-To: <20190219201539.GT14180@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Mon, Feb 18, 2019 at 11:49:18AM +1100, Michael Ellerman wrote:
>> Balbir Singh <bsingharora@gmail.com> writes:
>> > Fair enough, my point was that the compiler can help out. I'll see what
>> > -Wconversion finds on my local build :)
>>
>> I get about 43MB of warnings here :)
>
> Yes, -Wconversion complains about a lot of things that are idiomatic C.
> There is a reason -Wconversion is not in -Wall or -Wextra.
Actually a lot of those go away when I add -Wno-sign-conversion.
And what's left seems mostly reasonable, they all indicate the
possibility of a bug I think.
In fact this works and would have caught the bug:
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index d8c8d7c9df15..3114e3f368e2 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -904,7 +904,12 @@ static inline int pud_none(pud_t pud)
static inline int pud_present(pud_t pud)
{
+ __diag_push();
+ __diag_warn(GCC, 8, "-Wconversion", "ulong -> int");
+
return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PRESENT));
+
+ __diag_pop();
}
extern struct page *pud_page(pud_t pud);
Obviously we're not going to instrument every function like that. But we
could start instrumenting particular files.
cheers
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Russell Currey @ 2019-02-20 11:18 UTC (permalink / raw)
To: Akshay Adiga, Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20190220060353.GA21952@aks.ibm>
On Wed, 2019-02-20 at 11:34 +0530, Akshay Adiga wrote:
> On Tue, Feb 19, 2019 at 02:21:04PM +1000, Nicholas Piggin wrote:
> > Michael Ellerman's on February 8, 2019 11:04 am:
> > > Nicholas Piggin <npiggin@gmail.com> writes:
> > > > Russell Currey's on February 6, 2019 4:28 pm:
> > > > > Without restoring the IAMR after idle, execution prevention
> > > > > on POWER9
> > > > > with Radix MMU is overwritten and the kernel can freely
> > > > > execute userspace without
> > > > > faulting.
> > > > >
> > > > > This is necessary when returning from any stop state that
> > > > > modifies user
> > > > > state, as well as hypervisor state.
> > > > >
> > > > > To test how this fails without this patch, load the lkdtm
> > > > > driver and
> > > > > do the following:
> > > > >
> > > > > echo EXEC_USERSPACE > /sys/kernel/debug/provoke-
> > > > > crash/DIRECT
> > > > >
> > > > > which won't fault, then boot the kernel with powersave=off,
> > > > > where it
> > > > > will fault. Applying this patch will fix this.
> > > > >
> > > > > Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel
> > > > > execution of user
> > > > > space")
> > > > > Cc: <stable@vger.kernel.org>
> > > > > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > > >
> > > > Good catch and debugging. This really should be a quirk, we
> > > > don't want
> > > > to have to restore this thing on a thread switch.
> > >
> > > I'm not sure I follow. We don't context switch it on Radix, but
> > > we do
> > > on hash if pkeys are enabled.
> >
> > Badly worded, I mean a hardware quirk. It should follow thread
> > switches. Still, avoiding it for the no-loss case is better than
> > nothing. We can just revisit it as an optimization if future
> > hardware does not require the restore.
>
> Apparently, the POWER9 Processor User’s Manual v2.0 documents that
> IAMR can be lost, and that is not just the end.
>
> Pasting excerpt from "Section 23.5.9.2 State Loss and
> Restoration,Page 309"
>
> On the POWER9 core, the only state that can be lost for
> Stop levels less than four, when PSSCR[ESL] = ‘1’ are the
> following SPRs: CR, FPSCR, VSCR, XER, DSCR, AMR, IAMR, UAMOR,
> AMOR, DAWR, DAWRX.
>
> My observation is that AMOR is being used in kernel as of today
> and AMOR is also lost (recreated in similar scenarios where
> IAMR is lost).
>
I can add AMOR to this patch (or you can send a patch, either way).
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Russell Currey @ 2019-02-20 11:20 UTC (permalink / raw)
To: Akshay Adiga; +Cc: linuxppc-dev
In-Reply-To: <20190220085841.GA11633@aks.ibm>
On Wed, 2019-02-20 at 14:28 +0530, Akshay Adiga wrote:
> On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
> > Without restoring the IAMR after idle, execution prevention on
> > POWER9
> > with Radix MMU is overwritten and the kernel can freely execute
> > userspace without
> > faulting.
> >
> > This is necessary when returning from any stop state that modifies
> > user
> > state, as well as hypervisor state.
> >
> > To test how this fails without this patch, load the lkdtm driver
> > and
> > do the following:
> >
> > echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
> >
> > which won't fault, then boot the kernel with powersave=off, where
> > it
> > will fault. Applying this patch will fix this.
> >
> > Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
> > user
> > space")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > arch/powerpc/include/asm/cpuidle.h | 1 +
> > arch/powerpc/kernel/asm-offsets.c | 1 +
> > arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> > 3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/cpuidle.h
> > b/arch/powerpc/include/asm/cpuidle.h
> > index 43e5f31fe64d..ad67dbe59498 100644
> > --- a/arch/powerpc/include/asm/cpuidle.h
> > +++ b/arch/powerpc/include/asm/cpuidle.h
> > @@ -77,6 +77,7 @@ struct stop_sprs {
> > u64 mmcr1;
> > u64 mmcr2;
> > u64 mmcra;
> > + u64 iamr;
> > };
> >
> > #define PNV_IDLE_NAME_LEN 16
> > diff --git a/arch/powerpc/kernel/asm-offsets.c
> > b/arch/powerpc/kernel/asm-offsets.c
> > index 9ffc72ded73a..10e0314c2b0d 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -774,6 +774,7 @@ int main(void)
> > STOP_SPR(STOP_MMCR1, mmcr1);
> > STOP_SPR(STOP_MMCR2, mmcr2);
> > STOP_SPR(STOP_MMCRA, mmcra);
> > + STOP_SPR(STOP_IAMR, iamr);
> > #endif
> >
> > DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> > diff --git a/arch/powerpc/kernel/idle_book3s.S
> > b/arch/powerpc/kernel/idle_book3s.S
> > index 7f5ac2e8581b..bb4f552f6c7e 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -200,6 +200,12 @@ pnv_powersave_common:
> > /* Continue saving state */
> > SAVE_GPR(2, r1)
> > SAVE_NVGPRS(r1)
> > +
> > +BEGIN_FTR_SECTION
> > + mfspr r5, SPRN_IAMR
> > + std r5, STOP_IAMR(r13)
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
> > mfcr r5
> > std r5,_CCR(r1)
> > std r1,PACAR1(r13)
> > @@ -924,6 +930,13 @@ BEGIN_FTR_SECTION
> > END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> > REST_NVGPRS(r1)
> > REST_GPR(2, r1)
> > +
> > +BEGIN_FTR_SECTION
> > + ld r4, STOP_IAMR(r13)
> > + mtspr SPRN_IAMR, r4
> > + isync
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
> > ld r4,PACAKMSR(r13)
> > ld r5,_LINK(r1)
> > ld r6,_CCR(r1)
> > @@ -946,6 +959,13 @@ pnv_wakeup_noloss:
> > BEGIN_FTR_SECTION
> > CHECK_HMI_INTERRUPT
> > END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> > +
> > +BEGIN_FTR_SECTION
> > + ld r4, STOP_IAMR(r13)
> > + mtspr SPRN_IAMR, r4
> > + isync
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
>
> pnv_wakeup_noloss gets called from two paths:
> 1) cpu wakes up at 0x100 (ESL=EC=1)
> 2) cpu wakes up at next instruction (ESL=EC=0)
>
In v2 I drop it from noloss, is that still correct?
> We know for the fact that its not lost with ESL=EC=0 , so we
> should put it somewhere else.
>
> I would like to put the restore code in
> pnv_restore_hyp_resource_arch300
> it already has some work arounds we can add another.
>
> By this time we cr3 has comparison of SRR1[46:47] with 2
>
> 542 pnv_restore_hyp_resource_arch300:
> 543 /*
> 544 * Workaround for POWER9, if we lost resources, the
> ERAT
> 545 * might have been mixed up and needs flushing. We also
> need
> 546 * to reload MMCR0 (see comment above). We also need to
> set
> 547 * then clear bit 60 in MMCRA to ensure the PMU starts
> running.
> 548 */
> 549 blt cr3,1f
> 550 BEGIN_FTR_SECTION
> 551 PPC_INVALIDATE_ERAT
> 552 ld r1,PACAR1(r13)
> 553 ld r4,_MMCR0(r1)
> 554 mtspr SPRN_MMCR0,r4
> 555 END_FTR_SECTION_IFCLR(CPU_FTR_POWER9_DD2_1)
> 556 mfspr r4,SPRN_MMCRA
> 557 ori r4,r4,(1 << (63-60))
> 558 mtspr SPRN_MMCRA,r4
> 559 xori r4,r4,(1 << (63-60))
> 560 mtspr SPRN_MMCRA,r4
> + 561 BEGIN_FTR_SECTION
> + 562 ld r4,STOP_IAMR(r13)
> + 563 mtspr SPRN_IAMR,r4
> + 564 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> 565 1:
> 566 /*
>
> I have a patchset to handle both AMOR and IAMR,
> need to test it on power8 before posting.
If you think that's better, go ahead.
>
>
> > ld r4,PACAKMSR(r13)
> > ld r5,_NIP(r1)
> > ld r6,_CCR(r1)
> > --
> > 2.20.1
> >
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Russell Currey @ 2019-02-20 11:25 UTC (permalink / raw)
To: Akshay Adiga, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20190220071503.GA4708@aks.ibm>
On Wed, 2019-02-20 at 12:45 +0530, Akshay Adiga wrote:
> On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
> > Without restoring the IAMR after idle, execution prevention on
> > POWER9
> > with Radix MMU is overwritten and the kernel can freely execute
> > userspace without
> > faulting.
> >
> > This is necessary when returning from any stop state that modifies
> > user
> > state, as well as hypervisor state.
> >
> > To test how this fails without this patch, load the lkdtm driver
> > and
> > do the following:
> >
> > echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
> >
> > which won't fault, then boot the kernel with powersave=off, where
> > it
> > will fault. Applying this patch will fix this.
> >
> > Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
> > user
> > space")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > arch/powerpc/include/asm/cpuidle.h | 1 +
> > arch/powerpc/kernel/asm-offsets.c | 1 +
> > arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> > 3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/cpuidle.h
> > b/arch/powerpc/include/asm/cpuidle.h
> > index 43e5f31fe64d..ad67dbe59498 100644
> > --- a/arch/powerpc/include/asm/cpuidle.h
> > +++ b/arch/powerpc/include/asm/cpuidle.h
> > @@ -77,6 +77,7 @@ struct stop_sprs {
> > u64 mmcr1;
> > u64 mmcr2;
> > u64 mmcra;
> > + u64 iamr;
> > };
> >
> > #define PNV_IDLE_NAME_LEN 16
> > diff --git a/arch/powerpc/kernel/asm-offsets.c
> > b/arch/powerpc/kernel/asm-offsets.c
> > index 9ffc72ded73a..10e0314c2b0d 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -774,6 +774,7 @@ int main(void)
> > STOP_SPR(STOP_MMCR1, mmcr1);
> > STOP_SPR(STOP_MMCR2, mmcr2);
> > STOP_SPR(STOP_MMCRA, mmcra);
> > + STOP_SPR(STOP_IAMR, iamr);
> > #endif
> >
> > DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> > diff --git a/arch/powerpc/kernel/idle_book3s.S
> > b/arch/powerpc/kernel/idle_book3s.S
> > index 7f5ac2e8581b..bb4f552f6c7e 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -200,6 +200,12 @@ pnv_powersave_common:
> > /* Continue saving state */
> > SAVE_GPR(2, r1)
> > SAVE_NVGPRS(r1)
> > +
> > +BEGIN_FTR_SECTION
> > + mfspr r5, SPRN_IAMR
> > + std r5, STOP_IAMR(r13)
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
>
> Are we trying to add for both power8 and power9 ?
> power9 would be CPU_FTR_ARCH_300.
>
If I recall correctly I had this at P9 only but for some reason I
changed it - the reason is probably just that I had it confused for
something else. Michael can you confirm this should be P9 only?
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: Context switch IAMR on Power9
From: Russell Currey @ 2019-02-20 11:25 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: paulus, kvm-ppc
In-Reply-To: <20190220085500.29837-1-mpe@ellerman.id.au>
On Wed, 2019-02-20 at 19:55 +1100, Michael Ellerman wrote:
> kvmhv_p9_guest_entry() implements a fast-path guest entry for Power9
> when guest and host are both running with the Radix MMU.
>
> Currently in that path we don't save the host AMR (Authority Mask
> Register) value, and we always restore 0 on return to the host. That
> is OK at the moment because the AMR is not used for storage keys with
> the Radix MMU.
>
> However we plan to start using the AMR on Radix to prevent the kernel
> from reading/writing to userspace outside of copy_to/from_user(). In
> order to make that work we need to save/restore the AMR value.
>
> We only restore the value if it is different from the guest value,
> which is already in the register when we exit to the host. This
> should
> mean we rarely need to actually restore the value when running a
> modern Linux as a guest, because it will be using the same value as
> us.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Tested-by: Russell Currey <ruscur@russell.cc>
^ 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