* [PATCH 05/10] mm: replace get_vaddr_frames() write/force parameters with gup_flags
From: Lorenzo Stoakes @ 2016-10-13 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>
This patch removes the write and force parameters from get_vaddr_frames() and
replaces them with a gup_flags parameter to make the use of FOLL_FORCE explicit
in callers as use of this flag can result in surprising behaviour (and hence
bugs) within the mm subsystem.
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 3 ++-
drivers/media/platform/omap/omap_vout.c | 2 +-
drivers/media/v4l2-core/videobuf2-memops.c | 6 +++++-
include/linux/mm.h | 2 +-
mm/frame_vector.c | 13 ++-----------
5 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index aa92dec..fbd13fa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -488,7 +488,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev,
goto err_free;
}
- ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
+ ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
+ g2d_userptr->vec);
if (ret != npages) {
DRM_ERROR("failed to get user pages from userptr.\n");
if (ret < 0)
diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
index e668dde..a31b95c 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -214,7 +214,7 @@ static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
if (!vec)
return -ENOMEM;
- ret = get_vaddr_frames(virtp, 1, true, false, vec);
+ ret = get_vaddr_frames(virtp, 1, FOLL_WRITE, vec);
if (ret != 1) {
frame_vector_destroy(vec);
return -EINVAL;
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c b/drivers/media/v4l2-core/videobuf2-memops.c
index 3c3b517..1cd322e 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -42,6 +42,10 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
unsigned long first, last;
unsigned long nr;
struct frame_vector *vec;
+ unsigned int flags = FOLL_FORCE;
+
+ if (write)
+ flags |= FOLL_WRITE;
first = start >> PAGE_SHIFT;
last = (start + length - 1) >> PAGE_SHIFT;
@@ -49,7 +53,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
vec = frame_vector_create(nr);
if (!vec)
return ERR_PTR(-ENOMEM);
- ret = get_vaddr_frames(start & PAGE_MASK, nr, write, true, vec);
+ ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
if (ret < 0)
goto out_destroy;
/* We accept only complete set of PFNs */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ab538..5ff084f6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1305,7 +1305,7 @@ struct frame_vector {
struct frame_vector *frame_vector_create(unsigned int nr_frames);
void frame_vector_destroy(struct frame_vector *vec);
int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
- bool write, bool force, struct frame_vector *vec);
+ unsigned int gup_flags, struct frame_vector *vec);
void put_vaddr_frames(struct frame_vector *vec);
int frame_vector_to_pages(struct frame_vector *vec);
void frame_vector_to_pfns(struct frame_vector *vec);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 81b6749..db77dcb 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -11,10 +11,7 @@
* get_vaddr_frames() - map virtual addresses to pfns
* @start: starting user address
* @nr_frames: number of pages / pfns from start to map
- * @write: whether pages will be written to by the caller
- * @force: whether to force write access even if user mapping is
- * readonly. See description of the same argument of
- get_user_pages().
+ * @gup_flags: flags modifying lookup behaviour
* @vec: structure which receives pages / pfns of the addresses mapped.
* It should have space for@least nr_frames entries.
*
@@ -34,23 +31,17 @@
* This function takes care of grabbing mmap_sem as necessary.
*/
int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
- bool write, bool force, struct frame_vector *vec)
+ unsigned int gup_flags, struct frame_vector *vec)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
int ret = 0;
int err;
int locked;
- unsigned int gup_flags = 0;
if (nr_frames == 0)
return 0;
- if (write)
- gup_flags |= FOLL_WRITE;
- if (force)
- gup_flags |= FOLL_FORCE;
-
if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
nr_frames = vec->nr_allocated;
--
2.10.0
^ permalink raw reply related
* [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Lorenzo Stoakes @ 2016-10-13 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>
This patch removes the write and force parameters from get_user_pages_locked()
and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
explicit in callers as use of this flag can result in surprising behaviour (and
hence bugs) within the mm subsystem.
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
include/linux/mm.h | 2 +-
mm/frame_vector.c | 8 +++++++-
mm/gup.c | 12 +++---------
mm/nommu.c | 5 ++++-
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6adc4bc..27ab538 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages,
struct vm_area_struct **vmas);
long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages, int *locked);
+ unsigned int gup_flags, struct page **pages, int *locked);
long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 381bb07..81b6749 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -41,10 +41,16 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
int ret = 0;
int err;
int locked;
+ unsigned int gup_flags = 0;
if (nr_frames == 0)
return 0;
+ if (write)
+ gup_flags |= FOLL_WRITE;
+ if (force)
+ gup_flags |= FOLL_FORCE;
+
if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
nr_frames = vec->nr_allocated;
@@ -59,7 +65,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
vec->got_ref = true;
vec->is_pfns = false;
ret = get_user_pages_locked(start, nr_frames,
- write, force, (struct page **)(vec->ptrs), &locked);
+ gup_flags, (struct page **)(vec->ptrs), &locked);
goto out;
}
diff --git a/mm/gup.c b/mm/gup.c
index cfcb014..7a0d033 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -838,18 +838,12 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
* up_read(&mm->mmap_sem);
*/
long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages,
+ unsigned int gup_flags, struct page **pages,
int *locked)
{
- unsigned int flags = FOLL_TOUCH;
-
- if (write)
- flags |= FOLL_WRITE;
- if (force)
- flags |= FOLL_FORCE;
-
return __get_user_pages_locked(current, current->mm, start, nr_pages,
- pages, NULL, locked, true, flags);
+ pages, NULL, locked, true,
+ gup_flags | FOLL_TOUCH);
}
EXPORT_SYMBOL(get_user_pages_locked);
diff --git a/mm/nommu.c b/mm/nommu.c
index 7e27add..842cfdd 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,12 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
EXPORT_SYMBOL(get_user_pages);
long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages,
+ unsigned int gup_flags, struct page **pages,
int *locked)
{
+ int write = gup_flags & FOLL_WRITE;
+ int force = gup_flags & FOLL_FORCE;
+
return get_user_pages(start, nr_pages, write, force, pages, NULL);
}
EXPORT_SYMBOL(get_user_pages_locked);
--
2.10.0
^ permalink raw reply related
* [PATCH 03/10] mm: replace get_user_pages_unlocked() write/force parameters with gup_flags
From: Lorenzo Stoakes @ 2016-10-13 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>
This patch removes the write and force parameters from get_user_pages_unlocked()
and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
explicit in callers as use of this flag can result in surprising behaviour (and
hence bugs) within the mm subsystem.
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
arch/mips/mm/gup.c | 2 +-
arch/s390/mm/gup.c | 3 ++-
arch/sh/mm/gup.c | 3 ++-
arch/sparc/mm/gup.c | 3 ++-
arch/x86/mm/gup.c | 2 +-
drivers/media/pci/ivtv/ivtv-udma.c | 4 ++--
drivers/media/pci/ivtv/ivtv-yuv.c | 5 +++--
drivers/scsi/st.c | 5 ++---
drivers/video/fbdev/pvr2fb.c | 4 ++--
include/linux/mm.h | 2 +-
mm/gup.c | 14 ++++----------
mm/nommu.c | 11 ++---------
mm/util.c | 3 ++-
net/ceph/pagevec.c | 2 +-
14 files changed, 27 insertions(+), 36 deletions(-)
diff --git a/arch/mips/mm/gup.c b/arch/mips/mm/gup.c
index 42d124f..d8c3c15 100644
--- a/arch/mips/mm/gup.c
+++ b/arch/mips/mm/gup.c
@@ -287,7 +287,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,
- write, 0, pages);
+ pages, write ? FOLL_WRITE : 0);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/arch/s390/mm/gup.c b/arch/s390/mm/gup.c
index adb0c34..18d4107 100644
--- a/arch/s390/mm/gup.c
+++ b/arch/s390/mm/gup.c
@@ -266,7 +266,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
/* 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, write, 0, pages);
+ ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
+ write ? FOLL_WRITE : 0);
/* 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 40fa6c8..063c298 100644
--- a/arch/sh/mm/gup.c
+++ b/arch/sh/mm/gup.c
@@ -258,7 +258,8 @@ 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, write, 0, pages);
+ (end - start) >> PAGE_SHIFT, pages,
+ write ? FOLL_WRITE : 0);
/* 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 4e06750..cd0e32b 100644
--- a/arch/sparc/mm/gup.c
+++ b/arch/sparc/mm/gup.c
@@ -238,7 +238,8 @@ 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, write, 0, pages);
+ (end - start) >> PAGE_SHIFT, pages,
+ write ? FOLL_WRITE : 0);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/arch/x86/mm/gup.c b/arch/x86/mm/gup.c
index b8b6a60..0d4fb3e 100644
--- a/arch/x86/mm/gup.c
+++ b/arch/x86/mm/gup.c
@@ -435,7 +435,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
ret = get_user_pages_unlocked(start,
(end - start) >> PAGE_SHIFT,
- write, 0, pages);
+ pages, write ? FOLL_WRITE : 0);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
index 4769469..2c9232e 100644
--- a/drivers/media/pci/ivtv/ivtv-udma.c
+++ b/drivers/media/pci/ivtv/ivtv-udma.c
@@ -124,8 +124,8 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long ivtv_dest_addr,
}
/* Get user pages for DMA Xfer */
- err = get_user_pages_unlocked(user_dma.uaddr, user_dma.page_count, 0,
- 1, dma->map);
+ err = get_user_pages_unlocked(user_dma.uaddr, user_dma.page_count,
+ dma->map, FOLL_FORCE);
if (user_dma.page_count != err) {
IVTV_DEBUG_WARN("failed to map user pages, returned %d instead of %d\n",
diff --git a/drivers/media/pci/ivtv/ivtv-yuv.c b/drivers/media/pci/ivtv/ivtv-yuv.c
index b094054..f7299d3 100644
--- a/drivers/media/pci/ivtv/ivtv-yuv.c
+++ b/drivers/media/pci/ivtv/ivtv-yuv.c
@@ -76,11 +76,12 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct ivtv_user_dma *dma,
/* Get user pages for DMA Xfer */
y_pages = get_user_pages_unlocked(y_dma.uaddr,
- y_dma.page_count, 0, 1, &dma->map[0]);
+ y_dma.page_count, &dma->map[0], FOLL_FORCE);
uv_pages = 0; /* silence gcc. value is set and consumed only if: */
if (y_pages == y_dma.page_count) {
uv_pages = get_user_pages_unlocked(uv_dma.uaddr,
- uv_dma.page_count, 0, 1, &dma->map[y_pages]);
+ uv_dma.page_count, &dma->map[y_pages],
+ FOLL_FORCE);
}
if (y_pages != y_dma.page_count || uv_pages != uv_dma.page_count) {
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 7af5226..618422e 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4922,9 +4922,8 @@ static int sgl_map_user_pages(struct st_buffer *STbp,
res = get_user_pages_unlocked(
uaddr,
nr_pages,
- rw == READ,
- 0, /* don't force */
- pages);
+ pages,
+ rw == READ ? FOLL_WRITE : 0); /* don't force */
/* Errors and no page mapped should return here */
if (res < nr_pages)
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 3b1ca44..a2564ab 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -686,8 +686,8 @@ static ssize_t pvr2fb_write(struct fb_info *info, const char *buf,
if (!pages)
return -ENOMEM;
- ret = get_user_pages_unlocked((unsigned long)buf, nr_pages, WRITE,
- 0, pages);
+ ret = get_user_pages_unlocked((unsigned long)buf, nr_pages, pages,
+ FOLL_WRITE);
if (ret < nr_pages) {
nr_pages = ret;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2db98b6..6adc4bc 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1287,7 +1287,7 @@ long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages);
+ struct page **pages, unsigned int gup_flags);
int get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages);
diff --git a/mm/gup.c b/mm/gup.c
index 3d620dd..cfcb014 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -897,17 +897,10 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
* "force" parameter).
*/
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages)
+ struct page **pages, unsigned int gup_flags)
{
- unsigned int flags = FOLL_TOUCH;
-
- if (write)
- flags |= FOLL_WRITE;
- if (force)
- flags |= FOLL_FORCE;
-
return __get_user_pages_unlocked(current, current->mm, start, nr_pages,
- pages, flags);
+ pages, gup_flags | FOLL_TOUCH);
}
EXPORT_SYMBOL(get_user_pages_unlocked);
@@ -1525,7 +1518,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
start += nr << PAGE_SHIFT;
pages += nr;
- ret = get_user_pages_unlocked(start, nr_pages - nr, write, 0, pages);
+ ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
+ write ? FOLL_WRITE : 0);
/* Have to be a bit careful with return values */
if (nr > 0) {
diff --git a/mm/nommu.c b/mm/nommu.c
index 925dcc1..7e27add 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -197,17 +197,10 @@ long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
EXPORT_SYMBOL(__get_user_pages_unlocked);
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages)
+ struct page **pages, unsigned int gup_flags)
{
- unsigned int flags = 0;
-
- if (write)
- flags |= FOLL_WRITE;
- if (force)
- flags |= FOLL_FORCE;
-
return __get_user_pages_unlocked(current, current->mm, start, nr_pages,
- pages, flags);
+ pages, gup_flags);
}
EXPORT_SYMBOL(get_user_pages_unlocked);
diff --git a/mm/util.c b/mm/util.c
index 662cddf..4c685bd 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -283,7 +283,8 @@ EXPORT_SYMBOL_GPL(__get_user_pages_fast);
int __weak get_user_pages_fast(unsigned long start,
int nr_pages, int write, struct page **pages)
{
- return get_user_pages_unlocked(start, nr_pages, write, 0, pages);
+ return get_user_pages_unlocked(start, nr_pages, pages,
+ write ? FOLL_WRITE : 0);
}
EXPORT_SYMBOL_GPL(get_user_pages_fast);
diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
index 00d2601..1a7c9a7 100644
--- a/net/ceph/pagevec.c
+++ b/net/ceph/pagevec.c
@@ -26,7 +26,7 @@ struct page **ceph_get_direct_page_vector(const void __user *data,
while (got < num_pages) {
rc = get_user_pages_unlocked(
(unsigned long)data + ((unsigned long)got * PAGE_SIZE),
- num_pages - got, write_page, 0, pages + got);
+ num_pages - got, pages + got, write_page ? FOLL_WRITE : 0);
if (rc < 0)
break;
BUG_ON(rc == 0);
--
2.10.0
^ permalink raw reply related
* [PATCH 02/10] mm: remove write/force parameters from __get_user_pages_unlocked()
From: Lorenzo Stoakes @ 2016-10-13 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>
This patch removes the write and force parameters from
__get_user_pages_unlocked() to make the use of FOLL_FORCE explicit in callers as
use of this flag can result in surprising behaviour (and hence bugs) within the
mm subsystem.
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
include/linux/mm.h | 3 +--
mm/gup.c | 17 +++++++++--------
mm/nommu.c | 12 +++++++++---
mm/process_vm_access.c | 7 +++++--
virt/kvm/async_pf.c | 3 ++-
virt/kvm/kvm_main.c | 11 ++++++++---
6 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index e9caec6..2db98b6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1285,8 +1285,7 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages, int *locked);
long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages,
- unsigned int gup_flags);
+ struct page **pages, unsigned int gup_flags);
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages);
int get_user_pages_fast(unsigned long start, int nr_pages, int write,
diff --git a/mm/gup.c b/mm/gup.c
index ba83942..3d620dd 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -865,17 +865,11 @@ EXPORT_SYMBOL(get_user_pages_locked);
*/
__always_inline long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages,
- unsigned int gup_flags)
+ struct page **pages, unsigned int gup_flags)
{
long ret;
int locked = 1;
- if (write)
- gup_flags |= FOLL_WRITE;
- if (force)
- gup_flags |= FOLL_FORCE;
-
down_read(&mm->mmap_sem);
ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, NULL,
&locked, false, gup_flags);
@@ -905,8 +899,15 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages)
{
+ unsigned int flags = FOLL_TOUCH;
+
+ if (write)
+ flags |= FOLL_WRITE;
+ if (force)
+ flags |= FOLL_FORCE;
+
return __get_user_pages_unlocked(current, current->mm, start, nr_pages,
- write, force, pages, FOLL_TOUCH);
+ pages, flags);
}
EXPORT_SYMBOL(get_user_pages_unlocked);
diff --git a/mm/nommu.c b/mm/nommu.c
index 95daf81..925dcc1 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -185,8 +185,7 @@ EXPORT_SYMBOL(get_user_pages_locked);
long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
- int write, int force, struct page **pages,
- unsigned int gup_flags)
+ struct page **pages, unsigned int gup_flags)
{
long ret;
down_read(&mm->mmap_sem);
@@ -200,8 +199,15 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages)
{
+ unsigned int flags = 0;
+
+ if (write)
+ flags |= FOLL_WRITE;
+ if (force)
+ flags |= FOLL_FORCE;
+
return __get_user_pages_unlocked(current, current->mm, start, nr_pages,
- write, force, pages, 0);
+ pages, flags);
}
EXPORT_SYMBOL(get_user_pages_unlocked);
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index 07514d4..be8dc8d 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -88,12 +88,16 @@ static int process_vm_rw_single_vec(unsigned long addr,
ssize_t rc = 0;
unsigned long max_pages_per_loop = PVM_MAX_KMALLOC_PAGES
/ sizeof(struct pages *);
+ unsigned int flags = FOLL_REMOTE;
/* Work out address and page range required */
if (len == 0)
return 0;
nr_pages = (addr + len - 1) / PAGE_SIZE - addr / PAGE_SIZE + 1;
+ if (vm_write)
+ flags |= FOLL_WRITE;
+
while (!rc && nr_pages && iov_iter_count(iter)) {
int pages = min(nr_pages, max_pages_per_loop);
size_t bytes;
@@ -104,8 +108,7 @@ static int process_vm_rw_single_vec(unsigned long addr,
* current/current->mm
*/
pages = __get_user_pages_unlocked(task, mm, pa, pages,
- vm_write, 0, process_pages,
- FOLL_REMOTE);
+ process_pages, flags);
if (pages <= 0)
return -EFAULT;
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index db96688..8035cc1 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -84,7 +84,8 @@ static void async_pf_execute(struct work_struct *work)
* mm and might be done in another context, so we must
* use FOLL_REMOTE.
*/
- __get_user_pages_unlocked(NULL, mm, addr, 1, 1, 0, NULL, FOLL_REMOTE);
+ __get_user_pages_unlocked(NULL, mm, addr, 1, NULL,
+ FOLL_WRITE | FOLL_REMOTE);
kvm_async_page_present_sync(vcpu, apf);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 81dfc73..28510e7 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1416,10 +1416,15 @@ static int hva_to_pfn_slow(unsigned long addr, bool *async, bool write_fault,
down_read(¤t->mm->mmap_sem);
npages = get_user_page_nowait(addr, write_fault, page);
up_read(¤t->mm->mmap_sem);
- } else
+ } else {
+ unsigned int flags = FOLL_TOUCH | FOLL_HWPOISON;
+
+ if (write_fault)
+ flags |= FOLL_WRITE;
+
npages = __get_user_pages_unlocked(current, current->mm, addr, 1,
- write_fault, 0, page,
- FOLL_TOUCH|FOLL_HWPOISON);
+ page, flags);
+ }
if (npages != 1)
return npages;
--
2.10.0
^ permalink raw reply related
* [PATCH 01/10] mm: remove write/force parameters from __get_user_pages_locked()
From: Lorenzo Stoakes @ 2016-10-13 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>
This patch removes the write and force parameters from __get_user_pages_locked()
to make the use of FOLL_FORCE explicit in callers as use of this flag can result
in surprising behaviour (and hence bugs) within the mm subsystem.
Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
mm/gup.c | 47 +++++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 96b2b2f..ba83942 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -729,7 +729,6 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
struct mm_struct *mm,
unsigned long start,
unsigned long nr_pages,
- int write, int force,
struct page **pages,
struct vm_area_struct **vmas,
int *locked, bool notify_drop,
@@ -747,10 +746,6 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
if (pages)
flags |= FOLL_GET;
- if (write)
- flags |= FOLL_WRITE;
- if (force)
- flags |= FOLL_FORCE;
pages_done = 0;
lock_dropped = false;
@@ -846,9 +841,15 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages,
int *locked)
{
+ unsigned int flags = FOLL_TOUCH;
+
+ if (write)
+ flags |= FOLL_WRITE;
+ if (force)
+ flags |= FOLL_FORCE;
+
return __get_user_pages_locked(current, current->mm, start, nr_pages,
- write, force, pages, NULL, locked, true,
- FOLL_TOUCH);
+ pages, NULL, locked, true, flags);
}
EXPORT_SYMBOL(get_user_pages_locked);
@@ -869,9 +870,15 @@ __always_inline long __get_user_pages_unlocked(struct task_struct *tsk, struct m
{
long ret;
int locked = 1;
+
+ if (write)
+ gup_flags |= FOLL_WRITE;
+ if (force)
+ gup_flags |= FOLL_FORCE;
+
down_read(&mm->mmap_sem);
- ret = __get_user_pages_locked(tsk, mm, start, nr_pages, write, force,
- pages, NULL, &locked, false, gup_flags);
+ ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, NULL,
+ &locked, false, gup_flags);
if (locked)
up_read(&mm->mmap_sem);
return ret;
@@ -963,9 +970,15 @@ long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
int write, int force, struct page **pages,
struct vm_area_struct **vmas)
{
- return __get_user_pages_locked(tsk, mm, start, nr_pages, write, force,
- pages, vmas, NULL, false,
- FOLL_TOUCH | FOLL_REMOTE);
+ unsigned int flags = FOLL_TOUCH | FOLL_REMOTE;
+
+ if (write)
+ flags |= FOLL_WRITE;
+ if (force)
+ flags |= FOLL_FORCE;
+
+ return __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas,
+ NULL, false, flags);
}
EXPORT_SYMBOL(get_user_pages_remote);
@@ -979,9 +992,15 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages,
struct vm_area_struct **vmas)
{
+ unsigned int flags = FOLL_TOUCH;
+
+ if (write)
+ flags |= FOLL_WRITE;
+ if (force)
+ flags |= FOLL_FORCE;
+
return __get_user_pages_locked(current, current->mm, start, nr_pages,
- write, force, pages, vmas, NULL, false,
- FOLL_TOUCH);
+ pages, vmas, NULL, false, flags);
}
EXPORT_SYMBOL(get_user_pages);
--
2.10.0
^ permalink raw reply related
* [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags
From: Lorenzo Stoakes @ 2016-10-13 0:20 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adjusts functions in the get_user_pages* family such that
desired FOLL_* flags are passed as an argument rather than implied by flags.
The purpose of this change is to make the use of FOLL_FORCE explicit so it is
easier to grep for and clearer to callers that this flag is being used. The use
of FOLL_FORCE is an issue as it overrides missing VM_READ/VM_WRITE flags for the
VMA whose pages we are reading from/writing to, which can result in surprising
behaviour.
The patch series came out of the discussion around commit 38e0885, which
addressed a BUG_ON() being triggered when a page was faulted in with PROT_NONE
set but having been overridden by FOLL_FORCE. do_numa_page() was run on the
assumption the page _must_ be one marked for NUMA node migration as an actual
PROT_NONE page would have been dealt with prior to this code path, however
FOLL_FORCE introduced a situation where this assumption did not hold.
See https://marc.info/?l=linux-mm&m=147585445805166 for the patch proposal.
Lorenzo Stoakes (10):
mm: remove write/force parameters from __get_user_pages_locked()
mm: remove write/force parameters from __get_user_pages_unlocked()
mm: replace get_user_pages_unlocked() write/force parameters with gup_flags
mm: replace get_user_pages_locked() write/force parameters with gup_flags
mm: replace get_vaddr_frames() write/force parameters with gup_flags
mm: replace get_user_pages() write/force parameters with gup_flags
mm: replace get_user_pages_remote() write/force parameters with gup_flags
mm: replace __access_remote_vm() write parameter with gup_flags
mm: replace access_remote_vm() write parameter with gup_flags
mm: replace access_process_vm() write parameter with gup_flags
arch/alpha/kernel/ptrace.c | 9 ++--
arch/blackfin/kernel/ptrace.c | 5 ++-
arch/cris/arch-v32/drivers/cryptocop.c | 4 +-
arch/cris/arch-v32/kernel/ptrace.c | 4 +-
arch/ia64/kernel/err_inject.c | 2 +-
arch/ia64/kernel/ptrace.c | 14 +++---
arch/m32r/kernel/ptrace.c | 15 ++++---
arch/mips/kernel/ptrace32.c | 5 ++-
arch/mips/mm/gup.c | 2 +-
arch/powerpc/kernel/ptrace32.c | 5 ++-
arch/s390/mm/gup.c | 3 +-
arch/score/kernel/ptrace.c | 10 +++--
arch/sh/mm/gup.c | 3 +-
arch/sparc/kernel/ptrace_64.c | 24 +++++++----
arch/sparc/mm/gup.c | 3 +-
arch/x86/kernel/step.c | 3 +-
arch/x86/mm/gup.c | 2 +-
arch/x86/mm/mpx.c | 5 +--
arch/x86/um/ptrace_32.c | 3 +-
arch/x86/um/ptrace_64.c | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 ++-
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 ++-
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 3 +-
drivers/gpu/drm/i915/i915_gem_userptr.c | 6 ++-
drivers/gpu/drm/radeon/radeon_ttm.c | 3 +-
drivers/gpu/drm/via/via_dmablit.c | 4 +-
drivers/infiniband/core/umem.c | 6 ++-
drivers/infiniband/core/umem_odp.c | 7 ++-
drivers/infiniband/hw/mthca/mthca_memfree.c | 2 +-
drivers/infiniband/hw/qib/qib_user_pages.c | 3 +-
drivers/infiniband/hw/usnic/usnic_uiom.c | 5 ++-
drivers/media/pci/ivtv/ivtv-udma.c | 4 +-
drivers/media/pci/ivtv/ivtv-yuv.c | 5 ++-
drivers/media/platform/omap/omap_vout.c | 2 +-
drivers/media/v4l2-core/videobuf-dma-sg.c | 7 ++-
drivers/media/v4l2-core/videobuf2-memops.c | 6 ++-
drivers/misc/mic/scif/scif_rma.c | 3 +-
drivers/misc/sgi-gru/grufault.c | 2 +-
drivers/platform/goldfish/goldfish_pipe.c | 3 +-
drivers/rapidio/devices/rio_mport_cdev.c | 3 +-
drivers/scsi/st.c | 5 +--
.../interface/vchiq_arm/vchiq_2835_arm.c | 3 +-
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +-
drivers/video/fbdev/pvr2fb.c | 4 +-
drivers/virt/fsl_hypervisor.c | 4 +-
fs/exec.c | 9 +++-
fs/proc/base.c | 19 +++++---
include/linux/mm.h | 18 ++++----
kernel/events/uprobes.c | 6 ++-
kernel/ptrace.c | 16 ++++---
mm/frame_vector.c | 9 ++--
mm/gup.c | 50 ++++++++++------------
mm/memory.c | 16 ++++---
mm/mempolicy.c | 2 +-
mm/nommu.c | 38 +++++++---------
mm/process_vm_access.c | 7 ++-
mm/util.c | 8 ++--
net/ceph/pagevec.c | 2 +-
security/tomoyo/domain.c | 2 +-
virt/kvm/async_pf.c | 3 +-
virt/kvm/kvm_main.c | 11 +++--
61 files changed, 260 insertions(+), 187 deletions(-)
^ permalink raw reply
* [PATCH/RFT 07/12] USB: ohci-da8xx: Request gpios and handle interrupt in the driver
From: David Lechner @ 2016-10-12 23:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKXjFTMO_tCzLxc5wznrW_g6rxMYT8jBgHCtiFfpHq_6M=RZWA@mail.gmail.com>
On 10/12/2016 10:01 AM, Axel Haslam wrote:
> I agree that we should use a regulator for the vbus power.
> i will make that change. However, im not so sure about using the
> regulator for the overcurrent handling. There seems to be no other
> driver doing this, and as you mention, we would need to change the regulator
> framework, which might not be justifiable. I think there is not another way
> to handle the over current notification other than powering the port off.
The regulator framework has REGULATOR_EVENT_OVER_CURRENT already.
Perhaps this could be of some use? For example you could extend the
existing gpio-regulator driver with an optional overcurrent gpio pin.
>
> how about using regulator for vbus, but keeping gpio for overcurrent
> notifications?
See the suggestion above about extending the gpio-regulator driver.
> For the usersapce handling you describe above, maybe we should be able to
> listen for an usb overcurrent uevent in userspace? it seems this
> question was asked
> a couple of years back[1], but im not sure what the conclusion was. In any case,
> we could have DT and non-DT based ohci-da8xx working,
> And could work on a uevent notification for the scenario you describe
> above which
> i think is not specific to the ohci-da8xx.
>
> [1]http://linux-usb.vger.kernel.narkive.com/SjcUB5hk/how-best-to-get-over-current-notification-to-user-application
Thanks for the link. Too bad it seems nothing ever became of this. I
guess it will be up to me to bring up the discussion again if I really
want it.
^ permalink raw reply
* [PATCH v3 3/6] pwm: imx: support output polarity inversion
From: Stefan Agner @ 2016-10-12 23:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161013001540.7f759158@jawa>
On 2016-10-12 15:15, Lukasz Majewski wrote:
> Hi Stefan,
>
>> On 2016-10-07 08:11, Bhuvanchandra DV wrote:
>> > From: Lothar Wassmann <LW@KARO-electronics.de>
>> >
>> > The i.MX pwm unit on i.MX27 and newer SoCs provides a configurable
>> > output polarity. This patch adds support to utilize this feature
>> > where available.
>> >
>> > Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
>> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
>> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
>> > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> > Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +--
>> > drivers/pwm/pwm-imx.c | 51
>> > +++++++++++++++++++++-- 2 files changed, 51 insertions(+), 6
>> > deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> > b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> > index e00c2e9..c61bdf8 100644
>> > --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> > +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> > @@ -6,8 +6,8 @@ Required properties:
>> > - "fsl,imx1-pwm" for PWM compatible with the one integrated on
>> > i.MX1
>> > - "fsl,imx27-pwm" for PWM compatible with the one integrated on
>> > i.MX27
>> > - reg: physical base address and length of the controller's
>> > registers -- #pwm-cells: should be 2. See pwm.txt in this directory
>> > for a description of
>> > - the cells format.
>> > +- #pwm-cells: 2 for i.MX1 and 3 for i.MX27 and newer SoCs. See
>> > pwm.txt
>> > + in this directory for a description of the cells format.
>> > - clocks : Clock specifiers for both ipg and per clocks.
>> > - clock-names : Clock names should include both "ipg" and "per"
>> > See the clock consumer binding,
>> > @@ -17,7 +17,7 @@ See the clock consumer binding,
>> > Example:
>> >
>> > pwm1: pwm at 53fb4000 {
>> > - #pwm-cells = <2>;
>> > + #pwm-cells = <3>;
>> > compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
>> > reg = <0x53fb4000 0x4000>;
>> > clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
>> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
>> > index d600fd5..c37d223 100644
>> > --- a/drivers/pwm/pwm-imx.c
>> > +++ b/drivers/pwm/pwm-imx.c
>> > @@ -38,6 +38,7 @@
>> > #define MX3_PWMCR_DOZEEN (1 << 24)
>> > #define MX3_PWMCR_WAITEN (1 << 23)
>> > #define MX3_PWMCR_DBGEN (1 << 22)
>> > +#define MX3_PWMCR_POUTC (1 << 18)
>> > #define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16)
>> > #define MX3_PWMCR_CLKSRC_IPG (1 << 16)
>> > #define MX3_PWMCR_SWR (1 << 3)
>> > @@ -180,6 +181,9 @@ static int imx_pwm_config_v2(struct pwm_chip
>> > *chip, if (enable)
>> > cr |= MX3_PWMCR_EN;
>> >
>> > + if (pwm->args.polarity == PWM_POLARITY_INVERSED)
>> > + cr |= MX3_PWMCR_POUTC;
>> > +
>>
>> This seems wrong to me, the config callback is meant for period/duty
>> cycle only.
>
> If it is meant only for that, then the polarity should be removed from
> it.
>
> However after very quick testing, at least on my setup, it turns out
> that removing this lines causes polarity to _not_ being set (and the
> polarity is not inverted).
>
> I will investigate this further on my setup and hopefully sent proper
> patch.
>
>> The set_polarity callback should get called in case a
>> different polarity is requested.
>
> On my setup the pwm2 is set from DT and pwm_backlight_probe() calls
> pwm_apply_args(), so everything should work. However, as I mentioned
> above there still is some problem with inversion setting.
>
>>
>>
>> > writel(cr, imx->mmio_base + MX3_PWMCR);
>> >
>> > return 0;
>> > @@ -240,27 +244,62 @@ static void imx_pwm_disable(struct pwm_chip
>> > *chip, struct pwm_device *pwm)
>> > clk_disable_unprepare(imx->clk_per);
>> > }
>> >
>> > -static struct pwm_ops imx_pwm_ops = {
>> > +static int imx_pwm_set_polarity(struct pwm_chip *chip, struct
>> > pwm_device *pwm,
>> > + enum pwm_polarity polarity)
>> > +{
>> > + struct imx_chip *imx = to_imx_chip(chip);
>> > + u32 val;
>> > +
>> > + if (polarity == pwm->args.polarity)
>> > + return 0;
>>
>> I don't think that this is right. Today, pwm_apply_args (in
>> include/linux/pwm.h) copies the polarity from args to state.polarity,
>> which is then passed as polarity argument to this function. So this
>> will always return 0 afaict.
>
> Yes, I've overlooked it (that the state is copied).
>
> It can be dropped.
Did you do the above test with that line dropped?
>
>>
>> I would just drop that.
>>
>> There is probably one little problem in the current state of affairs:
>> If the bootloader makes use of a PWM channel with inverted state,
>> then the kernel would not know about that and currently assume a
>> wrong initial state... I guess at one point in time we should
>> implement the state retrieval callback and move to the new atomic PWM
>> API, which would mean to implement apply callback.
>
> Are there any patches on the horizon?
>
Not that I know of...
--
Stefan
>>
>> --
>> Stefan
>>
>>
>> > +
>> > + val = readl(imx->mmio_base + MX3_PWMCR);
>> > +
>> > + if (polarity == PWM_POLARITY_INVERSED)
>> > + val |= MX3_PWMCR_POUTC;
>> > + else
>> > + val &= ~MX3_PWMCR_POUTC;
>> > +
>> > + writel(val, imx->mmio_base + MX3_PWMCR);
>> > +
>> > + dev_dbg(imx->chip.dev, "%s: polarity set to %s\n",
>> > __func__,
>> > + polarity == PWM_POLARITY_INVERSED ? "inverted" :
>> > "normal"); +
>> > + return 0;
>> > +}
>> > +
>> > +static struct pwm_ops imx_pwm_ops_v1 = {
>> > .enable = imx_pwm_enable,
>> > .disable = imx_pwm_disable,
>> > .config = imx_pwm_config,
>> > .owner = THIS_MODULE,
>> > };
>> >
>> > +static struct pwm_ops imx_pwm_ops_v2 = {
>> > + .enable = imx_pwm_enable,
>> > + .disable = imx_pwm_disable,
>> > + .set_polarity = imx_pwm_set_polarity,
>> > + .config = imx_pwm_config,
>> > + .owner = THIS_MODULE,
>> > +};
>> > +
>> > struct imx_pwm_data {
>> > int (*config)(struct pwm_chip *chip,
>> > struct pwm_device *pwm, int duty_ns, int
>> > period_ns); void (*set_enable)(struct pwm_chip *chip, bool enable);
>> > + struct pwm_ops *pwm_ops;
>> > };
>> >
>> > static struct imx_pwm_data imx_pwm_data_v1 = {
>> > .config = imx_pwm_config_v1,
>> > .set_enable = imx_pwm_set_enable_v1,
>> > + .pwm_ops = &imx_pwm_ops_v1,
>> > };
>> >
>> > static struct imx_pwm_data imx_pwm_data_v2 = {
>> > .config = imx_pwm_config_v2,
>> > .set_enable = imx_pwm_set_enable_v2,
>> > + .pwm_ops = &imx_pwm_ops_v2,
>> > };
>> >
>> > static const struct of_device_id imx_pwm_dt_ids[] = {
>> > @@ -282,6 +321,8 @@ static int imx_pwm_probe(struct platform_device
>> > *pdev) if (!of_id)
>> > return -ENODEV;
>> >
>> > + data = of_id->data;
>> > +
>> > imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
>> > if (imx == NULL)
>> > return -ENOMEM;
>> > @@ -300,18 +341,22 @@ static int imx_pwm_probe(struct
>> > platform_device *pdev) return PTR_ERR(imx->clk_ipg);
>> > }
>> >
>> > - imx->chip.ops = &imx_pwm_ops;
>> > + imx->chip.ops = data->pwm_ops;
>> > imx->chip.dev = &pdev->dev;
>> > imx->chip.base = -1;
>> > imx->chip.npwm = 1;
>> > imx->chip.can_sleep = true;
>> > + if (data->pwm_ops->set_polarity) {
>> > + dev_dbg(&pdev->dev, "PWM supports output
>> > inversion\n");
>> > + imx->chip.of_xlate = of_pwm_xlate_with_flags;
>> > + imx->chip.of_pwm_n_cells = 3;
>> > + }
>> >
>> > r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
>> > if (IS_ERR(imx->mmio_base))
>> > return PTR_ERR(imx->mmio_base);
>> >
>> > - data = of_id->data;
>> > imx->config = data->config;
>> > imx->set_enable = data->set_enable;
>>
>
> Best regards,
>
> ?ukasz Majewski
^ permalink raw reply
* [PATCH] exynos-drm: Fix display manager failing to start without IOMMU problem
From: Shuah Khan @ 2016-10-12 23:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ce7a035c-f881-82ac-97f6-869442d1ed47@osg.samsung.com>
On 10/12/2016 05:11 PM, Shuah Khan wrote:
+ Fixing Krzysztof Kozlowski address.
> Hi Inki,
>
> On 08/15/2016 10:40 PM, Inki Dae wrote:
>
>>>
>>> okay the very first commit that added IOMMU support
>>> introduced the code that rejects non-contig gem memory
>>> type without IOMMU.
>>>
>>> commit 0519f9a12d0113caab78980c48a7902d2bd40c2c
>>> Author: Inki Dae <inki.dae@samsung.com>
>>> Date: Sat Oct 20 07:53:42 2012 -0700
>>>
>>> drm/exynos: add iommu support for exynos drm framework
>>>
>
> I haven't given up on this yet. I am still seeing the following failure:
>
> Additional debug messages I added:
> [ 15.287403] exynos_drm_gem_create_ioctl() 1
> [ 15.287419] exynos_drm_gem_create() flags 1
>
> [ 15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM memory is not supported.
>
> Additional debug message I added:
> [ 15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize framebuffer
>
> This is what happens:
>
> 1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
> 2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
> 3. exynos_user_fb_create() tries to associate GEM to fb and fails during
> check_fb_gem_memory_type()
>
> At this point, there is no recovery and lightdm fails
>
> xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
> allocations are not supported in some exynos drm versions: The following
> commit introduced this change:
>
> https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9
>
> excerpts from the diff:- if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
> - create_exynos.flags = EXYNOS_BO_CONTIG;
> - else
> - create_exynos.flags = EXYNOS_BO_NONCONTIG;
> +
> + /* Contiguous allocations are not supported in some exynos drm versions.
> + * When they are supported all allocations are effectively contiguous
> + * anyway, so for simplicity we always request non contiguous buffers.
> + */
> + create_exynos.flags = EXYNOS_BO_NONCONTIG;
>
> There might have been logic on exynos_drm that forced Contig when it coudn't
> support NONCONTIG. At least, that is what this comment suggests. This assumption
> doesn't appear to be a good one and not sure if this change was made to fix a bug.
>
> After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
> support, latest kernels have a mismatch with the installed xf86-video-armsoc
>
> This is what I am running into. This leads to the following question:
>
> 1. How do we ensure exynos_drm kernel changes don't break user-space
> specifically xf86-video-armsoc
> 2. This seems to have gone undetected for a while. I see a change in
> exynos_drm_gem_dumb_create() that is probably addressing this type
> of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
> handling for IOMMU NONCONTIG case.
>
> Anyway, I am interested in getting the exynos_drm kernel side code
> and xf86-video-armsoc in sync to resolve the issue.
>
> Could you recommend a going forward plan?
>
> I can submit a patch to xf86-video-armsoc. I am also looking ahead to
> see if we can avoid such breaks in the future by keeping kernel and
> xf86-video-armsoc in sync.
>
> thanks,
> -- Shuah
>
^ permalink raw reply
* [PATCHv2 1/4] arm64: dump: Make ptdump debugfs a separate option
From: Kees Cook @ 2016-10-12 23:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3d550df8-f850-d217-aaae-d69c8ea8243b@redhat.com>
On Wed, Oct 12, 2016 at 3:57 PM, Laura Abbott <labbott@redhat.com> wrote:
> On 10/12/2016 03:45 PM, Kees Cook wrote:
>>
>> On Wed, Oct 12, 2016 at 3:31 PM, Laura Abbott <labbott@redhat.com> wrote:
>>>
>>>
>>> ptdump_register currently initializes a set of page table information and
>>> registers debugfs. There are uses for the ptdump option without wanting
>>> the
>>> debugfs options. Split this out to make it a separate option.
>>>
>>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>>> ---
>>> v2: Minor style fixups per Mark Rutland, intialization is now separate
>>> from
>>> register since it never needed to be combined in the first place, EFI
>>> page table registration.
>>> ---
>>> arch/arm64/Kconfig.debug | 6 +++++-
>>> arch/arm64/include/asm/ptdump.h | 13 ++++++++-----
>>> arch/arm64/mm/Makefile | 3 ++-
>>> arch/arm64/mm/dump.c | 26 +++++---------------------
>>> arch/arm64/mm/ptdump_debugfs.c | 31 +++++++++++++++++++++++++++++++
>>> drivers/firmware/efi/arm-runtime.c | 5 ++---
>>> 6 files changed, 53 insertions(+), 31 deletions(-)
>>> create mode 100644 arch/arm64/mm/ptdump_debugfs.c
>>>
>>> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
>>> index b661fe7..21a5b74 100644
>>> --- a/arch/arm64/Kconfig.debug
>>> +++ b/arch/arm64/Kconfig.debug
>>> @@ -2,9 +2,13 @@ menu "Kernel hacking"
>>>
>>> source "lib/Kconfig.debug"
>>>
>>> -config ARM64_PTDUMP
>>> +config ARM64_PTDUMP_CORE
>>> + def_bool n
>>> +
>>> +config ARM64_PTDUMP_DEBUGFS
>>> bool "Export kernel pagetable layout to userspace via debugfs"
>>
>>
>> I think this can be a tristate now, yes? (I did this on x86 so I could
>> load ptdump as a module for Chrome OS image testing but then leave the
>> module off the read-only partition for release images...)
>>
>> https://git.kernel.org/linus/8609d1b5daa36350e020e737946c40887af1743a
>>
>> -Kees
>>
>
> I saw the module for x86. arm64 works slightly differently since there
> are multiple debugfs dumping sources, the init_mm and EFI page tables.
> These are built in and can call register but then have no way to
> actually register the debugfs entry when the module is inserted since
> the current config covers both/all of the exporters. I
> decided against trying to add the infrastructure to make modularity
> work because I was spending more time on that than the actual W^X checks.
Okay, sounds fine. I'll see if I can take a look at this in the future
if no one else beats me to it.
In the meantime, this whole series looks good to me. :)
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply
* [PATCH] exynos-drm: Fix display manager failing to start without IOMMU problem
From: Shuah Khan @ 2016-10-12 23:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57B29925.4020600@samsung.com>
Hi Inki,
On 08/15/2016 10:40 PM, Inki Dae wrote:
>>
>> okay the very first commit that added IOMMU support
>> introduced the code that rejects non-contig gem memory
>> type without IOMMU.
>>
>> commit 0519f9a12d0113caab78980c48a7902d2bd40c2c
>> Author: Inki Dae <inki.dae@samsung.com>
>> Date: Sat Oct 20 07:53:42 2012 -0700
>>
>> drm/exynos: add iommu support for exynos drm framework
>>
I haven't given up on this yet. I am still seeing the following failure:
Additional debug messages I added:
[ 15.287403] exynos_drm_gem_create_ioctl() 1
[ 15.287419] exynos_drm_gem_create() flags 1
[ 15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM memory is not supported.
Additional debug message I added:
[ 15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize framebuffer
This is what happens:
1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
3. exynos_user_fb_create() tries to associate GEM to fb and fails during
check_fb_gem_memory_type()
At this point, there is no recovery and lightdm fails
xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
allocations are not supported in some exynos drm versions: The following
commit introduced this change:
https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9
excerpts from the diff:- if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
- create_exynos.flags = EXYNOS_BO_CONTIG;
- else
- create_exynos.flags = EXYNOS_BO_NONCONTIG;
+
+ /* Contiguous allocations are not supported in some exynos drm versions.
+ * When they are supported all allocations are effectively contiguous
+ * anyway, so for simplicity we always request non contiguous buffers.
+ */
+ create_exynos.flags = EXYNOS_BO_NONCONTIG;
There might have been logic on exynos_drm that forced Contig when it coudn't
support NONCONTIG. At least, that is what this comment suggests. This assumption
doesn't appear to be a good one and not sure if this change was made to fix a bug.
After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
support, latest kernels have a mismatch with the installed xf86-video-armsoc
This is what I am running into. This leads to the following question:
1. How do we ensure exynos_drm kernel changes don't break user-space
specifically xf86-video-armsoc
2. This seems to have gone undetected for a while. I see a change in
exynos_drm_gem_dumb_create() that is probably addressing this type
of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
handling for IOMMU NONCONTIG case.
Anyway, I am interested in getting the exynos_drm kernel side code
and xf86-video-armsoc in sync to resolve the issue.
Could you recommend a going forward plan?
I can submit a patch to xf86-video-armsoc. I am also looking ahead to
see if we can avoid such breaks in the future by keeping kernel and
xf86-video-armsoc in sync.
thanks,
-- Shuah
^ permalink raw reply
* Low network throughput on i.MX28
From: Jörg Krause @ 2016-10-12 23:09 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I am using a custom i.MX28 board similar to the i.MX28-EVK. For Wi-Fi
the board assembles a BCM43362 from Broadcom and for Ethernet a
LAN8720A from Microchip. The board is running mainline Linux 4.7.
While both, wireless and wired network interfaces work, the TCP
throughput measured with iperf is low.
The bandwith for Ethernet is between 20-30 MBits/sec and for WLAN is
about 4-5 MBits/sec.
There exists an Application Note "i.MX28 Ethernet Performance on
Linux" [1] which shows a bandwith of > 60 MBits/sec. A user an the NXP
forum [2] told he achieved 20 MBits/sec with some Qualcom chip.
Note, that these values are most probably measured with the legacy
Linux Kernel 2.6.35 from NXP.
Does anybody has done throughput tests on i.MX28 with mainline Kernel?
If so, what are the results? What might be the bottleneck?
[1] http://cache.freescale.com/files/32bit/doc/app_note/AN4544.pdf
[2] https://community.nxp.com/thread/353921
Best regards
J?rg Krause
^ permalink raw reply
* [PATCHv2 1/4] arm64: dump: Make ptdump debugfs a separate option
From: Laura Abbott @ 2016-10-12 22:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5jLNUrt22Qg2gFmLj9rgStOTTTsOT7QGaw5efk==WbFwRg@mail.gmail.com>
On 10/12/2016 03:45 PM, Kees Cook wrote:
> On Wed, Oct 12, 2016 at 3:31 PM, Laura Abbott <labbott@redhat.com> wrote:
>>
>> ptdump_register currently initializes a set of page table information and
>> registers debugfs. There are uses for the ptdump option without wanting the
>> debugfs options. Split this out to make it a separate option.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> v2: Minor style fixups per Mark Rutland, intialization is now separate from
>> register since it never needed to be combined in the first place, EFI
>> page table registration.
>> ---
>> arch/arm64/Kconfig.debug | 6 +++++-
>> arch/arm64/include/asm/ptdump.h | 13 ++++++++-----
>> arch/arm64/mm/Makefile | 3 ++-
>> arch/arm64/mm/dump.c | 26 +++++---------------------
>> arch/arm64/mm/ptdump_debugfs.c | 31 +++++++++++++++++++++++++++++++
>> drivers/firmware/efi/arm-runtime.c | 5 ++---
>> 6 files changed, 53 insertions(+), 31 deletions(-)
>> create mode 100644 arch/arm64/mm/ptdump_debugfs.c
>>
>> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
>> index b661fe7..21a5b74 100644
>> --- a/arch/arm64/Kconfig.debug
>> +++ b/arch/arm64/Kconfig.debug
>> @@ -2,9 +2,13 @@ menu "Kernel hacking"
>>
>> source "lib/Kconfig.debug"
>>
>> -config ARM64_PTDUMP
>> +config ARM64_PTDUMP_CORE
>> + def_bool n
>> +
>> +config ARM64_PTDUMP_DEBUGFS
>> bool "Export kernel pagetable layout to userspace via debugfs"
>
> I think this can be a tristate now, yes? (I did this on x86 so I could
> load ptdump as a module for Chrome OS image testing but then leave the
> module off the read-only partition for release images...)
>
> https://git.kernel.org/linus/8609d1b5daa36350e020e737946c40887af1743a
>
> -Kees
>
I saw the module for x86. arm64 works slightly differently since there
are multiple debugfs dumping sources, the init_mm and EFI page tables.
These are built in and can call register but then have no way to
actually register the debugfs entry when the module is inserted since
the current config covers both/all of the exporters. I
decided against trying to add the infrastructure to make modularity
work because I was spending more time on that than the actual W^X checks.
Thanks,
Laura
>> depends on DEBUG_KERNEL
>> + select ARM64_PTDUMP_CORE
>> select DEBUG_FS
>> help
>> Say Y here if you want to show the kernel pagetable layout in a
>
>
>
^ permalink raw reply
* [PATCH V2 1/3] Revert "ACPI, PCI, IRQ: reduce static IRQ array size to 16"
From: Sinan Kaya @ 2016-10-12 22:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012221357.GB15116@localhost>
Hi Bjorn,
On 10/12/2016 6:13 PM, Bjorn Helgaas wrote:
> Hi Sinan,
>
> I have to apologize because I haven't followed all the discussion and
> now I'm trying to figure it out from the patches and changelogs. But
> I guess that's not all bad, because future interested folks *should*
> be able to figure things out from that :)
Sure, np. I figured you are busy with the new baseline. Then, I saw a
series of patches coming from you.
>
> On Tue, Oct 04, 2016 at 05:15:17PM -0400, Sinan Kaya wrote:
>> This reverts commit 5c5087a55390 ("ACPI,PCI,IRQ: reduce static IRQ array
>> size to 16").
>>
>> The code maintains a fixed size array for IRQ penalties. The array
>> gets updated by external calls such as acpi_penalize_sci_irq,
>> acpi_penalize_isa_irq to reflect the actual interrupt usage of the
>> system. Since the IRQ distribution is platform specific, this is
>> not known ahead of time. The IRQs get updated based on the SCI
>> interrupt number BIOS has chosen or the ISA IRQs that were assigned
>> to existing peripherals.
>>
>> By the time ACPI gets initialized, this code tries to determine an
>> IRQ number based on penalty values in this array. It will try to locate
>> the IRQ with the least penalty assignment so that interrupt sharing is
>> avoided if possible.
>>
>> A couple of notes about the external APIs:
>> 1. These API can be called before the ACPI is started. Therefore, one
>> cannot assume that the PCI link objects are initialized for calculating
>> penalties.
>
> Which API are you thinking about here? pcibios_penalize_isa_irq() is
> called by ACPI and PNP, which should both be after ACPI is started.
Correct, I was talking about acpi_penalize_sci_irq function here.
>
> My guess is you're thinking about acpi_penalize_sci_irq() (added back
> later in this series), which is called here, which is definitely
> before ACPI objects are available:
>
> setup_arch
> acpi_boot_init
> acpi_process_madt
> acpi_parse_madt_ioapic_entries
> acpi_table_parse_madt
> acpi_parse_int_src_ovr
> acpi_sci_ioapic_setup
> acpi_penalize_sci_irq # <---
>
>> 2. The polarity and trigger information passed via the
>> acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem
>> is reporting as the call might have been placed before the IRQ is
>> registered by the interrupt subsystem.
>>
>> The previous change was in the direction to remove these external API and
>> try to calculate the penalties at runtime for the ISA path as well. This
>> didn't work out well with the existing platforms.
>>
>> Restoring the old behavior for IRQ < 256 and the new behavior will remain
>> effective for IRQ >= 256.
>
> IIRC, this all started because we needed more than 256 IRQs, but we
> didn't know how to size a static table to be large enough without
> being wasteful.
Correct. We only need 1024 for ARM/ARM64. But, we wanted to remove this
restriction altogether to be arch proof. One of my earlier proposal was
to just resize the array to 1024. I was asked if I was wasting resources
by resizing to 1024.
>
> Prior to 5c5087a55390, we tracked penalties for IRQs 0-255. After it,
> we only tracked penalties for IRQs 0-15. I think this patch basically
> makes it so we track 0-255 again.
Yes, we went back to 256 interrupts after the revert.
>
> *This* patch only increases the range for pcibios_penalize_isa_irq()
> (and command-line hints, but hopefully nobody cares about those). A
> subsequent patch increases it for SCI as well.
>
> The name "ACPI_MAX_IRQS" is now slightly misleading (because we do
> support more than 256 IRQs) and the 256 value is sort of an
> unjustified magic number. 16 is explainable as the number of ISA
> IRQs, but I don't know what 256 is based on (other than historical
> practice, of course). ACPI device IRQs can be much larger, and I
> think the SCI IRQ can be, too (the FADT SCI_INT field is 16 bits).
>
> Can you tie this back to the specific problem on the broken machine
> somehow? Do we need a penalty for an IRQ in the 16-255 range?
The problem on the broken machine was SCI IRQ and PCI IRQ happened to be
same. It was IRQ 11. When SCI IRQ heavily penalized IRQ 11 due to
wrong interrupt type detection, PCI IRQs no longer worked as this line
prohibits using the IRQ.
if (acpi_irq_get_penalty(irq) >= PIRQ_PENALTY_ISA_ALWAYS) {
printk(KERN_ERR PREFIX "No IRQ available for %s [%s]. "
"Try pci=noacpi or acpi=off\n",
acpi_device_name(link->device),
acpi_device_bid(link->device));
return -ENODEV;
}
>
> In a subsequent patch, I see something about the IRQ type not being
> updated at the right time, but I can't quite connect the dots.
The reason why PCI IRQ 11 didn't work is above.
When we detected a problem with the SCI IRQ type, we were penalizing
the IRQ below.
static int acpi_irq_get_penalty(int irq)
{
...
if (irq == acpi_gbl_FADT.sci_interrupt) {
u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK;
if (type != IRQ_TYPE_LEVEL_LOW)
penalty += PIRQ_PENALTY_ISA_ALWAYS; <---- here
else
penalty += PIRQ_PENALTY_PCI_USING;
}
>
> To be clear, I'm not asking for any changes in the patch; I'm just
> trying to understand what's going on.
Sure, I hope this makes it clear now.
>
>> Tested-by: Jonathan Liu <net147@gmail.com>
>> Tested-by: Ondrej Zary <linux@rainbow-software.org>
>> Link: http://www.gossamer-threads.com/lists/linux/kernel/2537016#2537016
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/acpi/pci_link.c | 35 ++++++++++++++++++-----------------
>> 1 file changed, 18 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
>> index c983bf7..f3792f4 100644
>> --- a/drivers/acpi/pci_link.c
>> +++ b/drivers/acpi/pci_link.c
>> @@ -438,6 +438,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
>> * enabled system.
>> */
>>
>> +#define ACPI_MAX_IRQS 256
>> #define ACPI_MAX_ISA_IRQS 16
>>
>> #define PIRQ_PENALTY_PCI_POSSIBLE (16*16)
>> @@ -446,7 +447,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
>> #define PIRQ_PENALTY_ISA_USED (16*16*16*16*16)
>> #define PIRQ_PENALTY_ISA_ALWAYS (16*16*16*16*16*16)
>>
>> -static int acpi_isa_irq_penalty[ACPI_MAX_ISA_IRQS] = {
>> +static int acpi_irq_penalty[ACPI_MAX_IRQS] = {
>> PIRQ_PENALTY_ISA_ALWAYS, /* IRQ0 timer */
>> PIRQ_PENALTY_ISA_ALWAYS, /* IRQ1 keyboard */
>> PIRQ_PENALTY_ISA_ALWAYS, /* IRQ2 cascade */
>> @@ -511,7 +512,7 @@ static int acpi_irq_get_penalty(int irq)
>> }
>>
>> if (irq < ACPI_MAX_ISA_IRQS)
>> - return penalty + acpi_isa_irq_penalty[irq];
>> + return penalty + acpi_irq_penalty[irq];
>>
>> penalty += acpi_irq_pci_sharing_penalty(irq);
>> return penalty;
>> @@ -538,14 +539,14 @@ int __init acpi_irq_penalty_init(void)
>>
>> for (i = 0; i < link->irq.possible_count; i++) {
>> if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS)
>> - acpi_isa_irq_penalty[link->irq.
>> + acpi_irq_penalty[link->irq.
>> possible[i]] +=
>> penalty;
>> }
>>
>> } else if (link->irq.active &&
>> - (link->irq.active < ACPI_MAX_ISA_IRQS)) {
>> - acpi_isa_irq_penalty[link->irq.active] +=
>> + (link->irq.active < ACPI_MAX_IRQS)) {
>> + acpi_irq_penalty[link->irq.active] +=
>> PIRQ_PENALTY_PCI_POSSIBLE;
>> }
>> }
>> @@ -828,7 +829,7 @@ static void acpi_pci_link_remove(struct acpi_device *device)
>> }
>>
>> /*
>> - * modify acpi_isa_irq_penalty[] from cmdline
>> + * modify acpi_irq_penalty[] from cmdline
>> */
>> static int __init acpi_irq_penalty_update(char *str, int used)
>> {
>> @@ -837,24 +838,24 @@ static int __init acpi_irq_penalty_update(char *str, int used)
>> for (i = 0; i < 16; i++) {
>> int retval;
>> int irq;
>> - int new_penalty;
>>
>> retval = get_option(&str, &irq);
>>
>> if (!retval)
>> break; /* no number found */
>>
>> - /* see if this is a ISA IRQ */
>> - if ((irq < 0) || (irq >= ACPI_MAX_ISA_IRQS))
>> + if (irq < 0)
>> + continue;
>> +
>> + if (irq >= ARRAY_SIZE(acpi_irq_penalty))
>> continue;
>>
>> if (used)
>> - new_penalty = acpi_irq_get_penalty(irq) +
>> - PIRQ_PENALTY_ISA_USED;
>> + acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
>> + PIRQ_PENALTY_ISA_USED;
>> else
>> - new_penalty = 0;
>> + acpi_irq_penalty[irq] = 0;
>>
>> - acpi_isa_irq_penalty[irq] = new_penalty;
>> if (retval != 2) /* no next number */
>> break;
>> }
>> @@ -870,14 +871,14 @@ static int __init acpi_irq_penalty_update(char *str, int used)
>> */
>> void acpi_penalize_isa_irq(int irq, int active)
>> {
>> - if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty)))
>> - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
>> - (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
>> + if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty))
>> + acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
>> + (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
>> }
>>
>> bool acpi_isa_irq_available(int irq)
>> {
>> - return irq >= 0 && (irq >= ARRAY_SIZE(acpi_isa_irq_penalty) ||
>> + return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) ||
>> acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS);
>> }
>>
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCHv2 1/4] arm64: dump: Make ptdump debugfs a separate option
From: Kees Cook @ 2016-10-12 22:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476311522-15381-2-git-send-email-labbott@redhat.com>
On Wed, Oct 12, 2016 at 3:31 PM, Laura Abbott <labbott@redhat.com> wrote:
>
> ptdump_register currently initializes a set of page table information and
> registers debugfs. There are uses for the ptdump option without wanting the
> debugfs options. Split this out to make it a separate option.
>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v2: Minor style fixups per Mark Rutland, intialization is now separate from
> register since it never needed to be combined in the first place, EFI
> page table registration.
> ---
> arch/arm64/Kconfig.debug | 6 +++++-
> arch/arm64/include/asm/ptdump.h | 13 ++++++++-----
> arch/arm64/mm/Makefile | 3 ++-
> arch/arm64/mm/dump.c | 26 +++++---------------------
> arch/arm64/mm/ptdump_debugfs.c | 31 +++++++++++++++++++++++++++++++
> drivers/firmware/efi/arm-runtime.c | 5 ++---
> 6 files changed, 53 insertions(+), 31 deletions(-)
> create mode 100644 arch/arm64/mm/ptdump_debugfs.c
>
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index b661fe7..21a5b74 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -2,9 +2,13 @@ menu "Kernel hacking"
>
> source "lib/Kconfig.debug"
>
> -config ARM64_PTDUMP
> +config ARM64_PTDUMP_CORE
> + def_bool n
> +
> +config ARM64_PTDUMP_DEBUGFS
> bool "Export kernel pagetable layout to userspace via debugfs"
I think this can be a tristate now, yes? (I did this on x86 so I could
load ptdump as a module for Chrome OS image testing but then leave the
module off the read-only partition for release images...)
https://git.kernel.org/linus/8609d1b5daa36350e020e737946c40887af1743a
-Kees
> depends on DEBUG_KERNEL
> + select ARM64_PTDUMP_CORE
> select DEBUG_FS
> help
> Say Y here if you want to show the kernel pagetable layout in a
--
Kees Cook
Nexus Security
^ permalink raw reply
* [PATCHv2 4/4] arm64: dump: Add checking for writable and exectuable pages
From: Laura Abbott @ 2016-10-12 22:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476311522-15381-1-git-send-email-labbott@redhat.com>
Page mappings with full RWX permissions are a security risk. x86
has an option to walk the page tables and dump any bad pages.
(See e1a58320a38d ("x86/mm: Warn on W^X mappings")). Add a similar
implementation for arm64.
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Check only init_mm, style cleanups, UXN checks, compiliation fixes for
disabled case.
---
arch/arm64/Kconfig.debug | 30 ++++++++++++++++++++++++
arch/arm64/include/asm/ptdump.h | 8 +++++++
arch/arm64/mm/dump.c | 52 +++++++++++++++++++++++++++++++++++++++++
arch/arm64/mm/mmu.c | 2 ++
4 files changed, 92 insertions(+)
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index 21a5b74..3f627c94 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -42,6 +42,36 @@ config ARM64_RANDOMIZE_TEXT_OFFSET
of TEXT_OFFSET and platforms must not require a specific
value.
+config DEBUG_WX
+ bool "Warn on W+X mappings at boot"
+ select ARM64_PTDUMP_CORE
+ ---help---
+ Generate a warning if any W+X mappings are found at boot.
+
+ This is useful for discovering cases where the kernel is leaving
+ W+X mappings after applying NX, as such mappings are a security risk.
+ This check also includes UXN, which should be set on all kernel
+ mappings.
+
+ Look for a message in dmesg output like this:
+
+ arm64/mm: Checked W+X mappings: passed, no W+X pages found.
+
+ or like this, if the check failed:
+
+ arm64/mm: Checked W+X mappings: FAILED, <N> W+X pages found.
+
+ Note that even if the check fails, your kernel is possibly
+ still fine, as W+X mappings are not a security hole in
+ themselves, what they do is that they make the exploitation
+ of other unfixed kernel bugs easier.
+
+ There is no runtime or memory usage effect of this option
+ once the kernel has booted up - it's a one time check.
+
+ If in doubt, say "Y".
+
+
config DEBUG_SET_MODULE_RONX
bool "Set loadable kernel module data as NX and text as RO"
depends on MODULES
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 8fc0957..6afd847 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -42,5 +42,13 @@ static inline int ptdump_debugfs_register(struct ptdump_info *info,
return 0;
}
#endif
+void ptdump_check_wx(void);
+#endif /* CONFIG_ARM64_PTDUMP_CORE */
+
+#ifdef CONFIG_DEBUG_WX
+#define debug_checkwx() ptdump_check_wx()
+#else
+#define debug_checkwx() do { } while (0)
+#endif
#endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index bb36649..4913af5 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -74,6 +74,8 @@ struct pg_state {
unsigned long start_address;
unsigned level;
u64 current_prot;
+ bool check_wx;
+ unsigned long wx_pages;
};
struct prot_bits {
@@ -202,6 +204,35 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits,
}
}
+static void note_prot_uxn(struct pg_state *st, unsigned long addr)
+{
+ if (!st->check_wx)
+ return;
+
+ if ((st->current_prot & PTE_UXN) == PTE_UXN)
+ return;
+
+ WARN_ONCE(1, "arm64/mm: Found non-UXN mapping at address %p/%pS\n",
+ (void *)st->start_address, (void *)st->start_address);
+
+ st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
+}
+
+static void note_prot_wx(struct pg_state *st, unsigned long addr)
+{
+ if (!st->check_wx)
+ return;
+ if ((st->current_prot & PTE_RDONLY) == PTE_RDONLY)
+ return;
+ if ((st->current_prot & PTE_PXN) == PTE_PXN)
+ return;
+
+ WARN_ONCE(1, "arm64/mm: Found insecure W+X mapping at address %p/%pS\n",
+ (void *)st->start_address, (void *)st->start_address);
+
+ st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
+}
+
static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
u64 val)
{
@@ -219,6 +250,8 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
unsigned long delta;
if (st->current_prot) {
+ note_prot_uxn(st, addr);
+ note_prot_wx(st, addr);
pt_dump_seq_printf(st->seq, "0x%016lx-0x%016lx ",
st->start_address, addr);
@@ -344,6 +377,25 @@ static struct ptdump_info kernel_ptdump_info = {
.base_addr = VA_START,
};
+void ptdump_check_wx(void)
+{
+ struct pg_state st = {
+ .seq = NULL,
+ .marker = (struct addr_marker[]) {
+ { -1, NULL},
+ },
+ .check_wx = true,
+ };
+
+ walk_pgd(&st, &init_mm, 0);
+ note_page(&st, 0, 0, 0);
+ if (st.wx_pages)
+ pr_info("Checked W+X mappings: FAILED, %lu W+X pages found\n",
+ st.wx_pages);
+ else
+ pr_info("Checked W+X mappings: passed, no W+X pages found\n");
+}
+
static int ptdump_init(void)
{
ptdump_initialize();
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 05615a3..2cbe2fe 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -42,6 +42,7 @@
#include <asm/tlb.h>
#include <asm/memblock.h>
#include <asm/mmu_context.h>
+#include <asm/ptdump.h>
u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
@@ -396,6 +397,7 @@ void mark_rodata_ro(void)
section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata,
section_size, PAGE_KERNEL_RO);
+ debug_checkwx();
}
static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
--
2.7.4
^ permalink raw reply related
* [PATCHv2 3/4] arm64: dump: Remove max_addr
From: Laura Abbott @ 2016-10-12 22:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476311522-15381-1-git-send-email-labbott@redhat.com>
max_addr was added as part of struct ptdump_info but has never actually
been used. Remove it.
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
New for v2 of the series
---
arch/arm64/include/asm/ptdump.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 7c35689..8fc0957 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -30,7 +30,6 @@ struct ptdump_info {
struct mm_struct *mm;
const struct addr_marker *markers;
unsigned long base_addr;
- unsigned long max_addr;
};
void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
--
2.7.4
^ permalink raw reply related
* [PATCHv2 2/4] arm64: dump: Make the page table dumping seq_file optional
From: Laura Abbott @ 2016-10-12 22:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476311522-15381-1-git-send-email-labbott@redhat.com>
The page table dumping code always assumes it will be dumping to a
seq_file to userspace. Future code will be taking advantage of
the page table dumping code but will not need the seq_file. Make
the seq_file optional for these cases.
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Tweak commit text per Mark Rutland's suggestion
---
arch/arm64/mm/dump.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index f0f0be7..bb36649 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -50,6 +50,18 @@ static const struct addr_marker address_markers[] = {
{ -1, NULL },
};
+#define pt_dump_seq_printf(m, fmt, args...) \
+({ \
+ if (m) \
+ seq_printf(m, fmt, ##args); \
+})
+
+#define pt_dump_seq_puts(m, fmt) \
+({ \
+ if (m) \
+ seq_printf(m, fmt); \
+})
+
/*
* The page dumper groups page table entries of the same type into a single
* description. It uses pg_state to track the range information while
@@ -186,7 +198,7 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits,
s = bits->clear;
if (s)
- seq_printf(st->seq, " %s", s);
+ pt_dump_seq_printf(st->seq, " %s", s);
}
}
@@ -200,14 +212,14 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
st->level = level;
st->current_prot = prot;
st->start_address = addr;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
} else if (prot != st->current_prot || level != st->level ||
addr >= st->marker[1].start_address) {
const char *unit = units;
unsigned long delta;
if (st->current_prot) {
- seq_printf(st->seq, "0x%016lx-0x%016lx ",
+ pt_dump_seq_printf(st->seq, "0x%016lx-0x%016lx ",
st->start_address, addr);
delta = (addr - st->start_address) >> 10;
@@ -215,17 +227,17 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
delta >>= 10;
unit++;
}
- seq_printf(st->seq, "%9lu%c %s", delta, *unit,
+ pt_dump_seq_printf(st->seq, "%9lu%c %s", delta, *unit,
pg_level[st->level].name);
if (pg_level[st->level].bits)
dump_prot(st, pg_level[st->level].bits,
pg_level[st->level].num);
- seq_puts(st->seq, "\n");
+ pt_dump_seq_puts(st->seq, "\n");
}
if (addr >= st->marker[1].start_address) {
st->marker++;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
}
st->start_address = addr;
@@ -235,7 +247,7 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
if (addr >= st->marker[1].start_address) {
st->marker++;
- seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
}
}
--
2.7.4
^ permalink raw reply related
* [PATCHv2 1/4] arm64: dump: Make ptdump debugfs a separate option
From: Laura Abbott @ 2016-10-12 22:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476311522-15381-1-git-send-email-labbott@redhat.com>
ptdump_register currently initializes a set of page table information and
registers debugfs. There are uses for the ptdump option without wanting the
debugfs options. Split this out to make it a separate option.
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Minor style fixups per Mark Rutland, intialization is now separate from
register since it never needed to be combined in the first place, EFI
page table registration.
---
arch/arm64/Kconfig.debug | 6 +++++-
arch/arm64/include/asm/ptdump.h | 13 ++++++++-----
arch/arm64/mm/Makefile | 3 ++-
arch/arm64/mm/dump.c | 26 +++++---------------------
arch/arm64/mm/ptdump_debugfs.c | 31 +++++++++++++++++++++++++++++++
drivers/firmware/efi/arm-runtime.c | 5 ++---
6 files changed, 53 insertions(+), 31 deletions(-)
create mode 100644 arch/arm64/mm/ptdump_debugfs.c
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index b661fe7..21a5b74 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -2,9 +2,13 @@ menu "Kernel hacking"
source "lib/Kconfig.debug"
-config ARM64_PTDUMP
+config ARM64_PTDUMP_CORE
+ def_bool n
+
+config ARM64_PTDUMP_DEBUGFS
bool "Export kernel pagetable layout to userspace via debugfs"
depends on DEBUG_KERNEL
+ select ARM64_PTDUMP_CORE
select DEBUG_FS
help
Say Y here if you want to show the kernel pagetable layout in a
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 07b8ed0..7c35689 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -16,9 +16,10 @@
#ifndef __ASM_PTDUMP_H
#define __ASM_PTDUMP_H
-#ifdef CONFIG_ARM64_PTDUMP
+#ifdef CONFIG_ARM64_PTDUMP_CORE
#include <linux/mm_types.h>
+#include <linux/seq_file.h>
struct addr_marker {
unsigned long start_address;
@@ -32,13 +33,15 @@ struct ptdump_info {
unsigned long max_addr;
};
-int ptdump_register(struct ptdump_info *info, const char *name);
-
+void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
+#ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name);
#else
-static inline int ptdump_register(struct ptdump_info *info, const char *name)
+static inline int ptdump_debugfs_register(struct ptdump_info *info,
+ const char *name)
{
return 0;
}
-#endif /* CONFIG_ARM64_PTDUMP */
+#endif
#endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
index 54bb209..e703fb9 100644
--- a/arch/arm64/mm/Makefile
+++ b/arch/arm64/mm/Makefile
@@ -3,7 +3,8 @@ obj-y := dma-mapping.o extable.o fault.o init.o \
ioremap.o mmap.o pgd.o mmu.o \
context.o proc.o pageattr.o
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
-obj-$(CONFIG_ARM64_PTDUMP) += dump.o
+obj-$(CONFIG_ARM64_PTDUMP_CORE) += dump.o
+obj-$(CONFIG_ARM64_PTDUMP_DEBUGFS) += ptdump_debugfs.o
obj-$(CONFIG_NUMA) += numa.o
obj-$(CONFIG_KASAN) += kasan_init.o
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index 9c3e75d..f0f0be7 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -304,9 +304,8 @@ static void walk_pgd(struct pg_state *st, struct mm_struct *mm,
}
}
-static int ptdump_show(struct seq_file *m, void *v)
+void ptdump_walk_pgd(struct seq_file *m, struct ptdump_info *info)
{
- struct ptdump_info *info = m->private;
struct pg_state st = {
.seq = m,
.marker = info->markers,
@@ -315,33 +314,16 @@ static int ptdump_show(struct seq_file *m, void *v)
walk_pgd(&st, info->mm, info->base_addr);
note_page(&st, 0, 0, 0);
- return 0;
}
-static int ptdump_open(struct inode *inode, struct file *file)
+static void ptdump_initialize(void)
{
- return single_open(file, ptdump_show, inode->i_private);
-}
-
-static const struct file_operations ptdump_fops = {
- .open = ptdump_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-int ptdump_register(struct ptdump_info *info, const char *name)
-{
- struct dentry *pe;
unsigned i, j;
for (i = 0; i < ARRAY_SIZE(pg_level); i++)
if (pg_level[i].bits)
for (j = 0; j < pg_level[i].num; j++)
pg_level[i].mask |= pg_level[i].bits[j].mask;
-
- pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
- return pe ? 0 : -ENOMEM;
}
static struct ptdump_info kernel_ptdump_info = {
@@ -352,6 +334,8 @@ static struct ptdump_info kernel_ptdump_info = {
static int ptdump_init(void)
{
- return ptdump_register(&kernel_ptdump_info, "kernel_page_tables");
+ ptdump_initialize();
+ return ptdump_debugfs_register(&kernel_ptdump_info,
+ "kernel_page_tables");
}
device_initcall(ptdump_init);
diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c
new file mode 100644
index 0000000..eee4d86
--- /dev/null
+++ b/arch/arm64/mm/ptdump_debugfs.c
@@ -0,0 +1,31 @@
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#include <asm/ptdump.h>
+
+static int ptdump_show(struct seq_file *m, void *v)
+{
+ struct ptdump_info *info = m->private;
+ ptdump_walk_pgd(m, info);
+ return 0;
+}
+
+static int ptdump_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, ptdump_show, inode->i_private);
+}
+
+static const struct file_operations ptdump_fops = {
+ .open = ptdump_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name)
+{
+ struct dentry *pe;
+ pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
+ return pe ? 0 : -ENOMEM;
+
+}
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 7c75a8d..33d35e8 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -39,7 +39,7 @@ static struct mm_struct efi_mm = {
.mmlist = LIST_HEAD_INIT(efi_mm.mmlist),
};
-#ifdef CONFIG_ARM64_PTDUMP
+#ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
#include <asm/ptdump.h>
static struct ptdump_info efi_ptdump_info = {
@@ -53,10 +53,9 @@ static struct ptdump_info efi_ptdump_info = {
static int __init ptdump_init(void)
{
- return ptdump_register(&efi_ptdump_info, "efi_page_tables");
+ return ptdump_debugfs_register(&efi_ptdump_info, "efi_page_tables");
}
device_initcall(ptdump_init);
-
#endif
static bool __init efi_virtmap_init(void)
--
2.7.4
^ permalink raw reply related
* [PATCHv2 0/4] WX checking for arm64
From: Laura Abbott @ 2016-10-12 22:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This is v2 of the implementation to check for writable and executable pages on
arm64.
Major changes since v1:
- I realized my concerns about initialization and registration were unfounded
so registration to register page tables with debugfs is simplified.
- New patch to remove max_addr since it was pointed out it was unused.
- Rebased to include changes for the EFI page tables as well.
- Checking is now only done on the init_mm page tables. It was mentioned that
we should check the hyp page tables as well but that can be follow on work.
- Checking for UXN per suggestion from Mark Rutland.
Laura Abbott (4):
arm64: dump: Make ptdump debugfs a separate option
arm64: dump: Make the page table dumping seq_file optional
arm64: dump: Remove max_addr
arm64: dump: Add checking for writable and exectuable pages
arch/arm64/Kconfig.debug | 34 ++++++++++++++-
arch/arm64/include/asm/ptdump.h | 22 +++++++---
arch/arm64/mm/Makefile | 3 +-
arch/arm64/mm/dump.c | 89 ++++++++++++++++++++++++++------------
arch/arm64/mm/mmu.c | 2 +
arch/arm64/mm/ptdump_debugfs.c | 31 +++++++++++++
drivers/firmware/efi/arm-runtime.c | 5 +--
7 files changed, 147 insertions(+), 39 deletions(-)
create mode 100644 arch/arm64/mm/ptdump_debugfs.c
--
2.7.4
^ permalink raw reply
* [PATCH V3 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Baicar, Tyler @ 2016-10-12 22:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161011185236.GC1041@n2100.armlinux.org.uk>
Hello Russell,
Thank you for the feedback! Responses below
On 10/11/2016 12:52 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 07, 2016 at 03:31:14PM -0600, Tyler Baicar wrote:
>> +static void cper_estatus_print_section_v300(const char *pfx,
>> + const struct acpi_hest_generic_data_v300 *gdata)
>> +{
>> + __u8 hour, min, sec, day, mon, year, century, *timestamp;
>> +
>> + if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
>> + timestamp = (__u8 *)&(gdata->time_stamp);
>> + memcpy(&sec, timestamp, 1);
>> + memcpy(&min, timestamp + 1, 1);
>> + memcpy(&hour, timestamp + 2, 1);
>> + memcpy(&day, timestamp + 4, 1);
>> + memcpy(&mon, timestamp + 5, 1);
>> + memcpy(&year, timestamp + 6, 1);
>> + memcpy(¢ury, timestamp + 7, 1);
> This is utterly silly. Why are you using memcpy() to access individual
> bytes of a u8 pointer? What's wrong with:
>
> sec = timestamp[0];
> min = timestamp[1];
> hour = timestamp[2];
> day = timestamp[4];
> mon = timestamp[5];
> year = timestamp[6];
> century = timestamp[7];
>
> or even do the conversion here:
>
> sec = bcd2bin(timestamp[0]);
> ... etc ...
Yes, that will be a lot cleaner especially with moving the conversion here.
>
>> + printk("%stime: ", pfx);
>> + printk("%7s", 0x01 & *(timestamp + 3) ? "precise" : "");
>> + printk(" %02d:%02d:%02d %02d%02d-%02d-%02d\n",
>> + bcd2bin(hour), bcd2bin(min), bcd2bin(sec),
>> + bcd2bin(century), bcd2bin(year), bcd2bin(mon),
>> + bcd2bin(day));
>> + }
> It's also a good idea to (as much as possible) keep to single printk()
> statements - which makes the emission of the string more atomic wrt
> other CPUs and contexts. So, this should probably become (with the
> conversion being done at the assignment of sec etc):
>
> printk("%stime: %7s %02d:%02d:%02d %02d%02d-%02d-%02d\n",
> pfx, 0x01 & timestamp[3] ? "precise" : "",
> hour, min, sec, century, year, mon, day);
>
> which, IMHO, looks a lot nicer and doesn't risk some other printk()
> getting between each individual part of the line.
I will make this change in the next version. This printk does look a lot
nicer and avoids other prints from getting in the middle (I actually
just saw that happen in testing a couple days ago)
>> +}
>> +
>> static void cper_estatus_print_section(
>> - const char *pfx, const struct acpi_hest_generic_data *gdata, int sec_no)
>> + const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
>> {
>> uuid_le *sec_type = (uuid_le *)gdata->section_type;
>> __u16 severity;
>> char newpfx[64];
>>
>> + if ((gdata->revision >> 8) >= 0x03)
>> + cper_estatus_print_section_v300(pfx,
>> + (const struct acpi_hest_generic_data_v300 *)gdata);
>> +
>> severity = gdata->error_severity;
>> printk("%s""Error %d, type: %s\n", pfx, sec_no,
>> cper_severity_str(severity));
> Not sure why you have the "" here - %sError works just as well and the
> "" is just obfuscation - the compiler will eliminate the double-double
> quote and merge the strings anyway.
>
I will remove the "" in the next version.
Thanks,
Tyler
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v3 3/6] pwm: imx: support output polarity inversion
From: Lukasz Majewski @ 2016-10-12 22:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5325a332099473a2d8382530f79c5a8e@agner.ch>
Hi Stefan,
> On 2016-10-07 08:11, Bhuvanchandra DV wrote:
> > From: Lothar Wassmann <LW@KARO-electronics.de>
> >
> > The i.MX pwm unit on i.MX27 and newer SoCs provides a configurable
> > output polarity. This patch adds support to utilize this feature
> > where available.
> >
> > Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +--
> > drivers/pwm/pwm-imx.c | 51
> > +++++++++++++++++++++-- 2 files changed, 51 insertions(+), 6
> > deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > index e00c2e9..c61bdf8 100644
> > --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > @@ -6,8 +6,8 @@ Required properties:
> > - "fsl,imx1-pwm" for PWM compatible with the one integrated on
> > i.MX1
> > - "fsl,imx27-pwm" for PWM compatible with the one integrated on
> > i.MX27
> > - reg: physical base address and length of the controller's
> > registers -- #pwm-cells: should be 2. See pwm.txt in this directory
> > for a description of
> > - the cells format.
> > +- #pwm-cells: 2 for i.MX1 and 3 for i.MX27 and newer SoCs. See
> > pwm.txt
> > + in this directory for a description of the cells format.
> > - clocks : Clock specifiers for both ipg and per clocks.
> > - clock-names : Clock names should include both "ipg" and "per"
> > See the clock consumer binding,
> > @@ -17,7 +17,7 @@ See the clock consumer binding,
> > Example:
> >
> > pwm1: pwm at 53fb4000 {
> > - #pwm-cells = <2>;
> > + #pwm-cells = <3>;
> > compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
> > reg = <0x53fb4000 0x4000>;
> > clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > index d600fd5..c37d223 100644
> > --- a/drivers/pwm/pwm-imx.c
> > +++ b/drivers/pwm/pwm-imx.c
> > @@ -38,6 +38,7 @@
> > #define MX3_PWMCR_DOZEEN (1 << 24)
> > #define MX3_PWMCR_WAITEN (1 << 23)
> > #define MX3_PWMCR_DBGEN (1 << 22)
> > +#define MX3_PWMCR_POUTC (1 << 18)
> > #define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16)
> > #define MX3_PWMCR_CLKSRC_IPG (1 << 16)
> > #define MX3_PWMCR_SWR (1 << 3)
> > @@ -180,6 +181,9 @@ static int imx_pwm_config_v2(struct pwm_chip
> > *chip, if (enable)
> > cr |= MX3_PWMCR_EN;
> >
> > + if (pwm->args.polarity == PWM_POLARITY_INVERSED)
> > + cr |= MX3_PWMCR_POUTC;
> > +
>
> This seems wrong to me, the config callback is meant for period/duty
> cycle only.
If it is meant only for that, then the polarity should be removed from
it.
However after very quick testing, at least on my setup, it turns out
that removing this lines causes polarity to _not_ being set (and the
polarity is not inverted).
I will investigate this further on my setup and hopefully sent proper
patch.
> The set_polarity callback should get called in case a
> different polarity is requested.
On my setup the pwm2 is set from DT and pwm_backlight_probe() calls
pwm_apply_args(), so everything should work. However, as I mentioned
above there still is some problem with inversion setting.
>
>
> > writel(cr, imx->mmio_base + MX3_PWMCR);
> >
> > return 0;
> > @@ -240,27 +244,62 @@ static void imx_pwm_disable(struct pwm_chip
> > *chip, struct pwm_device *pwm)
> > clk_disable_unprepare(imx->clk_per);
> > }
> >
> > -static struct pwm_ops imx_pwm_ops = {
> > +static int imx_pwm_set_polarity(struct pwm_chip *chip, struct
> > pwm_device *pwm,
> > + enum pwm_polarity polarity)
> > +{
> > + struct imx_chip *imx = to_imx_chip(chip);
> > + u32 val;
> > +
> > + if (polarity == pwm->args.polarity)
> > + return 0;
>
> I don't think that this is right. Today, pwm_apply_args (in
> include/linux/pwm.h) copies the polarity from args to state.polarity,
> which is then passed as polarity argument to this function. So this
> will always return 0 afaict.
Yes, I've overlooked it (that the state is copied).
It can be dropped.
>
> I would just drop that.
>
> There is probably one little problem in the current state of affairs:
> If the bootloader makes use of a PWM channel with inverted state,
> then the kernel would not know about that and currently assume a
> wrong initial state... I guess at one point in time we should
> implement the state retrieval callback and move to the new atomic PWM
> API, which would mean to implement apply callback.
Are there any patches on the horizon?
>
> --
> Stefan
>
>
> > +
> > + val = readl(imx->mmio_base + MX3_PWMCR);
> > +
> > + if (polarity == PWM_POLARITY_INVERSED)
> > + val |= MX3_PWMCR_POUTC;
> > + else
> > + val &= ~MX3_PWMCR_POUTC;
> > +
> > + writel(val, imx->mmio_base + MX3_PWMCR);
> > +
> > + dev_dbg(imx->chip.dev, "%s: polarity set to %s\n",
> > __func__,
> > + polarity == PWM_POLARITY_INVERSED ? "inverted" :
> > "normal"); +
> > + return 0;
> > +}
> > +
> > +static struct pwm_ops imx_pwm_ops_v1 = {
> > .enable = imx_pwm_enable,
> > .disable = imx_pwm_disable,
> > .config = imx_pwm_config,
> > .owner = THIS_MODULE,
> > };
> >
> > +static struct pwm_ops imx_pwm_ops_v2 = {
> > + .enable = imx_pwm_enable,
> > + .disable = imx_pwm_disable,
> > + .set_polarity = imx_pwm_set_polarity,
> > + .config = imx_pwm_config,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > struct imx_pwm_data {
> > int (*config)(struct pwm_chip *chip,
> > struct pwm_device *pwm, int duty_ns, int
> > period_ns); void (*set_enable)(struct pwm_chip *chip, bool enable);
> > + struct pwm_ops *pwm_ops;
> > };
> >
> > static struct imx_pwm_data imx_pwm_data_v1 = {
> > .config = imx_pwm_config_v1,
> > .set_enable = imx_pwm_set_enable_v1,
> > + .pwm_ops = &imx_pwm_ops_v1,
> > };
> >
> > static struct imx_pwm_data imx_pwm_data_v2 = {
> > .config = imx_pwm_config_v2,
> > .set_enable = imx_pwm_set_enable_v2,
> > + .pwm_ops = &imx_pwm_ops_v2,
> > };
> >
> > static const struct of_device_id imx_pwm_dt_ids[] = {
> > @@ -282,6 +321,8 @@ static int imx_pwm_probe(struct platform_device
> > *pdev) if (!of_id)
> > return -ENODEV;
> >
> > + data = of_id->data;
> > +
> > imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
> > if (imx == NULL)
> > return -ENOMEM;
> > @@ -300,18 +341,22 @@ static int imx_pwm_probe(struct
> > platform_device *pdev) return PTR_ERR(imx->clk_ipg);
> > }
> >
> > - imx->chip.ops = &imx_pwm_ops;
> > + imx->chip.ops = data->pwm_ops;
> > imx->chip.dev = &pdev->dev;
> > imx->chip.base = -1;
> > imx->chip.npwm = 1;
> > imx->chip.can_sleep = true;
> > + if (data->pwm_ops->set_polarity) {
> > + dev_dbg(&pdev->dev, "PWM supports output
> > inversion\n");
> > + imx->chip.of_xlate = of_pwm_xlate_with_flags;
> > + imx->chip.of_pwm_n_cells = 3;
> > + }
> >
> > r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
> > if (IS_ERR(imx->mmio_base))
> > return PTR_ERR(imx->mmio_base);
> >
> > - data = of_id->data;
> > imx->config = data->config;
> > imx->set_enable = data->set_enable;
>
Best regards,
?ukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161013/c11675fd/attachment-0001.sig>
^ permalink raw reply
* [PATCH V2 1/3] Revert "ACPI,PCI,IRQ: reduce static IRQ array size to 16"
From: Bjorn Helgaas @ 2016-10-12 22:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-2-git-send-email-okaya@codeaurora.org>
Hi Sinan,
I have to apologize because I haven't followed all the discussion and
now I'm trying to figure it out from the patches and changelogs. But
I guess that's not all bad, because future interested folks *should*
be able to figure things out from that :)
On Tue, Oct 04, 2016 at 05:15:17PM -0400, Sinan Kaya wrote:
> This reverts commit 5c5087a55390 ("ACPI,PCI,IRQ: reduce static IRQ array
> size to 16").
>
> The code maintains a fixed size array for IRQ penalties. The array
> gets updated by external calls such as acpi_penalize_sci_irq,
> acpi_penalize_isa_irq to reflect the actual interrupt usage of the
> system. Since the IRQ distribution is platform specific, this is
> not known ahead of time. The IRQs get updated based on the SCI
> interrupt number BIOS has chosen or the ISA IRQs that were assigned
> to existing peripherals.
>
> By the time ACPI gets initialized, this code tries to determine an
> IRQ number based on penalty values in this array. It will try to locate
> the IRQ with the least penalty assignment so that interrupt sharing is
> avoided if possible.
>
> A couple of notes about the external APIs:
> 1. These API can be called before the ACPI is started. Therefore, one
> cannot assume that the PCI link objects are initialized for calculating
> penalties.
Which API are you thinking about here? pcibios_penalize_isa_irq() is
called by ACPI and PNP, which should both be after ACPI is started.
My guess is you're thinking about acpi_penalize_sci_irq() (added back
later in this series), which is called here, which is definitely
before ACPI objects are available:
setup_arch
acpi_boot_init
acpi_process_madt
acpi_parse_madt_ioapic_entries
acpi_table_parse_madt
acpi_parse_int_src_ovr
acpi_sci_ioapic_setup
acpi_penalize_sci_irq # <---
> 2. The polarity and trigger information passed via the
> acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem
> is reporting as the call might have been placed before the IRQ is
> registered by the interrupt subsystem.
>
> The previous change was in the direction to remove these external API and
> try to calculate the penalties at runtime for the ISA path as well. This
> didn't work out well with the existing platforms.
>
> Restoring the old behavior for IRQ < 256 and the new behavior will remain
> effective for IRQ >= 256.
IIRC, this all started because we needed more than 256 IRQs, but we
didn't know how to size a static table to be large enough without
being wasteful.
Prior to 5c5087a55390, we tracked penalties for IRQs 0-255. After it,
we only tracked penalties for IRQs 0-15. I think this patch basically
makes it so we track 0-255 again.
*This* patch only increases the range for pcibios_penalize_isa_irq()
(and command-line hints, but hopefully nobody cares about those). A
subsequent patch increases it for SCI as well.
The name "ACPI_MAX_IRQS" is now slightly misleading (because we do
support more than 256 IRQs) and the 256 value is sort of an
unjustified magic number. 16 is explainable as the number of ISA
IRQs, but I don't know what 256 is based on (other than historical
practice, of course). ACPI device IRQs can be much larger, and I
think the SCI IRQ can be, too (the FADT SCI_INT field is 16 bits).
Can you tie this back to the specific problem on the broken machine
somehow? Do we need a penalty for an IRQ in the 16-255 range?
In a subsequent patch, I see something about the IRQ type not being
updated at the right time, but I can't quite connect the dots.
To be clear, I'm not asking for any changes in the patch; I'm just
trying to understand what's going on.
> Tested-by: Jonathan Liu <net147@gmail.com>
> Tested-by: Ondrej Zary <linux@rainbow-software.org>
> Link: http://www.gossamer-threads.com/lists/linux/kernel/2537016#2537016
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/acpi/pci_link.c | 35 ++++++++++++++++++-----------------
> 1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index c983bf7..f3792f4 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -438,6 +438,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
> * enabled system.
> */
>
> +#define ACPI_MAX_IRQS 256
> #define ACPI_MAX_ISA_IRQS 16
>
> #define PIRQ_PENALTY_PCI_POSSIBLE (16*16)
> @@ -446,7 +447,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
> #define PIRQ_PENALTY_ISA_USED (16*16*16*16*16)
> #define PIRQ_PENALTY_ISA_ALWAYS (16*16*16*16*16*16)
>
> -static int acpi_isa_irq_penalty[ACPI_MAX_ISA_IRQS] = {
> +static int acpi_irq_penalty[ACPI_MAX_IRQS] = {
> PIRQ_PENALTY_ISA_ALWAYS, /* IRQ0 timer */
> PIRQ_PENALTY_ISA_ALWAYS, /* IRQ1 keyboard */
> PIRQ_PENALTY_ISA_ALWAYS, /* IRQ2 cascade */
> @@ -511,7 +512,7 @@ static int acpi_irq_get_penalty(int irq)
> }
>
> if (irq < ACPI_MAX_ISA_IRQS)
> - return penalty + acpi_isa_irq_penalty[irq];
> + return penalty + acpi_irq_penalty[irq];
>
> penalty += acpi_irq_pci_sharing_penalty(irq);
> return penalty;
> @@ -538,14 +539,14 @@ int __init acpi_irq_penalty_init(void)
>
> for (i = 0; i < link->irq.possible_count; i++) {
> if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS)
> - acpi_isa_irq_penalty[link->irq.
> + acpi_irq_penalty[link->irq.
> possible[i]] +=
> penalty;
> }
>
> } else if (link->irq.active &&
> - (link->irq.active < ACPI_MAX_ISA_IRQS)) {
> - acpi_isa_irq_penalty[link->irq.active] +=
> + (link->irq.active < ACPI_MAX_IRQS)) {
> + acpi_irq_penalty[link->irq.active] +=
> PIRQ_PENALTY_PCI_POSSIBLE;
> }
> }
> @@ -828,7 +829,7 @@ static void acpi_pci_link_remove(struct acpi_device *device)
> }
>
> /*
> - * modify acpi_isa_irq_penalty[] from cmdline
> + * modify acpi_irq_penalty[] from cmdline
> */
> static int __init acpi_irq_penalty_update(char *str, int used)
> {
> @@ -837,24 +838,24 @@ static int __init acpi_irq_penalty_update(char *str, int used)
> for (i = 0; i < 16; i++) {
> int retval;
> int irq;
> - int new_penalty;
>
> retval = get_option(&str, &irq);
>
> if (!retval)
> break; /* no number found */
>
> - /* see if this is a ISA IRQ */
> - if ((irq < 0) || (irq >= ACPI_MAX_ISA_IRQS))
> + if (irq < 0)
> + continue;
> +
> + if (irq >= ARRAY_SIZE(acpi_irq_penalty))
> continue;
>
> if (used)
> - new_penalty = acpi_irq_get_penalty(irq) +
> - PIRQ_PENALTY_ISA_USED;
> + acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
> + PIRQ_PENALTY_ISA_USED;
> else
> - new_penalty = 0;
> + acpi_irq_penalty[irq] = 0;
>
> - acpi_isa_irq_penalty[irq] = new_penalty;
> if (retval != 2) /* no next number */
> break;
> }
> @@ -870,14 +871,14 @@ static int __init acpi_irq_penalty_update(char *str, int used)
> */
> void acpi_penalize_isa_irq(int irq, int active)
> {
> - if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty)))
> - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
> - (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
> + if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty))
> + acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
> + (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
> }
>
> bool acpi_isa_irq_available(int irq)
> {
> - return irq >= 0 && (irq >= ARRAY_SIZE(acpi_isa_irq_penalty) ||
> + return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) ||
> acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS);
> }
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V3 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Baicar, Tyler @ 2016-10-12 22:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3f17d0a8-6b63-5792-903a-341effaae432@arm.com>
Hello Suzuki,
Thank you for the feedback! Responses below.
On 10/11/2016 11:28 AM, Suzuki K Poulose wrote:
> On 07/10/16 22:31, Tyler Baicar wrote:
>> Currently when a RAS error is reported it is not timestamped.
>> The ACPI 6.1 spec adds the timestamp field to the generic error
>> data entry v3 structure. The timestamp of when the firmware
>> generated the error is now being reported.
>>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>
> Please could you keep the people who reviewed/commented on your series
> in the past,
> whenever you post a new version ?
Do you mean to just send the new version to their e-mail directly in
addition to the lists? If so, I will do that next time.
I know you provided good feedback on the previous patchset, but I did
not have anyone specifically respond to add "reviewed-by:...". I don't
think I should add reviewed-by for someone unless they specifically add
it in a response :)
>
>> ---
>> drivers/acpi/apei/ghes.c | 25 ++++++++++--
>> drivers/firmware/efi/cper.c | 97
>> +++++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 105 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 3021f0e..c8488f1 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -80,6 +80,10 @@
>> ((struct acpi_hest_generic_status *) \
>> ((struct ghes_estatus_node *)(estatus_node) + 1))
>>
>> +#define acpi_hest_generic_data_version(gdata) \
>> + (gdata->revision >> 8)
>
> ...
>
>> +inline void *acpi_hest_generic_data_payload(struct
>> acpi_hest_generic_data *gdata)
>> +{
>> + return acpi_hest_generic_data_version(gdata) >= 3 ?
>> + (void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
>> + gdata + 1;
>> +}
>> +
>
>
>
>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>> index d425374..9fa1317 100644
>> --- a/drivers/firmware/efi/cper.c
>> +++ b/drivers/firmware/efi/cper.c
>
>> +#define acpi_hest_generic_data_version(gdata) \
>> + (gdata->revision >> 8)
>> +
>
> ...
>
>> +static inline void *acpi_hest_generic_data_payload(struct
>> acpi_hest_generic_data *gdata)
>> +{
>> + return acpi_hest_generic_data_version(gdata) >= 3 ?
>> + (void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
>> + gdata + 1;
>> +}
>
> Could these go to a header file, so that we don't need duplicate
> definitions of these helpers in
> different files ?
>
I think that should work to avoid duplication. I will move them to a
header file in the next patchset.
>> +
>> +static void cper_estatus_print_section_v300(const char *pfx,
>> + const struct acpi_hest_generic_data_v300 *gdata)
>> +{
>> + __u8 hour, min, sec, day, mon, year, century, *timestamp;
>> +
>> + if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
>> + timestamp = (__u8 *)&(gdata->time_stamp);
>> + memcpy(&sec, timestamp, 1);
>> + memcpy(&min, timestamp + 1, 1);
>> + memcpy(&hour, timestamp + 2, 1);
>> + memcpy(&day, timestamp + 4, 1);
>> + memcpy(&mon, timestamp + 5, 1);
>> + memcpy(&year, timestamp + 6, 1);
>> + memcpy(¢ury, timestamp + 7, 1);
>> + printk("%stime: ", pfx);
>> + printk("%7s", 0x01 & *(timestamp + 3) ? "precise" : "");
>
> What format is the (timestamp + 3) stored in ? Does it need conversion ?
The third byte of the timestamp is currently only used to determine if
the time is precise or not. Bit 0 is used to specify that and the other
bits in this byte are marked as reserved. This is shown in table 247 of
the UEFI spec 2.6:
Byte 3:
Bit 0 ? Timestamp is precise if this bit is set and correlates to the
time of the error event.
Bit 7:1 ? Reserved
>
>> + printk(" %02d:%02d:%02d %02d%02d-%02d-%02d\n",
>> + bcd2bin(hour), bcd2bin(min), bcd2bin(sec),
>> + bcd2bin(century), bcd2bin(year), bcd2bin(mon),
>> + bcd2bin(day));
>> + }
>
> minor nit: Would it be easier to order/parse the error messages if the
> date
> is printed first followed by time ?
>
> i.e,
> 17:20:14 2016-09-15 Mon
> vs
> 2016-09-15 Mon 17:20:14
>
> e.g, people looking at a huge log, looking for logs from a specific
> date might
> find the latter more useful to skip the messages.
>
The latter does seem like it would be better for parsing large logs. I
can rearrange the order in the next patchset.
>> +}
>> +
>> static void cper_estatus_print_section(
>> - const char *pfx, const struct acpi_hest_generic_data *gdata, int
>> sec_no)
>> + const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
>> {
>> uuid_le *sec_type = (uuid_le *)gdata->section_type;
>> __u16 severity;
>> char newpfx[64];
>>
>> + if ((gdata->revision >> 8) >= 0x03)
>
> Could we use the helper defined above ?
Yes, I'll change this to use acpi_hest_generic_data_version(gdata) instead.
>
>> @@ -451,12 +497,22 @@ void cper_estatus_print(const char *pfx,
>> printk("%s""event severity: %s\n", pfx,
>> cper_severity_str(severity));
>> data_len = estatus->data_length;
>> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
>> + if ((gdata->revision >> 8) >= 0x03)
>
> Same as above, use the macro ?
Yes, I'll change this to use acpi_hest_generic_data_version(gdata) instead.
>
>> + gdata_v3 = (struct acpi_hest_generic_data_v300 *)gdata;
>> +
>> snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> +
>> while (data_len >= sizeof(*gdata)) {
>> gedata_len = gdata->error_data_length;
>> cper_estatus_print_section(newpfx, gdata, sec_no);
>> - data_len -= gedata_len + sizeof(*gdata);
>> - gdata = (void *)(gdata + 1) + gedata_len;
>> + if(gdata_v3) {
>> + data_len -= gedata_len + sizeof(*gdata_v3);
>> + gdata_v3 = (void *)(gdata_v3 + 1) + gedata_len;
>> + gdata = (struct acpi_hest_generic_data *)gdata_v3;
>> + } else {
>> + data_len -= gedata_len + sizeof(*gdata);
>> + gdata = (void *)(gdata + 1) + gedata_len;
>> + }
>> sec_no++;
>> }
>
> ...
>
>>
>> @@ -486,15 +543,29 @@ int cper_estatus_check(const struct
>> acpi_hest_generic_status *estatus)
>> return rc;
>> data_len = estatus->data_length;
>> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
>> - while (data_len >= sizeof(*gdata)) {
>> - gedata_len = gdata->error_data_length;
>> - if (gedata_len > data_len - sizeof(*gdata))
>> +
>> + if ((gdata->revision >> 8) >= 0x03) {
>> + gdata_v3 = (struct acpi_hest_generic_data_v300 *)gdata;
>> + while (data_len >= sizeof(*gdata_v3)) {
>> + gedata_len = gdata_v3->error_data_length;
>> + if (gedata_len > data_len - sizeof(*gdata_v3))
>> + return -EINVAL;
>> + data_len -= gedata_len + sizeof(*gdata_v3);
>> + gdata_v3 = (void *)(gdata_v3 + 1) + gedata_len;
>> + }
>> + if (data_len)
>> + return -EINVAL;
>> + } else {
>> + while (data_len >= sizeof(*gdata)) {
>> + gedata_len = gdata->error_data_length;
>> + if (gedata_len > data_len - sizeof(*gdata))
>> + return -EINVAL;
>> + data_len -= gedata_len + sizeof(*gdata);
>> + gdata = (void *)(gdata + 1) + gedata_len;
>> + }
>> + if (data_len)
>
> As mentioned in the previous version, would it make sense to add some
> more
> helpers to deal with record versions ? We seem to be doing the version
> switch and
> code duplication at different places.
>
> Does the following help ? Thoughts ?
>
> #define acpi_hest_generic_data_error_length(gdata) (((struct
> acpi_hest_generic_data *)(gdata))->error_data_length)
> #define acpi_hest_generic_data_size(gdata) \
> ((acpi_hest_generic_data_version(gdata) >= 3) ? \
> sizeof(struct acpi_hest_generic_data_v300) : \
> sizeof(struct acpi_hest_generic_data))
> #define acpi_hest_generic_data_record_size(gdata)
> (acpi_hest_generic_data_size(gdata) + \
> acpi_hest_generic_data_error_length(gdata))
> #define acpi_hest_generic_data_next(gdata) \
> ((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
>
>
> Suzuki
These helpers will definitely help consolidate this code. I will use
these in the next version to remove the code duplication here.
Thanks,
Tyler
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [arm-soc:to-build 1/3] arch/x86/include/asm/thread_info.h:174:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe
From: kbuild test robot @ 2016-10-12 22:06 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git to-build
head: b0a8f2c6d232f3f8463955e5927c56dd057495b1
commit: 3ddcb978f68b765b3af38bf45bf4a1a4b0a87df1 [1/3] Revert "Disable "maybe-uninitialized" warning globally"
config: i386-randconfig-s1-201641 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 3ddcb978f68b765b3af38bf45bf4a1a4b0a87df1
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/thread_info.h:69:0,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:59,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/mm.h:9,
from mm/usercopy.c:17:
arch/x86/include/asm/thread_info.h: In function 'check_stack_object':
>> arch/x86/include/asm/thread_info.h:174:11: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
oldframe = __builtin_frame_address(1);
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/thread_info.h:176:9: warning: calling '__builtin_frame_address' with a nonzero argument is unsafe [-Wframe-address]
frame = __builtin_frame_address(2);
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/__builtin_frame_address +174 arch/x86/include/asm/thread_info.h
0f60a8ef Kees Cook 2016-07-12 158 * Walks up the stack frames to make sure that the specified object is
0f60a8ef Kees Cook 2016-07-12 159 * entirely contained by a single stack frame.
0f60a8ef Kees Cook 2016-07-12 160 *
0f60a8ef Kees Cook 2016-07-12 161 * Returns:
0f60a8ef Kees Cook 2016-07-12 162 * 1 if within a frame
0f60a8ef Kees Cook 2016-07-12 163 * -1 if placed across a frame boundary (or outside stack)
0f60a8ef Kees Cook 2016-07-12 164 * 0 unable to determine (no frame pointers, etc)
0f60a8ef Kees Cook 2016-07-12 165 */
0f60a8ef Kees Cook 2016-07-12 166 static inline int arch_within_stack_frames(const void * const stack,
0f60a8ef Kees Cook 2016-07-12 167 const void * const stackend,
0f60a8ef Kees Cook 2016-07-12 168 const void *obj, unsigned long len)
0f60a8ef Kees Cook 2016-07-12 169 {
0f60a8ef Kees Cook 2016-07-12 170 #if defined(CONFIG_FRAME_POINTER)
0f60a8ef Kees Cook 2016-07-12 171 const void *frame = NULL;
0f60a8ef Kees Cook 2016-07-12 172 const void *oldframe;
0f60a8ef Kees Cook 2016-07-12 173
0f60a8ef Kees Cook 2016-07-12 @174 oldframe = __builtin_frame_address(1);
0f60a8ef Kees Cook 2016-07-12 175 if (oldframe)
0f60a8ef Kees Cook 2016-07-12 176 frame = __builtin_frame_address(2);
0f60a8ef Kees Cook 2016-07-12 177 /*
0f60a8ef Kees Cook 2016-07-12 178 * low ----------------------------------------------> high
0f60a8ef Kees Cook 2016-07-12 179 * [saved bp][saved ip][args][local vars][saved bp][saved ip]
0f60a8ef Kees Cook 2016-07-12 180 * ^----------------^
0f60a8ef Kees Cook 2016-07-12 181 * allow copies only within here
0f60a8ef Kees Cook 2016-07-12 182 */
:::::: The code at line 174 was first introduced by commit
:::::: 0f60a8efe4005ab5e65ce000724b04d4ca04a199 mm: Implement stack frame object validation
:::::: TO: Kees Cook <keescook@chromium.org>
:::::: CC: Kees Cook <keescook@chromium.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 24527 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161013/8e639a65/attachment-0001.gz>
^ 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