Re: [PATCH] schedule: use unlikely()
* Mikulas Patocka wrote:
>
>
> On Fri, 24 Nov 2017, Ingo Molnar wrote:
>
> > * Mikulas Patocka wrote:
> >
> > > A small patch for schedule(), so that the code goes straght in the common
> > > case.
> > >
> > > Signed-off-by: Mikulas Patocka
> > >
> > > ---
> > > include/linux/blkdev.h |2 +-
> > > kernel/sched/core.c|2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > Index: linux-2.6/include/linux/blkdev.h
> > > ===
> > > --- linux-2.6.orig/include/linux/blkdev.h
> > > +++ linux-2.6/include/linux/blkdev.h
> > > @@ -1308,7 +1308,7 @@ static inline bool blk_needs_flush_plug(
> > > {
> > > struct blk_plug *plug = tsk->plug;
> > >
> > > - return plug &&
> > > + return unlikely(plug != NULL) &&
> > > (!list_empty(&plug->list) ||
> > >!list_empty(&plug->mq_list) ||
> > >!list_empty(&plug->cb_list));
> >
> > That's an unrelated change.
>
> It is related, because this function gets inlined into schedule().
That needs to be in the changelog and the patch needs to be Cc:-ed to the
affected maintainer as well.
> > > Index: linux-2.6/kernel/sched/core.c
> > > ===
> > > --- linux-2.6.orig/kernel/sched/core.c
> > > +++ linux-2.6/kernel/sched/core.c
> > > @@ -3405,7 +3405,7 @@ void __noreturn do_task_dead(void)
> > >
> > > static inline void sched_submit_work(struct task_struct *tsk)
> > > {
> > > - if (!tsk->state || tsk_is_pi_blocked(tsk))
> > > + if (!tsk->state || unlikely(tsk_is_pi_blocked(tsk)))
> > > return;
> > > /*
> > >* If we are going to sleep and we have plugged IO queued,
> >
> > Do these changes actually change the generated assembly code?
> >
> > Thanks,
> >
> > Ingo
>
> Yes. It saves two jumps because the compiler falsely assumes that
> tsk_is_pi_blocked would return true. Likewise, the compiler falsely
> assumes that tsk->plug would be true.
>
> The static branch prediction in gcc assumes that pointers are usually not
> NULL, but in this case tsk->pi_blocked_on and tsk->plug are usually NULL.
That too should be in the changelog.
Thanks,
Ingo
Re: [PATCH v2] doc: add maintainer book
On Sat, Nov 25, 2017 at 08:56:23AM +0100, Greg Kroah-Hartman wrote: > On Sat, Nov 25, 2017 at 08:44:19AM +1100, Tobin C. Harding wrote: > > There is currently very little documentation in the kernel on maintainer > > level tasks. In particular there are no documents on creating pull > > requests to submit to Linus. > > > > Quoting Greg Kroah-Hartman on LKML: > > > > Anyway, this actually came up at the kernel summit / maintainer > > meeting a few weeks ago, in that "how do I make a > > good pull request to Linus" is something we need to document. > > > > Here's what I do, and it seems to work well, so maybe we should turn > > it into the start of the documentation for how to do it. > > > > (quote references: kernel summit, Europe 2017) > > > > Create a new kernel documentation book 'how to be a maintainer' > > (suggested by Jonathan Corbet). Add chapters on 'configuring git' and > > 'creating a pull request'. > > > > Most of the content was written by Linus Torvalds and Greg Kroah-Hartman > > in discussion on LKML. This is stated at the start of one of the > > chapters and the original email thread is referenced in > > 'pull-requests.rst'. > > > > Signed-off-by: Tobin C. Harding > > --- > > You dropped my reviewed-by :( Oh, I didn't realize I was able to keep it between versions. I realize this was a reasonably trivial change but in general how much change is ok while keeping the reviewed-by? Who's call is it, the original author, the reviewed-by dev or the maintainer? So v3 with your reviewed-by in the commit log below the signed-off tag. (ain't nothin like noise ;) thanks, Tobin.
Re: [PATCH v2] doc: add maintainer book
On Sat, Nov 25, 2017 at 07:22:36PM +1100, Tobin C. Harding wrote: > On Sat, Nov 25, 2017 at 08:56:23AM +0100, Greg Kroah-Hartman wrote: > > On Sat, Nov 25, 2017 at 08:44:19AM +1100, Tobin C. Harding wrote: > > > There is currently very little documentation in the kernel on maintainer > > > level tasks. In particular there are no documents on creating pull > > > requests to submit to Linus. > > > > > > Quoting Greg Kroah-Hartman on LKML: > > > > > > Anyway, this actually came up at the kernel summit / maintainer > > > meeting a few weeks ago, in that "how do I make a > > > good pull request to Linus" is something we need to document. > > > > > > Here's what I do, and it seems to work well, so maybe we should turn > > > it into the start of the documentation for how to do it. > > > > > > (quote references: kernel summit, Europe 2017) > > > > > > Create a new kernel documentation book 'how to be a maintainer' > > > (suggested by Jonathan Corbet). Add chapters on 'configuring git' and > > > 'creating a pull request'. > > > > > > Most of the content was written by Linus Torvalds and Greg Kroah-Hartman > > > in discussion on LKML. This is stated at the start of one of the > > > chapters and the original email thread is referenced in > > > 'pull-requests.rst'. > > > > > > Signed-off-by: Tobin C. Harding > > > --- > > > > You dropped my reviewed-by :( > > Oh, I didn't realize I was able to keep it between versions. I realize > this was a reasonably trivial change but in general how much change is > ok while keeping the reviewed-by? Who's call is it, the original > author, the reviewed-by dev or the maintainer? Use your judgement, usually for tiny changes it's good to keep it so people don't have to keep reviewing it and adding it again. thanks, greg k-h
Re: [PATCH] schedule: use unlikely()
On Mon, Nov 13, 2017 at 02:00:45PM -0500, Mikulas Patocka wrote: > A small patch for schedule(), so that the code goes straght in the common > case. > > Signed-off-by: Mikulas Patocka Was this a measurable difference? If so, great, please provide the numbers and how you tested in the changelog. If it can't be measured, then it is not worth it to add these markings as the CPU/compiler almost always knows better. thanks, greg k-h
Re: [PATCH 2/6] clk: lpc32xx: pr_err() strings should end with newlines
Hello Arvind, thank you for the change. On 11/24/2017 08:55 AM, Arvind Yadav wrote: > pr_err() messages should end with a new-line to avoid other messages > being concatenated. > > Signed-off-by: Arvind Yadav Acked-by: Vladimir Zapolskiy -- With best wishes, Vladimir
Re: [PATCH v3 RESEND] f2fs: add bug_on when f2fs_gc even fails to get one victim
Ok, I have found a panic with this bug_on for generic/027 today:
[ 5157.753224] F2FS-fs (loop2): Mounted with checkpoint version = 2e2
generic/027[ 5168.741251] run fstests generic/027 at 2017-11-25 04:46:40
[ 5189.445989] F2FS-fs (loop3): Found nat_bits in checkpoint
[ 5189.510872] F2FS-fs (loop3): Mounted with checkpoint version = 165da00b
[ 5250.613849] [ cut here ]
[ 5250.616840] kernel BUG at
/opt/s00293685/src/kernel/jaegeuk/f2fs/fs/f2fs/gc.c:1038!
[ 5250.628467] invalid opcode: [#1] SMP
[ 5250.628467] Modules linked in:
[ 5250.628467] CPU: 7 PID: 3173 Comm: xfs_io Not tainted 4.14.0-rc4+ #128
[ 5250.628467] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
[ 5250.628467] task: 880130f2be80 task.stack: c9000acd
[ 5250.628467] RIP: 0010:f2fs_gc+0x9da/0xa80
[ 5250.628467] RSP: 0018:c9000acd3b48 EFLAGS: 0246
[ 5250.628467] RAX: 001b RBX: 880134fa2648 RCX:
880134fa2f00
[ 5250.628467] RDX: 0006 RSI: 0200 RDI:
0001
[ 5250.628467] RBP: c9000acd3c38 R08: 001b R09:
0001
[ 5250.628467] R10: R11: 0001 R12:
[ 5250.628467] R13: 0001 R14: 880138472000 R15:
0002
[ 5250.628467] FS: 01666880() GS:88013fdc()
knlGS:
[ 5250.628467] CS: 0010 DS: ES: CR0: 80050033
[ 5250.628467] CR2: 006ef120 CR3: 000130f48000 CR4:
06e0
[ 5250.628467] Call Trace:
[ 5250.628467] f2fs_balance_fs+0x13c/0x1f0
[ 5250.628467] f2fs_create+0x146/0x260
[ 5250.628467] path_openat+0xe31/0x12c0
[ 5250.628467] do_filp_open+0x7e/0xd0
[ 5250.628467] ? kmem_cache_alloc+0x92/0x160
[ 5250.628467] ? getname_flags+0x4f/0x1f0
[ 5250.628467] do_sys_open+0x115/0x1f0
[ 5250.628467] SyS_open+0x1e/0x20
[ 5250.628467] entry_SYSCALL_64_fastpath+0x13/0x94
[ 5250.628467] RIP: 0033:0x4171d0
[ 5250.628467] RSP: 002b:7fff9a45b678 EFLAGS: 0246 ORIG_RAX:
0002
[ 5250.628467] RAX: ffda RBX: 0001 RCX:
004171d0
[ 5250.628467] RDX: 0180 RSI: 0042 RDI:
7fff9a45c1cb
[ 5250.628467] RBP: 7fff9a45c1bf R08: 7fff9a45b7f0 R09:
0001
[ 5250.628467] R10: 004bd8d3 R11: 0246 R12:
0006
[ 5250.628467] R13: 7fff9a45b830 R14: 0180 R15:
[ 5250.628467] Code: 00 bb c3 ff ff ff e9 2c fa ff ff 4d 8b 27 bb fb ff
ff ff c7 44 24 7c 00 00 00 00 c7 84 24 80 00 00 00 00 00 00 00 e9 0c fa
ff ff <0f> 0b 41 8b 96 fc 03 00 00 41 8b be f4 03 00 00 4c 8b 21 45 8b
[ 5250.628467] RIP: f2fs_gc+0x9da/0xa80 RSP: c9000acd3b48
[ 5250.685538] ---[ end trace 00b8c84c59632b32 ]---
Let me fix it one by one.
On 2017/11/23 21:05, Chao Yu wrote:
On 2017/11/22 11:50, Yunlong Song wrote:
ping again...
On 2017/11/17 9:09, Yunlong Song wrote:
This can help to find potential bugs on some corner case.
Could you test this patch with fstest suit? if there are any testcases
can trigger this bug_on, it will be better to fix them all together.
Thanks,
Signed-off-by: Yunlong Song
---
fs/f2fs/gc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 5d5bba4..c89128b 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1035,6 +1035,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
goto stop;
}
if (!__get_victim(sbi, &segno, gc_type)) {
+f2fs_bug_on(sbi, !total_freed && has_not_enough_free_secs(sbi, 0, 0));
ret = -ENODATA;
goto stop;
}
.
--
Thanks,
Yunlong Song
Re: [crash] PANIC: double fault, error_code: 0x0
* Ingo Molnar wrote: > > * Andy Lutomirski wrote: > > > > Note that if *any* of those 4 padding sequences is removed, the kernel > > > starts > > > crashing again. Also note that the exact size of the padding appears to > > > be not > > > material - it could be larger as well, i.e. it's not an alignment bug I > > > think. > > > > > > In any case it's not a problem in the actual assembly code paths itself > > > it > > > appears. > > > > > > One guess would be tha it's some sort of sizing bug: maybe the padding > > > forces a > > > key piece of data or code on another page - but I'm too tired to root > > > cause it > > > right now. > > > > > > Any ideas? > > > > This smells like a pagerable setup bug. Either the pagetables are a bit > > broken or they're totally busted and the passing gets something in a more > > TLB-friendly place. > > Also note that the delta patch below also keeps it working, i.e. doubling the > first padding and eliminating the second padding. > > I.e. it's the total per IRQ entry padding that matters, not the exact > placement of > the padding. > > I.e. some sort of sizing bug - IDT and/or the pagetables. > > (Also note that in my config NR_CPUS is at 128 - defconfigs are 64.) The simplest padding I found is the one below - this indicates some sort of section sizing or page table setup bug (or page alignment bug) and makes races and other bugs less likely. Thanks, Ingo => arch/x86/entry/entry_64.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 4ac952080869..ea992ca4e74f 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -547,6 +547,8 @@ END(irq_entries_start) ud2 .Lokay_\@: addq $8, %rsp +#else + .rep 64; nop; .endr #endif .endm
[PATCH v2] devres: use MACRO instead of function for devm_ioremap
Default ioremap is ioremap_nocache, so devm_ioremap has the same function
with devm_ioremap_nocache, which may just be killed. However, there are
many places which use devm_ioremap_nocache, while many use devm_ioremap.
This patch is to use MACRO for devm_ioremap, which will reduce the size of
devres.o from 206824 Bytes to 203768 Bytes.
Signed-off-by: Yisheng Xie
---
v2:
* MACRO devm_ioremap instead of devm_ioremap_nocache
include/linux/io.h | 4 ++--
lib/devres.c | 30 +-
2 files changed, 3 insertions(+), 31 deletions(-)
diff --git a/include/linux/io.h b/include/linux/io.h
index 32e30e8..3759882 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -73,8 +73,6 @@ static inline void devm_ioport_unmap(struct device *dev, void
__iomem *addr)
#define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
-void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
- resource_size_t size);
void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
resource_size_t size);
void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
@@ -84,6 +82,8 @@ int check_signature(const volatile void __iomem *io_addr,
const unsigned char *signature, int length);
void devm_ioremap_release(struct device *dev, void *res);
+#define devm_ioremap devm_ioremap_nocache
+
void *devm_memremap(struct device *dev, resource_size_t offset,
size_t size, unsigned long flags);
void devm_memunmap(struct device *dev, void *addr);
diff --git a/lib/devres.c b/lib/devres.c
index 5f2aedd..ebfaab1 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -16,34 +16,6 @@ static int devm_ioremap_match(struct device *dev, void *res,
void *match_data)
}
/**
- * devm_ioremap - Managed ioremap()
- * @dev: Generic device to remap IO address for
- * @offset: Resource address to map
- * @size: Size of map
- *
- * Managed ioremap(). Map is automatically unmapped on driver detach.
- */
-void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
- resource_size_t size)
-{
- void __iomem **ptr, *addr;
-
- ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- addr = ioremap(offset, size);
- if (addr) {
- *ptr = addr;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return addr;
-}
-EXPORT_SYMBOL(devm_ioremap);
-
-/**
* devm_ioremap_nocache - Managed ioremap_nocache()
* @dev: Generic device to remap IO address for
* @offset: Resource address to map
@@ -153,7 +125,7 @@ void __iomem *devm_ioremap_resource(struct device *dev,
struct resource *res)
return IOMEM_ERR_PTR(-EBUSY);
}
- dest_ptr = devm_ioremap(dev, res->start, size);
+ dest_ptr = devm_ioremap_nocache(dev, res->start, size);
if (!dest_ptr) {
dev_err(dev, "ioremap failed for resource %pR\n", res);
devm_release_mem_region(dev, res->start, size);
--
1.7.12.4
Re: [crash] PANIC: double fault, error_code: 0x0
* Ingo Molnar wrote: > > (Also note that in my config NR_CPUS is at 128 - defconfigs are 64.) > > The simplest padding I found is the one below - this indicates some sort of > section sizing or page table setup bug (or page alignment bug) and makes > races and > other bugs less likely. > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index 4ac952080869..ea992ca4e74f 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -547,6 +547,8 @@ END(irq_entries_start) > ud2 > .Lokay_\@: > addq $8, %rsp > +#else > + .rep 64; nop; .endr Also note that turning off CONFIG_UNWINDER_ORC also solves the crash. I did that in an attempt to get a different backtrace. So it's either unwinder related, or seemingly minor changes to code alignment/placement will make the bug go away. Thanks, Ingo
Re: [crash] PANIC: double fault, error_code: 0x0
* Ingo Molnar wrote: > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > > index 4ac952080869..ea992ca4e74f 100644 > > --- a/arch/x86/entry/entry_64.S > > +++ b/arch/x86/entry/entry_64.S > > @@ -547,6 +547,8 @@ END(irq_entries_start) > > ud2 > > .Lokay_\@: > > addq $8, %rsp > > +#else > > + .rep 64; nop; .endr > > Also note that turning off CONFIG_UNWINDER_ORC also solves the crash. I did > that > in an attempt to get a different backtrace. > > So it's either unwinder related, or seemingly minor changes to code > alignment/placement will make the bug go away. Ok, I think the Orc unwinder is innocent: I just forced a build with frame pointers but with ORC debuginfo and unwinder, and that is booting fine too. So it's the specific code size and alignment present in the config I sent that is triggering the bug. Fudging that alignment/sizing with the workaround patch above makes the crash go away. Thanks, Ingo
[PATCH] arm64: fix missing 'const' qualifiers
It was discovered during LTO-enabled compilation with gcc/ld.bfd.
Signed-off-by: Yury Norov
---
arch/arm64/kernel/cpu_ops.c | 7 ---
drivers/clk/hisilicon/crg-hi3516cv300.c | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index d16978213c5b..36178002f0c3 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -31,13 +31,14 @@ extern const struct cpu_operations cpu_psci_ops;
const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
-static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
+static const struct cpu_operations *const dt_supported_cpu_ops[] __initconst =
{
&smp_spin_table_ops,
&cpu_psci_ops,
NULL,
};
-static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
+static const struct cpu_operations
+ *const acpi_supported_cpu_ops[] __initconst = {
#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
&acpi_parking_protocol_ops,
#endif
@@ -47,7 +48,7 @@ static const struct cpu_operations *acpi_supported_cpu_ops[]
__initconst = {
static const struct cpu_operations * __init cpu_get_ops(const char *name)
{
- const struct cpu_operations **ops;
+ const struct cpu_operations *const *ops;
ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
diff --git a/drivers/clk/hisilicon/crg-hi3516cv300.c
b/drivers/clk/hisilicon/crg-hi3516cv300.c
index 2007123832bb..53450b651e4c 100644
--- a/drivers/clk/hisilicon/crg-hi3516cv300.c
+++ b/drivers/clk/hisilicon/crg-hi3516cv300.c
@@ -204,7 +204,7 @@ static const struct hisi_crg_funcs hi3516cv300_crg_funcs = {
/* hi3516CV300 sysctrl CRG */
#define HI3516CV300_SYSCTRL_NR_CLKS 16
-static const char *wdt_mux_p[] __initconst = { "3m", "apb" };
+static const char *const wdt_mux_p[] __initconst = { "3m", "apb" };
static u32 wdt_mux_table[] = {0, 1};
static const struct hisi_mux_clock hi3516cv300_sysctrl_mux_clks[] = {
--
2.11.0
Re: [PATCH v2] ARM: dts: imx6qdl-nitrogen6x: Add SPI NOR partitions
On Fri, Nov 24, 2017 at 3:00 PM, Otavio Salvador wrote: > This adds the partitions definition for the SPI NOR to provide > backward compatibility with the documented[1] layout used with > Boundary Devices BSP. > > 1. https://boundarydevices.com/boot-flash-access-linux/ > > It exports to Linux: > > mtd0: bootloader > mtd1: env > mtd2: splash > > Signed-off-by: Otavio Salvador After thinking a bit about Fabio's recommendation to use 'read-only' to protect the partitions, I think it'd be better to use 'lock' so it is still possible to write them on Linux but it requires a unlock prior changing it. In my case, I am interested in being capable of upgrading the bootloader from Linux. What people think? -- Otavio Salvador O.S. Systems http://www.ossystems.com.brhttp://code.ossystems.com.br Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750
Re: [PATCH] pata_pdc2027x: Remove unnecessary error check and coding style error.
On 11/25/2017 7:15 AM, Arvind Yadav wrote:
Here, The function pdc_hardware_init always return zero. So it is not
necessary to check its return value.
Fix these checkpatch.pl error:
ERROR: space prohibited after that '~' (ctx:WxW)
+ mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
ERROR: spaces required around that '?' (ctx:VxW)
+ long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
ERROR: that open brace { should be on the previous line
+ const struct ata_port_info *ppi[] =
+ { &pdc2027x_port_info[board_idx], NULL };
Please fix the checkpatch.pl errors in a sperate patch.
Signed-off-by: Arvind Yadav
[...]
MBR, Sergei
Re: [PATCH] pata_pdc2027x: Remove unnecessary error check and coding style error.
Hi Sergei,
On Saturday 25 November 2017 03:30 PM, Sergei Shtylyov wrote:
On 11/25/2017 7:15 AM, Arvind Yadav wrote:
Here, The function pdc_hardware_init always return zero. So it is not
necessary to check its return value.
Fix these checkpatch.pl error:
ERROR: space prohibited after that '~' (ctx:WxW)
+mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
ERROR: spaces required around that '?' (ctx:VxW)
+long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
ERROR: that open brace { should be on the previous line
+const struct ata_port_info *ppi[] =
+{ &pdc2027x_port_info[board_idx], NULL };
Please fix the checkpatch.pl errors in a sperate patch.
Please find a patch v3, which does not include checkpatch.pl error fix.
Signed-off-by: Arvind Yadav
[...]
MBR, Sergei
~arvind
[PATCH v3] pata_pdc2027x: Remove unnecessary error check
Here, The function pdc_hardware_init always return zero. So it is not
necessary to check its return value.
Signed-off-by: Arvind Yadav
---
changes in v2 :
Make function return type 'void' instead of 'int.
Add sapce between ':'.
changes in v3 :
Fix the checkpatch.pl errors in a sperate patch.
drivers/ata/pata_pdc2027x.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 82bfd51..6348a83 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -649,7 +649,7 @@ static long pdc_detect_pll_input_clock(struct ata_host
*host)
* @host: target ATA host
* @board_idx: board identifier
*/
-static int pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
+static void pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
{
long pll_clock;
@@ -665,8 +665,6 @@ static int pdc_hardware_init(struct ata_host *host,
unsigned int board_idx)
/* Adjust PLL control register */
pdc_adjust_pll(host, pll_clock, board_idx);
-
- return 0;
}
/**
@@ -753,8 +751,7 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
//pci_enable_intx(pdev);
/* initialize adapter */
- if (pdc_hardware_init(host, board_idx) != 0)
- return -EIO;
+ pdc_hardware_init(host, board_idx);
pci_set_master(pdev);
return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt,
@@ -778,8 +775,7 @@ static int pdc2027x_reinit_one(struct pci_dev *pdev)
else
board_idx = PDC_UDMA_133;
- if (pdc_hardware_init(host, board_idx))
- return -EIO;
+ pdc_hardware_init(host, board_idx);
ata_host_resume(host);
return 0;
--
2.7.4
[PATCH] pata_pdc2027x: Fix coding sytle error
Fix these checkpatch.pl error:
ERROR: space prohibited before open square bracket '['.
ERROR: space prohibited after that '~' (ctx:WxW)
+ mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
ERROR: spaces required around that '?' (ctx:VxW)
+ long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
ERROR: that open brace { should be on the previous line
+ const struct ata_port_info *ppi[] =
+ { &pdc2027x_port_info[board_idx], NULL };
Signed-off-by: Arvind Yadav
---
changes in v1:
These cheange was a part of '[PATCH] pata_pdc2027x:
Remove unnecessary error check and coding style error'
and got comment 'Please fix the checkpatch.pl errors
in a sperate patch'.
drivers/ata/pata_pdc2027x.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 6348a83..d1e8b63 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -84,7 +84,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct
ata_device **r_failed
*/
static struct pdc2027x_pio_timing {
u8 value0, value1, value2;
-} pdc2027x_pio_timing_tbl [] = {
+} pdc2027x_pio_timing_tbl[] = {
{ 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
{ 0x46, 0x29, 0xa4 }, /* PIO mode 1 */
{ 0x23, 0x26, 0x64 }, /* PIO mode 2 */
@@ -94,7 +94,7 @@ static struct pdc2027x_pio_timing {
static struct pdc2027x_mdma_timing {
u8 value0, value1;
-} pdc2027x_mdma_timing_tbl [] = {
+} pdc2027x_mdma_timing_tbl[] = {
{ 0xdf, 0x5f }, /* MDMA mode 0 */
{ 0x6b, 0x27 }, /* MDMA mode 1 */
{ 0x69, 0x25 }, /* MDMA mode 2 */
@@ -102,7 +102,7 @@ static struct pdc2027x_mdma_timing {
static struct pdc2027x_udma_timing {
u8 value0, value1, value2;
-} pdc2027x_udma_timing_tbl [] = {
+} pdc2027x_udma_timing_tbl[] = {
{ 0x4a, 0x0f, 0xd5 }, /* UDMA mode 0 */
{ 0x3a, 0x0a, 0xd0 }, /* UDMA mode 1 */
{ 0x2a, 0x07, 0xcd }, /* UDMA mode 2 */
@@ -277,7 +277,7 @@ static unsigned long pdc2027x_mode_filter(struct ata_device
*adev, unsigned long
ATA_ID_PROD_LEN + 1);
/* If the master is a maxtor in UDMA6 then the slave should not use
UDMA 6 */
if (strstr(model_num, "Maxtor") == NULL && pair->dma_mode ==
XFER_UDMA_6)
- mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
+ mask &= ~(1 << (6 + ATA_SHIFT_UDMA));
return mask;
}
@@ -520,7 +520,7 @@ static void pdc_adjust_pll(struct ata_host *host, long
pll_clock, unsigned int b
void __iomem *mmio_base = host->iomap[PDC_MMIO_BAR];
u16 pll_ctl;
long pll_clock_khz = pll_clock / 1000;
- long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
+ long pout_required = board_idx ? PDC_133_MHZ : PDC_100_MHZ;
long ratio = pout_required / pll_clock_khz;
int F, R;
@@ -705,8 +705,8 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
static const unsigned long cmd_offset[] = { 0x17c0, 0x15c0 };
static const unsigned long bmdma_offset[] = { 0x1000, 0x1008 };
unsigned int board_idx = (unsigned int) ent->driver_data;
- const struct ata_port_info *ppi[] =
- { &pdc2027x_port_info[board_idx], NULL };
+ const struct ata_port_info *ppi[] = {
+ &pdc2027x_port_info[board_idx], NULL };
struct ata_host *host;
void __iomem *mmio_base;
int i, rc;
--
2.7.4
Re: [PATCH v2 2/2] x86: disable IRQs before changing CR4
On Fri, 24 Nov 2017, Nadav Amit wrote: > /* Set in this cpu's CR4. */ > -static inline void cr4_set_bits(unsigned long mask) > +static inline void cr4_set_bits_irqs_off(unsigned long mask) This change is kinda weird. I'd expect that there is a corresponding function cr4_set_bits() which takes care of disabling interrupts. But there is not. All it does is creating a lot of pointless churn. > static __always_inline void setup_smep(struct cpuinfo_x86 *c) > static __init int setup_disable_smap(char *arg) > static __always_inline void setup_pku(struct cpuinfo_x86 *c) Why are you not doing this at the call site around all calls which fiddle with cr4, i.e. in identify_cpu() ? identify_cpu() is called from two places: identify_boot_cpu() and identify_secondary_cpu() identify_secondary_cpu is called with interrupts disabled anyway and there is no reason why we can't enforce interrupts being disabled around identify_cpu() completely. But if we actually do the right thing, i.e. having cr4_set_bit() and cr4_set_bit_irqsoff() all of this churn goes away magically. Then the only place which needs to be changed is the context switch because here interrupts are already disabled and we really care about performance. > @@ -293,7 +303,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct > task_struct *next_p, > } > > if ((tifp ^ tifn) & _TIF_NOTSC) > - cr4_toggle_bits(X86_CR4_TSD); > + cr4_toggle_bits_irqs_off(X86_CR4_TSD); > Thanks, tglx
Re: [PATCH] pata_pdc2027x: Fix coding sytle error
On Sat, 2017-11-25 at 16:04 +0530, Arvind Yadav wrote:
[]
> diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
[]
> @@ -84,7 +84,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct
> ata_device **r_failed
> */
> static struct pdc2027x_pio_timing {
> u8 value0, value1, value2;
> -} pdc2027x_pio_timing_tbl [] = {
> +} pdc2027x_pio_timing_tbl[] = {
> { 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
> { 0x46, 0x29, 0xa4 }, /* PIO mode 1 */
> { 0x23, 0x26, 0x64 }, /* PIO mode 2 */
trivia:
It seems all the _timing_tbl structs should be const
Re: [PATCH] pata_pdc2027x: Fix coding sytle error
Hi Joe,
On Saturday 25 November 2017 04:32 PM, Joe Perches wrote:
On Sat, 2017-11-25 at 16:04 +0530, Arvind Yadav wrote:
[]
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
[]
@@ -84,7 +84,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct
ata_device **r_failed
*/
static struct pdc2027x_pio_timing {
u8 value0, value1, value2;
-} pdc2027x_pio_timing_tbl [] = {
+} pdc2027x_pio_timing_tbl[] = {
{ 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
{ 0x46, 0x29, 0xa4 }, /* PIO mode 1 */
{ 0x23, 0x26, 0x64 }, /* PIO mode 2 */
trivia:
It seems all the _timing_tbl structs should be const
Yes, It should be const. I will push anther patch.
~arvind
[PATCH] pata_pdc2027x : make pdc2027x_*_timing structures const
Make these pdc2027x_*_timing structures const as it is never modified.
Signed-off-by: Arvind Yadav
---
drivers/ata/pata_pdc2027x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index d1e8b63..141bf81 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -82,7 +82,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct
ata_device **r_failed
* is issued to the device. However, if the controller clock is 133MHz,
* the following tables must be used.
*/
-static struct pdc2027x_pio_timing {
+static const struct pdc2027x_pio_timing {
u8 value0, value1, value2;
} pdc2027x_pio_timing_tbl[] = {
{ 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
@@ -92,7 +92,7 @@ static struct pdc2027x_pio_timing {
{ 0x23, 0x09, 0x25 }, /* PIO mode 4, IORDY on, Prefetch off */
};
-static struct pdc2027x_mdma_timing {
+static const struct pdc2027x_mdma_timing {
u8 value0, value1;
} pdc2027x_mdma_timing_tbl[] = {
{ 0xdf, 0x5f }, /* MDMA mode 0 */
@@ -100,7 +100,7 @@ static struct pdc2027x_mdma_timing {
{ 0x69, 0x25 }, /* MDMA mode 2 */
};
-static struct pdc2027x_udma_timing {
+static const struct pdc2027x_udma_timing {
u8 value0, value1, value2;
} pdc2027x_udma_timing_tbl[] = {
{ 0x4a, 0x0f, 0xd5 }, /* UDMA mode 0 */
--
2.7.4
[PATCH] x86/mm/kaiser: Fix IRQ entries text section mapping
* Ingo Molnar wrote: > > So it's either unwinder related, or seemingly minor changes to code > > alignment/placement will make the bug go away. > > Ok, I think the Orc unwinder is innocent: I just forced a build with frame > pointers but with ORC debuginfo and unwinder, and that is booting fine too. > > So it's the specific code size and alignment present in the config I sent > that is > triggering the bug. Fudging that alignment/sizing with the workaround patch > above > makes the crash go away. Ok, after a few hours of debugging with Thomas today we found the bug: it was caused by Kaiser not mapping the __irqentry_text_start...__irqentry_text_end kernel virtual memory range, which the IRQ entry code requires before it switches to the kernel page tables. This bug was hidden in most configs by virtue of: kaiser_add_user_map_ptrs_early(__entry_text_start, __entry_text_end, __PAGE_KERNEL_RX | _PAGE_GLOBAL); because the __irqentry_text_start...__irqentry_text_end kernel code section comes right after this section and is pretty small - so it usually fits into the last page of the mapping. But for certain configs the __entry_text_start address crossed the next page boundary, and was not mapped - resulting in a page fault and then in a double fault as the entry stack overflowed. The patch below fixes the crash, and after some testing I'll backmerge it to the core Kaiser patch in tip:WIP.x86/mm. There's a few takeaways from this incident though: - Entry stack overflow debugging is pretty crude and should probably be improved. - We should better isolate all Kaiser-mapped sections and pad them out to page boundary. This would immediately trigger any simila false-sharing side-effect mapping bugs and they wouldn't be .config dependent Heisenbugs. Thanks, Ingo => >From 19bbacd5f7c66b4716716bad97848a9cacd6fe68 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Sat, 25 Nov 2017 12:10:38 +0100 Subject: [PATCH] x86/mm/kaiser: Fix IRQ entries text section mapping Backmerge to: f9e7e52fa076: x86/mm/kaiser: Unmap kernel from userspace page tables (core patch) Signed-off-by: Ingo Molnar --- arch/x86/mm/kaiser.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/mm/kaiser.c b/arch/x86/mm/kaiser.c index 1eb27b410556..ea8887bf550a 100644 --- a/arch/x86/mm/kaiser.c +++ b/arch/x86/mm/kaiser.c @@ -412,6 +412,8 @@ void __init kaiser_init(void) kaiser_add_user_map_ptrs_early(__entry_text_start, __entry_text_end, __PAGE_KERNEL_RX | _PAGE_GLOBAL); + kaiser_add_user_map_ptrs_early(__irqentry_text_start, __irqentry_text_end, + __PAGE_KERNEL_RX | _PAGE_GLOBAL); /* the fixed map address of the idt_table */ kaiser_add_user_map_early((void *)idt_descr.address,
[PATCH] nvmem: core: Deduplicate bus_find_device() by name matching
No need to reinvent the wheel, we have bus_find_device_by_name().
Signed-off-by: Lukas Wunner
---
drivers/nvmem/core.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 5a5cefd12153..93084ab61e0f 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -600,16 +600,11 @@ static void __nvmem_device_put(struct nvmem_device *nvmem)
mutex_unlock(&nvmem_mutex);
}
-static int nvmem_match(struct device *dev, void *data)
-{
- return !strcmp(dev_name(dev), data);
-}
-
static struct nvmem_device *nvmem_find(const char *name)
{
struct device *d;
- d = bus_find_device(&nvmem_bus_type, NULL, (void *)name, nvmem_match);
+ d = bus_find_device_by_name(&nvmem_bus_type, NULL, name);
if (!d)
return NULL;
--
2.11.0
Re: [PATCH 15/43] x86/entry/64: Create a percpu SYSCALL entry trampoline
On Fri, Nov 24, 2017 at 06:23:43PM +0100, Ingo Molnar wrote:
> From: Andy Lutomirski
>
> Handling SYSCALL is tricky: the SYSCALL handler is entered with every
> single register (except FLAGS), including RSP, live. It somehow needs
> to set RSP to point to a valid stack, which means it needs to save the
> user RSP somewhere and find its own stack pointer. The canonical way
> to do this is with SWAPGS, which lets us access percpu data using the
> %gs prefix.
>
> With KAISER-like pagetable switching, this is problematic. Without a
> scratch register, switching CR3 is impossible, so %gs-based percpu
> memory would need to be mapped in the user pagetables. Doing that
> without information leaks is difficult or impossible.
>
> Instead, use a different sneaky trick. Map a copy of the first part
> of the SYSCALL asm at a different address for each CPU. Now RIP
> varies depending on the CPU, so we can use RIP-relative memory access
> to access percpu memory. By putting the relevant information (one
> scratch slot and the stack address) at a constant offset relative to
> RIP, we can make SYSCALL work without relying on %gs.
>
> A nice thing about this approach is that we can easily switch it on
> and off if we want pagetable switching to be configurable.
>
> The compat variant of SYSCALL doesn't have this problem in the first
> place -- there are plenty of scratch registers, since we don't care
> about preserving r8-r15. This patch therefore doesn't touch SYSCALL32
> at all.
>
> XXX: Whenever we settle how KAISER gets turned on and off, we should do
> the same to this.
>
> Signed-off-by: Andy Lutomirski
> Signed-off-by: Thomas Gleixner
> Cc: Borislav Petkov
> Cc: Brian Gerst
> Cc: Dave Hansen
> Cc: Josh Poimboeuf
> Cc: Linus Torvalds
> Cc: Peter Zijlstra
> Link:
> https://lkml.kernel.org/r/b95ccae0a5a2f090c901e49fce7c9e8ff6acd40d.1511497875.git.l...@kernel.org
> Signed-off-by: Ingo Molnar
> ---
> arch/x86/entry/entry_64.S | 48
> +++
> arch/x86/include/asm/fixmap.h | 2 ++
> arch/x86/kernel/asm-offsets.c | 1 +
> arch/x86/kernel/cpu/common.c | 12 ++-
> arch/x86/kernel/vmlinux.lds.S | 10 +
> 5 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 426b8c669d6a..0cde243b7542 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -140,6 +140,54 @@ END(native_usergs_sysret64)
> * with them due to bugs in both AMD and Intel CPUs.
> */
>
> + .pushsection .entry_trampoline, "ax"
> +
> +/*
> + * The code in here gets remapped into cpu_entry_area's trampoline. This
> means
> + * that the assembler and linker have the wrong idea as to where this code
> + * lives (and, in fact, it's mapped more than once, so it's not even at a
> + * fixed address). So we can't reference any symbols outside the entry
> + * trampoline and expect it to work.
> + *
> + * Instead, we carefully abuse %rip-relative addressing.
> + * .Lentry_trampoline(%rip) refers to the start of the remapped) entry
_entry_trampoline(%rip) I'd guess.
> + * trampoline. We can thus find cpu_entry_area with this macro:
Uuh, fun. :)
> + */
> +
> +#define CPU_ENTRY_AREA \
> + _entry_trampoline - CPU_ENTRY_AREA_entry_trampoline(%rip)
So this generates
_entry_trampoline - 16384(%rip)
here. IINM, the layout looks like this
[ GDT | TSS | TSS | TSS | trampoline ]
where each section is a page, and we have 4 pages per CPU. Just for my
own understanding...
> +
> +/* The top word of the SYSENTER stack is hot and is usable as scratch space.
> */
> +#define RSP_SCRATCH CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + \
> + SIZEOF_SYSENTER_stack - 8 + CPU_ENTRY_AREA
I'm wondering if it would be easier to make SYSENTER_stack part of
struct cpu_entry_area and thus simplify that wild offset math here :)
Also, pls align:
#define RSP_SCRATCH CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + \
SIZEOF_SYSENTER_stack - 8 + CPU_ENTRY_AREA
> +
> +ENTRY(entry_SYSCALL_64_trampoline)
> + UNWIND_HINT_EMPTY
> + swapgs
> +
> + /* Stash the user RSP. */
> + movq%rsp, RSP_SCRATCH
> +
> + /* Load the top of the task stack into RSP */
> + movqCPU_ENTRY_AREA_tss + TSS_sp1 + CPU_ENTRY_AREA, %rsp
Yeah, let's put CPU_ENTRY_AREA first because then it reads easier:
movqCPU_ENTRY_AREA + CPU_ENTRY_AREA_tss + TSS_sp1, %rsp
i.e., pointer to cpu_entry_area, offset to tss within said
cpu_entry_area and then inside that, sp1.
Ditto for above.
...
> @@ -1417,10 +1424,13 @@ static DEFINE_PER_CPU_PAGE_ALIGNED(char,
> exception_stacks
> /* May not be marked __init: used by software suspend */
> void syscall_init(void)
> {
> + extern char _entry_trampoline[];
> + extern char entry_SYSCALL_64_trampoline[];
> +
> int cpu = smp_processor_id();
>
> wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
> - wrms
[PATCH] nvmem: core: switch to device_property_present for reading property "read-only"
Switch to more generic device_property_present to consider also non-DT
properties.
Signed-off-by: Heiner Kallweit
---
drivers/nvmem/core.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 5a5cefd12..ba0e3b453 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -444,7 +444,6 @@ static int nvmem_setup_compat(struct nvmem_device *nvmem,
struct nvmem_device *nvmem_register(const struct nvmem_config *config)
{
struct nvmem_device *nvmem;
- struct device_node *np;
int rval;
if (!config->dev)
@@ -473,13 +472,12 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->priv = config->priv;
nvmem->reg_read = config->reg_read;
nvmem->reg_write = config->reg_write;
- np = config->dev->of_node;
- nvmem->dev.of_node = np;
+ nvmem->dev.of_node = config->dev->of_node;
dev_set_name(&nvmem->dev, "%s%d",
config->name ? : "nvmem",
config->name ? config->id : nvmem->id);
- nvmem->read_only = of_property_read_bool(np, "read-only") |
+ nvmem->read_only = device_property_present(config->dev, "read-only") |
config->read_only;
if (config->root_only)
--
2.15.0
Re: [f2fs-dev] [PATCH] f2fs: remove an excess variable
On 2017/11/25 11:46, LiFan wrote: > Remove the variable page_idx which no one would miss. > > Signed-off-by: Fan li Reviewed-by: Chao Yu Thanks,
Re: [PATCH 16/43] x86/irq: Remove an old outdated comment about context tracking races
On Fri, Nov 24, 2017 at 06:23:44PM +0100, Ingo Molnar wrote: > From: Andy Lutomirski > > That race has been fixed and code cleaned up for a while now. > > Signed-off-by: Andy Lutomirski > Signed-off-by: Thomas Gleixner > Cc: Borislav Petkov > Cc: Brian Gerst > Cc: Dave Hansen > Cc: Josh Poimboeuf > Cc: Linus Torvalds > Cc: Peter Zijlstra > Link: > https://lkml.kernel.org/r/12e75976dbbb7ece2b0a64238f1d3892dfed1e16.1511497875.git.l...@kernel.org > Signed-off-by: Ingo Molnar > --- > arch/x86/kernel/irq.c | 12 > 1 file changed, 12 deletions(-) > > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > index 49cfd9fe7589..68e1867cca80 100644 > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c > @@ -219,18 +219,6 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs > *regs) > /* high bit used in ret_from_ code */ > unsigned vector = ~regs->orig_ax; > > - /* > - * NB: Unlike exception entries, IRQ entries do not reliably > - * handle context tracking in the low-level entry code. This is > - * because syscall entries execute briefly with IRQs on before > - * updating context tracking state, so we can take an IRQ from > - * kernel mode with CONTEXT_USER. The low-level entry code only > - * updates the context if we came from user mode, so we won't > - * switch to CONTEXT_KERNEL. We'll fix that once the syscall > - * code is cleaned up enough that we can cleanly defer enabling > - * IRQs. > - */ > - > entering_irq(); > > /* entering_irq() tells RCU that we're not quiescent. Check it. */ > -- Reviewed-by: Borislav Petkov Also, fixes like that should move to the top of the patchset. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 17/43] x86/irq/64: In the stack overflow warning, print the offending IP
On Fri, Nov 24, 2017 at 06:23:45PM +0100, Ingo Molnar wrote:
> From: Andy Lutomirski
>
> In case something goes wrong with unwind (not unlikely in case of
> overflow), print the offending IP where we detected the overflow.
>
> Signed-off-by: Andy Lutomirski
> Signed-off-by: Thomas Gleixner
> Cc: Borislav Petkov
> Cc: Brian Gerst
> Cc: Dave Hansen
> Cc: Josh Poimboeuf
> Cc: Linus Torvalds
> Cc: Peter Zijlstra
> Link:
> https://lkml.kernel.org/r/6fcf700cc5ee884fb739b67d1246ab4185c41409.1511497875.git.l...@kernel.org
> Signed-off-by: Ingo Molnar
> ---
> arch/x86/kernel/irq_64.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/irq_64.c b/arch/x86/kernel/irq_64.c
> index 020efbf5786b..d86e344f5b3d 100644
> --- a/arch/x86/kernel/irq_64.c
> +++ b/arch/x86/kernel/irq_64.c
> @@ -57,10 +57,10 @@ static inline void stack_overflow_check(struct pt_regs
> *regs)
> if (regs->sp >= estack_top && regs->sp <= estack_bottom)
> return;
>
> - WARN_ONCE(1, "do_IRQ(): %s has overflown the kernel stack
> (cur:%Lx,sp:%lx,irq stk top-bottom:%Lx-%Lx,exception stk
> top-bottom:%Lx-%Lx)\n",
> + WARN_ONCE(1, "do_IRQ(): %s has overflown the kernel stack
> (cur:%Lx,sp:%lx,irq stk top-bottom:%Lx-%Lx,exception stk
> top-bottom:%Lx-%Lx,ip:%pF)\n",
> current->comm, curbase, regs->sp,
> irq_stack_top, irq_stack_bottom,
> - estack_top, estack_bottom);
> + estack_top, estack_bottom, (void *)regs->ip);
>
> if (sysctl_panic_on_stackoverflow)
> panic("low stack detected by irq handler - check messages\n");
> --
Reviewed-by: Borislav Petkov
Also move to the beginning of the patchset.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 18/43] x86/entry/64: Move the IST stacks into cpu_entry_area
On Fri, Nov 24, 2017 at 06:23:46PM +0100, Ingo Molnar wrote: > From: Andy Lutomirski > > The IST stacks are needed when an IST exception occurs and are > accessed before any kernel code at all runs. Move them into > cpu_entry_area. > > Signed-off-by: Andy Lutomirski > Signed-off-by: Thomas Gleixner > Cc: Borislav Petkov > Cc: Brian Gerst > Cc: Dave Hansen > Cc: Josh Poimboeuf > Cc: Linus Torvalds > Cc: Peter Zijlstra > Link: > https://lkml.kernel.org/r/0ffddccdc0ce1953f950a553142662cf68258fb7.1511497875.git.l...@kernel.org > Signed-off-by: Ingo Molnar > --- > arch/x86/include/asm/fixmap.h | 10 ++ > arch/x86/kernel/cpu/common.c | 40 +--- > 2 files changed, 35 insertions(+), 15 deletions(-) Reviewed-by: Borislav Petkov -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
[tip:x86/urgent] x86/tlb: Refactor CR4 setting and shadow write
Commit-ID: 0c3292ca8025c5aef44dc389ac3a6bf4a325e0be
Gitweb: https://git.kernel.org/tip/0c3292ca8025c5aef44dc389ac3a6bf4a325e0be
Author: Nadav Amit
AuthorDate: Fri, 24 Nov 2017 19:29:06 -0800
Committer: Thomas Gleixner
CommitDate: Sat, 25 Nov 2017 13:28:43 +0100
x86/tlb: Refactor CR4 setting and shadow write
Refactor the write to CR4 and its shadow value. This is done in
preparation for the addition of an assertion to check that IRQs are
disabled during CR4 update.
No functional change.
Signed-off-by: Nadav Amit
Signed-off-by: Thomas Gleixner
Cc: nadav.a...@gmail.com
Cc: Andy Lutomirski
Cc: linux-e...@vger.kernel.org
Link: https://lkml.kernel.org/r/20171125032907.2241-2-na...@vmware.com
---
arch/x86/include/asm/tlbflush.h | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 509046c..e736f7f 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -173,17 +173,20 @@ static inline void cr4_init_shadow(void)
this_cpu_write(cpu_tlbstate.cr4, __read_cr4());
}
+static inline void __cr4_set(unsigned long cr4)
+{
+ this_cpu_write(cpu_tlbstate.cr4, cr4);
+ __write_cr4(cr4);
+}
+
/* Set in this cpu's CR4. */
static inline void cr4_set_bits(unsigned long mask)
{
unsigned long cr4;
cr4 = this_cpu_read(cpu_tlbstate.cr4);
- if ((cr4 | mask) != cr4) {
- cr4 |= mask;
- this_cpu_write(cpu_tlbstate.cr4, cr4);
- __write_cr4(cr4);
- }
+ if ((cr4 | mask) != cr4)
+ __cr4_set(cr4 | mask);
}
/* Clear in this cpu's CR4. */
@@ -192,11 +195,8 @@ static inline void cr4_clear_bits(unsigned long mask)
unsigned long cr4;
cr4 = this_cpu_read(cpu_tlbstate.cr4);
- if ((cr4 & ~mask) != cr4) {
- cr4 &= ~mask;
- this_cpu_write(cpu_tlbstate.cr4, cr4);
- __write_cr4(cr4);
- }
+ if ((cr4 & ~mask) != cr4)
+ __cr4_set(cr4 & ~mask);
}
static inline void cr4_toggle_bits(unsigned long mask)
@@ -204,9 +204,7 @@ static inline void cr4_toggle_bits(unsigned long mask)
unsigned long cr4;
cr4 = this_cpu_read(cpu_tlbstate.cr4);
- cr4 ^= mask;
- this_cpu_write(cpu_tlbstate.cr4, cr4);
- __write_cr4(cr4);
+ __cr4_set(cr4 ^ mask);
}
/* Read the CR4 shadow. */
[tip:x86/urgent] x86/tlb: Disable interrupts when changing CR4
Commit-ID: 9d0b62328d34c7044114d4f4281981d4c537c4ba
Gitweb: https://git.kernel.org/tip/9d0b62328d34c7044114d4f4281981d4c537c4ba
Author: Nadav Amit
AuthorDate: Fri, 24 Nov 2017 19:29:07 -0800
Committer: Thomas Gleixner
CommitDate: Sat, 25 Nov 2017 13:28:43 +0100
x86/tlb: Disable interrupts when changing CR4
CR4 modifications are implemented as RMW operations which update a shadow
variable and write the result to CR4. The RMW operation is protected by
preemption disable, but there is no enforcement or debugging mechanism.
CR4 modifications happen also in interrupt context via
__native_flush_tlb_global(). This implementation does not affect a
interrupted thread context CR4 operation, because the CR4 toggle restores
the original content and does not modify the shadow variable.
So the current situation seems to be safe, but a recent patch tried to add
an actual RMW operation in interrupt context, which will cause subtle
corruptions.
To prevent that and make the CR4 handling future proof:
- Add a lockdep assertion to __cr4_set() which will catch interrupt
enabled invocations
- Disable interrupts in the cr4 manipulator inlines
- Rename cr4_toggle_bits() to cr4_toggle_bits_irqsoff(). This is called
from __switch_to_xtra() where interrupts are already disabled and
performance matters.
All other call sites are not performance critical, so the extra overhead of
an additional local_irq_save/restore() pair is not a problem. If new call
sites care about performance then the necessary _irqsoff() variants can be
added.
[ tglx: Condensed the patch by moving the irq protection inside the
manipulator functions. Updated changelog ]
Signed-off-by: Nadav Amit
Signed-off-by: Thomas Gleixner
Cc: Luck
Cc: Radim Krčmář
Cc: Andy Lutomirski
Cc: Paolo Bonzini
Cc: Borislav Petkov
Cc: nadav.a...@gmail.com
Cc: linux-e...@vger.kernel.org
Link: https://lkml.kernel.org/r/20171125032907.2241-3-na...@vmware.com
---
arch/x86/include/asm/tlbflush.h | 11 ---
arch/x86/kernel/process.c | 2 +-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index e736f7f..877b5c1 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -175,6 +175,7 @@ static inline void cr4_init_shadow(void)
static inline void __cr4_set(unsigned long cr4)
{
+ lockdep_assert_irqs_disabled();
this_cpu_write(cpu_tlbstate.cr4, cr4);
__write_cr4(cr4);
}
@@ -182,24 +183,28 @@ static inline void __cr4_set(unsigned long cr4)
/* Set in this cpu's CR4. */
static inline void cr4_set_bits(unsigned long mask)
{
- unsigned long cr4;
+ unsigned long cr4, flags;
+ local_irq_save(flags);
cr4 = this_cpu_read(cpu_tlbstate.cr4);
if ((cr4 | mask) != cr4)
__cr4_set(cr4 | mask);
+ local_irq_restore(flags);
}
/* Clear in this cpu's CR4. */
static inline void cr4_clear_bits(unsigned long mask)
{
- unsigned long cr4;
+ unsigned long cr4, flags;
+ local_irq_save(flags);
cr4 = this_cpu_read(cpu_tlbstate.cr4);
if ((cr4 & ~mask) != cr4)
__cr4_set(cr4 & ~mask);
+ local_irq_restore(flags);
}
-static inline void cr4_toggle_bits(unsigned long mask)
+static inline void cr4_toggle_bits_irqsoff(unsigned long mask)
{
unsigned long cr4;
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 97fb3e5..bb988a2 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -299,7 +299,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct
task_struct *next_p,
}
if ((tifp ^ tifn) & _TIF_NOTSC)
- cr4_toggle_bits(X86_CR4_TSD);
+ cr4_toggle_bits_irqsoff(X86_CR4_TSD);
if ((tifp ^ tifn) & _TIF_NOCPUID)
set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
Re: [PATCH 22/43] x86/mm/kaiser: Prepare assembly for entry/exit CR3 switching
On Sat, 25 Nov 2017, Thomas Gleixner wrote: > On Fri, 24 Nov 2017, Ingo Molnar wrote: > > @@ -1288,6 +1308,8 @@ ENTRY(error_entry) > > * from user mode due to an IRET fault. > > */ > > SWAPGS > > + /* We have user CR3. Change to kernel CR3. */ > > + SWITCH_TO_KERNEL_CR3 scratch_reg=%rax > > > > .Lerror_entry_from_usermode_after_swapgs: > > /* Put us onto the real thread stack. */ > > @@ -1333,6 +1355,7 @@ ENTRY(error_entry) > > * gsbase and proceed. We'll fix up the exception and land in > > * .Lgs_change's error handler with kernel gsbase. > > */ > > + SWITCH_TO_KERNEL_CR3 scratch_reg=%rax > > SWAPGS > > This is wrong. SWAPGS needs to come first. With this fixed: Reviewed-by: Thomas Gleixner
( Compensation Reinbursement )
View the enclosed file for your Compensation Reinbursement. United Nations Compensation Unit.docx Description: Binary data
Re: [PATCH v5 10/11] intel_sgx: glue code for in-kernel LE
On Fri, Nov 17, 2017 at 03:07:05PM -0800, Darren Hart wrote:
> On Mon, Nov 13, 2017 at 09:45:27PM +0200, Jarkko Sakkinen wrote:
> > Glue code for hosting in-kernel Launch Enclave (LE) by using the user
> > space helper framework.
> >
> > Tokens for launching enclaves are generated with by the following
> > protocol:
> >
> > 1. The driver sends a SIGSTRUCT blob to the LE hosting process
> >to the input pipe.
> > 2. The LE hosting process reads the SIGSTRUCT blob from the input
> >pipe.
> > 3. After generating a EINITTOKEN blob, the LE hosting process writes
> >it to the output pipe.
> > 4. The driver reads the EINITTOKEN blob from the output pipe.
> >
> > If IA32_SGXLEPUBKEYHASH* MSRs are writable and they don't have the
> > public key hash of the LE they will be updated.
> >
>
> A few nits throughout to keep in mind:
>
> * #includes in alphabetical order in general
> * function local variables declared in order of decreasing line length
> * don't insert newlines where coding_style doesn't compel you to
>
> > Signed-off-by: Jarkko Sakkinen
> > -
> ...--
> > diff --git a/drivers/platform/x86/intel_sgx/sgx_le.c
> > b/drivers/platform/x86/intel_sgx/sgx_le.c
> > new file mode 100644
> > index ..d49c58f09db6
> > --- /dev/null
> > +++ b/drivers/platform/x86/intel_sgx/sgx_le.c
> > @@ -0,0 +1,313 @@
> ...
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> alphabetical order
> ...
> > +static int sgx_le_create_pipe(struct sgx_le_ctx *ctx,
> > + unsigned int fd)
> > +{
> > + struct file *files[2];
> > + int ret;
> > +
> > + ret = create_pipe_files(files, 0);
> > + if (ret)
> > + goto out;
>
> Fairly inconsistent in the use of the goto out: model and returning
> inline where there is no cleanup to be done. Whatever you do, please be
> consistent within the file.
>
> If there is no cleanup to do, a local return is fine.
It is cruft that I haven't remembered to clean up eg there used to be
clean up. Thanks for spotting that.
/Jarkko
IRQ not handled on my Acer Aspire R3-131T
Hi there! My machine is an Acer Aspire R3-131T, the firmware installed is the latest (1.17, released 2017/01/04). My problem is that my kernel log is overflooded with the following error: [5.356743] irq 173, desc: 9c9bb7b48800, depth: 1, count: 0, unhandled: 0 [5.356745] ->handle_irq(): 9b0e0a90, [5.356747] handle_bad_irq+0x0/0x260 [5.356748] ->irq_data.chip(): c0713120, [5.356751] chv_pinctrl_exit+0x4533/0x413 [pinctrl_cherryview] [5.356752] ->action(): (null) [5.356753]IRQ_NOPROBE set You already discussed something similar last Year (Oct 2016) here: http://lkml.iu.edu/hypermail/linux/kernel/1610.2/03006.html Here are the same logs, which you also asked for at the discussion mentioned above. LTS (everything is fine) - dmesg: https://pastebin.com/3XnFtpgB - interrupts: https://pastebin.com/XdN1juKb - pins: https://pastebin.com/hwX3vvZN 4.13.12 (what a mess!) - dmesg: https://pastebin.com/5fhLM0sn - interrupts: https://pastebin.com/FLELSX3Q - pins: https://pastebin.com/uPes8624 Thanks in advance! P.S: I would appreciate it if you could CC me personally at answers/comments since I am not yet subscribed to the kernel mailing list.
06222d856e ("x86/mm/kaiser: Use PCID feature to make user and kernel switches faster"): BUG: kernel hang in boot stage
Hi Dave, Here are two more error messages for commit https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/mm commit 06222d856e45d727c18665ed37419d653f1dbef5 Author: Dave Hansen AuthorDate: Wed Nov 22 16:35:09 2017 -0800 Commit: Ingo Molnar CommitDate: Fri Nov 24 08:29:51 2017 +0100 x86/mm/kaiser: Use PCID feature to make user and kernel switches faster Short summary: Use x86 PCID feature to avoid flushing the TLB at all interrupts and syscalls. Speed them up. Makes context switches and TLB flushing slower. [...] Signed-off-by: Dave Hansen Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Daniel Gruss Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Hugh Dickins Cc: Josh Poimboeuf Cc: Kees Cook Cc: Linus Torvalds Cc: Michael Schwarz Cc: Moritz Lipp Cc: Peter Zijlstra Cc: Richard Fellner Cc: Thomas Gleixner Cc: linux...@kvack.org Link: http://lkml.kernel.org/r/20171123003509.ec42d...@viggo.jf.intel.com Signed-off-by: Ingo Molnar 5ab2af1e02 x86/mm: Allow flushing for future ASID switches 06222d856e x86/mm/kaiser: Use PCID feature to make user and kernel switches faster 850f70b234 x86/mm/kaiser: Add Kconfig 1b46550a68 Merge branch 'WIP.x86/mm' +---+++++ | | 5ab2af1e02 | 06222d856e | 850f70b234 | 1b46550a68 | +---+++++ | boot_successes| 353| 134 | 114| 120| | boot_failures | 40 | 24 | 28 | 29 | | WARNING:at_drivers/pci/pci-sysfs.c:#pci_mmap_resource | 36 | 10 | 8 | 8 | | RIP:pci_mmap_resource | 37 | 10 | 8 | 9 | | BUG:workqueue_lockup-pool | 3 | 1 ||| | BUG:kernel_hang_in_boot_stage | 0 | 9 | 9 | 4 | | kernel_BUG_at_arch/x86/kernel/mpparse.c | 0 | 4 | 11 | 16 | | PANIC:early_exception | 0 | 4 | 11 | 16 | | RIP:default_get_smp_config| 0 | 4 | 11 | 16 | +---+++++ [0.004000] ... CHAINHASH_SIZE: 32768 [0.004000] memory used by lock dependency info: 7359 kB [0.004000] per task-struct memory footprint: 1920 bytes [0.004000] ODEBUG: selftest passed [0.004000] ACPI: Core revision 20170728 BUG: kernel hang in boot stage [0.00] Initmem setup node 0 [mem 0x1000-0x1ffdbfff] [0.00] On node 0 totalpages: 130938 [0.00] DMA zone: 64 pages used for memmap [0.00] DMA zone: 21 pages reserved [0.00] DMA zone: 3998 pages, LIFO batch:0 [0.00] DMA32 zone: 1984 pages used for memmap [0.00] DMA32 zone: 126940 pages, LIFO batch:31 [0.00] Intel MultiProcessor Specification v1.0 [0.00] [ cut here ] [0.00] kernel BUG at arch/x86/kernel/mpparse.c:559! PANIC: early exception 0x06 IP 10:81f1ceb4 error 0 cr2 0x880002b6d000 [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.14.0-01247-g06222d8 #112 [0.00] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 [0.00] task: 81c1a500 task.stack: 81c0 [0.00] RIP: 0010:default_get_smp_config+0x73c/0x7da [0.00] RSP: :81c03e38 EFLAGS: 00010046 ORIG_RAX: [0.00] RAX: 0027 RBX: 86db21edabd071e4 RCX: 81c03ce4 [0.00] RDX: 0001 RSI: RDI: [0.00] RBP: 81c03eb8 R08: 0001 R09: [0.00] R10: 81c03d88 R11: 82919e0c R12: 88001ffcfe25 [0.00] R13: 003d6e25 R14: ff200a90 R15: [0.00] FS: () GS:81c39000() knlGS: [0.00] CS: 0010 DS: ES: CR0: 80050033 [0.00] CR2: 880002b6d000 CR3: 01c15000 CR4: 000606b0 [0.00] Call Trace: [0.00] ? dmi_check_system+0x15/0x3a [0.00] ? acpi_boot_init+0x34/0x84f [0.00] ? fill_pte+0xba/0xee [0.00] ? early_pci_scan_bus+0x62/0x30e [0.00] ? __set_pte_vaddr+0x36/0x40 [
Re: [PATCH 0/3] scsi: arcmsr: add driver module parameter - (linux-kernel: dan.carpen...@oracle.com exclusive) msi_enable, msix_enable
On 24.11.2017 15:53, Dan Carpenter - dan.carpen...@oracle.com wrote: > Is there a crash or a performance issue? What does the bug in the > current code look like from a user perspective? It looks like this: https://bugzilla.kernel.org/show_bug.cgi?id=197877 dmesg output can be found in the bug report. Disabling MSI interrupts with msi_enable=0 (and Ching's patches) does resolve the issue, but whether it should be considered a fix or a workaround is not for me to say. Regards, Kristian Rasmussen
21729f81ce ("x86/mm: Provide general kernel support for memory encryption"): BUG: kernel reboot-without-warning in early-boot stage, last printk: early console in setup code
Greetings, 0day kernel testing robot got the below dmesg and the first bad commit is https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master commit 21729f81ce8ae76a6995681d40e16f7ce8075db4 Author: Tom Lendacky AuthorDate: Mon Jul 17 16:10:07 2017 -0500 Commit: Ingo Molnar CommitDate: Tue Jul 18 11:38:00 2017 +0200 x86/mm: Provide general kernel support for memory encryption Changes to the existing page table macros will allow the SME support to be enabled in a simple fashion with minimal changes to files that use these macros. Since the memory encryption mask will now be part of the regular pagetable macros, we introduce two new macros (_PAGE_TABLE_NOENC and _KERNPG_TABLE_NOENC) to allow for early pagetable creation/initialization without the encryption mask before SME becomes active. Two new pgprot() macros are defined to allow setting or clearing the page encryption mask. The FIXMAP_PAGE_NOCACHE define is introduced for use with MMIO. SME does not support encryption for MMIO areas so this define removes the encryption mask from the page attribute. Two new macros are introduced (__sme_pa() / __sme_pa_nodebug()) to allow creating a physical address with the encryption mask. These are used when working with the cr3 register so that the PGD can be encrypted. The current __va() macro is updated so that the virtual address is generated based off of the physical address without the encryption mask thus allowing the same virtual address to be generated regardless of whether encryption is enabled for that physical location or not. Also, an early initialization function is added for SME. If SME is active, this function: - Updates the early_pmd_flags so that early page faults create mappings with the encryption mask. - Updates the __supported_pte_mask to include the encryption mask. - Updates the protection_map entries to include the encryption mask so that user-space allocations will automatically have the encryption mask applied. Signed-off-by: Tom Lendacky Reviewed-by: Thomas Gleixner Reviewed-by: Borislav Petkov Cc: Alexander Potapenko Cc: Andrey Ryabinin Cc: Andy Lutomirski Cc: Arnd Bergmann Cc: Borislav Petkov Cc: Brijesh Singh Cc: Dave Young Cc: Dmitry Vyukov Cc: Jonathan Corbet Cc: Konrad Rzeszutek Wilk Cc: Larry Woodman Cc: Linus Torvalds Cc: Matt Fleming Cc: Michael S. Tsirkin Cc: Paolo Bonzini Cc: Peter Zijlstra Cc: Radim Krčmář Cc: Rik van Riel Cc: Toshimitsu Kani Cc: kasan-...@googlegroups.com Cc: k...@vger.kernel.org Cc: linux-a...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux...@kvack.org Link: http://lkml.kernel.org/r/b36e952c4c39767ae7f0a41cf5345adf27438480.1500319216.git.thomas.lenda...@amd.com Signed-off-by: Ingo Molnar fd7e315988 x86/mm: Simplify p[g4um]d_page() macros 21729f81ce x86/mm: Provide general kernel support for memory encryption 7753ea0964 Merge tag 'kvm-4.15-2' of git://git.kernel.org/pub/scm/virt/kvm/kvm 6fc478f80f Add linux-next specific files for 20171124 +---++++---+ | | fd7e315988 | 21729f81ce | 7753ea0964 | next-20171124 | +---++++---+ | boot_successes | 14 | 0 | 0 | 21| | boot_failures | 26 | 11 | 19 | 4 | | WARNING:at_drivers/pci/pci-sysfs.c:#pci_mmap_resource | 17 | 0 | 0 | 4 | | RIP:pci_mmap_resource | 17 | 0 | 0 | 4 | | BUG:KASAN:slab-out-of-bounds_in_c | 5 ||| | | WARNING:bad_unlock_balance_detected | 7 ||| | | is_trying_to_release_lock(rcu_preempt_state)at | 7 ||| | | calltrace:SyS_setpriority
Re: [PATCH v4] iio : Add cm3218 smbus ara and acpi support
On Tue, 21 Nov 2017 09:22:16 +0800
Phil Reid wrote:
> On 20/11/2017 18:57, Mika Westerberg wrote:
> > +Jarkko
> >
> > On Sun, Nov 19, 2017 at 04:35:51PM +, Jonathan Cameron wrote:
> >> On Thu, 2 Nov 2017 16:04:07 +0100
> >> Wolfram Sang wrote:
> >>
> >>> On Thu, Nov 02, 2017 at 02:35:50PM +, Jonathan Cameron wrote:
> On Fri, 27 Oct 2017 18:27:02 +0200
> Marc CAPDEVILLE wrote:
>
> > On asus T100, Capella cm3218 chip is implemented as ambiant light
> > sensor. This chip expose an smbus ARA protocol device on standard
> > address 0x0c. The chip is not functional before all alerts are
> > acknowledged.
> > On asus T100, this device is enumerated on ACPI bus and the
> > description give tow I2C connection. The first is the connection to
> > the ARA device and the second gives the real address of the device.
> >
> > So, on device probe,If the i2c address is the ARA address and the
> > device is enumerated via acpi, we lookup for the real address in
> > the ACPI resource list and change it in the client structure.
> > if an interrupt resource is given, and only for cm3218 chip,
> > we declare an smbus_alert device.
> >
> > Signed-off-by: Marc CAPDEVILLE
>
> Wolfram - this needs input from you on how to neatly handle
> an ACPI registered ARA.
> >>>
> >>> ACPI is really not my field. Try asking the I2C ACPI maintainers or
> >>> Benjamin Tissoires who did work on SMBus
> >>> interrupts recently.
> >>>
> >> Hi Mika, Benjamin,
> >>
> >> So we've lost most of the context in this thread, but the basic question
> >> is how to handle smbus ARA support with ACPI.
> >>
> >> https://patchwork.kernel.org/patch/10030309/
> >>
> >> Has the proposal made in this driver which is really not terribly nice
> >> (as it registers the ARA device by messing with the address and registering
> >> a second device).
> >>
> >> As I understood it the ARA device registration should be handled by the
> >> i2c master, but there are very few examples.
> >>
> >> Phil pointed out that equivalent OF support recently got taken from him..
> >> https://www.spinics.net/lists/devicetree/msg191947.html
> >> https://www.spinics.net/lists/linux-i2c/msg31173.html
> >>
> >> Any thoughts on the right way to do this?
> >
> > There does not seem to be any way in ACPI to tell which "connection" is
> > used to describe ARA so that part currently is something each driver
> > needs to handle as they know the device the best. I don't think we have
> > any means to handle it in generic way in I2C core except to provide some
> > helpers that work on top of i2c_setup_smbus_alert() but understand ACPI
> > resources. Say provide function like this:
> >
> >int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index);
> >
> > Which then extracts automatically I2cSerialBus connection from "index"
> > and calls i2c_setup_smbus_alert() accordingly.
> >
> > In the long run we could introduce _DSD property that can be used to
> > name the connection in the same way DT does;
> >
> > Name (_CRS, ResourceTemplate () {
> > I2cSerialBus () { ... } // ARA
> > I2cSerialBus () { ... } // normal device address
> > })
> >
> > Name (_DSD, Package () {
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > Package () {
> > Package () {"smbus_alert", 0} // Where 0 means the first
> > I2cSerialBus
> > ...
> > }
> > })
> >
>
> I wonder if it's worth involving the smbus_alert driver in this case.
> The cm3218 driver doesn't appear to be using the alert callback in strcut
> i2c_driver.
True - though it really should be..
> So the smbus_alert driver is not going to notifiy the cm3218 driver.
> Are there more than one alert/ara capable devices on the bus?
I'm not keen on taking a work around for one board on the basis that
it only has this one device on the bus - we will probably get a different
one down the line where this isn't true - then we end up ripping up what
has been done so far and starting again.
I don't mind having some ACPI matching code in the driver but it needs
to use the ARA infrastructure to actually handle things...
If we then get nice generic ACPI code via some other means in the future
we can move the driver over to that.
Sometimes I wonder if some people just write ACPI tables with no though
to generalization at all, or that the code running might be shared across
different machines. (sometimes that assumption is valid, but not that
often... oh well - usual case of if we all work together everyone wins
but not worth one company trying to do things right...)
> Perhaps a workaround in this case is if that acpi entry is defined the cm3218
> driver
> handles that ara request directly to clear the interrupt.
>
Jonathan
Re: [PATCH v4] iio : Add cm3218 smbus ara and acpi support
On Mon, 20 Nov 2017 12:57:56 +0200
Mika Westerberg wrote:
> +Jarkko
>
> On Sun, Nov 19, 2017 at 04:35:51PM +, Jonathan Cameron wrote:
> > On Thu, 2 Nov 2017 16:04:07 +0100
> > Wolfram Sang wrote:
> >
> > > On Thu, Nov 02, 2017 at 02:35:50PM +, Jonathan Cameron wrote:
> > > > On Fri, 27 Oct 2017 18:27:02 +0200
> > > > Marc CAPDEVILLE wrote:
> > > >
> > > > > On asus T100, Capella cm3218 chip is implemented as ambiant light
> > > > > sensor. This chip expose an smbus ARA protocol device on standard
> > > > > address 0x0c. The chip is not functional before all alerts are
> > > > > acknowledged.
> > > > > On asus T100, this device is enumerated on ACPI bus and the
> > > > > description give tow I2C connection. The first is the connection to
> > > > > the ARA device and the second gives the real address of the device.
> > > > >
> > > > > So, on device probe,If the i2c address is the ARA address and the
> > > > > device is enumerated via acpi, we lookup for the real address in
> > > > > the ACPI resource list and change it in the client structure.
> > > > > if an interrupt resource is given, and only for cm3218 chip,
> > > > > we declare an smbus_alert device.
> > > > >
> > > > > Signed-off-by: Marc CAPDEVILLE
> > > >
> > > > Wolfram - this needs input from you on how to neatly handle
> > > > an ACPI registered ARA.
> > >
> > > ACPI is really not my field. Try asking the I2C ACPI maintainers or
> > > Benjamin Tissoires who did work on SMBus
> > > interrupts recently.
> > >
> > Hi Mika, Benjamin,
> >
> > So we've lost most of the context in this thread, but the basic question
> > is how to handle smbus ARA support with ACPI.
> >
> > https://patchwork.kernel.org/patch/10030309/
> >
> > Has the proposal made in this driver which is really not terribly nice
> > (as it registers the ARA device by messing with the address and registering
> > a second device).
> >
> > As I understood it the ARA device registration should be handled by the
> > i2c master, but there are very few examples.
> >
> > Phil pointed out that equivalent OF support recently got taken from him..
> > https://www.spinics.net/lists/devicetree/msg191947.html
> > https://www.spinics.net/lists/linux-i2c/msg31173.html
> >
> > Any thoughts on the right way to do this?
>
> There does not seem to be any way in ACPI to tell which "connection" is
> used to describe ARA so that part currently is something each driver
> needs to handle as they know the device the best. I don't think we have
> any means to handle it in generic way in I2C core except to provide some
> helpers that work on top of i2c_setup_smbus_alert() but understand ACPI
> resources. Say provide function like this:
>
> int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index);
>
> Which then extracts automatically I2cSerialBus connection from "index"
> and calls i2c_setup_smbus_alert() accordingly.
>
> In the long run we could introduce _DSD property that can be used to
> name the connection in the same way DT does;
>
> Name (_CRS, ResourceTemplate () {
> I2cSerialBus () { ... } // ARA
> I2cSerialBus () { ... } // normal device address
> })
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () {"smbus_alert", 0} // Where 0 means the first
> I2cSerialBus
> ...
> }
> })
>
> But it does not help the existing systems.
I'm curious - how would we go about promoting this piece of common sense?
Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 6/7] typec: tcpm: Represent source supply through power_supply class
Hi,
On 11/24/2017 01:19 PM, Heikki Krogerus wrote:
Hi,
On Tue, Nov 14, 2017 at 11:44:47AM +, Adam Thomson wrote:
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 78983e1..7c26c3d 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -277,6 +278,10 @@ struct tcpm_port {
u32 current_limit;
u32 supply_voltage;
+ /* Used to export TA voltage and current */
+ struct power_supply *psy;
+ struct power_supply_desc psy_desc;
+
u32 bist_request;
/* PD state for Vendor Defined Messages */
@@ -1756,6 +1761,7 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port)
int ret = -EINVAL;
port->pps_data.supported = false;
+ port->psy_desc.type = POWER_SUPPLY_TYPE_USB_PD;
/*
* Select the source PDO providing the most power while staying within
@@ -1775,8 +1781,10 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port)
mv = pdo_min_voltage(pdo);
break;
case PDO_TYPE_APDO:
- if (pdo_apdo_type(pdo) == APDO_TYPE_PPS)
+ if (pdo_apdo_type(pdo) == APDO_TYPE_PPS) {
port->pps_data.supported = true;
+ port->psy_desc.type =
POWER_SUPPLY_TYPE_USB_PD_PPS;
+ }
continue;
default:
tcpm_log(port, "Invalid PDO type, ignoring");
@@ -2248,6 +2256,9 @@ static void tcpm_reset_port(struct tcpm_port *port)
port->try_snk_count = 0;
port->supply_voltage = 0;
port->current_limit = 0;
+ port->psy_desc.type = POWER_SUPPLY_TYPE_USB_TYPE_C;
Is it OK to everybody that the type of the psy is changed like that?
Hans?!
We do have drivers that already change the type, for example
drivers/power/supply/isp1704_charger.c, but what does the user space
expect? The ABI for the power supply class was never documented I
guess.
The main userspace consumer of the power_supply sysfs class is upower,
upower knows about 3 types: "Mains", "Battery" and "USB", anything
else it gives a type of UP_DEVICE_KIND_UNKNOWN. So POWER_SUPPLY_TYPE_USB_TYPE_C
vs POWER_SUPPLY_TYPE_USB_PD_PPS does matter to it.
Regards,
Hans
Re: [PATCH v2] iio: adc: aspeed: Deassert reset in probe
On Mon, 20 Nov 2017 15:22:38 +1030 Joel Stanley wrote: > On Mon, Nov 20, 2017 at 2:33 AM, Jonathan Cameron wrote: > > On Thu, 2 Nov 2017 14:49:32 + > > Jonathan Cameron wrote: > >> IIO is closed for this cycle anyway now. > >> Otherwise, series looks good. > >> > >> Will pick up when back with my main PC as traveling for this week and > >> next. > > > > Forgot to ask, do you want me to pick this up as a fix? > > Also does it make sense to tag it for stable? > > > > If not I can pick it up for the coming cycle. Given the code changes > > are small and well isolated I'm happy to do any of the 3 options, > > it really depends on whether the rest of the platform works well enough > > to be worth rushing these through? > > Without the clock driver upstream there's no rush to merge this. I'm > still waiting on review from the clock guys. > > Please queue them for 4.16. Thanks! Done - applied to the togreg branch of iio.git which will be pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > Cheers, > > Joel
Re: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection
On Sun, 19 Nov 2017 20:48:21 -0500 harinath Nampally wrote: > > > This patch adds following related changes: > > > - defines pulse event related registers > > > - enables and handles single pulse interrupt for fxls8471 > > > - handles IIO_EV_DIR_EITHER in read/write callbacks (because > > > event direction for pulse is either rising or falling) > > > - configures read/write event value for pulse latency register > > > using IIO_EV_INFO_HYSTERESIS > > > - adds multiple events like pulse and tranient event spec > > > as elements of event_spec array named 'mma8452_accel_events' > > > > > > Except mma8653 chip all other chips like mma845x and > > > fxls8471 have single tap detection feature. > > > Tested thoroughly using iio_event_monitor application on > > > imx6ul-evk board which has fxls8471. > > > > > > Signed-off-by: Harinath Nampally > > > > The use of an either direction magnitude event is misleading. > > Tap detection requires a timed sequence of events - a rapid > > increase in acceleration followed by a rapid decrease. > Yes I agree. > > > So.. I'd propose (and I want as much feedback on this as possible) > > > > IIO_EV_TYPE_TAP > > > > Then abuse iio_event_direction to specify single or double (the concept > > of direction doesn't really apply to these in that sense so we'd always > > have them set to none). > Sure, I think this would be very useful as there are lot of modern > accelerometers > with tap event support. It's been true since the start of IIO but we just kept kicking this question into the long grass as we didn't really have the surrounding support that would be ideally needed. They are only really useful in the general sense if we finally do something with the iio-input bridge and put together the in kernel event consumer interface we have never actually written... It's been on the todo list for at least 5 years... oops :( > > Thanks, > Harinath > > On Sun, Nov 19, 2017 at 9:47 AM, Jonathan Cameron wrote: > > On Wed, 8 Nov 2017 22:12:57 -0500 > > Harinath Nampally wrote: > > > >> This patch adds following related changes: > >> - defines pulse event related registers > >> - enables and handles single pulse interrupt for fxls8471 > >> - handles IIO_EV_DIR_EITHER in read/write callbacks (because > >> event direction for pulse is either rising or falling) > >> - configures read/write event value for pulse latency register > >> using IIO_EV_INFO_HYSTERESIS > >> - adds multiple events like pulse and tranient event spec > >> as elements of event_spec array named 'mma8452_accel_events' > >> > >> Except mma8653 chip all other chips like mma845x and > >> fxls8471 have single tap detection feature. > >> Tested thoroughly using iio_event_monitor application on > >> imx6ul-evk board which has fxls8471. > >> > >> Signed-off-by: Harinath Nampally > > > > The use of an either direction magnitude event is misleading. > > Tap detection requires a timed sequence of events - a rapid > > increase in acceleration followed by a rapid decrease. > > > > I don't think we can fit tap detection into the existing > > event types (which is annoying but there we are). > > > > So.. I'd propose (and I want as much feedback on this as possible) > > > > IIO_EV_TYPE_TAP > > > > Then abuse iio_event_direction to specify single or double (the concept > > of direction doesn't really apply to these in that sense so we'd always > > have them set to none). > > > > The alternative would be to full out for an abstract representation of > > these events (like step detection) and define a channel type > > IIO_TAP then detect a change in the number of taps (IIO_DIR_NONE), but > > given we have directional taps we would need the modifier for that. > > Hence no means of describing whether it is a single or double tap > > short of burning channel taps. I'm also not sure we want to > > remove the association with a particular sensor channel. > > > > Hence I think I prefer option 1 but would like other's input on this... > > > > Jonathan > > > >> --- > >> drivers/iio/accel/mma8452.c | 156 > >> ++-- > >> 1 file changed, 151 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c > >> index 43c3a6b..36f1b56 100644 > >> --- a/drivers/iio/accel/mma8452.c > >> +++ b/drivers/iio/accel/mma8452.c > >> @@ -72,6 +72,19 @@ > >> #define MMA8452_TRANSIENT_THS_MASK GENMASK(6, 0) > >> #define MMA8452_TRANSIENT_COUNT 0x20 > >> #define MMA8452_TRANSIENT_CHAN_SHIFT 1 > >> +#define MMA8452_PULSE_CFG0x21 > >> +#define MMA8452_PULSE_CFG_CHAN(chan) BIT(chan * 2) > >> +#define MMA8452_PULSE_CFG_ELEBIT(6) > >> +#define MMA8452_PULSE_SRC0x22 > >> +#define MMA8452_PULSE_SRC_XPULSE BIT(4) > >> +#define MMA8452_PULSE_SRC_YPULSE BIT(5) > >> +#define MMA8452_PULSE_SRC_ZPULSE BIT(6) > >> +#def
Re: [PATCH] nvmem: core: Deduplicate bus_find_device() by name matching
Thanks for the patch,
On 25/11/17 11:31, Lukas Wunner wrote:
No need to reinvent the wheel, we have bus_find_device_by_name().
Signed-off-by: Lukas Wunner
---
drivers/nvmem/core.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
It looks good for me, I will queue it up!
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 5a5cefd12153..93084ab61e0f 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -600,16 +600,11 @@ static void __nvmem_device_put(struct nvmem_device *nvmem)
mutex_unlock(&nvmem_mutex);
}
-static int nvmem_match(struct device *dev, void *data)
-{
- return !strcmp(dev_name(dev), data);
-}
-
static struct nvmem_device *nvmem_find(const char *name)
{
struct device *d;
- d = bus_find_device(&nvmem_bus_type, NULL, (void *)name, nvmem_match);
+ d = bus_find_device_by_name(&nvmem_bus_type, NULL, name);
if (!d)
return NULL;
Re: [PATCH] nvmem: core: switch to device_property_present for reading property "read-only"
Thanks for the patch,
On 25/11/17 11:56, Heiner Kallweit wrote:
Switch to more generic device_property_present to consider also non-DT
properties.
Signed-off-by: Heiner Kallweit
---
drivers/nvmem/core.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
It looks good for me, I will queue it up!
index 5a5cefd12..ba0e3b453 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -444,7 +444,6 @@ static int nvmem_setup_compat(struct nvmem_device *nvmem,
struct nvmem_device *nvmem_register(const struct nvmem_config *config)
{
struct nvmem_device *nvmem;
- struct device_node *np;
int rval;
if (!config->dev)
@@ -473,13 +472,12 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->priv = config->priv;
nvmem->reg_read = config->reg_read;
nvmem->reg_write = config->reg_write;
- np = config->dev->of_node;
- nvmem->dev.of_node = np;
+ nvmem->dev.of_node = config->dev->of_node;
dev_set_name(&nvmem->dev, "%s%d",
config->name ? : "nvmem",
config->name ? config->id : nvmem->id);
- nvmem->read_only = of_property_read_bool(np, "read-only") |
+ nvmem->read_only = device_property_present(config->dev, "read-only") |
config->read_only;
if (config->root_only)
Re: [PATCH net v3] net: thunderx: Fix TCP/UDP checksum offload for IPv6 pkts
From: Aleksey Makarov
Date: Thu, 23 Nov 2017 22:34:31 +0300
> From: Sunil Goutham
>
> Don't offload IP header checksum to NIC.
>
> This fixes a previous patch which enabled checksum offloading
> for both IPv4 and IPv6 packets. So L3 checksum offload was
> getting enabled for IPv6 pkts. And HW is dropping these pkts
> as it assumes the pkt is IPv4 when IP csum offload is set
> in the SQ descriptor.
>
> Fixes: 3a9024f52c2e ("net: thunderx: Enable TSO and checksum offloads for
> ipv6")
> Signed-off-by: Sunil Goutham
> Signed-off-by: Aleksey Makarov
> Reviewed-by: Eric Dumazet
Applied.
Re: [PATCH] net: thunderbolt: Stop using zero to mean no valid DMA mapping
From: Mika Westerberg
Date: Fri, 24 Nov 2017 14:05:36 +0300
> Commit 86dabda426ac ("net: thunderbolt: Clear finished Tx frame bus
> address in tbnet_tx_callback()") fixed a DMA-API violation where the
> driver called dma_unmap_page() in tbnet_free_buffers() for a bus address
> that might already be unmapped. The fix was to zero out the bus address
> of a frame in tbnet_tx_callback().
>
> However, as pointed out by David Miller, zero might well be valid
> mapping (at least in theory) so it is not good idea to use it here.
>
> It turns out that we don't need the whole map/unmap dance for Tx buffers
> at all. Instead we can map the buffers when they are initially allocated
> and unmap them when the interface is brought down. In between we just
> DMA sync the buffers for the CPU or device as needed.
>
> Signed-off-by: Mika Westerberg
Applied, thanks.
Re: [PATCH 15/43] x86/entry/64: Create a percpu SYSCALL entry trampoline
On Sat, Nov 25, 2017 at 3:40 AM, Borislav Petkov wrote: > On Fri, Nov 24, 2017 at 06:23:43PM +0100, Ingo Molnar wrote: >> From: Andy Lutomirski >> >> Handling SYSCALL is tricky: the SYSCALL handler is entered with every >> single register (except FLAGS), including RSP, live. It somehow needs >> to set RSP to point to a valid stack, which means it needs to save the >> user RSP somewhere and find its own stack pointer. The canonical way >> to do this is with SWAPGS, which lets us access percpu data using the >> %gs prefix. >> >> With KAISER-like pagetable switching, this is problematic. Without a >> scratch register, switching CR3 is impossible, so %gs-based percpu >> memory would need to be mapped in the user pagetables. Doing that >> without information leaks is difficult or impossible. >> >> Instead, use a different sneaky trick. Map a copy of the first part >> of the SYSCALL asm at a different address for each CPU. Now RIP >> varies depending on the CPU, so we can use RIP-relative memory access >> to access percpu memory. By putting the relevant information (one >> scratch slot and the stack address) at a constant offset relative to >> RIP, we can make SYSCALL work without relying on %gs. >> >> A nice thing about this approach is that we can easily switch it on >> and off if we want pagetable switching to be configurable. >> >> The compat variant of SYSCALL doesn't have this problem in the first >> place -- there are plenty of scratch registers, since we don't care >> about preserving r8-r15. This patch therefore doesn't touch SYSCALL32 >> at all. >> >> XXX: Whenever we settle how KAISER gets turned on and off, we should do >> the same to this. >> >> Signed-off-by: Andy Lutomirski >> Signed-off-by: Thomas Gleixner >> Cc: Borislav Petkov >> Cc: Brian Gerst >> Cc: Dave Hansen >> Cc: Josh Poimboeuf >> Cc: Linus Torvalds >> Cc: Peter Zijlstra >> Link: >> https://lkml.kernel.org/r/b95ccae0a5a2f090c901e49fce7c9e8ff6acd40d.1511497875.git.l...@kernel.org >> Signed-off-by: Ingo Molnar >> --- >> arch/x86/entry/entry_64.S | 48 >> +++ >> arch/x86/include/asm/fixmap.h | 2 ++ >> arch/x86/kernel/asm-offsets.c | 1 + >> arch/x86/kernel/cpu/common.c | 12 ++- >> arch/x86/kernel/vmlinux.lds.S | 10 + >> 5 files changed, 72 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >> index 426b8c669d6a..0cde243b7542 100644 >> --- a/arch/x86/entry/entry_64.S >> +++ b/arch/x86/entry/entry_64.S >> @@ -140,6 +140,54 @@ END(native_usergs_sysret64) >> * with them due to bugs in both AMD and Intel CPUs. >> */ >> >> + .pushsection .entry_trampoline, "ax" >> + >> +/* >> + * The code in here gets remapped into cpu_entry_area's trampoline. This >> means >> + * that the assembler and linker have the wrong idea as to where this code >> + * lives (and, in fact, it's mapped more than once, so it's not even at a >> + * fixed address). So we can't reference any symbols outside the entry >> + * trampoline and expect it to work. >> + * >> + * Instead, we carefully abuse %rip-relative addressing. >> + * .Lentry_trampoline(%rip) refers to the start of the remapped) entry > > _entry_trampoline(%rip) I'd guess. Indeed. It used to be .L, but then I put it in the linker script the obvious way and it wasn't any more. > >> + * trampoline. We can thus find cpu_entry_area with this macro: > > Uuh, fun. :) That's what I thought! > >> + */ >> + >> +#define CPU_ENTRY_AREA \ >> + _entry_trampoline - CPU_ENTRY_AREA_entry_trampoline(%rip) > > So this generates > > _entry_trampoline - 16384(%rip) > > here. IINM, the layout looks like this > > [ GDT | TSS | TSS | TSS | trampoline ] > > where each section is a page, and we have 4 pages per CPU. Just for my > own understanding... Indeed. > >> + >> +/* The top word of the SYSENTER stack is hot and is usable as scratch >> space. */ >> +#define RSP_SCRATCH CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + \ >> + SIZEOF_SYSENTER_stack - 8 + CPU_ENTRY_AREA > > I'm wondering if it would be easier to make SYSENTER_stack part of > struct cpu_entry_area and thus simplify that wild offset math here :) It's like that with the last patch that I haven't send out applied, actually :) > > Also, pls align: > > #define RSP_SCRATCH CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + \ > SIZEOF_SYSENTER_stack - 8 + CPU_ENTRY_AREA Done. > >> + >> +ENTRY(entry_SYSCALL_64_trampoline) >> + UNWIND_HINT_EMPTY >> + swapgs >> + >> + /* Stash the user RSP. */ >> + movq%rsp, RSP_SCRATCH >> + >> + /* Load the top of the task stack into RSP */ >> + movqCPU_ENTRY_AREA_tss + TSS_sp1 + CPU_ENTRY_AREA, %rsp > > Yeah, let's put CPU_ENTRY_AREA first because then it reads easier: > > movqCPU_ENTRY_AREA + CPU_ENTRY_AREA_tss + TSS_sp1, %rsp Nope, it won't build. That would expand like 0x1(%rip) + 0x2, which isn't valid. > > i.e., poin
Re: [PATCH] iio: accel: kxsd9-i2c: add missing MODULE_AUTHOR/DESCRIPTION/LICENSE
On Mon, 20 Nov 2017 12:55:18 -0800
Jesse Chan wrote:
> This change resolves a new compile-time warning
> when built as a loadable module:
>
> WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/accel/kxsd9-i2c.o
> see include/linux/module.h for more information
>
> This adds the license as "GPL v2", which matches the header of the file.
>
> MODULE_DESCRIPTION and MODULE_AUTHOR are also added.
Linus sent patches fixing the license and description the other day.
Also wise to actually cc Linus!
Right now they are only publicly visible in the testing
branch of iio.git on kernel.org as I'll be rebasing once rc1 is out...
Thanks,
Jonathan
>
> Signed-off-by: Jesse Chan
> ---
> drivers/iio/accel/kxsd9-i2c.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
> index 98fbb628d5bd..40f82ada6a52 100644
> --- a/drivers/iio/accel/kxsd9-i2c.c
> +++ b/drivers/iio/accel/kxsd9-i2c.c
> @@ -63,3 +63,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
> .id_table = kxsd9_i2c_id,
> };
> module_i2c_driver(kxsd9_i2c_driver);
> +
> +MODULE_AUTHOR("Linus Walleij ");
> +MODULE_DESCRIPTION("Kionix KXSD9 I2C driver");
> +MODULE_LICENSE("GPL v2");
Re: [PATCH] iio: adc: qcom-vadc: add missing MODULE_DESCRIPTION/LICENSE
On Mon, 20 Nov 2017 12:55:51 -0800
Jesse Chan wrote:
> This change resolves a new compile-time warning
> when built as a loadable module:
>
> WARNING: modpost: missing MODULE_LICENSE() in
> drivers/iio/adc/qcom-vadc-common.o
> see include/linux/module.h for more information
>
> This adds the license as "GPL v2", which matches the header of the file.
>
> MODULE_DESCRIPTION is also added.
>
> Signed-off-by: Jesse Chan
Again, already handled by Linus Walleij.
I'm also far from keen on pushing the header include up into
the header...
> ---
> drivers/iio/adc/qcom-pm8xxx-xoadc.c | 1 -
> drivers/iio/adc/qcom-spmi-vadc.c| 1 -
> drivers/iio/adc/qcom-vadc-common.c | 3 +++
> drivers/iio/adc/qcom-vadc-common.h | 2 ++
> 4 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> index b093ecddf1a8..e7ba66798362 100644
> --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> @@ -11,7 +11,6 @@
>
> #include
> #include
> -#include
> #include
> #include
> #include
> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c
> b/drivers/iio/adc/qcom-spmi-vadc.c
> index 3680e0d47412..569f88ce9385 100644
> --- a/drivers/iio/adc/qcom-spmi-vadc.c
> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
> @@ -19,7 +19,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> diff --git a/drivers/iio/adc/qcom-vadc-common.c
> b/drivers/iio/adc/qcom-vadc-common.c
> index 47d24ae5462f..8078a0cbdc46 100644
> --- a/drivers/iio/adc/qcom-vadc-common.c
> +++ b/drivers/iio/adc/qcom-vadc-common.c
> @@ -229,3 +229,6 @@ int qcom_vadc_decimation_from_dt(u32 value)
> return __ffs64(value / VADC_DECIMATION_MIN);
> }
> EXPORT_SYMBOL(qcom_vadc_decimation_from_dt);
> +
> +MODULE_DESCRIPTION("Qualcomm voltage ADC driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/adc/qcom-vadc-common.h
> b/drivers/iio/adc/qcom-vadc-common.h
> index 1d5354ff5c72..c62500147bb1 100644
> --- a/drivers/iio/adc/qcom-vadc-common.h
> +++ b/drivers/iio/adc/qcom-vadc-common.h
> @@ -6,6 +6,8 @@
> #ifndef QCOM_VADC_COMMON_H
> #define QCOM_VADC_COMMON_H
>
> +#include
> +
> #define VADC_CONV_TIME_MIN_US2000
> #define VADC_CONV_TIME_MAX_US2100
>
Re: [PATCH 19/43] x86/entry/64: Remove the SYSENTER stack canary
On Fri, Nov 24, 2017 at 06:23:47PM +0100, Ingo Molnar wrote: > From: Andy Lutomirski > > Now that the SYSENTER stack has a guard page, there's no need for a > canary to detect overflow after the fact. > > Signed-off-by: Andy Lutomirski > Signed-off-by: Thomas Gleixner > Cc: Borislav Petkov > Cc: Brian Gerst > Cc: Dave Hansen > Cc: Josh Poimboeuf > Cc: Linus Torvalds > Cc: Peter Zijlstra > Link: > https://lkml.kernel.org/r/be3179c0a38c392fa44ebeb7dd89391ff5c010c3.1511497875.git.l...@kernel.org > Signed-off-by: Ingo Molnar > --- > arch/x86/include/asm/processor.h | 1 - > arch/x86/kernel/dumpstack.c | 3 +-- > arch/x86/kernel/process.c| 1 - > arch/x86/kernel/traps.c | 7 --- > 4 files changed, 1 insertion(+), 11 deletions(-) Nice. Reviewed-by: Borislav Petkov -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver
On Fri, Nov 17, 2017 at 10:33:55AM +0100, jacopo mondi wrote: > Hi Sakari! > > On Fri, Nov 17, 2017 at 02:36:51AM +0200, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Wed, Nov 15, 2017 at 03:25:11PM +0100, jacopo mondi wrote: > > > Hi Sakari, > > >thanks for review! > > > > You're welcome! > > > > > On Wed, Nov 15, 2017 at 02:45:51PM +0200, Sakari Ailus wrote: > > > > Hi Jacopo, > > > > > > > > Could you remove the original driver and send the patch using git > > > > send-email -C ? That way a single patch would address converting it to a > > > > proper V4L2 driver as well as move it to the correct location. The > > > > changes > > > > would be easier to review that way since then, well, it'd be easier to > > > > see > > > > the changes. :-) > > > > > > Actually I prefer not to remove the existing driver at the moment. See > > > the cover letter for reasons why not to do so right now... > > > > So it's about testing mostly? Does someone (possibly you) have those boards > > to test? I'd like to see this patchset to remove that last remaining SoC > > camera bridge driver. :-) > > Well, we agreed that for most of those platforms, compile testing it > would be enough (let's believe in "if it compiles, it works"). I > personally don't have access to those boards, and frankly I'm not even > sure there are many of them around these days (I guess most of them > are not even produced anymore). > > > > > > > > > Also, there's not that much code from the old driver in here, surely > > > less than the default 50% -C and -M options of 'git format-patch' use > > > as a threshold for detecting copies iirc.. > > > > Oh, if that's so, then makes sense to review it as a new driver. > > thanks :) > > > > > > > > > > The same goes for the two V4L2 SoC camera sensor / video decoder > > > > drivers at > > > > the end of the set. > > > > > > > > > > Also in this case I prefer not to remove existing code, as long as > > > there are platforms using it.. > > > > Couldn't they use this driver instead? > > Oh, they will eventually, I hope :) > > I would like to make sure we're all on the same page with this. My > preference would be: > > 1) Have renesas-ceu.c driver merged with Migo-R ported to use this new > driver as an 'example'. > 2) Do not remove any of the existing soc_camera code at this point > 3) Port all other 4 SH users of sh_mobile_ceu_camera to use the now > merged renesas-ceu driver > 4) Remove sh_mobile_ceu_camera and soc_camera sensor drivers whose > only users were those 4 SH boards > 5) Remove soc_camera completely. For my understanding there are some > PXA platforms still using soc_camera provided utilities somewhere. > Hans knows better, but we can discuss this once we'll get there. The first point here is basically done by this patchset and your intent would be to proceed with the rest, right? The above seems good; what I wanted to say was that I'd like to avoid ending up in a permanent situation where some hardware works with the new driver and some will continue to use the old one. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH] arm64: allwinner: a64: Enable AXP803 for Orangepi Win
On Thu, Nov 23, 2017 at 11:05:36PM +0530, Jagan Teki wrote: > Enable AXP803 PMIC and regulators for Orangepi Win. > > Signed-off-by: Jagan Teki Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
On Thu, Nov 23, 2017 at 09:14:44AM +0800, Yong wrote: > > On Wed, Nov 22, 2017 at 09:33:06AM +0800, Yong wrote: > > > > On Thu, Jul 27, 2017 at 01:01:35PM +0800, Yong Deng wrote: > > > > > Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface > > > > > and CSI1 is used for parallel interface. This is not documented in > > > > > datasheet but by testing and guess. > > > > > > > > > > This patch implement a v4l2 framework driver for it. > > > > > > > > > > Currently, the driver only support the parallel interface. MIPI-CSI2, > > > > > ISP's support are not included in this patch. > > > > > > > > > > Signed-off-by: Yong Deng > > > > > > > > Thanks again for this driver. > > > > > > > > It seems like at least this iteration is behaving in a weird way with > > > > DMA transfers for at least YU12 and NV12 (and I would assume YV12). > > > > > > > > Starting a transfer of multiple frames in either of these formats, > > > > using either ffmpeg (ffmpeg -f v4l2 -video_size 640x480 -framerate 30 > > > > -i /dev/video0 output.mkv) or yavta (yavta -c80 -p -F --skip 0 -f NV12 > > > > -s 640x480 $(media-c tl -e 'sun6i-csi')) will end up in a panic. > > > > > > > > The panic seems to be generated with random data going into parts of > > > > the kernel memory, the pattern being in my case something like > > > > 0x8287868a which is very odd (always around 0x88) > > > > > > > > It turns out that when you cover the sensor, the values change to > > > > around 0x28, so it really seems like it's pixels that have been copied > > > > there. > > > > > > > > I've looked quickly at the DMA setup, and it seems reasonable to > > > > me. Do you have the same issue on your side? Have you been able to > > > > test those formats using your hardware? > > > > > > I had tested the following formats with BT1120 input: > > > V4L2_PIX_FMT_NV12 -> NV12 > > > V4L2_PIX_FMT_NV21 -> NV21 > > > V4L2_PIX_FMT_NV16 -> NV16 > > > V4L2_PIX_FMT_NV61 -> NV61 > > > V4L2_PIX_FMT_YUV420 -> YU12 > > > V4L2_PIX_FMT_YVU420 -> YV12 > > > V4L2_PIX_FMT_YUV422P -> 422P > > > And they all work fine. > > > > Ok, that's good to know. > > > > > > Given that they all are planar formats and YUYV and the likes work > > > > just fine, maybe we can leave them aside for now? > > > > > > V4L2_PIX_FMT_YUV422P and V4L2_PIX_FMT_YUYV is OK, and V4L2_PIX_FMT_NV12 > > > is bad? It's really weird. > > > > > > What's your input bus code format, type and width? > > > > The sensor is an ov5640, so the MBUS code for the bus is > > MEDIA_BUS_FMT_YUYV8_2X8. > > Did you test on V3s? No, this is on an H3, but that would be the first difference so far. > I haven't tested it with MEDIA_BUS_FMT_YUYV8_2X8. Ok, it's good to know that at least it works on your end, it's useful for us to debug things :) > The Allwinner CSI's DMA is definitely weird. Ondřej Jirman thought > that CSI has an internal queue (Ondřej's commit has explained in detail). > I think CSI just pick up the buffer address before the frame done > interrupt triggered. > The patch in attachment can deal with this. You can see if it is > useful to solve your problem. I'll test that on monday, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH v1 08/10] media: i2c: ov772x: Remove soc_camera dependencies
On Fri, Nov 17, 2017 at 10:14:51AM +0100, jacopo mondi wrote:
> Hi Sakari!
>
> On Fri, Nov 17, 2017 at 02:43:15AM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Nov 15, 2017 at 11:56:01AM +0100, Jacopo Mondi wrote:
> > >
>
> [snip]
>
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -25,8 +26,8 @@
> > > #include
> > >
> > > #include
> > > -#include
> > > -#include
> > > +
> > > +#include
> >
> > Alphabetical order would be nice.
>
> ups!
>
> >
> > > #include
> > > #include
> > > #include
> > > @@ -393,7 +394,7 @@ struct ov772x_win_size {
> > > struct ov772x_priv {
> > > struct v4l2_subdevsubdev;
> > > struct v4l2_ctrl_handler hdl;
> > > - struct v4l2_clk *clk;
> > > + struct clk *clk;
> > > struct ov772x_camera_info*info;
> > > const struct ov772x_color_format *cfmt;
> > > const struct ov772x_win_size *win;
> > > @@ -550,7 +551,7 @@ static int ov772x_reset(struct i2c_client *client)
> > > }
> > >
> > > /*
> > > - * soc_camera_ops function
> > > + * subdev ops
> > > */
> > >
> > > static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
> > > @@ -650,13 +651,36 @@ static int ov772x_s_register(struct v4l2_subdev *sd,
> > > }
> > > #endif
> > >
> > > +static int ov772x_power_on(struct ov772x_priv *priv)
> > > +{
> > > + int ret;
> > > +
> > > + if (priv->info->platform_enable) {
> > > + ret = priv->info->platform_enable();
> > > + if (ret)
> > > + return ret;
> >
> > What does this do, enable the regulator?
>
> Well, it depends on what function the platform code stores in
> 'platform_enable' pointer, doesn't it?
>
> As you can see in [05/10] of this series, for Migo-R it's not about
> a regulator, but switching between the two available video inputs
> (OV7725 and TW9910) toggling their 'enable' pins.
Ok. That's not a very nice design.
Fair enough. I guess it's good to proceed one thing at a time.
If someone has this sensor on a board with DT support, we can use the
regulator framework and just ignore the platform callbacks.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
[PATCH 0/2] video: ssd1307fb: Adjustments for ssd1307fb_probe()
From: Markus Elfring Date: Sat, 25 Nov 2017 16:56:46 +0100 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation Improve a size determination drivers/video/fbdev/ssd1307fb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.15.0
[PATCH 1/2] video: ssd1307fb: Delete an error message for a failed memory allocation in ssd1307fb_probe()
From: Markus Elfring
Date: Sat, 25 Nov 2017 16:45:56 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/video/fbdev/ssd1307fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index f599520374dd..fb0ef1922d49 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -630,7 +630,6 @@ static int ssd1307fb_probe(struct i2c_client *client,
ssd1307fb_defio = devm_kzalloc(&client->dev, sizeof(struct
fb_deferred_io), GFP_KERNEL);
if (!ssd1307fb_defio) {
- dev_err(&client->dev, "Couldn't allocate deferred io.\n");
ret = -ENOMEM;
goto fb_alloc_error;
}
--
2.15.0
[PATCH 2/2] video: ssd1307fb: Improve a size determination in ssd1307fb_probe()
From: Markus Elfring
Date: Sat, 25 Nov 2017 16:50:26 +0100
Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/video/fbdev/ssd1307fb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index fb0ef1922d49..bb32b2dd1f7a 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -628,7 +628,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
goto fb_alloc_error;
}
- ssd1307fb_defio = devm_kzalloc(&client->dev, sizeof(struct
fb_deferred_io), GFP_KERNEL);
+ ssd1307fb_defio = devm_kzalloc(&client->dev, sizeof(*ssd1307fb_defio),
+ GFP_KERNEL);
if (!ssd1307fb_defio) {
ret = -ENOMEM;
goto fb_alloc_error;
--
2.15.0
Re: [PATCH] x86/mm/kaiser: Fix IRQ entries text section mapping
On Sat, 25 Nov 2017, Ingo Molnar wrote: > kaiser_add_user_map_ptrs_early(__entry_text_start, __entry_text_end, > __PAGE_KERNEL_RX | _PAGE_GLOBAL); > + kaiser_add_user_map_ptrs_early(__irqentry_text_start, > __irqentry_text_end, > +__PAGE_KERNEL_RX | _PAGE_GLOBAL); The bad news is that this maps more stuff than actually needed: 81ab1c20 T __irqentry_text_start 81ab1c20 T apic_timer_interrupt 81ab1cf0 T x86_platform_ipi 81ab1dc0 T threshold_interrupt 81ab1e90 T deferred_error_interrupt 81ab1f60 T thermal_interrupt 81ab2030 T call_function_single_interrupt 81ab2100 T call_function_interrupt 81ab21d0 T reschedule_interrupt 81ab22a0 T error_interrupt 81ab2370 T spurious_interrupt 81ab2440 T irq_work_interrupt The above are the real entry points. The below is already C-Code which has nothing to do with the entry region. 81ab2510 T do_IRQ 81ab2610 T smp_x86_platform_ipi 81ab2840 T smp_irq_work_interrupt 81ab2a50 T smp_deferred_error_interrupt 81ab2c60 T smp_threshold_interrupt 81ab2e70 T smp_thermal_interrupt 81ab3080 T smp_reschedule_interrupt 81ab3280 T smp_call_function_interrupt 81ab3490 T smp_call_function_single_interrupt 81ab36a0 T smp_apic_timer_interrupt 81ab3900 T smp_spurious_interrupt 81ab3b40 T smp_error_interrupt 81ab3e20 T smp_irq_move_cleanup_interrupt 81ab3ecc T __irqentry_text_end irqentry_text is checked by kasan, kprobes, tracing and the unwinder. kasan uses it to filter irq stacks, i.e. to limit the stack entries to the point where it hits something inside irqentry_text. Shouldn't be a problem to restrict it to the actual entry code. kprobes has a similar thing. The comment says: Do not optimize in the entry code due to the unstable stack and register handling. The C functions are not affected by that Tracing uses it to stop the printout of the function graph. Should be safe to print the C-Code functions. The unwinder might get confused. The comment there says: Don't warn if the unwinder got lost due to an interrupt in entry code or in the C handler before the first frame pointer got set up: I think the unwinder one is easy to solve by having a separate section for the C-code portion. Thanks, tglx
Re: [PATCH v5 1/2] iio : Add cm3218 smbus ara and acpi support
On Wed, 22 Nov 2017 23:52:33 +0100
Marc CAPDEVILLE wrote:
> On asus T100, Capella cm3218 chip is implemented as ambiant light
> sensor. This chip expose an smbus ARA protocol device on standard
> address 0x0c. The chip is not functional before all alerts are
> acknowledged.
> On asus T100, this device is enumerated on ACPI bus and the description
> give two I2C connection. The first is the connection to the ARA device
> and the second gives the real address of the device.
>
> So, on device probe,If the i2c address is the ARA address and the
> device is enumerated via acpi, we lookup for the real address in
> the ACPI resource list and create an i2c dummy device with that address.
> If the client address is already the real one, we create an i2c dummy
> device for the ara function.
>
> if an interrupt resource is given, we request a treaded interrupt to
threaded
> acknowledge the smbus alert for cm3218 chip.
I'm afraid I really want this to use the standard ARA infrastructure.
Sooner or later we'll have a device where the bus has multiple ARA
capable devices given that's the whole point of ARA in the first
place (as opposed to a standard interrupt).
I'd do it by 'adjusting' the automatically created device to
have the correct address then calling i2c_setup_smbus_alert.
If the i2c_setup_smbus_alert fails it gets interesting
as it may mean there is already another device registering from
ACPI in a similar way to this one so we already have an alert
master. Might be worth adding something to the i2c core to detect
that rather than a normal error.
This is all really ugly given that all the ARA stuff should
be part of the bus, not the devices on the bus. Ah well.
You'll need to provide an alert callback for the i2c driver
to actually handle the resulting alert and it should all
work reasonably cleanly.
I live in hope that sensible ACPI unified handling of this
will one day occur but in the meantime I think this is the
best we can do.
Sorry I didn't get back to you before this version!
Jonathan
>
> Signed-off-by: Marc CAPDEVILLE
> ---
> v5 :
>- dont use smbus_alert driver anymore
>- implement threaded interrupt to acknowledge alert from cm3218
>- dont touch address in i2c_client structur
>- register new dummy i2c client for second address
>
> v4 :
>- rework acpi i2c adress lookup due to bad commit being sent.
>
> v3 :
>- rework acpi i2c adress lookup
>- comment style cleanup
>- add prefix cm32181_to constantes and macros
>
> v2 :
>- cm3218 support always build
>- Cleanup of unneeded #if statement
>- Beter identifying chip in platform device, acpi and of_device
>
> drivers/iio/light/cm32181.c | 247
> ++--
> 1 file changed, 239 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index d6fd0dace74f..9b412dbb3248 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -18,6 +18,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> /* Registers Address */
> #define CM32181_REG_ADDR_CMD 0x00
> @@ -47,6 +50,11 @@
> #define CM32181_CALIBSCALE_RESOLUTION1000
> #define MLUX_PER_LUX 1000
>
> +#define CM32181_ID 0x81
> +#define CM3218_ID0x18
> +
> +#define CM3218_ARA_ADDR 0x0c
> +
> static const u8 cm32181_reg[CM32181_CONF_REG_NUM] = {
> CM32181_REG_ADDR_CMD,
> };
> @@ -56,7 +64,9 @@ static const int als_it_value[] = {25000, 5, 10,
> 20, 40,
> 80};
>
> struct cm32181_chip {
> - struct i2c_client *client;
> + int chip_id;
> + struct i2c_client *als;
> + struct i2c_client *ara;
> struct mutex lock;
> u16 conf_regs[CM32181_CONF_REG_NUM];
> int calibscale;
> @@ -72,7 +82,7 @@ struct cm32181_chip {
> */
> static int cm32181_reg_init(struct cm32181_chip *cm32181)
> {
> - struct i2c_client *client = cm32181->client;
> + struct i2c_client *client = cm32181->als;
> int i;
> s32 ret;
>
> @@ -81,7 +91,7 @@ static int cm32181_reg_init(struct cm32181_chip *cm32181)
> return ret;
>
> /* check device ID */
> - if ((ret & 0xFF) != 0x81)
> + if ((ret & 0xFF) != cm32181->chip_id)
> return -ENODEV;
>
> /* Default Values */
> @@ -138,7 +148,7 @@ static int cm32181_read_als_it(struct cm32181_chip
> *cm32181, int *val2)
> */
> static int cm32181_write_als_it(struct cm32181_chip *cm32181, int val)
> {
> - struct i2c_client *client = cm32181->client;
> + struct i2c_client *client = cm32181->als;
> u16 als_it;
> int ret, i, n;
>
> @@ -175,7 +185,7 @@ static int cm32181_write_als_it(struct cm32181_chip
> *cm32181, int val)
> */
> static int cm32181_get_lux(struct cm32181_chip *cm32181)
> {
> - struct i2c_client *client = cm32181->client;
> + stru
Re: [PATCH 20/43] x86/entry: Clean up SYSENTER_stack code
On Fri, Nov 24, 2017 at 06:23:48PM +0100, Ingo Molnar wrote:
> From: Andy Lutomirski
>
> The existing code was a mess, mainly because C arrays are nasty.
> Turn SYSENTER_stack into a struct, add a helper to find it, and do
> all the obvious cleanups this enables.
>
> Signed-off-by: Andy Lutomirski
> Signed-off-by: Thomas Gleixner
> Cc: Borislav Petkov
> Cc: Brian Gerst
> Cc: Dave Hansen
> Cc: Josh Poimboeuf
> Cc: Linus Torvalds
> Cc: Peter Zijlstra
> Link:
> https://lkml.kernel.org/r/38ff640712c9b591b32de24a080daf13afaba234.1511497875.git.l...@kernel.org
> Signed-off-by: Ingo Molnar
> ---
...
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index 61b1af88ac07..46c0995344aa 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -94,10 +94,8 @@ void common(void) {
> BLANK();
> DEFINE(PTREGS_SIZE, sizeof(struct pt_regs));
>
> - /* Offset from cpu_tss to SYSENTER_stack */
> - OFFSET(CPU_TSS_SYSENTER_stack, tss_struct, SYSENTER_stack);
> - /* Size of SYSENTER_stack */
> - DEFINE(SIZEOF_SYSENTER_stack, sizeof(((struct tss_struct
> *)0)->SYSENTER_stack));
> + OFFSET(TSS_STRUCT_SYSENTER_stack, tss_struct, SYSENTER_stack);
> + DEFINE(SIZEOF_SYSENTER_stack, sizeof(struct SYSENTER_stack));
>
> /* Layout info for cpu_entry_area */
> OFFSET(CPU_ENTRY_AREA_tss, cpu_entry_area, tss);
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 6b949e6ea0f9..f9c7e6852874 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1332,12 +1332,7 @@ void enable_sep_cpu(void)
>
> tss->x86_tss.ss1 = __KERNEL_CS;
> wrmsr(MSR_IA32_SYSENTER_CS, tss->x86_tss.ss1, 0);
> -
> - wrmsr(MSR_IA32_SYSENTER_ESP,
> - (unsigned long)&get_cpu_entry_area(cpu)->tss +
> - offsetofend(struct tss_struct, SYSENTER_stack),
> - 0);
> -
> + wrmsr(MSR_IA32_SYSENTER_ESP, (unsigned long)(cpu_SYSENTER_stack(cpu) +
> 1), 0);
> wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0);
Right, so we have now two TSS thingies, AFAICT:
tss = &per_cpu(cpu_tss, cpu);
which is cpu_tss and then indirectly, we have also:
&get_cpu_entry_area((cpu))->tss
And those are two different things in my guest here:
[0.044002] tss: 0xf5747000
[0.044706] entry area tss: 0xffef1000
What is the logic here? We carry two TSSs per CPU - one which is RO
for the entry area and the other is the actual cpu_tss thing? Or am I
misreading it?
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 1/2] iio: adc: ina2xx: Make calibration register value fixed
On Wed, 22 Nov 2017 16:32:14 +0100
Maciej Purski wrote:
> Calibration register is used for calculating current register in
> hardware according to datasheet:
> current = shunt_volt * calib_register / 2048 (ina 226)
> current = shunt_volt * calib_register / 4096 (ina 219)
>
> Fix calib_register value to 2048 for ina226 and 4096 for ina 219 in
> order to avoid truncation error and provide best precision allowed
> by shunt_voltage measurement. Make current scale value follow changes
> of shunt_resistor from sysfs as calib_register value is now fixed.
>
> Power_lsb value should also follow shunt_resistor changes as stated in
> datasheet:
> power_lsb = 25 * current_lsb (ina 226)
> power_lsb = 20 * current_lsb (ina 219)
>
> Signed-off-by: Maciej Purski
I like the patch but would like to let it sit for a little longer
to see if others wish to comment.
Looks good to me but there are users out there and I want some
feedback from others that this won't break them...
Whilst the change is fully within the ABI we might have
known user scripts that make assumptions about the scale for
example and need time to fix those before we push this out.
If no one shouts in a few weeks I'll probably take it to force
the issue!
Remind me if I seem to have forgotten it.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ina2xx-adc.c | 59
>
> 1 file changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 84a4387..0f25087 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -110,11 +110,11 @@ enum ina2xx_ids { ina219, ina226 };
>
> struct ina2xx_config {
> u16 config_default;
> - int calibration_factor;
> + int calibration_value;
> int shunt_div;
> int bus_voltage_shift;
> int bus_voltage_lsb;/* uV */
> - int power_lsb; /* uW */
> + int power_lsb_factor;
> enum ina2xx_ids chip_id;
> };
>
> @@ -124,6 +124,8 @@ struct ina2xx_chip_info {
> const struct ina2xx_config *config;
> struct mutex state_lock;
> unsigned int shunt_resistor_uohm;
> + unsigned int current_lsb_uA;
> + unsigned int power_lsb_uW;
> int avg;
> int int_time_vbus; /* Bus voltage integration time uS */
> int int_time_vshunt; /* Shunt voltage integration time uS */
> @@ -133,20 +135,20 @@ struct ina2xx_chip_info {
> static const struct ina2xx_config ina2xx_config[] = {
> [ina219] = {
> .config_default = INA219_CONFIG_DEFAULT,
> - .calibration_factor = 4096,
> + .calibration_value = 4096,
> .shunt_div = 100,
> .bus_voltage_shift = 3,
> .bus_voltage_lsb = 4000,
> - .power_lsb = 2,
> + .power_lsb_factor = 20,
> .chip_id = ina219,
> },
> [ina226] = {
> .config_default = INA226_CONFIG_DEFAULT,
> - .calibration_factor = 512,
> + .calibration_value = 2048,
> .shunt_div = 400,
> .bus_voltage_shift = 0,
> .bus_voltage_lsb = 1250,
> - .power_lsb = 25000,
> + .power_lsb_factor = 25,
> .chip_id = ina226,
> },
> };
> @@ -210,14 +212,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>
> case INA2XX_POWER:
> /* processed (mW) = raw*lsb (uW) / 1000 */
> - *val = chip->config->power_lsb;
> + *val = chip->power_lsb_uW;
> *val2 = 1000;
> return IIO_VAL_FRACTIONAL;
>
> case INA2XX_CURRENT:
> - /* processed (mA) = raw (mA) */
> - *val = 1;
> - return IIO_VAL_INT;
> + /* processed (mA) = raw*lsb (uA) / 1000 */
> + *val = chip->current_lsb_uA;
> + *val2 = 1000;
> + return IIO_VAL_FRACTIONAL;
> }
> }
>
> @@ -434,28 +437,35 @@ static ssize_t ina2xx_allow_async_readout_store(struct
> device *dev,
> }
>
> /*
> - * Set current LSB to 1mA, shunt is in uOhms
> - * (equation 13 in datasheet). We hardcode a Current_LSB
> - * of 1.0 x10-3. The only remaining parameter is RShunt.
> - * There is no need to expose the CALIBRATION register
> - * to the user for now. But we need to reset this register
> - * if the user updates RShunt after driver init, e.g upon
> - * reading an EEPROM/Probe-type value.
> + * Calibration register is set to the best value, which eliminates
> + * truncation errors on calculating current register in hardware.
> + * According to datasheet (eq. 3) the best values are 2048 for
> + * ina226 and 4096 for ina219. They are hardcoded as calibration_value.
> */
> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)
> {
> - u16 reg
Re: [PATCH] iio: accel: remove redundant pointer pdata
On Wed, 22 Nov 2017 14:32:11 +
Colin King wrote:
> From: Colin Ian King
>
> Pointer pdata is being assigned but it is never being used, hence
> it is redundant and can be removed. Cleans up clang warning:
>
> drivers/iio/accel/st_accel_core.c:952:3: warning: Value stored to 'pdata'
> is never read
>
> Signed-off-by: Colin Ian King
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> ---
> drivers/iio/accel/st_accel_core.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/iio/accel/st_accel_core.c
> b/drivers/iio/accel/st_accel_core.c
> index 460aa58e0159..6fe995cf16a6 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -920,8 +920,6 @@ static const struct iio_trigger_ops st_accel_trigger_ops
> = {
> int st_accel_common_probe(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *adata = iio_priv(indio_dev);
> - struct st_sensors_platform_data *pdata =
> - (struct st_sensors_platform_data *)adata->dev->platform_data;
> int irq = adata->get_irq_data_ready(indio_dev);
> int err;
>
> @@ -948,9 +946,6 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
> &adata->sensor_settings->fs.fs_avl[0];
> adata->odr = adata->sensor_settings->odr.odr_avl[0].hz;
>
> - if (!pdata)
> - pdata = (struct st_sensors_platform_data *)&default_accel_pdata;
> -
> err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
> if (err < 0)
> goto st_accel_power_off;
Re: [PATCH 20/43] x86/entry: Clean up SYSENTER_stack code
On Sat, 25 Nov 2017, Borislav Petkov wrote: > > - > > + wrmsr(MSR_IA32_SYSENTER_ESP, (unsigned long)(cpu_SYSENTER_stack(cpu) + > > 1), 0); > > wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0); > > Right, so we have now two TSS thingies, AFAICT: > > tss = &per_cpu(cpu_tss, cpu); > > which is cpu_tss and then indirectly, we have also: > > &get_cpu_entry_area((cpu))->tss > > And those are two different things in my guest here: > > [0.044002] tss: 0xf5747000 > [0.044706] entry area tss: 0xffef1000 > > What is the logic here? We carry two TSSs per CPU - one which is RO > for the entry area and the other is the actual cpu_tss thing? Or am I > misreading it? entry area tss is a alias mapping of cpu_tss + set_percpu_fixmap_pages(get_cpu_entry_area_index(cpu, tss), + &per_cpu(cpu_tss, cpu), + sizeof(struct tss_struct) / PAGE_SIZE, + PAGE_KERNEL); Thanks, tglx
Re: [PATCH 20/43] x86/entry: Clean up SYSENTER_stack code
> On Nov 25, 2017, at 9:50 AM, Thomas Gleixner wrote: > > On Sat, 25 Nov 2017, Borislav Petkov wrote: >>> - >>> +wrmsr(MSR_IA32_SYSENTER_ESP, (unsigned long)(cpu_SYSENTER_stack(cpu) + >>> 1), 0); >>>wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0); >> >> Right, so we have now two TSS thingies, AFAICT: >> >>tss = &per_cpu(cpu_tss, cpu); >> >> which is cpu_tss and then indirectly, we have also: >> >>&get_cpu_entry_area((cpu))->tss >> >> And those are two different things in my guest here: >> >> [0.044002] tss: 0xf5747000 >> [0.044706] entry area tss: 0xffef1000 >> >> What is the logic here? We carry two TSSs per CPU - one which is RO >> for the entry area and the other is the actual cpu_tss thing? Or am I >> misreading it? > > entry area tss is a alias mapping of cpu_tss > > + set_percpu_fixmap_pages(get_cpu_entry_area_index(cpu, tss), > + &per_cpu(cpu_tss, cpu), > + sizeof(struct tss_struct) / PAGE_SIZE, > + PAGE_KERNEL); > Exactly. And, in the patch I haven't emailed, the alias is RO on x86_64. Maybe I should rename cpu_tss to cpu_tss_rw in that patch. > Thanks, > >tglx
Re: [PATCH 20/43] x86/entry: Clean up SYSENTER_stack code
On Sat, 25 Nov 2017, Andy Lutomirski wrote: > > On Nov 25, 2017, at 9:50 AM, Thomas Gleixner wrote: > > > > On Sat, 25 Nov 2017, Borislav Petkov wrote: > >>> - > >>> +wrmsr(MSR_IA32_SYSENTER_ESP, (unsigned long)(cpu_SYSENTER_stack(cpu) > >>> + 1), 0); > >>>wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0); > >> > >> Right, so we have now two TSS thingies, AFAICT: > >> > >>tss = &per_cpu(cpu_tss, cpu); > >> > >> which is cpu_tss and then indirectly, we have also: > >> > >>&get_cpu_entry_area((cpu))->tss > >> > >> And those are two different things in my guest here: > >> > >> [0.044002] tss: 0xf5747000 > >> [0.044706] entry area tss: 0xffef1000 > >> > >> What is the logic here? We carry two TSSs per CPU - one which is RO > >> for the entry area and the other is the actual cpu_tss thing? Or am I > >> misreading it? > > > > entry area tss is a alias mapping of cpu_tss > > > > + set_percpu_fixmap_pages(get_cpu_entry_area_index(cpu, tss), > > + &per_cpu(cpu_tss, cpu), > > + sizeof(struct tss_struct) / PAGE_SIZE, > > + PAGE_KERNEL); > > > > Exactly. And, in the patch I haven't emailed, the alias is RO on x86_64. Btw, I don't think you have to worry about making it RO. The SDM is blurry about that for 64bit. RW is a must for 32bit though. > Maybe I should rename cpu_tss to cpu_tss_rw in that patch. For clarity that would be nice. Thanks, tglx
Re: [PATCH 20/43] x86/entry: Clean up SYSENTER_stack code
On Sat, Nov 25, 2017 at 06:03:36PM +0100, Thomas Gleixner wrote: > > Maybe I should rename cpu_tss to cpu_tss_rw in that patch. > > For clarity that would be nice. + a comment stating the alias mapping. It took tglx and me a while on IRC to figure it out. :-) Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 24/43] x86/mm/kaiser: Mark per-cpu data structures required for entry/exit
On Fri, 24 Nov 2017, Ingo Molnar wrote:
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index aab4fe9f49f8..300090d1c209 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -46,7 +46,7 @@ struct gdt_page {
> struct desc_struct gdt[GDT_ENTRIES];
> } __attribute__((aligned(PAGE_SIZE)));
>
> -DECLARE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page);
> +DECLARE_PER_CPU_PAGE_ALIGNED_USER_MAPPED(struct gdt_page, gdt_page);
This patch is obsolete. We have the alias mappings in the cpu_entry_area
already.
Thanks,
tglx
Re: [PATCH v2 2/2] x86: disable IRQs before changing CR4
Thomas Gleixner wrote: > On Fri, 24 Nov 2017, Nadav Amit wrote: >> /* Set in this cpu's CR4. */ >> -static inline void cr4_set_bits(unsigned long mask) >> +static inline void cr4_set_bits_irqs_off(unsigned long mask) > > This change is kinda weird. I'd expect that there is a corresponding > function cr4_set_bits() which takes care of disabling interrupts. But there > is not. All it does is creating a lot of pointless churn. > >> static __always_inline void setup_smep(struct cpuinfo_x86 *c) >> static __init int setup_disable_smap(char *arg) >> static __always_inline void setup_pku(struct cpuinfo_x86 *c) > > Why are you not doing this at the call site around all calls which fiddle > with cr4, i.e. in identify_cpu() ? > > identify_cpu() is called from two places: > >identify_boot_cpu() and identify_secondary_cpu() > > identify_secondary_cpu is called with interrupts disabled anyway and there > is no reason why we can't enforce interrupts being disabled around > identify_cpu() completely. > > But if we actually do the right thing, i.e. having cr4_set_bit() and > cr4_set_bit_irqsoff() all of this churn goes away magically. > > Then the only place which needs to be changed is the context switch because > here interrupts are already disabled and we really care about performance. > >> @@ -293,7 +303,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct >> task_struct *next_p, >> } >> >> if ((tifp ^ tifn) & _TIF_NOTSC) >> -cr4_toggle_bits(X86_CR4_TSD); >> +cr4_toggle_bits_irqs_off(X86_CR4_TSD); You make a good point. I will add cr4_set_bit(). I will leave identify_cpu() as is, since it is rather hard to maintain code that enables/disables irqs at one point and rely on these operations at a completely different place. As you said, it is less of an issue once cr4_set_bit() and friends are introduced. Thanks, Nadav
Re: [PATCH] lib: memmove: Use optimised memcpy if possible
Hi,
On 4 October 2017 at 22:26, PrasannaKumar Muralidharan
wrote:
> When there is no overlap between src and dst use optimised memcpy if it
> is available.
>
> Signed-off-by: Paul Burton
> Signed-off-by: PrasannaKumar Muralidharan
> ---
> This change is a small part of a patch [1] from Paul Burton. I have
> added his Signed-off by. I do not know whether it is correct. Please let
> me know if it has to be changed, I will send a v2.
>
> This patch is boot tested with qemu for MIPS architecture by removing
> mips's memmove routine. This patch does not contain MIPS changes. I
> will try to find out why [1] was not taken already and figure out what
> to do.
>
> 1. https://patchwork.linux-mips.org/patch/14517/
>
> lib/string.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/lib/string.c b/lib/string.c
> index 9921dc2..462ab7b 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -825,6 +825,17 @@ void *memmove(void *dest, const void *src, size_t count)
> char *tmp;
> const char *s;
>
> +#ifdef __HAVE_ARCH_MEMCPY
> + /* Use optimised memcpy when there is no overlap */
> + const char *s_end = src + count;
> + const char *d = dest;
> + char *d_end = dest + count;
> +
> + s = src;
> + if ((d_end <= s) || (s_end <= d))
> + return memcpy(dest, src, count);
> +#endif /* __HAVE_ARCH_MEMCPY */
> +
> if (dest <= src) {
> tmp = dest;
> s = src;
> --
> 2.10.0
>
Is there anything more that I have to do for this patch?
Regards,
PrasannaKumar
Re: [PATCH v2 2/2] x86: disable IRQs before changing CR4
On Sat, 25 Nov 2017, Nadav Amit wrote: > Thomas Gleixner wrote: > > > On Fri, 24 Nov 2017, Nadav Amit wrote: > >> /* Set in this cpu's CR4. */ > >> -static inline void cr4_set_bits(unsigned long mask) > >> +static inline void cr4_set_bits_irqs_off(unsigned long mask) > > > > This change is kinda weird. I'd expect that there is a corresponding > > function cr4_set_bits() which takes care of disabling interrupts. But there > > is not. All it does is creating a lot of pointless churn. > > > >> static __always_inline void setup_smep(struct cpuinfo_x86 *c) > >> static __init int setup_disable_smap(char *arg) > >> static __always_inline void setup_pku(struct cpuinfo_x86 *c) > > > > Why are you not doing this at the call site around all calls which fiddle > > with cr4, i.e. in identify_cpu() ? > > > > identify_cpu() is called from two places: > > > >identify_boot_cpu() and identify_secondary_cpu() > > > > identify_secondary_cpu is called with interrupts disabled anyway and there > > is no reason why we can't enforce interrupts being disabled around > > identify_cpu() completely. > > > > But if we actually do the right thing, i.e. having cr4_set_bit() and > > cr4_set_bit_irqsoff() all of this churn goes away magically. > > > > Then the only place which needs to be changed is the context switch because > > here interrupts are already disabled and we really care about performance. > > > >> @@ -293,7 +303,7 @@ void __switch_to_xtra(struct task_struct *prev_p, > >> struct task_struct *next_p, > >>} > >> > >>if ((tifp ^ tifn) & _TIF_NOTSC) > >> - cr4_toggle_bits(X86_CR4_TSD); > >> + cr4_toggle_bits_irqs_off(X86_CR4_TSD); > > You make a good point. I will add cr4_set_bit(). I will leave identify_cpu() > as is, since it is rather hard to maintain code that enables/disables irqs > at one point and rely on these operations at a completely different place. > As you said, it is less of an issue once cr4_set_bit() and friends are > introduced. I fixed that up already as I wanted to have it done, see the tip-bot mail in your inbox. Thanks, tglx
Re: [PATCH 20/43] x86/entry: Clean up SYSENTER_stack code
On Sat, Nov 25, 2017 at 9:10 AM, Borislav Petkov wrote: > On Sat, Nov 25, 2017 at 06:03:36PM +0100, Thomas Gleixner wrote: >> > Maybe I should rename cpu_tss to cpu_tss_rw in that patch. >> >> For clarity that would be nice. > > + a comment stating the alias mapping. It took tglx and me a while on > IRC to figure it out. :-) I'm adding this comment to the top of cpu_entry_area: + * Every field is a virtual alias of some other allocated backing store. + * There is no direct allocation of a struct cpu_entry_area.
[PATCH] x86/orc: Don't bail on stack overflow
If we overflow the stack into a guard page and then try to unwind
it with ORC, it should work perfectly: by construction, there can't
be any meaningful data in the guard page because no writes to the
guard page will have succeeded.
ORC seems entirely capable of unwinding in this situation, except
that it doesn't even try. Adjust its initial stack check so that
it's willing to try unwinding.
I tested this by intentionally overflowing the task stack. The
result is an accurate call trace instead of a trace consisting
purely of '?' entries.
Signed-off-by: Andy Lutomirski
---
Hi all-
Ingo, this would have fixed half the debugging problem you had, I think.
To really nail it, we'd want some kind of magic to annotate the trace
so that page_fault (and async_page_fault) entries show CR2 and error_code.
Josh, any ideas of how to do that cleanly? We could easily hard-code it
in the OOPS unwinder, I guess.
arch/x86/kernel/unwind_orc.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index a3f973b2c97a..7f6e3935666b 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -553,8 +553,18 @@ void __unwind_start(struct unwind_state *state, struct
task_struct *task,
}
if (get_stack_info((unsigned long *)state->sp, state->task,
- &state->stack_info, &state->stack_mask))
- return;
+ &state->stack_info, &state->stack_mask)) {
+ /*
+* We weren't on a valid stack. It's possible that
+* we overflowed a valid stack into a guard page.
+* See if the next page up is valid so that we can
+* generate some kind of backtrace if this happens.
+*/
+ void *next_page = (void *)PAGE_ALIGN((unsigned long)regs->sp);
+ if (get_stack_info(next_page, state->task, &state->stack_info,
+ &state->stack_mask))
+ return;
+ }
/*
* The caller can provide the address of the first frame directly
--
2.13.6
Re: [PATCH v2 2/2] x86: disable IRQs before changing CR4
Thomas Gleixner wrote: > On Sat, 25 Nov 2017, Nadav Amit wrote: >> Thomas Gleixner wrote: >> >>> On Fri, 24 Nov 2017, Nadav Amit wrote: /* Set in this cpu's CR4. */ -static inline void cr4_set_bits(unsigned long mask) +static inline void cr4_set_bits_irqs_off(unsigned long mask) >>> >>> This change is kinda weird. I'd expect that there is a corresponding >>> function cr4_set_bits() which takes care of disabling interrupts. But there >>> is not. All it does is creating a lot of pointless churn. >>> static __always_inline void setup_smep(struct cpuinfo_x86 *c) static __init int setup_disable_smap(char *arg) static __always_inline void setup_pku(struct cpuinfo_x86 *c) >>> >>> Why are you not doing this at the call site around all calls which fiddle >>> with cr4, i.e. in identify_cpu() ? >>> >>> identify_cpu() is called from two places: >>> >>> identify_boot_cpu() and identify_secondary_cpu() >>> >>> identify_secondary_cpu is called with interrupts disabled anyway and there >>> is no reason why we can't enforce interrupts being disabled around >>> identify_cpu() completely. >>> >>> But if we actually do the right thing, i.e. having cr4_set_bit() and >>> cr4_set_bit_irqsoff() all of this churn goes away magically. >>> >>> Then the only place which needs to be changed is the context switch because >>> here interrupts are already disabled and we really care about performance. >>> @@ -293,7 +303,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p, } if ((tifp ^ tifn) & _TIF_NOTSC) - cr4_toggle_bits(X86_CR4_TSD); + cr4_toggle_bits_irqs_off(X86_CR4_TSD); >> >> You make a good point. I will add cr4_set_bit(). I will leave identify_cpu() >> as is, since it is rather hard to maintain code that enables/disables irqs >> at one point and rely on these operations at a completely different place. >> As you said, it is less of an issue once cr4_set_bit() and friends are >> introduced. > > I fixed that up already as I wanted to have it done, see the tip-bot mail > in your inbox. Thanks, your changes made it much better. At some point it might be better to make the MTRR code to use these interfaces too instead of meddling with CR4 directly. Anyhow, that is a different story. Regards, Nadav signature.asc Description: Message signed with OpenPGP
Re: [PATCH 1/2] iio: chemical: sgpxx: Support Sensirion SGPxx sensors
On Tue, 21 Nov 2017 22:46:07 +0100 (CET) Peter Meerwald-Stadler wrote: > Hello, > > some quick comments on this driver below A few additional bits from me but I think Peter got most of the key stuff. > > I think documentation is missing and the ABI is a bit problematic and > unusual > > > Support Sensirion SGP30 and SGPC3 multi-pixel I2C gas sensors > > generally, we tend to avoid wildcard driver names; sgp30 would be > preferred over sgpxx > > > Supported Features: > > > > * Indoor Air Quality (IAQ) concentrations for > > SGP30 and SGPC3: > > - tVOC (in_concentration_voc_input) > > SGP30 only: > > - CO2eq (in_concentration_co2_input) > > IAQ must first be initialized by writing a non-empty value to > > out_iaq_init. After initializing IAQ, at least one IAQ signal must > > be read out every second (SGP30) / every two seconds (SGPC3) for the > > sensor to correctly maintain its internal baseline > > * Baseline support for IAQ (in_iaq_baseline, out_iaq_baseline) > > * Gas concentration signals for > > SGP30 and SGPC3: > > - Ethanol (in_concentration_ethanol_raw) > > SGP30 only: > > - H2 (in_concentration_h2_raw) > > * On-chip self test (in_selftest) > > The self test interferes with IAQ operations. If needed, first > > retrieve the current baseline, then reset it after the self test > > * Sensor interface version (in_feature_set_version) > > * Sensor serial number (in_serial_id) > > * Humidity compensation for SGP30 > > With the help of a humidity signal, the gas signals can be > > humidity-compensated. > > * Checksummed I2C communication > > > > > For all features, refer to the data sheet or the documentation in > > Documentation/iio/chemical/sgpxx.txt for more details. > > may some brief TODOs; heat controller? > > > Signed-off-by: Andreas Brauchli > > --- > > Documentation/iio/chemical/sgpxx.txt | 112 + > > drivers/iio/chemical/Kconfig | 13 + > > drivers/iio/chemical/Makefile| 1 + > > drivers/iio/chemical/sgpxx.c | 894 > > +++ > > 4 files changed, 1020 insertions(+) > > create mode 100644 Documentation/iio/chemical/sgpxx.txt > > create mode 100644 drivers/iio/chemical/sgpxx.c > > > > diff --git a/Documentation/iio/chemical/sgpxx.txt > > b/Documentation/iio/chemical/sgpxx.txt > > new file mode 100644 > > index ..f49b2f365df3 > > --- /dev/null > > +++ b/Documentation/iio/chemical/sgpxx.txt > > @@ -0,0 +1,112 @@ > > +sgpxx: Industrial IO driver for Sensirion i2c Multi-Pixel Gas Sensors > > + > > +1. Overview > > + > > +The sgpxx driver supports the Sensirion SGP30 and SGPC3 multi-pixel gas > > sensors. > > + > > +Datasheets: > > +https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/9_Gas_Sensors/Sensirion_Gas_Sensors_SGP30_Datasheet_EN.pdf > > +https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/9_Gas_Sensors/Sensirion_Gas_Sensors_SGPC3_Datasheet_EN.pdf Put this stuff in the driver itself. > > + > > +2. Modes of Operation > > + > > +2.1. Driver Instantiation > > + > > +The sgpxx driver must be instantiated on the corresponding i2c bus with the > > +product name (sgp30 or sgpc3) and i2c address (0x58). > > + > > +Example instantiation of an sgp30 on i2c bus 1 (i2c-1): > > + > > +$ echo sgp30 0x58 | sudo tee /sys/bus/i2c/devices/i2c-1/new_device > > + > > +Using the wrong product name results in an instantiation error. Check > > dmesg. > > I'd rather drop this section, the only specific information is the I2C > address Which belongs in the device tree bindings not here. > > > + > > +2.2. Indoor Air Quality (IAQ) concentrations > > + > > +* tVOC (in_concentration_voc_input) at ppb precision (1e-9) > > +* CO2eq (in_concentration_co2_input) at ppm precision (1e-6) -- SGP30 only > > + > > +2.2.1. IAQ Initialization > > +Before Indoor Air Quality (IAQ) values can be read, the IAQ mode must be > > +initialized by writing a non-empty value to out_iaq_init: > > + > > +$ echo init > out_iaq_init > > can't this be done on probe()? It very much needs to be - userspace code for this sort of sensor should be generic and hence not need to know about how to initialize your particular sensor. It will assume if the driver is there, the device is on - or will be dynamically enabled when it wants to talk to it. > > in any case, private API should be documented under > Documentation/ABI/testing/sysfs-bus-iio-* > > > +After initializing IAQ, at least one IAQ signal must be read out every > > second > > +(SGP30) / every two seconds (SGPC3) for the sensor to correctly maintain > > its > > +internal baseline: > > shouldn't the driver do this? Absolutely! As I understand it, the requirement is also to prevent the hardware being read more often than this so it needs to be under control of the driver. > > > + > > +SGP30: > > +$ watch -n1 cat in_concentration_voc_input > > + > > +SGPC3: > > +$ wa
Re: [PATCH 2/2] iio: chemical: sgpxx: triggered buffer support
On Tue, 21 Nov 2017 17:11:29 +0100
Andreas Brauchli wrote:
> Support triggered buffer for use with e.g. hrtimer for automated
> polling to ensure that the sensor's internal baseline is correctly
> updated independently of the use-case.
Given the really strict timing requirements for this device and slow
read rates, I wouldn't involve a triggered buffer at all, but actually
do it with a thread / timer within the initial driver. The
sysfs interface is more than adequate for a 1Hz device so using
the buffered option is just adding unnecessary complexity...
I reviewed the code anyway. Key thing is though the file would be
small, there should be a separate .c file for the buffered support
if you are going to make it optional. That way we don't get ifdefs
in the c file, but rather stubs provided in the header.
You could decide to add stubs to include/linux/iio/triggered_buffer.h
/buffer.h
and then use __maybe_unusued to mark your trigger function.
The compiler would drop it anyway and this would suppress build
warnings.
However, I don't think this device wants to be supported via the
buffered interfaces at all. More smartness needed in the core
driver to maintain the timing etc.
Jonathan
>
> Triggered buffer support is only enabled when IIO_BUFFER is set.
>
> Signed-off-by: Andreas Brauchli
> ---
> drivers/iio/chemical/Kconfig | 3 +++
> drivers/iio/chemical/sgpxx.c | 38 ++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index 4574dd687513..6710fbfc6451 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -42,12 +42,15 @@ config SENSIRION_SGPXX
> tristate "Sensirion SGPxx gas sensors"
> depends on I2C
> select CRC8
> + select IIO_TRIGGERED_BUFFER if (IIO_BUFFER)
> help
> Say Y here to build I2C interface support for the following
> Sensirion SGP gas sensors:
> * SGP30 gas sensor
> * SGPC3 gas sensor
>
> + Also select IIO_BUFFER to enable triggered buffers.
> +
> To compile this driver as module, choose M here: the
> module will be called sgpxx.
>
> diff --git a/drivers/iio/chemical/sgpxx.c b/drivers/iio/chemical/sgpxx.c
> index aea55e41d4cc..025206448f73 100644
> --- a/drivers/iio/chemical/sgpxx.c
> +++ b/drivers/iio/chemical/sgpxx.c
> @@ -27,6 +27,10 @@
> #include
> #include
> #include
> +#ifdef CONFIG_IIO_BUFFER
> +#include
> +#include
> +#endif /* CONFIG_IIO_BUFFER */
> #include
>
> #define SGP_WORD_LEN 2
> @@ -789,6 +793,26 @@ static const struct of_device_id sgp_dt_ids[] = {
> { }
> };
>
> +#ifdef CONFIG_IIO_BUFFER
All this ifdef fun is rather messy.
Split it out to a separate file like most other drivers with
optional buffer support.
General rule (more or less) of kernel drivers is that
any optional ifdef stuff should be in headers to provide stubs
when code isn't available, not down in the drivers making them
harder to read.
> +static irqreturn_t sgp_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct sgp_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = sgp_get_measurement(data, data->measure_iaq_cmd,
> + SGP_MEASURE_MODE_IAQ);
> + if (!ret)
> + iio_push_to_buffers_with_timestamp(indio_dev,
> +&data->buffer.start,
> +pf->timestamp);
> +
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +#endif /* CONFIG_IIO_BUFFER */
> +
> static int sgp_probe(struct i2c_client *client,
>const struct i2c_device_id *id)
> {
> @@ -846,6 +870,17 @@ static int sgp_probe(struct i2c_client *client,
> indio_dev->channels = chip->channels;
> indio_dev->num_channels = chip->num_channels;
>
> +#ifdef CONFIG_IIO_BUFFER
> + ret = iio_triggered_buffer_setup(indio_dev,
> + iio_pollfunc_store_time,
> + sgp_trigger_handler,
> + NULL);
> + if (ret) {
> + dev_err(&client->dev, "failed to setup iio triggered buffer\n");
> + goto fail_free;
> + }
> +#endif /* CONFIG_IIO_BUFFER */
> +
> ret = devm_iio_device_register(&client->dev, indio_dev);
> if (!ret)
> return ret;
> @@ -863,6 +898,9 @@ static int sgp_remove(struct i2c_client *client)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> +#ifdef CONFIG_IIO_BUFFER
> + iio_triggered_buffer_cleanup(indio_dev);
> +#endif /* CONFIG_IIO_BUFFER */
I would prefer stubs being added to the header for this case to having
ifdefs in here.
> devm_iio_device_unregister(&client->dev, indio_dev);
>
[PATCH] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c
This is a patch to the ddk750_sii164.c file that fixes line length warnings
found by the checkpatch.pl script
Signed-off-by: Jeremy Lacomis
---
drivers/staging/sm750fb/ddk750_sii164.c | 39 +++--
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c
b/drivers/staging/sm750fb/ddk750_sii164.c
index c787a74c4f9c..d081ecbb3e3d 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -39,8 +39,10 @@ unsigned short sii164GetVendorID(void)
{
unsigned short vendorID;
- vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
SII164_VENDOR_ID_HIGH) << 8) |
- (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
SII164_VENDOR_ID_LOW);
+ vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
+ SII164_VENDOR_ID_HIGH) << 8) |
+ (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
+ SII164_VENDOR_ID_LOW);
return vendorID;
}
@@ -56,15 +58,20 @@ unsigned short sii164GetDeviceID(void)
{
unsigned short deviceID;
- deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
SII164_DEVICE_ID_HIGH) << 8) |
- (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
SII164_DEVICE_ID_LOW);
+ deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
+ SII164_DEVICE_ID_HIGH) << 8) |
+ (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
+ SII164_DEVICE_ID_LOW);
return deviceID;
}
-/* DVI.C will handle all SiI164 chip stuffs and try it best to make code
minimal and useful */
+/*
+ * DVI.C will handle all SiI164 chip stuffs and try it best to make code
minimal
+ * and useful
+ */
/*
* sii164InitChip
@@ -72,10 +79,10 @@ unsigned short sii164GetDeviceID(void)
*
* Input:
* edgeSelect - Edge Select:
- * 0 = Input data is falling edge latched
(falling edge
- * latched first in dual edge mode)
- * 1 = Input data is rising edge latched (rising
edge
- * latched first in dual edge mode)
+ * 0 = Input data is falling edge latched (falling
+ * edge latched first in dual edge mode)
+ * 1 = Input data is rising edge latched (rising
+ * edge latched first in dual edge mode)
* busSelect - Input Bus Select:
* 0 = Input data bus is 12-bits wide
* 1 = Input data bus is 24-bits wide
@@ -135,7 +142,8 @@ long sii164InitChip(unsigned char edgeSelect,
#endif
/* Check if SII164 Chip exists */
- if ((sii164GetVendorID() == SII164_VENDOR_ID) && (sii164GetDeviceID()
== SII164_DEVICE_ID)) {
+ if ((sii164GetVendorID() == SII164_VENDOR_ID) &&
+ (sii164GetDeviceID() == SII164_DEVICE_ID)) {
/*
* Initialize SII164 controller chip.
*/
@@ -260,8 +268,8 @@ void sii164ResetChip(void)
/*
* sii164GetChipString
- * This function returns a char string name of the current DVI Controller
chip.
- * It's convenient for application need to display the chip name.
+ * This function returns a char string name of the current DVI Controller
+ * chip. It's convenient for application need to display the chip name.
*/
char *sii164GetChipString(void)
{
@@ -336,8 +344,9 @@ void sii164EnableHotPlugDetection(unsigned char
enableHotPlug)
detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT);
- /* Depending on each DVI controller, need to enable the hot plug based
on each
-* individual chip design.
+ /*
+* Depending on each DVI controller, need to enable the hot plug based
+* on each individual chip design.
*/
if (enableHotPlug != 0)
sii164SelectHotPlugDetectionMode(SII164_HOTPLUG_USE_MDI);
@@ -402,5 +411,3 @@ void sii164ClearInterrupt(void)
#endif
#endif
-
-
--
2.11.0
Re: [PATCH v5 10/11] intel_sgx: glue code for in-kernel LE
On Fri, Nov 17, 2017 at 03:07:05PM -0800, Darren Hart wrote: > No incremental cleanup here - appears to all be handled through > sgx_le_stop - do I have that right? Yes. This is correct. /Jarkko
Re: [GIT PULL] afs: Fixes
On Fri, Nov 24, 2017 at 4:22 AM, David Howells wrote: > > (2) Don't write to a page that's being written out, but wait for it to > complete. So I see in the commit message why afs needs to do this, but it's worth pointing out that it's (a) impossible to avoid the "inconsistent data" case for writable mmap'ed pages (b) can cause some really nasty latency issues So the "page->private" issue does mean that write-vs-writeback clearly needs that serialization (and that's not an issue for the mapped page being changed at the same time), but in general filesystem people should be aware that data under writeback may still be further dirtied while the writeback is active (and the page marked dirty again). It can obviously screw with things like checksums etc, and make for temporarily inconsistent state if the filesystem does those kinds of things. Linus
Re: [PATCH 2/2] staging: rtl8188eu: Fix private WEXT IOCTL calls
Hi, What was broken was private/device specific IOCTL calls implemented by this driver. The standard IOCTL calls worked and the driver worked as it was in client mode. But in AP mode with hostapd (https://w1.fi/hostapd/) the rtl871xdrv driver of hostapd (which is required for using devices that use this driver in AP mode) invokes private/device specific IOCTL calls and in the existing driver they could only be invoked through the ndo_do_ioctl interface of the driver. Support for which was removed in the mentioned commit by Johannes Berg. Hence the driver stopped working in AP mode with hostapd using rtl871xdrv driver. The solution was to implement equivalent versions of the existing private/device specific IOCTLs which will be invoked by the iw_handler_def interface. Cheers, Ishraq On 11/25/2017 05:40 AM, Dan Carpenter wrote: > I added Johannes to the CC list again because this is sort of his > fault... To be honest, I'm a little bit puzzled. I would have thought > Johannes's change would have made some code unused and that we could > delete it now. I haven't looked at this.
Re: [PATCH v1 03/10] v4l: platform: Add Renesas CEU driver
Hi Sakari! On Sat, Nov 25, 2017 at 05:56:14PM +0200, Sakari Ailus wrote: > On Fri, Nov 17, 2017 at 10:33:55AM +0100, jacopo mondi wrote: > > Hi Sakari! > > [snip] > > I would like to make sure we're all on the same page with this. My > > preference would be: > > > > 1) Have renesas-ceu.c driver merged with Migo-R ported to use this new > > driver as an 'example'. > > 2) Do not remove any of the existing soc_camera code at this point > > 3) Port all other 4 SH users of sh_mobile_ceu_camera to use the now > > merged renesas-ceu driver > > 4) Remove sh_mobile_ceu_camera and soc_camera sensor drivers whose > > only users were those 4 SH boards > > 5) Remove soc_camera completely. For my understanding there are some > > PXA platforms still using soc_camera provided utilities somewhere. > > Hans knows better, but we can discuss this once we'll get there. > > The first point here is basically done by this patchset and your intent > would be to proceed with the rest, right? Yep, you're right! > > The above seems good; what I wanted to say was that I'd like to avoid > ending up in a permanent situation where some hardware works with the new > driver and some will continue to use the old one. I hope that being the last users of soc_camera, there will be enough motivations to complete all the above points :) Thanks j > > -- > Kind regards, > > Sakari Ailus > e-mail: sakari.ai...@iki.fi
Re: [PATCH] x86/orc: Don't bail on stack overflow
On Sat, Nov 25, 2017 at 9:28 AM, Andy Lutomirski wrote: > If we overflow the stack into a guard page and then try to unwind > it with ORC, it should work perfectly: by construction, there can't > be any meaningful data in the guard page because no writes to the > guard page will have succeeded. > > ORC seems entirely capable of unwinding in this situation, except > that it doesn't even try. Adjust its initial stack check so that > it's willing to try unwinding. > > I tested this by intentionally overflowing the task stack. The > result is an accurate call trace instead of a trace consisting > purely of '?' entries. > > Signed-off-by: Andy Lutomirski > --- > > Hi all- > > Ingo, this would have fixed half the debugging problem you had, I think. > To really nail it, we'd want some kind of magic to annotate the trace > so that page_fault (and async_page_fault) entries show CR2 and error_code. > > Josh, any ideas of how to do that cleanly? We could easily hard-code it > in the OOPS unwinder, I guess. Actually, this does pretty well. We don't get CR2, but, when I added an intentional bug kind of along the lines of the one you debugged, the intermediate page fault successfully dumps all the regs in the stack trace, so we get the faulting instruction *and* the registers. We also get ORIG_RAX, which tells us the error code. We could be fancy and decode that.
[PATCH 2/2] staging: sm750b: Fix coding style issues in sm750_accel.c
This is a patch to sm750_accel.c that fixes 80-character line length
warnings found by checkpatch.pl. It also fixes some grammatical errors
in comments and moves parameter-specific comments from inline to before
the function.
Signed-off-by: Jeremy Lacomis
---
drivers/staging/sm750fb/sm750_accel.c | 184 ++
1 file changed, 98 insertions(+), 86 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_accel.c
b/drivers/staging/sm750fb/sm750_accel.c
index 1035e91e7cd3..085e5064188f 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+/* SPDX-License-Identifier: GPL-2.0 */
#include
#include
#include
@@ -68,11 +68,10 @@ void sm750_hw_de_init(struct lynx_accel *accel)
}
/*
- * set2dformat only be called from setmode functions
- * but if you need dual framebuffer driver,need call set2dformat
- * every time you use 2d function
+ * set2dformat can only be called from setmode functions, but if you need a
dual
+ * framebuffer driver, set2dformat must be called every time a 2D function is
+ * used
*/
-
void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt)
{
u32 reg;
@@ -94,7 +93,7 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
if (accel->de_wait() != 0) {
/*
-* int time wait and always busy,seems hardware
+* int time wait and always busy, seems hardware
* got something error
*/
pr_debug("De engine always busy\n");
@@ -130,80 +129,85 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
return 0;
}
-int sm750_hw_copyarea(
-struct lynx_accel *accel,
-unsigned int sBase, /* Address of source: offset in frame buffer */
-unsigned int sPitch, /* Pitch value of source surface in BYTE */
-unsigned int sx,
-unsigned int sy, /* Starting coordinate of source surface */
-unsigned int dBase, /* Address of destination: offset in frame buffer */
-unsigned int dPitch, /* Pitch value of destination surface in BYTE */
-unsigned int Bpp,/* Color depth of destination surface */
-unsigned int dx,
-unsigned int dy, /* Starting coordinate of destination surface */
-unsigned int width,
-unsigned int height, /* width and height of rectangle in pixel value */
-unsigned int rop2) /* ROP value */
+/*
+ * Notes on these parameters:
+ * sBase: Address of the source offset in the frame buffer
+ * sPitch: Pitch value of the source surface in BYTE
+ * sx, sy: Starting coordinate of the source surface
+ * dBase: Address of the destination offset in the frame buffer
+ * dPitch: Pitch value of the destination surface in BYTE
+ * Bpp: Color depth of the destination surface
+ * dx, dy: Starting coordinate of the destination surface
+ * width, height: Dimensions of the rectangle in pixels
+ * rop2: ROP value
+ */
+int sm750_hw_copyarea(struct lynx_accel *accel, unsigned int sBase,
+ unsigned int sPitch, unsigned int sx, unsigned int sy,
+ unsigned int dBase, unsigned int dPitch, unsigned int Bpp,
+ unsigned int dx, unsigned int dy, unsigned int width,
+ unsigned int height, unsigned int rop2)
{
unsigned int nDirection, de_ctrl;
nDirection = LEFT_TO_RIGHT;
- /* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left
*/
+ /*
+* Direction of ROP2 operation:
+* 1 = Left to Right
+* -1 = Right to Left
+*/
de_ctrl = 0;
- /* If source and destination are the same surface, need to check for
overlay cases */
+ /*
+* If the source and destination are the same surface, need to check for
+* overlay cases
+*/
if (sBase == dBase && sPitch == dPitch) {
/* Determine direction of operation */
- if (sy < dy) {
- /* +--+
-* |S |
-* | +--+
-* | | | |
-* | | | |
-* +---|--+ |
-* | D|
-* +--+
-*/
+ /* +--+
+* |S |
+* | +--+
+* | | | |
+* | | | |
+* +---|--+ |
+* | D|
+* +--+
+*/
+ if (sy < dy) {
nDirection = BOTTOM_TO_TOP;
- } else if (sy > dy) {
- /* +--+
-* |D |
-* | +--+
-* | | | |
-
[PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h
This patch to ddk750_sii164.h fixes line length warnings found by the
checkpatch.pl script and reformats comments uniformly.
Signed-off-by: Jeremy Lacomis
---
drivers/staging/sm750fb/ddk750_sii164.h | 57 +
1 file changed, 22 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_sii164.h
b/drivers/staging/sm750fb/ddk750_sii164.h
index 2e9a88cd6af3..393a3c5be3ae 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ b/drivers/staging/sm750fb/ddk750_sii164.h
@@ -4,15 +4,20 @@
#define USE_DVICHIP
-/* Hot Plug detection mode structure */
+/*
+ * Hot Plug detection mode structure:
+ * Disable Hot Plug output bit (always high).
+ * Use Monitor Detect Interrupt bit.
+ * Use Receiver Sense detect bit.
+ * Use Hot Plug detect bit.
+ */
enum sii164_hot_plug_mode {
- SII164_HOTPLUG_DISABLE = 0, /* Disable Hot Plug output bit
(always high). */
- SII164_HOTPLUG_USE_MDI, /* Use Monitor Detect Interrupt
bit. */
- SII164_HOTPLUG_USE_RSEN,/* Use Receiver Sense detect bit. */
- SII164_HOTPLUG_USE_HTPLG/* Use Hot Plug detect bit. */
+ SII164_HOTPLUG_DISABLE = 0,
+ SII164_HOTPLUG_USE_MDI,
+ SII164_HOTPLUG_USE_RSEN,
+ SII164_HOTPLUG_USE_HTPLG
};
-
/* Silicon Image SiI164 chip prototype */
long sii164InitChip(unsigned char edgeSelect,
unsigned char busSelect,
@@ -28,7 +33,6 @@ long sii164InitChip(unsigned char edgeSelect,
unsigned short sii164GetVendorID(void);
unsigned short sii164GetDeviceID(void);
-
#ifdef SII164_FULL_FUNCTIONS
void sii164ResetChip(void);
char *sii164GetChipString(void);
@@ -39,35 +43,26 @@ unsigned char sii164CheckInterrupt(void);
void sii164ClearInterrupt(void);
#endif
/*
- * below register definition is used for
+ * The below register definition is used for the
* Silicon Image SiI164 DVI controller chip
*/
-/*
- * Vendor ID registers
- */
+
+/* Vendor ID registers */
#define SII164_VENDOR_ID_LOW0x00
#define SII164_VENDOR_ID_HIGH 0x01
-/*
- * Device ID registers
- */
+/* Device ID registers */
#define SII164_DEVICE_ID_LOW0x02
#define SII164_DEVICE_ID_HIGH 0x03
-/*
- * Device Revision
- */
+/* Device Revision */
#define SII164_DEVICE_REVISION 0x04
-/*
- * Frequency Limitation registers
- */
+/* Frequency Limitation registers */
#define SII164_FREQUENCY_LIMIT_LOW 0x06
#define SII164_FREQUENCY_LIMIT_HIGH 0x07
-/*
- * Power Down and Input Signal Configuration registers
- */
+/* Power Down and Input Signal Configuration registers */
#define SII164_CONFIGURATION0x08
/* Power down (PD) */
@@ -95,9 +90,7 @@ void sii164ClearInterrupt(void);
#define SII164_CONFIGURATION_VSYNC_FORCE_LOW0x00
#define SII164_CONFIGURATION_VSYNC_AS_IS0x20
-/*
- * Detection registers
- */
+/* Detection registers */
#define SII164_DETECT 0x09
/* Monitor Detect Interrupt (MDI) */
@@ -127,9 +120,7 @@ void sii164ClearInterrupt(void);
#define SII164_DETECT_MONITOR_SENSE_OUTPUT_HTPLG0x30
#define SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG 0x30
-/*
- * Skewing registers
- */
+/* Skewing registers */
#define SII164_DESKEW 0x0A
/* General Purpose Input (CTL[3:1]) */
@@ -149,14 +140,10 @@ void sii164ClearInterrupt(void);
#define SII164_DESKEW_7_STEP0xC0
#define SII164_DESKEW_8_STEP0xE0
-/*
- * User Configuration Data registers (CFG 7:0)
- */
+/* User Configuration Data registers (CFG 7:0) */
#define SII164_USER_CONFIGURATION 0x0B
-/*
- * PLL registers
- */
+/* PLL registers */
#define SII164_PLL 0x0C
/* PLL Filter Value (PLLF) */
--
2.11.0
[PATCH v3] staging: rtl8188eu: Fix private WEXT IOCTL calls
From: Ishraq Ibne Ashraf
Commit 8bfb36766064 ("wireless: wext: remove ndo_do_ioctl fallback") breaks
private WEXT
IOCTL calls of this driver as these are not invoked through ndo_do_ioctl
interface anymore. As a result hostapd stops working with this driver. In
this patch this problem is solved by implementing equivalent private IOCTL
functions of the existing ones which are accessed via iw_handler_def
interface.
Signed-off-by: Ishraq Ibne Ashraf
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 1077
1 file changed, 1077 insertions(+)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index c0664dc..8d99f99 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -3061,6 +3061,1081 @@ static iw_handler rtw_handlers[] = {
NULL, /*---hole---*/
};
+static int get_private_handler_ieee_param(struct adapter *padapter,
+ union iwreq_data *wrqu,
+ void *param)
+{
+ /*
+* This function is expected to be called in master mode, which allows
no
+* power saving. So we just check hw_init_completed.
+*/
+ if (!padapter->hw_init_completed)
+ return -EPERM;
+
+ if (copy_from_user(param, wrqu->data.pointer, wrqu->data.length))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int rtw_hostapd_sta_flush_pvt(struct net_device *dev,
+struct iw_request_info *info,
+union iwreq_data *wrqu,
+char *extra)
+{
+ struct adapter *padapter = (struct adapter *)rtw_netdev_priv(dev);
+ flush_all_cam_entry(padapter); /* Clear CAM. */
+ return rtw_sta_flush(padapter);
+}
+
+static int rtw_add_sta_pvt(struct net_device *dev,
+ struct iw_request_info *info,
+ union iwreq_data *wrqu,
+ char *extra)
+{
+ int ret;
+ int flags;
+ struct sta_info *psta;
+ struct ieee_param *param;
+ struct adapter *padapter = (struct adapter *)rtw_netdev_priv(dev);
+ struct sta_priv *pstapriv = &padapter->stapriv;
+ struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
+
+ param = (struct ieee_param *)kmalloc(wrqu->data.length, GFP_KERNEL);
+ if (!param)
+ return -ENOMEM;
+
+ ret = get_private_handler_ieee_param(padapter, wrqu, param);
+ if (ret)
+ goto err_free_param;
+
+ DBG_88E("rtw_add_sta(aid =%d) =%pM\n",
+ param->u.add_sta.aid,
+ (param->sta_addr));
+
+ if (!check_fwstate(pmlmepriv, (_FW_LINKED|WIFI_AP_STATE))) {
+ ret = -EINVAL;
+ goto err_free_param;
+ }
+
+ if (param->sta_addr[0] == 0xff && param->sta_addr[1] == 0xff &&
+ param->sta_addr[2] == 0xff && param->sta_addr[3] == 0xff &&
+ param->sta_addr[4] == 0xff && param->sta_addr[5] == 0xff) {
+ ret = -EINVAL;
+ goto err_free_param;
+ }
+
+ psta = rtw_get_stainfo(pstapriv, param->sta_addr);
+ if (!psta) {
+ ret = -ENOMEM;
+ goto err_free_param;
+ }
+
+ flags = param->u.add_sta.flags;
+ psta->aid = param->u.add_sta.aid; /* aid = 1~2007. */
+
+ memcpy(psta->bssrateset, param->u.add_sta.tx_supp_rates, 16);
+
+ /* Check WMM cap. */
+ if (WLAN_STA_WME&flags)
+ psta->qos_option = 1;
+ else
+ psta->qos_option = 0;
+
+ if (pmlmepriv->qospriv.qos_option == 0)
+ psta->qos_option = 0;
+
+ /* Check 802.11n HT cap. */
+ if (WLAN_STA_HT&flags) {
+ psta->htpriv.ht_option = true;
+ psta->qos_option = 1;
+ memcpy(&psta->htpriv.ht_cap,
+ ¶m->u.add_sta.ht_cap,
+ sizeof(struct ieee80211_ht_cap));
+ } else {
+ psta->htpriv.ht_option = false;
+ }
+
+ if (pmlmepriv->htpriv.ht_option == false)
+ psta->htpriv.ht_option = false;
+
+ update_sta_info_apmode(padapter, psta);
+
+ if (copy_to_user(wrqu->data.pointer, param, wrqu->data.length)) {
+ ret = -EFAULT;
+ goto err_free_param;
+ }
+
+ kfree(param);
+ return 0;
+
+err_free_param:
+ kfree(param);
+ return ret;
+}
+
+static int rtw_del_sta_pvt(struct net_device *dev,
+ struct iw_request_info *info,
+ union iwreq_data *wrqu,
+ char *extra)
+{
+ int ret;
+ int updated;
+ struct sta_info *psta;
+ struct ieee_param *param;
+ struct adapter *padapter = (struct adapter *)rtw_netdev_priv(
[PATCH] scsi: libfc: fix ELS request handling
The modification of fc_lport_recv_els_req() in commit fcabb09e59a7 (merged in 4.12-rc1) caused certain requests not to be handled at all. Fix that. Fixes: fcabb09e59a7 "scsi: libfc: directly call ELS request handlers" Signed-off-by: Martin Wilck --- drivers/scsi/libfc/fc_lport.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index 2fd0ec651170..787e82435241 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -904,10 +904,14 @@ static void fc_lport_recv_els_req(struct fc_lport *lport, case ELS_FLOGI: if (!lport->point_to_multipoint) fc_lport_recv_flogi_req(lport, fp); + else + fc_rport_recv_req(lport, fp); break; case ELS_LOGO: if (fc_frame_sid(fp) == FC_FID_FLOGI) fc_lport_recv_logo_req(lport, fp); + else + fc_rport_recv_req(lport, fp); break; case ELS_RSCN: lport->tt.disc_recv_req(lport, fp); -- 2.15.0
Re: [PATCH 2/2] hwmon: (ina2xx) Make calibration register value fixed
On 11/22/2017 07:32 AM, Maciej Purski wrote:
Calibration register is used for calculating current register in
hardware according to datasheet:
current = shunt_volt * calib_register / 2048 (ina 226)
current = shunt_volt * calib_register / 4096 (ina 219)
Fix calib_register value to 2048 for ina226 and 4096 for ina 219 in
order to avoid truncation error and provide best precision allowed
by shunt_voltage measurement. Make current scale value follow changes
of shunt_resistor from sysfs as calib_register value is now fixed.
Power_lsb value should also follow shunt_resistor changes as stated in
datasheet:
power_lsb = 25 * current_lsb (ina 226)
power_lsb = 20 * current_lsb (ina 219)
Signed-off-by: Maciej Purski
Setting the calibration register to a specific value may optimize precision,
but limits the supported value range, which is the whole point of providing
a calibration register. What am I missing here ?
Guenter
---
drivers/hwmon/ina2xx.c | 87 +-
1 file changed, 50 insertions(+), 37 deletions(-)
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 62e38fa..e362a93 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -95,18 +95,20 @@ enum ina2xx_ids { ina219, ina226 };
struct ina2xx_config {
u16 config_default;
- int calibration_factor;
+ int calibration_value;
int registers;
int shunt_div;
int bus_voltage_shift;
int bus_voltage_lsb;/* uV */
- int power_lsb; /* uW */
+ int power_lsb_factor;
};
struct ina2xx_data {
const struct ina2xx_config *config;
long rshunt;
+ long current_lsb_uA;
+ long power_lsb_uW;
struct mutex config_lock;
struct regmap *regmap;
@@ -116,21 +118,21 @@ struct ina2xx_data {
static const struct ina2xx_config ina2xx_config[] = {
[ina219] = {
.config_default = INA219_CONFIG_DEFAULT,
- .calibration_factor = 4096,
+ .calibration_value = 4096,
.registers = INA219_REGISTERS,
.shunt_div = 100,
.bus_voltage_shift = 3,
.bus_voltage_lsb = 4000,
- .power_lsb = 2,
+ .power_lsb_factor = 20,
},
[ina226] = {
.config_default = INA226_CONFIG_DEFAULT,
- .calibration_factor = 512,
+ .calibration_value = 2048,
.registers = INA226_REGISTERS,
.shunt_div = 400,
.bus_voltage_shift = 0,
.bus_voltage_lsb = 1250,
- .power_lsb = 25000,
+ .power_lsb_factor = 25,
},
};
@@ -169,12 +171,16 @@ static u16 ina226_interval_to_reg(int interval)
return INA226_SHIFT_AVG(avg_bits);
}
+/*
+ * Calibration register is set to the best value, which eliminates
+ * truncation errors on calculating current register in hardware.
+ * According to datasheet (eq. 3) the best values are 2048 for
+ * ina226 and 4096 for ina219. They are hardcoded as calibration_value.
+ */
static int ina2xx_calibrate(struct ina2xx_data *data)
{
- u16 val = DIV_ROUND_CLOSEST(data->config->calibration_factor,
- data->rshunt);
-
- return regmap_write(data->regmap, INA2XX_CALIBRATION, val);
+ return regmap_write(data->regmap, INA2XX_CALIBRATION,
+ data->config->calibration_value);
}
/*
@@ -187,10 +193,6 @@ static int ina2xx_init(struct ina2xx_data *data)
if (ret < 0)
return ret;
- /*
-* Set current LSB to 1mA, shunt is in uOhms
-* (equation 13 in datasheet).
-*/
return ina2xx_calibrate(data);
}
@@ -268,15 +270,15 @@ static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
val = DIV_ROUND_CLOSEST(val, 1000);
break;
case INA2XX_POWER:
- val = regval * data->config->power_lsb;
+ val = regval * data->power_lsb_uW;
break;
case INA2XX_CURRENT:
- /* signed register, LSB=1mA (selected), in mA */
- val = (s16)regval;
+ /* signed register, result in mA */
+ val = regval * data->current_lsb_uA;
+ val = DIV_ROUND_CLOSEST(val, 1000);
break;
case INA2XX_CALIBRATION:
- val = DIV_ROUND_CLOSEST(data->config->calibration_factor,
- regval);
+ val = regval;
break;
default:
/* programmer goofed */
@@ -304,9 +306,32 @@ static ssize_t ina2xx_show_value(struct device *dev,
ina2xx_get_value(data, attr->index, regval));
}
-static ssize_t ina2xx_set_shunt(struct device *dev,
- struct device_attribute *da,
-
Re: [patch V4 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses
On Fri 2017-11-17 15:06:39, Mauro Carvalho Chehab wrote: > Hi Thomas, > > Em Fri, 17 Nov 2017 11:00:33 +0100 (CET) > Thomas Gleixner escreveu: > > > Subject: Documentation: Add license-rules.rst to describe how to properly > > identify file licenses > > From: Thomas Gleixner > > Date: Fri, 10 Nov 2017 09:30:00 +0100 > > > > Add a file to the Documentation directory to describe how file licenses > > should be described in all kernel files, using the SPDX identifier, as well > > as where all licenses should be in the kernel source tree for people to > > refer to (LICENSES/). > > > > Thanks to Kate, Greg and Jonathan for review and editing and Jonas for the > > suggestions concerning the meta tags in the licenses files. > > > > Signed-off-by: Thomas Gleixner > > The document itself looks good, but I think it should also mention > what would be the expected values for the MODULE_LICENSE() macro and > how each license would be mapped into it. > > Right now, include/linux/module.h says: > > /* > * The following license idents are currently accepted as indicating free > * software modules > * > *"GPL" [GNU Public License v2 or later] Hmm. AFAICT Greg translated GPL as GPL v1 or later. That seemed wrong... and now it seems even more wrong. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
hi3521a syscon-reboot issue, reboot fails oddly.
Greetings,
Having a slight issue with getting reboot to work on the board I'm tinkering
with; according to the documentation writing any value to 0x12050004 should
reset the system, as such I have the following snippet in my dts to make it
work:
sysctrl: system-controller@1205 {
compatible = "hisilicon,hi3521a-sysctrl", "syscon";
reg = <0x1205 0x1000>;
#clock-cells = <1>;
#reset-cells = <2>;
};
reboot {
compatible = "syscon-reboot";
regmap = <&sysctrl>;
offset = <0x4>;
mask = <0xdeadbeef>;
};
The above is primarily based on the hi3519.dtsi setup, whose docs indicate
it resets in the same manner.
However, issuing a `reboot' ends up putting the cpu into a wierd state, with
the serial port getting spammed with '0x0a2020202020...0a20202020...' in what
appears to be an infinite amount of time (let it sit in this state overnight,
remaining in this state.
It doesn't make much sense to me, as I can't find anything in the bsp regarding
reset excluding a small bit in linux-3.10.y/arch/arm/mach-hi3521a/core.c:
void hi3521a_restart(char mode, const char *cmd)
{
writel(~0, __io_address(REG_BASE_SCTL + REG_SC_SYSRES));
}
which effectively does the same thing as syscon-reboot.
This happens whether I reboot via command or by killing the process feeding the
watchdog (procd on lede's initramfs).
Was hoping you guys could perhaps provide some guidance regarding this matter.
Regards,
Marty
--
2.15.0
Re: [patch V4 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses
On Wed 2017-11-22 14:48:04, Greg Kroah-Hartman wrote: > On Wed, Nov 22, 2017 at 09:51:17AM -0200, Mauro Carvalho Chehab wrote: > > Em Wed, 22 Nov 2017 12:12:04 +0100 (CET) > > Thomas Gleixner escreveu: > > > > > On Fri, 17 Nov 2017, Christoph Hellwig wrote: > > > > On Fri, Nov 17, 2017 at 07:11:41PM +0100, Thomas Gleixner wrote: > > > > > Introcude a MODULE_LICENSE_SPDX macro which flags the module info > > > > > storage > > > > > as 'SPDXIFY' and let the postprocessor do: > > > > > > > > Shouldn;t this be a FILE_LICENSE_SPDX? I'd also much prefer that over > > > > the nasty C99 comments to start with. And while I'm a bit behind on > > > > email I still haven't managed to find a good rationale for those to > > > > start with. > > > > Yeah, I also find nasty to have things like this on each C file: > > > > // SPDX-License-Identifier: GPL-2.0 > > /* > > * Copyright ... > > * ... > > */ > > > > Also, one may forget that headers use /**/ and end by doing the wrong > > thing, as a common practice is to just cut-and-paste the same copyright > > header on both C and H files at development time. > > You break the build when you get it wrong, so you will notice it. For > most "internal" .h files, using // is just fine. > > Yes, it's "ugly", but again, that's what Linus said he wanted it to look > like, take it up with him :) Linus said: # So in general, the _hope_ is that we can just end up replacing # existing boilerplate comments with that single line SPDX comment # (using "//" in *.[ch] files, but obviously some other kinds of files # end up having a different comment character, typically '#'). That does not sound like he was deciding between /* */ and //. And actually this was in context of files with no existing license. You made the ugly patches. Stop hiding behind Linus. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [patch V4 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses
On Sat, Nov 25, 2017 at 9:04 AM, Pavel Machek wrote: > > That does not sound like he was deciding between /* */ and //. And > actually this was in context of files with no existing license. You > made the ugly patches. Stop hiding behind Linus. No, Linus happily stands up for //. I really don't like one-liner /* */ comments. Let's face it, // was the _one_ thing C++ got right, among all the horrible bad decisions. I think we should just start moving to // in general, but only when adding new ones (ie I don't want to see any automatic conversion patches). And it's _particularly_ true for the whole case where we mix one-liners like the SPDX line with block comments. That's where /* */ really sucks. Linus
Re: WTF? Re: [PATCH] License cleanup: add SPDX GPL-2.0 license identifier to files with no license
Hi!
> > This would be even better:
> >
> > /*
> > * Driver for SMSC USB3503 USB 2.0 hub controller driver
> > *
> > * Copyright (c) 2012-2013 Dongjin Kim (tobet...@gmail.com)
> > */
> > ...
> > SPDX_MODULE_LICENSE("GPL-2.0+")
> >
> > So yes, SPDX can be improvement. But in current implementation it is
> > not.
>
> Again, as people seem to keep still missing this point, Linus asked for
> the format to look like it does today, using // at the top. Thomas and
> I originally did it first the way with the SPDX line in the big comment
> block.
>
> If you don't like the format, complain and convince him otherwise, you
> are not getting anywhere by responding to this old topic about it
> again.
Hey, Linus. This // SPDX at the begining of file looks really
ugly. Can we get something that looks less bad?
And BTW I responded to this uglyness before, but you just tried to
make me shut up, and then did not reply. Given what quality you
normally expect from patch submitters, you are doing pretty poor job
here.
> Having it be the first line of the file is good, it's obvious, and
> stands out, which is the point, you want it to, it's a license :)
What is good about that? License is about the least interesting thing
about the file. Point of SPDX conversion (see the mail I was replying
to?) was to make license information _less_ intrusive, not more. Tools
can find SPDX anywhere in the file for the people that really care.
That's how it works in U-Boot, which people are using as example of
reasonable SPDX conversion:
/*
* Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
* Copyright (c) 2010-2011 NVIDIA Corporation
* NVIDIA Corporation
*
* SPDX-License-Identifier: GPL-2.0+
*/
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
[PATCH] net: openvswitch: datapath: fix data type in queue_gso_packets
gso_type is being used in binary AND operations together with SKB_GSO_UDP.
The issue is that variable gso_type is of type unsigned short and
SKB_GSO_UDP expands to more than 16 bits:
SKB_GSO_UDP = 1 << 16
this makes any binary AND operation between gso_type and SKB_GSO_UDP to
be always zero, hence making some code unreachable and likely causing
undesired behavior.
Fix this by changing the data type of variable gso_type to unsigned int.
Addresses-Coverity-ID: 1462223
Fixes: 0c19f846d582 ("net: accept UFO datagrams from tuntap and packet")
Signed-off-by: Gustavo A. R. Silva
---
net/openvswitch/datapath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 99cfafc..ef38e5a 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -308,7 +308,7 @@ static int queue_gso_packets(struct datapath *dp, struct
sk_buff *skb,
const struct dp_upcall_info *upcall_info,
uint32_t cutlen)
{
- unsigned short gso_type = skb_shinfo(skb)->gso_type;
+ unsigned int gso_type = skb_shinfo(skb)->gso_type;
struct sw_flow_key later_key;
struct sk_buff *segs, *nskb;
int err;
--
2.7.4
Re: [patch V4 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses
On Sat 2017-11-25 09:11:58, Linus Torvalds wrote: > On Sat, Nov 25, 2017 at 9:04 AM, Pavel Machek wrote: > > > > That does not sound like he was deciding between /* */ and //. And > > actually this was in context of files with no existing license. You > > made the ugly patches. Stop hiding behind Linus. > > No, Linus happily stands up for //. > > I really don't like one-liner /* */ comments. Let's face it, // was > the _one_ thing C++ got right, among all the horrible bad decisions. > > I think we should just start moving to // in general, but only when > adding new ones (ie I don't want to see any automatic conversion > patches). > > And it's _particularly_ true for the whole case where we mix > one-liners like the SPDX line with block comments. That's where /* */ > really sucks. I agree that we should not do /* SPDX-License-Identifier: GPL-2.0+ */ /* * Driver for SMSC USB3503 USB 2.0 hub controller driver * * Copyright (c) 2012-2013 Dongjin Kim (tobet...@gmail.com) */ There's logical place in the comment, and it should look like this: /* * Driver for SMSC USB3503 USB 2.0 hub controller driver * * SPDX-License-Identifier: GPL-2.0+ * Copyright (c) 2012-2013 Dongjin Kim (tobet...@gmail.com) */ So I'm not saying "use one-line comments with SPDX identifier". I'm saying place it where people expect it. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH 42/43] x86/mm/kaiser: Allow KAISER to be enabled/disabled at runtime
On Fri, 24 Nov 2017, Ingo Molnar wrote: > From: Dave Hansen > > The KAISER CR3 switches are expensive for many reasons. Not all systems > benefit from the protection provided by KAISER. Some of them can not > pay the high performance cost. > > This patch adds a debugfs file. To disable KAISER, you do: > > echo 0 > /sys/kernel/debug/x86/kaiser-enabled > > and to re-enable it, you can: > > echo 1 > /sys/kernel/debug/x86/kaiser-enabled > > This is a *minimal* implementation. There are certainly plenty of > optimizations that can be done on top of this by using ALTERNATIVES > among other things. It's not only minimal. It's naive and broken. That thing explodes when toggled in the wrong moment. I did not even attempt to debug that, because I think the approach is wrong. If you really want to make it runtime switchable, then: - the shadow tables need to be updated unconditionally. I did not check whether thats done right now, but explosions are simpler to achieve when switching it back on. Though switching it off crashes as well. - you need to make sure that no task is in user space or on the way to it. The much I hate stop_machine(), that's probably the right tool. Once everything is in stomp_machine() the switch can be flipped. - the poisoning/unpoisoning of the kernel tables does not need to be done from stop_machine(). That can be done from regular context with a TIF flag, so you can make sure that every task is up to date before returning to user space. Though that needs a lot of thought. For now I really want to see that removed entirely and replaced by a simple boot time switch. We can use the global variable for now and optimize it later on. Thanks, tglx
Re: [PATCH v2] VSOCK: Don't call vsock_stream_has_data in atomic context
From: Jorgen Hansen Date: Fri, 24 Nov 2017 06:25:28 -0800 > When using the host personality, VMCI will grab a mutex for any > queue pair access. In the detach callback for the vmci vsock > transport, we call vsock_stream_has_data while holding a spinlock, > and vsock_stream_has_data will access a queue pair. > > To avoid this, we can simply omit calling vsock_stream_has_data > for host side queue pairs, since the QPs are empty per default > when the guest has detached. > > This bug affects users of VMware Workstation using kernel version > 4.4 and later. > > Testing: Ran vsock tests between guest and host, and verified that > with this change, the host isn't calling vsock_stream_has_data > during detach. Ran mixedTest between guest and host using both > guest and host as server. > > v2: Rebased on top of recent change to sk_state values > Reviewed-by: Adit Ranadive > Reviewed-by: Aditya Sarwade > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Jorgen Hansen Applied, thank you.
Re: [PATCH net] net: dsa: fix 'increment on 0' warning
From: Vivien Didelot
Date: Fri, 24 Nov 2017 11:36:06 -0500
> Setting the refcount to 0 when allocating a tree to match the number of
> switch devices it holds may cause an 'increment on 0; use-after-free',
> if CONFIG_REFCOUNT_FULL is enabled.
>
> To fix this, do not decrement the refcount of a newly allocated tree,
> increment it when an already allocated tree is found, and decrement it
> after the probing of a switch, as done with the previous behavior.
>
> At the same time, make dsa_tree_get and dsa_tree_put accept a NULL
> argument to simplify callers, and return the tree after incrementation,
> as most kref users like of_node_get and of_node_put do.
>
> Fixes: 8e5bf9759a06 ("net: dsa: simplify tree reference counting")
> Signed-off-by: Vivien Didelot
Applied, thanks Vivien.
lib/zstd/decompress.c:416:2: warning: argument 1 null where non-null expected
Hi Nick, First bad commit (maybe != root cause): tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 844056fd74ebdd826bd23a7d989597e15f478acb commit: 5c1aab1dd5445ed8bdcdbb575abc1b0d7ee5b2e7 btrfs: Add zstd support date: 3 months ago config: x86_64-randconfig-h0-11260236 (attached as .config) compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 reproduce: git checkout 5c1aab1dd5445ed8bdcdbb575abc1b0d7ee5b2e7 # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): Cyclomatic Complexity 1 lib/zstd/error_private.h:ERR_isError Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le64_to_cpup Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le32_to_cpup Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le16_to_cpup Cyclomatic Complexity 1 include/linux/unaligned/access_ok.h:get_unaligned_le16 Cyclomatic Complexity 1 include/linux/unaligned/access_ok.h:get_unaligned_le32 Cyclomatic Complexity 1 include/linux/unaligned/access_ok.h:get_unaligned_le64 Cyclomatic Complexity 1 lib/zstd/mem.h:ZSTD_32bits Cyclomatic Complexity 1 lib/zstd/mem.h:ZSTD_readLE16 Cyclomatic Complexity 1 lib/zstd/mem.h:ZSTD_readLE24 Cyclomatic Complexity 1 lib/zstd/mem.h:ZSTD_readLE32 Cyclomatic Complexity 1 lib/zstd/mem.h:ZSTD_readLE64 Cyclomatic Complexity 2 lib/zstd/mem.h:ZSTD_readLEST Cyclomatic Complexity 1 lib/zstd/bitstream.h:BIT_lookBits Cyclomatic Complexity 1 lib/zstd/bitstream.h:BIT_lookBitsFast Cyclomatic Complexity 1 lib/zstd/bitstream.h:BIT_skipBits Cyclomatic Complexity 1 lib/zstd/bitstream.h:BIT_readBits Cyclomatic Complexity 1 lib/zstd/bitstream.h:BIT_readBitsFast Cyclomatic Complexity 1 lib/zstd/fse.h:FSE_peekSymbol Cyclomatic Complexity 1 lib/zstd/fse.h:FSE_updateState Cyclomatic Complexity 1 lib/zstd/zstd_internal.h:ZSTD_copy8 Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_copy4 Cyclomatic Complexity 31 lib/zstd/decompress.c:ZSTD_execSequence Cyclomatic Complexity 32 lib/zstd/decompress.c:ZSTD_decodeSequenceLong_generic Cyclomatic Complexity 31 lib/zstd/decompress.c:ZSTD_execSequenceLong Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_refDictContent Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_DDictDictContent Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_DDictDictSize Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_limitCopy Cyclomatic Complexity 4 lib/zstd/decompress.c:ZSTD_frameHeaderSize Cyclomatic Complexity 3 lib/zstd/decompress.c:ZSTD_checkContinuity Cyclomatic Complexity 11 lib/zstd/bitstream.h:BIT_reloadDStream Cyclomatic Complexity 1 lib/zstd/fse.h:FSE_initDState Cyclomatic Complexity 4 lib/zstd/zstd_internal.h:ZSTD_wildcopy Cyclomatic Complexity 20 lib/zstd/decompress.c:ZSTD_decodeSequence Cyclomatic Complexity 3 lib/zstd/decompress.c:ZSTD_copyRawBlock Cyclomatic Complexity 5 lib/zstd/decompress.c:ZSTD_setRleBlock Cyclomatic Complexity 15 lib/zstd/decompress.c:ZSTD_buildSeqTable Cyclomatic Complexity 1 lib/zstd/bitstream.h:BIT_highbit32 Cyclomatic Complexity 17 lib/zstd/bitstream.h:BIT_initDStream Cyclomatic Complexity 1 lib/zstd/zstd_internal.h:ZSTD_highbit32 Cyclomatic Complexity 3 lib/zstd/decompress.c:ZSTD_decodeSequenceLong Cyclomatic Complexity 17 lib/zstd/decompress.c:ZSTD_execSequenceLast7 Cyclomatic Complexity 27 lib/zstd/decompress.c:ZSTD_loadEntropy Cyclomatic Complexity 6 lib/zstd/decompress.c:ZSTD_decompress_insertDictionary Cyclomatic Complexity 6 lib/zstd/decompress.c:ZSTD_loadEntropy_inDDict Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_DCtxWorkspaceBound Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_decompressBegin Cyclomatic Complexity 5 lib/zstd/decompress.c:ZSTD_refDDict Cyclomatic Complexity 11 lib/zstd/decompress.c:ZSTD_createDCtx_advanced Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_initDCtx Cyclomatic Complexity 3 lib/zstd/decompress.c:ZSTD_freeDCtx Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_copyDCtx Cyclomatic Complexity 7 lib/zstd/decompress.c:ZSTD_isFrame Cyclomatic Complexity 27 lib/zstd/decompress.c:ZSTD_getFrameParams Cyclomatic Complexity 14 lib/zstd/decompress.c:ZSTD_decodeFrameHeader Cyclomatic Complexity 6 lib/zstd/decompress.c:ZSTD_getFrameContentSize Cyclomatic Complexity 7 lib/zstd/decompress.c:ZSTD_getcBlockSize Cyclomatic Complexity 35 lib/zstd/decompress.c:ZSTD_decodeLiteralsBlock Cyclomatic Complexity 18 lib/zstd/decompress.c:ZSTD_decodeSeqHeaders Cyclomatic Complexity 20 lib/zstd/decompress.c:ZSTD_decompressSequencesLong Cyclomatic Complexity 14 lib/zstd/decompress.c:ZSTD_decompressSequences Cyclomatic Complexity 6 lib/zstd/decompress.c:ZSTD_decompressBlock_internal Cyclomatic Complexity 1 lib/zstd/decompress.c:ZSTD_decompressBlock Cyclomatic Complexity 1
[PATCH v6 05/11] x86: add SGX MSRs to msr-index.h
From: Haim Cohen These MSRs hold the SHA256 checksum of the currently configured root key for enclave signatures. Signed-off-by: Haim Cohen Signed-off-by: Jarkko Sakkinen --- arch/x86/include/asm/msr-index.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index b35cb98b5d60..22e27d46d046 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -436,6 +436,7 @@ #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX (1<<1) #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX (1<<2) #define FEATURE_CONTROL_SGX_ENABLE (1<<18) +#define FEATURE_CONTROL_SGX_LAUNCH_CONTROL_ENABLE (1<<17) #define FEATURE_CONTROL_LMCE (1<<20) #define MSR_IA32_APICBASE 0x001b @@ -502,6 +503,12 @@ #define PACKAGE_THERM_INT_LOW_ENABLE (1 << 1) #define PACKAGE_THERM_INT_PLN_ENABLE (1 << 24) +/* Intel SGX MSRs */ +#define MSR_IA32_SGXLEPUBKEYHASH0 0x008C +#define MSR_IA32_SGXLEPUBKEYHASH1 0x008D +#define MSR_IA32_SGXLEPUBKEYHASH2 0x008E +#define MSR_IA32_SGXLEPUBKEYHASH3 0x008F + /* Thermal Thresholds Support */ #define THERM_INT_THRESHOLD0_ENABLE(1 << 15) #define THERM_SHIFT_THRESHOLD08 -- 2.14.1
[PATCH v6 02/11] x86: add SGX definition to cpufeature
From: Kai Huang
Added X86_FEATURE_SGX, which identifies that CPU supports software guard
extensions (SGX).
Signed-off-by: Kai Huang
Signed-off-by: Jarkko Sakkinen
---
arch/x86/include/asm/cpufeatures.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/include/asm/cpufeatures.h
b/arch/x86/include/asm/cpufeatures.h
index 2519c6c801c9..31a7d1c0f204 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -219,6 +219,7 @@
/* Intel-defined CPU features, CPUID level 0x0007:0 (ebx), word 9 */
#define X86_FEATURE_FSGSBASE ( 9*32+ 0) /* {RD/WR}{FS/GS}BASE instructions*/
#define X86_FEATURE_TSC_ADJUST ( 9*32+ 1) /* TSC adjustment MSR 0x3b */
+#define X86_FEATURE_SGX( 9*32+ 2) /* Software Guard Extensions
*/
#define X86_FEATURE_BMI1 ( 9*32+ 3) /* 1st group bit manipulation
extensions */
#define X86_FEATURE_HLE( 9*32+ 4) /* Hardware Lock Elision */
#define X86_FEATURE_AVX2 ( 9*32+ 5) /* AVX2 instructions */
--
2.14.1
[PATCH v6 01/11] intel_sgx: updated MAINTAINERS
Signed-off-by: Jarkko Sakkinen --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 2d3d750b19c0..30a4b7f97a93 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14932,6 +14932,11 @@ L: linux...@kvack.org S: Maintained F: mm/zswap.c +INTEL SGX +M: Jarkko Sakkinen +L: intel-sgx-kernel-...@lists.01.org +Q: https://patchwork.kernel.org/project/intel-sgx/list/ + THE REST M: Linus Torvalds L: linux-kernel@vger.kernel.org -- 2.14.1
