* Re: BUG: KASAN: stack-out-of-bounds
From: Andrey Ryabinin @ 2019-02-27 9:19 UTC (permalink / raw)
To: Christophe Leroy, Alexander Potapenko, Dmitry Vyukov
Cc: linux-mm, linuxppc-dev, kasan-dev, Daniel Axtens
In-Reply-To: <c6d80735-0cfe-b4ab-0349-673fc65b2e15@c-s.fr>
On 2/27/19 11:25 AM, Christophe Leroy wrote:
> With version v8 of the series implementing KASAN on 32 bits powerpc (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm now able to activate KASAN on a mac99 is QEMU.
>
> Then I get the following reports at startup. Which of the two reports I get seems to depend on the option used to build the kernel, but for a given kernel I always get the same report.
>
> Is that a real bug, in which case how could I spot it ? Or is it something wrong in my implementation of KASAN ?
>
> I checked that after kasan_init(), the entire shadow memory is full of 0 only.
>
> I also made a try with the strong STACK_PROTECTOR compiled in, but no difference and nothing detected by the stack protector.
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
> Read of size 1 at addr c0ecdd40 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
> Call Trace:
> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
> [c0e9dd10] [c089579c] memchr+0x24/0x74
> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
> --- interrupt: c0e9df00 at 0x400f330
> LR = init_stack+0x1f00/0x2000
> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
> [c0e9df50] [c0c16434] start_kernel+0x310/0x488
> [c0e9dff0] [00003484] 0x3484
>
> The buggy address belongs to the variable:
> __log_buf+0xec0/0x4020
> The buggy address belongs to the page:
> page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
> flags: 0x1000(reserved)
> raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
> ^
> c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
This one doesn't look good. Notice that it says stack-out-of-bounds, but at the same time there is
"The buggy address belongs to the variable: __log_buf+0xec0/0x4020"
which is printed by following code:
if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
pr_err("The buggy address belongs to the variable:\n");
pr_err(" %pS\n", addr);
}
So the stack unrelated address got stack-related poisoning. This could be a stack overflow, did you increase THREAD_SHIFT?
KASAN with stack instrumentation significantly increases stack usage.
^ permalink raw reply
* Re: [PATCH V5 0/5] NestMMU pte upgrade workaround for mprotect
From: Aneesh Kumar K.V @ 2019-02-27 8:58 UTC (permalink / raw)
To: Andrew Morton; +Cc: x86, npiggin, linux-mm, paulus, linuxppc-dev
In-Reply-To: <20190226153733.2552bb48dd195ae3bd46c3ef@linux-foundation.org>
Andrew Morton <akpm@linux-foundation.org> writes:
> [patch 1/5]: unreviewed and has unaddressed comments from mpe.
> [patch 2/5]: ditto
> [patch 3/5]: ditto
> [patch 4/5]: seems ready
> [patch 5/5]: reviewed by mpe, but appears to need more work
That was mostly variable naming preferences. I like the christmas
tree style not the inverted christmas tree. There is one detail about
commit message, which indicate the change may be required by other
architecture too. Was not sure whether that needed a commit message
update.
I didn't send an updated series because after replying to most of them I
didn't find a strong request to get the required changes in. If you want
me update the series with this variable name ordering and commit message
update I can send a new series today.
-aneesh
^ permalink raw reply
* Re: [PATCH V7 0/4] mm/kvm/vfio/ppc64: Migrate compound pages out of CMA region
From: Aneesh Kumar K.V @ 2019-02-27 8:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrea Arcangeli, Alexey Kardashevskiy, linux-kernel,
Michal Hocko, linux-mm, linuxppc-dev, David Gibson
In-Reply-To: <20190226155324.e99d5200cc6293138ac5c6fa@linux-foundation.org>
Andrew Morton <akpm@linux-foundation.org> writes:
> [patch 1/4]: OK. I guess. Was this worth consuming our last PF_ flag?
That was done based on request from Andrea and it also helps in avoiding
allocating pages from CMA region where we know we are anyway going to
migrate them out. So yes, this helps.
> [patch 2/4]: unreviewed
> [patch 3/4]: unreviewed, mpe still unhappy, I expect?
I did reply to that email. I guess mpe is ok with that?
> [patch 4/4]: unreviewed
-aneesh
^ permalink raw reply
* [PATCH kernel] KVM: PPC: Allocate guest TCEs on demand too
From: Alexey Kardashevskiy @ 2019-02-27 8:42 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, kvm-ppc, David Gibson
We already allocate hardware TCE tables in multiple levels and skip
intermediate levels when we can, now it is a turn of the KVM TCE tables.
Thankfully these are allocated already in 2 levels.
This moves the table's last level allocation from the creating helper to
kvmppc_tce_put() and kvm_spapr_tce_fault().
This adds kvmppc_rm_ioba_validate() to do an additional test if
the consequent kvmppc_tce_put() needs a page which has not been allocated;
if this is the case, we bail out to virtual mode handlers.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
For NVLink2 passthrough guests with 128TiB DMA windows (when we push GPU RAM
above the PCI MMIO window in the guest) and very fragmented system RAM
the difference is about 16GiB of RAM before and after this patch.
---
arch/powerpc/kvm/book3s_64_vio.c | 21 ++++------
arch/powerpc/kvm/book3s_64_vio_hv.c | 62 ++++++++++++++++++++++++++---
2 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index f02b049..281b56b 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -228,7 +228,8 @@ static void release_spapr_tce_table(struct rcu_head *head)
unsigned long i, npages = kvmppc_tce_pages(stt->size);
for (i = 0; i < npages; i++)
- __free_page(stt->pages[i]);
+ if (stt->pages[i])
+ __free_page(stt->pages[i]);
kfree(stt);
}
@@ -241,6 +242,12 @@ static vm_fault_t kvm_spapr_tce_fault(struct vm_fault *vmf)
if (vmf->pgoff >= kvmppc_tce_pages(stt->size))
return VM_FAULT_SIGBUS;
+ if (!stt->pages[vmf->pgoff]) {
+ stt->pages[vmf->pgoff] = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!stt->pages[vmf->pgoff])
+ return VM_FAULT_OOM;
+ }
+
page = stt->pages[vmf->pgoff];
get_page(page);
vmf->page = page;
@@ -296,7 +303,6 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
struct kvmppc_spapr_tce_table *siter;
unsigned long npages, size = args->size;
int ret = -ENOMEM;
- int i;
if (!args->size || args->page_shift < 12 || args->page_shift > 34 ||
(args->offset + args->size > (ULLONG_MAX >> args->page_shift)))
@@ -320,12 +326,6 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
stt->kvm = kvm;
INIT_LIST_HEAD_RCU(&stt->iommu_tables);
- for (i = 0; i < npages; i++) {
- stt->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
- if (!stt->pages[i])
- goto fail;
- }
-
mutex_lock(&kvm->lock);
/* Check this LIOBN hasn't been previously allocated */
@@ -352,11 +352,6 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
if (ret >= 0)
return ret;
- fail:
- for (i = 0; i < npages; i++)
- if (stt->pages[i])
- __free_page(stt->pages[i]);
-
kfree(stt);
fail_acct:
kvmppc_account_memlimit(kvmppc_stt_pages(npages), false);
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index 2206bc7..fbb920d 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -158,23 +158,69 @@ static u64 *kvmppc_page_address(struct page *page)
return (u64 *) page_address(page);
}
+/*
+ * TCEs pages are allocated in kvmppc_tce_put() which won't be able to do so
+ * in real mode.
+ * Check if kvmppc_tce_put() can succeed in real mode, i.e. a TCEs page is
+ * allocated or not required (when clearing a tce entry).
+ */
+static long kvmppc_rm_ioba_validate(struct kvmppc_spapr_tce_table *stt,
+ unsigned long ioba, unsigned long npages, bool clearing)
+{
+ unsigned long i, sttpage, sttpages;
+ unsigned long ret = kvmppc_ioba_validate(stt, ioba, npages);
+
+ if (ret)
+ return ret;
+ /*
+ * clearing==true says kvmppc_tce_put won't be allocating pages
+ * for empty tces.
+ */
+ if (clearing)
+ return H_SUCCESS;
+
+ sttpage = ((ioba >> stt->page_shift) - stt->offset) / TCES_PER_PAGE;
+ sttpages = (npages + TCES_PER_PAGE - 1) / TCES_PER_PAGE;
+ for (i = sttpage; i < sttpage + sttpages; ++i)
+ if (!stt->pages[i])
+ return H_TOO_HARD;
+
+ return H_SUCCESS;
+}
+
/*
* Handles TCE requests for emulated devices.
* Puts guest TCE values to the table and expects user space to convert them.
* Called in both real and virtual modes.
* Cannot fail so kvmppc_tce_validate must be called before it.
*
- * WARNING: This will be called in real-mode on HV KVM and virtual
- * mode on PR KVM
+ * WARNING: This will be called in real-mode on HV HPT KVM and virtual
+ * mode on PR KVM or HV radix KVM
*/
void kvmppc_tce_put(struct kvmppc_spapr_tce_table *stt,
unsigned long idx, unsigned long tce)
{
struct page *page;
u64 *tbl;
+ unsigned long sttpage;
idx -= stt->offset;
- page = stt->pages[idx / TCES_PER_PAGE];
+ sttpage = idx / TCES_PER_PAGE;
+ page = stt->pages[sttpage];
+
+ if (!page) {
+ /* We allow any TCE, not just with read|write permissions */
+ if (!tce)
+ return;
+ /*
+ * We must not end up here in real mode,
+ * kvmppc_rm_ioba_validate() takes care of this.
+ */
+ page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (WARN_ON_ONCE(!page))
+ return;
+ stt->pages[sttpage] = page;
+ }
tbl = kvmppc_page_address(page);
tbl[idx % TCES_PER_PAGE] = tce;
@@ -381,7 +427,7 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
if (!stt)
return H_TOO_HARD;
- ret = kvmppc_ioba_validate(stt, ioba, 1);
+ ret = kvmppc_rm_ioba_validate(stt, ioba, 1, tce == 0);
if (ret != H_SUCCESS)
return ret;
@@ -480,7 +526,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
if (tce_list & (SZ_4K - 1))
return H_PARAMETER;
- ret = kvmppc_ioba_validate(stt, ioba, npages);
+ ret = kvmppc_rm_ioba_validate(stt, ioba, npages, false);
if (ret != H_SUCCESS)
return ret;
@@ -583,7 +629,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
if (!stt)
return H_TOO_HARD;
- ret = kvmppc_ioba_validate(stt, ioba, npages);
+ ret = kvmppc_rm_ioba_validate(stt, ioba, npages, tce_value == 0);
if (ret != H_SUCCESS)
return ret;
@@ -635,6 +681,10 @@ long kvmppc_h_get_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
idx = (ioba >> stt->page_shift) - stt->offset;
page = stt->pages[idx / TCES_PER_PAGE];
+ if (!page) {
+ vcpu->arch.regs.gpr[4] = 0;
+ return H_SUCCESS;
+ }
tbl = (u64 *)page_address(page);
vcpu->arch.regs.gpr[4] = tbl[idx % TCES_PER_PAGE];
--
2.17.1
^ permalink raw reply related
* Re: BUG: KASAN: stack-out-of-bounds
From: Dmitry Vyukov @ 2019-02-27 8:34 UTC (permalink / raw)
To: Christophe Leroy
Cc: kasan-dev, Linux-MM, Alexander Potapenko, Andrey Ryabinin,
linuxppc-dev, Daniel Axtens
In-Reply-To: <c6d80735-0cfe-b4ab-0349-673fc65b2e15@c-s.fr>
On Wed, Feb 27, 2019 at 9:25 AM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
> With version v8 of the series implementing KASAN on 32 bits powerpc
> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309),
> I'm now able to activate KASAN on a mac99 is QEMU.
>
> Then I get the following reports at startup. Which of the two reports I
> get seems to depend on the option used to build the kernel, but for a
> given kernel I always get the same report.
>
> Is that a real bug, in which case how could I spot it ? Or is it
> something wrong in my implementation of KASAN ?
What is the state of your source tree?
Please pass output through some symbolization script, function offsets
are not too useful.
There was some in scripts/ dir IIRC, but here is another one (though,
never tested on powerpc):
https://github.com/google/sanitizers/blob/master/address-sanitizer/tools/kasan_symbolize.py
> I checked that after kasan_init(), the entire shadow memory is full of 0
> only.
>
> I also made a try with the strong STACK_PROTECTOR compiled in, but no
> difference and nothing detected by the stack protector.
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
> Read of size 1 at addr c0ecdd40 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
> Call Trace:
> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
> [c0e9dd10] [c089579c] memchr+0x24/0x74
> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
> --- interrupt: c0e9df00 at 0x400f330
> LR = init_stack+0x1f00/0x2000
> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
> [c0e9df50] [c0c16434] start_kernel+0x310/0x488
> [c0e9dff0] [00003484] 0x3484
>
> The buggy address belongs to the variable:
> __log_buf+0xec0/0x4020
> The buggy address belongs to the page:
> page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
> flags: 0x1000(reserved)
> raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
> ^
> c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1ec/0x600
> Read of size 1 at addr f6f37de0 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1134
> Call Trace:
> [c0ff7d60] [c01fe808] print_address_description+0x6c/0x2b0 (unreliable)
> [c0ff7d90] [c01fe4fc] kasan_report+0x13c/0x1ac
> [c0ff7dd0] [c0d34324] pmac_nvram_init+0x1ec/0x600
> [c0ff7ef0] [c0d31148] pmac_setup_arch+0x280/0x308
> [c0ff7f20] [c0d2c30c] setup_arch+0x250/0x280
> [c0ff7f50] [c0d26354] start_kernel+0xb8/0x4d8
> [c0ff7ff0] [00003484] 0x3484
>
>
> Memory state around the buggy address:
> f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> ^
> f6f37e00: 00 00 00 00 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
> f6f37e80: 00 00 01 f2 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in memchr+0xa0/0xac
> Read of size 1 at addr c17cdd30 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1135
> Call Trace:
> [c179dc90] [c032fe28] print_address_description+0x64/0x2bc (unreliable)
> [c179dcc0] [c033020c] kasan_report+0xfc/0x180
> [c179dd00] [c115ef50] memchr+0xa0/0xac
> [c179dd20] [c01297f8] msg_print_text+0xc8/0x67c
> [c179ddd0] [c012bc8c] console_unlock+0x17c/0x818
> [c179de40] [c012f420] vprintk_emit+0x188/0x1c4
> --- interrupt: c179df30 at 0x400def0
> LR = init_stack+0x1ef0/0x2000
> [c179de80] [c012fff0] printk+0xa8/0xcc (unreliable)
> [c179df20] [c150b4b8] early_irq_init+0x38/0x108
> [c179df50] [c14ef7f8] start_kernel+0x30c/0x530
> [c179dff0] [00003484] 0x3484
>
> The buggy address belongs to the variable:
> __log_buf+0xeb0/0x4020
> The buggy address belongs to the page:
> page:c6ebe9a0 count:1 mapcount:0 mapping:00000000 index:0x0
> flags: 0x1000(reserved)
> raw: 00001000 c6ebe9a4 c6ebe9a4 00000000 00000000 00000000 ffffffff 00000001
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> c17cdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c17cdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >c17cdd00: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 f3 f3
> ^
> c17cdd80: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c17cde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x228/0xae0
> Read of size 1 at addr f6f37dd0 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1136
> Call Trace:
> [c1c37d50] [c03f7e88] print_address_description+0x6c/0x2b0 (unreliable)
> [c1c37d80] [c03f7bd4] kasan_report+0x10c/0x16c
> [c1c37dc0] [c19879b4] pmac_nvram_init+0x228/0xae0
> [c1c37ef0] [c19826bc] pmac_setup_arch+0x578/0x6a8
> [c1c37f20] [c19792bc] setup_arch+0x5f4/0x620
> [c1c37f50] [c196f898] start_kernel+0xb8/0x588
> [c1c37ff0] [00003484] 0x3484
>
>
> Memory state around the buggy address:
> f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >f6f37d80: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
> ^
> f6f37e00: 01 f2 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
> f6f37e80: 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1ec/0x5ec
> Read of size 1 at addr f6f37de0 by task swapper/0
>
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1137
> Call Trace:
> [c0fb7d60] [c01f8184] print_address_description+0x6c/0x2b0 (unreliable)
> [c0fb7d90] [c01f7ed0] kasan_report+0x10c/0x16c
> [c0fb7dd0] [c0d1dfe8] pmac_nvram_init+0x1ec/0x5ec
> [c0fb7ef0] [c0d1ae90] pmac_setup_arch+0x280/0x308
> [c0fb7f20] [c0d16138] setup_arch+0x250/0x280
> [c0fb7f50] [c0d1032c] start_kernel+0xb8/0x4a4
> [c0fb7ff0] [00003484] 0x3484
>
>
> Memory state around the buggy address:
> f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
> ^
> f6f37e00: 00 00 01 f2 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
> f6f37e80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
> ==================================================================
>
> Thanks
> Christophe
^ permalink raw reply
* BUG: KASAN: stack-out-of-bounds
From: Christophe Leroy @ 2019-02-27 8:25 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
Cc: linux-mm, linuxppc-dev, kasan-dev, Daniel Axtens
With version v8 of the series implementing KASAN on 32 bits powerpc
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309),
I'm now able to activate KASAN on a mac99 is QEMU.
Then I get the following reports at startup. Which of the two reports I
get seems to depend on the option used to build the kernel, but for a
given kernel I always get the same report.
Is that a real bug, in which case how could I spot it ? Or is it
something wrong in my implementation of KASAN ?
I checked that after kasan_init(), the entire shadow memory is full of 0
only.
I also made a try with the strong STACK_PROTECTOR compiled in, but no
difference and nothing detected by the stack protector.
==================================================================
BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74
Read of size 1 at addr c0ecdd40 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133
Call Trace:
[c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable)
[c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180
[c0e9dd10] [c089579c] memchr+0x24/0x74
[c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574
[c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8
[c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4
--- interrupt: c0e9df00 at 0x400f330
LR = init_stack+0x1f00/0x2000
[c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable)
[c0e9df20] [c0c28e44] early_irq_init+0x38/0x108
[c0e9df50] [c0c16434] start_kernel+0x310/0x488
[c0e9dff0] [00003484] 0x3484
The buggy address belongs to the variable:
__log_buf+0xec0/0x4020
The buggy address belongs to the page:
page:c6eac9a0 count:1 mapcount:0 mapping:00000000 index:0x0
flags: 0x1000(reserved)
raw: 00001000 c6eac9a4 c6eac9a4 00000000 00000000 00000000 ffffffff 00000001
page dumped because: kasan: bad access detected
Memory state around the buggy address:
c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
^
c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
==================================================================
BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1ec/0x600
Read of size 1 at addr f6f37de0 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1134
Call Trace:
[c0ff7d60] [c01fe808] print_address_description+0x6c/0x2b0 (unreliable)
[c0ff7d90] [c01fe4fc] kasan_report+0x13c/0x1ac
[c0ff7dd0] [c0d34324] pmac_nvram_init+0x1ec/0x600
[c0ff7ef0] [c0d31148] pmac_setup_arch+0x280/0x308
[c0ff7f20] [c0d2c30c] setup_arch+0x250/0x280
[c0ff7f50] [c0d26354] start_kernel+0xb8/0x4d8
[c0ff7ff0] [00003484] 0x3484
Memory state around the buggy address:
f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
^
f6f37e00: 00 00 00 00 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
f6f37e80: 00 00 01 f2 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
==================================================================
BUG: KASAN: stack-out-of-bounds in memchr+0xa0/0xac
Read of size 1 at addr c17cdd30 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1135
Call Trace:
[c179dc90] [c032fe28] print_address_description+0x64/0x2bc (unreliable)
[c179dcc0] [c033020c] kasan_report+0xfc/0x180
[c179dd00] [c115ef50] memchr+0xa0/0xac
[c179dd20] [c01297f8] msg_print_text+0xc8/0x67c
[c179ddd0] [c012bc8c] console_unlock+0x17c/0x818
[c179de40] [c012f420] vprintk_emit+0x188/0x1c4
--- interrupt: c179df30 at 0x400def0
LR = init_stack+0x1ef0/0x2000
[c179de80] [c012fff0] printk+0xa8/0xcc (unreliable)
[c179df20] [c150b4b8] early_irq_init+0x38/0x108
[c179df50] [c14ef7f8] start_kernel+0x30c/0x530
[c179dff0] [00003484] 0x3484
The buggy address belongs to the variable:
__log_buf+0xeb0/0x4020
The buggy address belongs to the page:
page:c6ebe9a0 count:1 mapcount:0 mapping:00000000 index:0x0
flags: 0x1000(reserved)
raw: 00001000 c6ebe9a4 c6ebe9a4 00000000 00000000 00000000 ffffffff 00000001
page dumped because: kasan: bad access detected
Memory state around the buggy address:
c17cdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c17cdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>c17cdd00: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 f3 f3
^
c17cdd80: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c17cde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
==================================================================
BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x228/0xae0
Read of size 1 at addr f6f37dd0 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1136
Call Trace:
[c1c37d50] [c03f7e88] print_address_description+0x6c/0x2b0 (unreliable)
[c1c37d80] [c03f7bd4] kasan_report+0x10c/0x16c
[c1c37dc0] [c19879b4] pmac_nvram_init+0x228/0xae0
[c1c37ef0] [c19826bc] pmac_setup_arch+0x578/0x6a8
[c1c37f20] [c19792bc] setup_arch+0x5f4/0x620
[c1c37f50] [c196f898] start_kernel+0xb8/0x588
[c1c37ff0] [00003484] 0x3484
Memory state around the buggy address:
f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>f6f37d80: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
^
f6f37e00: 01 f2 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
f6f37e80: 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
==================================================================
==================================================================
BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1ec/0x5ec
Read of size 1 at addr f6f37de0 by task swapper/0
CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1137
Call Trace:
[c0fb7d60] [c01f8184] print_address_description+0x6c/0x2b0 (unreliable)
[c0fb7d90] [c01f7ed0] kasan_report+0x10c/0x16c
[c0fb7dd0] [c0d1dfe8] pmac_nvram_init+0x1ec/0x5ec
[c0fb7ef0] [c0d1ae90] pmac_setup_arch+0x280/0x308
[c0fb7f20] [c0d16138] setup_arch+0x250/0x280
[c0fb7f50] [c0d1032c] start_kernel+0xb8/0x4a4
[c0fb7ff0] [00003484] 0x3484
Memory state around the buggy address:
f6f37c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f6f37d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>f6f37d80: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
^
f6f37e00: 00 00 01 f2 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2
f6f37e80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
==================================================================
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Andrew Donnellan @ 2019-02-27 8:18 UTC (permalink / raw)
To: Alastair D'Silva, 'Alastair D'Silva'
Cc: 'Arnd Bergmann', 'Greg Kroah-Hartman',
'Greg Kurz', linux-kernel, 'Frederic Barrat',
linuxppc-dev
In-Reply-To: <158101d4ce73$11ff4550$35fdcff0$@d-silva.org>
On 27/2/19 7:04 pm, Alastair D'Silva wrote:
>> -----Original Message-----
>> From: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> Sent: Wednesday, 27 February 2019 6:55 PM
>> To: Alastair D'Silva <alastair@d-silva.org>; 'Alastair D'Silva'
>> <alastair@au1.ibm.com>
>> Cc: 'Greg Kurz' <groug@kaod.org>; 'Frederic Barrat'
>> <fbarrat@linux.ibm.com>; 'Arnd Bergmann' <arnd@arndb.de>; 'Greg Kroah-
>> Hartman' <gregkh@linuxfoundation.org>; linuxppc-dev@lists.ozlabs.org;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
>>
>> On 27/2/19 6:34 pm, Alastair D'Silva wrote:>>> diff --git
>> a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index
>>>>> e6a607488f8a..16eb8a60d5c7 100644
>>>>> --- a/drivers/misc/ocxl/file.c
>>>>> +++ b/drivers/misc/ocxl/file.c
>>>>> @@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct
>>>>> ocxl_context *ctx,
>>>>>
>>>>> if (status == ATTACHED) {
>>>>> int rc;
>>>>> - struct link *link = ctx->afu->fn->link;
>>>>> + void *link = ctx->afu->fn->link;
>>>>
>>>> This doesn't look like a rename...
>>>
>>> That corrects the type to what the member (and prototype for
>> ocxl_link_update_pe) declare it as.
>>>
>>> The struct link there is bogus, it shouldn't even compile (since the intended
>> struct link is defined in a different compilation unit), but instead picks up a
>> different definition of 'struct link' from elsewhere.
>>>
>>
>> Given there's only a handful of struct links defined across the entire kernel,
>> I'm going to guess that the definition it's picking up is in fact the ocxl one.
>>
>
> Unlikely, since that's never in a header. It wasn't caught since it was assigned to/from a void*.
Ah, yeah that'd explain it... and it's a pointer so it never needs to
know its size. I'm clearly not very good at C.
>
>> I think the better solution here is to move struct ocxl_link into
>> ocxl_internal.h, change ocxl_fn::link to be struct ocxl_link * rather than void
>> *, and update the function signature for ocxl_link_update_pe() as well.
>
> Not move it, but we could have an opaque declaration there.
>
Putting it there would fit with all the other ocxl_* structs, but either
way, we definitely need a declaration in there and get rid of the void*, t
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* RE: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Alastair D'Silva @ 2019-02-27 8:04 UTC (permalink / raw)
To: 'Andrew Donnellan', 'Alastair D'Silva'
Cc: 'Arnd Bergmann', 'Greg Kroah-Hartman',
'Greg Kurz', linux-kernel, 'Frederic Barrat',
linuxppc-dev
In-Reply-To: <a36b088b-b8d7-8725-50db-6aad58672784@au1.ibm.com>
> -----Original Message-----
> From: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Sent: Wednesday, 27 February 2019 6:55 PM
> To: Alastair D'Silva <alastair@d-silva.org>; 'Alastair D'Silva'
> <alastair@au1.ibm.com>
> Cc: 'Greg Kurz' <groug@kaod.org>; 'Frederic Barrat'
> <fbarrat@linux.ibm.com>; 'Arnd Bergmann' <arnd@arndb.de>; 'Greg Kroah-
> Hartman' <gregkh@linuxfoundation.org>; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
>
> On 27/2/19 6:34 pm, Alastair D'Silva wrote:>>> diff --git
> a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index
> >>> e6a607488f8a..16eb8a60d5c7 100644
> >>> --- a/drivers/misc/ocxl/file.c
> >>> +++ b/drivers/misc/ocxl/file.c
> >>> @@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct
> >>> ocxl_context *ctx,
> >>>
> >>> if (status == ATTACHED) {
> >>> int rc;
> >>> - struct link *link = ctx->afu->fn->link;
> >>> + void *link = ctx->afu->fn->link;
> >>
> >> This doesn't look like a rename...
> >
> > That corrects the type to what the member (and prototype for
> ocxl_link_update_pe) declare it as.
> >
> > The struct link there is bogus, it shouldn't even compile (since the intended
> struct link is defined in a different compilation unit), but instead picks up a
> different definition of 'struct link' from elsewhere.
> >
>
> Given there's only a handful of struct links defined across the entire kernel,
> I'm going to guess that the definition it's picking up is in fact the ocxl one.
>
Unlikely, since that's never in a header. It wasn't caught since it was assigned to/from a void*.
> I think the better solution here is to move struct ocxl_link into
> ocxl_internal.h, change ocxl_fn::link to be struct ocxl_link * rather than void
> *, and update the function signature for ocxl_link_update_pe() as well.
Not move it, but we could have an opaque declaration there.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva msn: alastair@d-silva.org
blog: http://alastair.d-silva.org Twitter: @EvilDeece
^ permalink raw reply
* Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Andrew Donnellan @ 2019-02-27 7:54 UTC (permalink / raw)
To: Alastair D'Silva, 'Alastair D'Silva'
Cc: 'Arnd Bergmann', 'Greg Kroah-Hartman',
'Greg Kurz', linux-kernel, 'Frederic Barrat',
linuxppc-dev
In-Reply-To: <156e01d4ce6e$ee42d9f0$cac88dd0$@d-silva.org>
On 27/2/19 6:34 pm, Alastair D'Silva wrote:>>> diff --git
a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index
>>> e6a607488f8a..16eb8a60d5c7 100644
>>> --- a/drivers/misc/ocxl/file.c
>>> +++ b/drivers/misc/ocxl/file.c
>>> @@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct
>>> ocxl_context *ctx,
>>>
>>> if (status == ATTACHED) {
>>> int rc;
>>> - struct link *link = ctx->afu->fn->link;
>>> + void *link = ctx->afu->fn->link;
>>
>> This doesn't look like a rename...
>
> That corrects the type to what the member (and prototype for ocxl_link_update_pe) declare it as.
>
> The struct link there is bogus, it shouldn't even compile (since the intended struct link is defined in a different compilation unit), but instead picks up a different definition of 'struct link' from elsewhere.
>
Given there's only a handful of struct links defined across the entire
kernel, I'm going to guess that the definition it's picking up is in
fact the ocxl one.
I think the better solution here is to move struct ocxl_link into
ocxl_internal.h, change ocxl_fn::link to be struct ocxl_link * rather
than void *, and update the function signature for ocxl_link_update_pe()
as well.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* RE: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Alastair D'Silva @ 2019-02-27 7:34 UTC (permalink / raw)
To: 'Andrew Donnellan', 'Alastair D'Silva'
Cc: 'Arnd Bergmann', 'Greg Kroah-Hartman',
'Greg Kurz', linux-kernel, 'Frederic Barrat',
linuxppc-dev
In-Reply-To: <8c0300fc-4319-bd66-a47e-4ffda6ac795c@au1.ibm.com>
> -----Original Message-----
> From: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Sent: Wednesday, 27 February 2019 6:16 PM
> To: Alastair D'Silva <alastair@au1.ibm.com>; alastair@d-silva.org
> Cc: Greg Kurz <groug@kaod.org>; Frederic Barrat <fbarrat@linux.ibm.com>;
> Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
>
> On 27/2/19 3:57 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > The term 'link' is ambiguous (especially when the struct is used for a
> > list), so rename it for clarity.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> > drivers/misc/ocxl/file.c | 2 +-
> > drivers/misc/ocxl/link.c | 36 ++++++++++++++++++------------------
> > 2 files changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index
> > e6a607488f8a..16eb8a60d5c7 100644
> > --- a/drivers/misc/ocxl/file.c
> > +++ b/drivers/misc/ocxl/file.c
> > @@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct
> > ocxl_context *ctx,
> >
> > if (status == ATTACHED) {
> > int rc;
> > - struct link *link = ctx->afu->fn->link;
> > + void *link = ctx->afu->fn->link;
>
> This doesn't look like a rename...
That corrects the type to what the member (and prototype for ocxl_link_update_pe) declare it as.
The struct link there is bogus, it shouldn't even compile (since the intended struct link is defined in a different compilation unit), but instead picks up a different definition of 'struct link' from elsewhere.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva msn: alastair@d-silva.org
blog: http://alastair.d-silva.org Twitter: @EvilDeece
^ permalink raw reply
* Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Andrew Donnellan @ 2019-02-27 7:15 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-2-alastair@au1.ibm.com>
On 27/2/19 3:57 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> The term 'link' is ambiguous (especially when the struct is used for a
> list), so rename it for clarity.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
> drivers/misc/ocxl/file.c | 2 +-
> drivers/misc/ocxl/link.c | 36 ++++++++++++++++++------------------
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index e6a607488f8a..16eb8a60d5c7 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
>
> if (status == ATTACHED) {
> int rc;
> - struct link *link = ctx->afu->fn->link;
> + void *link = ctx->afu->fn->link;
This doesn't look like a rename...
>
> rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
> if (rc)
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index d50b861d7e57..8d2690a1a9de 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -76,7 +76,7 @@ struct spa {
> * limited number of opencapi slots on a system and lookup is only
> * done when the device is probed
> */
> -struct link {
> +struct ocxl_link {
> struct list_head list;
> struct kref ref;
> int domain;
> @@ -179,7 +179,7 @@ static void xsl_fault_handler_bh(struct work_struct *fault_work)
>
> static irqreturn_t xsl_fault_handler(int irq, void *data)
> {
> - struct link *link = (struct link *) data;
> + struct ocxl_link *link = (struct ocxl_link *) data;
> struct spa *spa = link->spa;
> u64 dsisr, dar, pe_handle;
> struct pe_data *pe_data;
> @@ -256,7 +256,7 @@ static int map_irq_registers(struct pci_dev *dev, struct spa *spa)
> &spa->reg_tfc, &spa->reg_pe_handle);
> }
>
> -static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
> +static int setup_xsl_irq(struct pci_dev *dev, struct ocxl_link *link)
> {
> struct spa *spa = link->spa;
> int rc;
> @@ -311,7 +311,7 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
> return rc;
> }
>
> -static void release_xsl_irq(struct link *link)
> +static void release_xsl_irq(struct ocxl_link *link)
> {
> struct spa *spa = link->spa;
>
> @@ -323,7 +323,7 @@ static void release_xsl_irq(struct link *link)
> unmap_irq_registers(spa);
> }
>
> -static int alloc_spa(struct pci_dev *dev, struct link *link)
> +static int alloc_spa(struct pci_dev *dev, struct ocxl_link *link)
> {
> struct spa *spa;
>
> @@ -350,7 +350,7 @@ static int alloc_spa(struct pci_dev *dev, struct link *link)
> return 0;
> }
>
> -static void free_spa(struct link *link)
> +static void free_spa(struct ocxl_link *link)
> {
> struct spa *spa = link->spa;
>
> @@ -364,12 +364,12 @@ static void free_spa(struct link *link)
> }
> }
>
> -static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
> +static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_link)
> {
> - struct link *link;
> + struct ocxl_link *link;
> int rc;
>
> - link = kzalloc(sizeof(struct link), GFP_KERNEL);
> + link = kzalloc(sizeof(struct ocxl_link), GFP_KERNEL);
> if (!link)
> return -ENOMEM;
>
> @@ -405,7 +405,7 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
> return rc;
> }
>
> -static void free_link(struct link *link)
> +static void free_link(struct ocxl_link *link)
> {
> release_xsl_irq(link);
> free_spa(link);
> @@ -415,7 +415,7 @@ static void free_link(struct link *link)
> int ocxl_link_setup(struct pci_dev *dev, int PE_mask, void **link_handle)
> {
> int rc = 0;
> - struct link *link;
> + struct ocxl_link *link;
>
> mutex_lock(&links_list_lock);
> list_for_each_entry(link, &links_list, list) {
> @@ -442,7 +442,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_setup);
>
> static void release_xsl(struct kref *ref)
> {
> - struct link *link = container_of(ref, struct link, ref);
> + struct ocxl_link *link = container_of(ref, struct ocxl_link, ref);
>
> list_del(&link->list);
> /* call platform code before releasing data */
> @@ -452,7 +452,7 @@ static void release_xsl(struct kref *ref)
>
> void ocxl_link_release(struct pci_dev *dev, void *link_handle)
> {
> - struct link *link = (struct link *) link_handle;
> + struct ocxl_link *link = (struct ocxl_link *) link_handle;
>
> mutex_lock(&links_list_lock);
> kref_put(&link->ref, release_xsl);
> @@ -488,7 +488,7 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
> void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
> void *xsl_err_data)
> {
> - struct link *link = (struct link *) link_handle;
> + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> struct spa *spa = link->spa;
> struct ocxl_process_element *pe;
> int pe_handle, rc = 0;
> @@ -558,7 +558,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_add_pe);
>
> int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
> {
> - struct link *link = (struct link *) link_handle;
> + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> struct spa *spa = link->spa;
> struct ocxl_process_element *pe;
> int pe_handle, rc;
> @@ -594,7 +594,7 @@ int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
>
> int ocxl_link_remove_pe(void *link_handle, int pasid)
> {
> - struct link *link = (struct link *) link_handle;
> + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> struct spa *spa = link->spa;
> struct ocxl_process_element *pe;
> struct pe_data *pe_data;
> @@ -666,7 +666,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_remove_pe);
>
> int ocxl_link_irq_alloc(void *link_handle, int *hw_irq, u64 *trigger_addr)
> {
> - struct link *link = (struct link *) link_handle;
> + struct ocxl_link *link = (struct ocxl_link *) link_handle;
> int rc, irq;
> u64 addr;
>
> @@ -687,7 +687,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_irq_alloc);
>
> void ocxl_link_free_irq(void *link_handle, int hw_irq)
> {
> - struct link *link = (struct link *) link_handle;
> + struct ocxl_link *link = (struct ocxl_link *) link_handle;
>
> pnv_ocxl_free_xive_irq(hw_irq);
> atomic_inc(&link->irq_available);
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v8 02/11] powerpc: prepare string/mem functions for KASAN
From: Daniel Axtens @ 2019-02-27 6:54 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <54c5d48a557647be5e0269a22be277891bbdd7f7.1551161392.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> CONFIG_KASAN implements wrappers for memcpy() memmove() and memset()
> Those wrappers are doing the verification then call respectively
> __memcpy() __memmove() and __memset(). The arches are therefore
> expected to rename their optimised functions that way.
>
> For files on which KASAN is inhibited, #defines are used to allow
> them to directly call optimised versions of the functions without
> going through the KASAN wrappers.
>
> See commit 393f203f5fd5 ("x86_64: kasan: add interceptors for
> memset/memmove/memcpy functions") for details.
>
> Other string / mem functions do not (yet) have kasan wrappers,
> we therefore have to fallback to the generic versions when
> KASAN is active, otherwise KASAN checks will be skipped.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/kasan.h | 15 +++++++++++++++
> arch/powerpc/include/asm/string.h | 32 +++++++++++++++++++++++++++++---
> arch/powerpc/kernel/prom_init_check.sh | 10 +++++++++-
> arch/powerpc/lib/Makefile | 11 ++++++++---
> arch/powerpc/lib/copy_32.S | 15 +++++++++------
> arch/powerpc/lib/mem_64.S | 11 +++++++----
> arch/powerpc/lib/memcpy_64.S | 5 +++--
> 7 files changed, 80 insertions(+), 19 deletions(-)
> create mode 100644 arch/powerpc/include/asm/kasan.h
>
> diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
> new file mode 100644
> index 000000000000..c3161b8fc017
> --- /dev/null
> +++ b/arch/powerpc/include/asm/kasan.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_KASAN_H
> +#define __ASM_KASAN_H
> +
> +#ifdef CONFIG_KASAN
> +#define _GLOBAL_KASAN(fn) .weak fn ; _GLOBAL(__##fn) ; _GLOBAL(fn)
> +#define _GLOBAL_TOC_KASAN(fn) .weak fn ; _GLOBAL_TOC(__##fn) ; _GLOBAL_TOC(fn)
> +#define EXPORT_SYMBOL_KASAN(fn) EXPORT_SYMBOL(__##fn) ; EXPORT_SYMBOL(fn)
[FWIW, and this shouldn't block your patch:] This doesn't seem to work
with the 64bit elf abi v1, as we have symbols and dot symbols - our
_GLOBAL* doesn't just create a symtab entry. I don't fully understand
the inner workings just yet, but Aneesh and Balbir have solutions that
use .set instead of creating two entries.
What I am also struggling with is why we export the __symbol
version. I know the x86 version does this, but I can't figure that out
either - why would a module need an uninstrumented copy?
Anyway, I am getting some issues such as:
WARNING: EXPORT symbol "__memcpy" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "__memset" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "__memmove" [vmlinux] version generation failed, symbol will not be versioned.
I think Balbir and Aneesh avoided this by just not ever exporting the
__symbol versions, but perhaps that won't fly for the final version. It
looks like we can also avoid this by jumping through some extra hoops
and creating new weak symbols - I'll keep working on it and let you know
how I go.
As I said, I don't think this should necessarily block your patches -
it's just notes on ppc64 progress.
Regards,
Daniel
> +#else
> +#define _GLOBAL_KASAN(fn) _GLOBAL(fn)
> +#define _GLOBAL_TOC_KASAN(fn) _GLOBAL_TOC(fn)
> +#define EXPORT_SYMBOL_KASAN(fn) EXPORT_SYMBOL(fn)
> +#endif
> +
> +#endif
> diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
> index 1647de15a31e..9bf6dffb4090 100644
> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -4,14 +4,17 @@
>
> #ifdef __KERNEL__
>
> +#ifndef CONFIG_KASAN
> #define __HAVE_ARCH_STRNCPY
> #define __HAVE_ARCH_STRNCMP
> +#define __HAVE_ARCH_MEMCHR
> +#define __HAVE_ARCH_MEMCMP
> +#define __HAVE_ARCH_MEMSET16
> +#endif
> +
> #define __HAVE_ARCH_MEMSET
> #define __HAVE_ARCH_MEMCPY
> #define __HAVE_ARCH_MEMMOVE
> -#define __HAVE_ARCH_MEMCMP
> -#define __HAVE_ARCH_MEMCHR
> -#define __HAVE_ARCH_MEMSET16
> #define __HAVE_ARCH_MEMCPY_FLUSHCACHE
>
> extern char * strcpy(char *,const char *);
> @@ -27,7 +30,27 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
> extern void * memchr(const void *,int,__kernel_size_t);
> extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>
> +void *__memset(void *s, int c, __kernel_size_t count);
> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
> +void *__memmove(void *to, const void *from, __kernel_size_t n);
> +
> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
> +/*
> + * For files that are not instrumented (e.g. mm/slub.c) we
> + * should use not instrumented version of mem* functions.
> + */
> +#define memcpy(dst, src, len) __memcpy(dst, src, len)
> +#define memmove(dst, src, len) __memmove(dst, src, len)
> +#define memset(s, c, n) __memset(s, c, n)
> +
> +#ifndef __NO_FORTIFY
> +#define __NO_FORTIFY /* FORTIFY_SOURCE uses __builtin_memcpy, etc. */
> +#endif
> +
> +#endif
> +
> #ifdef CONFIG_PPC64
> +#ifndef CONFIG_KASAN
> #define __HAVE_ARCH_MEMSET32
> #define __HAVE_ARCH_MEMSET64
>
> @@ -49,8 +72,11 @@ static inline void *memset64(uint64_t *p, uint64_t v, __kernel_size_t n)
> {
> return __memset64(p, v, n * 8);
> }
> +#endif
> #else
> +#ifndef CONFIG_KASAN
> #define __HAVE_ARCH_STRLEN
> +#endif
>
> extern void *memset16(uint16_t *, uint16_t, __kernel_size_t);
> #endif
> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> index 667df97d2595..181fd10008ef 100644
> --- a/arch/powerpc/kernel/prom_init_check.sh
> +++ b/arch/powerpc/kernel/prom_init_check.sh
> @@ -16,8 +16,16 @@
> # If you really need to reference something from prom_init.o add
> # it to the list below:
>
> +grep "^CONFIG_KASAN=y$" .config >/dev/null
> +if [ $? -eq 0 ]
> +then
> + MEM_FUNCS="__memcpy __memset"
> +else
> + MEM_FUNCS="memcpy memset"
> +fi
> +
> WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
> -_end enter_prom memcpy memset reloc_offset __secondary_hold
> +_end enter_prom $MEM_FUNCS reloc_offset __secondary_hold
> __secondary_hold_acknowledge __secondary_hold_spinloop __start
> strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
> reloc_got2 kernstart_addr memstart_addr linux_banner _stext
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 79396e184bca..47a4de434c22 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -8,9 +8,14 @@ ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
> CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>
> -obj-y += string.o alloc.o code-patching.o feature-fixups.o
> +obj-y += alloc.o code-patching.o feature-fixups.o
>
> -obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o strlen_32.o
> +ifndef CONFIG_KASAN
> +obj-y += string.o memcmp_$(BITS).o
> +obj-$(CONFIG_PPC32) += strlen_32.o
> +endif
> +
> +obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
>
> obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
>
> @@ -34,7 +39,7 @@ obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o \
> test_emulate_step_exec_instr.o
>
> obj-y += checksum_$(BITS).o checksum_wrappers.o \
> - string_$(BITS).o memcmp_$(BITS).o
> + string_$(BITS).o
>
> obj-y += sstep.o ldstfp.o quad.o
> obj64-y += quad.o
> diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
> index ba66846fe973..fc4fa7246200 100644
> --- a/arch/powerpc/lib/copy_32.S
> +++ b/arch/powerpc/lib/copy_32.S
> @@ -14,6 +14,7 @@
> #include <asm/ppc_asm.h>
> #include <asm/export.h>
> #include <asm/code-patching-asm.h>
> +#include <asm/kasan.h>
>
> #define COPY_16_BYTES \
> lwz r7,4(r4); \
> @@ -68,6 +69,7 @@ CACHELINE_BYTES = L1_CACHE_BYTES
> LG_CACHELINE_BYTES = L1_CACHE_SHIFT
> CACHELINE_MASK = (L1_CACHE_BYTES-1)
>
> +#ifndef CONFIG_KASAN
> _GLOBAL(memset16)
> rlwinm. r0 ,r5, 31, 1, 31
> addi r6, r3, -4
> @@ -81,6 +83,7 @@ _GLOBAL(memset16)
> sth r4, 4(r6)
> blr
> EXPORT_SYMBOL(memset16)
> +#endif
>
> /*
> * Use dcbz on the complete cache lines in the destination
> @@ -91,7 +94,7 @@ EXPORT_SYMBOL(memset16)
> * We therefore skip the optimised bloc that uses dcbz. This jump is
> * replaced by a nop once cache is active. This is done in machine_init()
> */
> -_GLOBAL(memset)
> +_GLOBAL_KASAN(memset)
> cmplwi 0,r5,4
> blt 7f
>
> @@ -150,7 +153,7 @@ _GLOBAL(memset)
> 9: stbu r4,1(r6)
> bdnz 9b
> blr
> -EXPORT_SYMBOL(memset)
> +EXPORT_SYMBOL_KASAN(memset)
>
> /*
> * This version uses dcbz on the complete cache lines in the
> @@ -163,12 +166,12 @@ EXPORT_SYMBOL(memset)
> * We therefore jump to generic_memcpy which doesn't use dcbz. This jump is
> * replaced by a nop once cache is active. This is done in machine_init()
> */
> -_GLOBAL(memmove)
> +_GLOBAL_KASAN(memmove)
> cmplw 0,r3,r4
> bgt backwards_memcpy
> /* fall through */
>
> -_GLOBAL(memcpy)
> +_GLOBAL_KASAN(memcpy)
> 1: b generic_memcpy
> patch_site 1b, patch__memcpy_nocache
>
> @@ -242,8 +245,8 @@ _GLOBAL(memcpy)
> stbu r0,1(r6)
> bdnz 40b
> 65: blr
> -EXPORT_SYMBOL(memcpy)
> -EXPORT_SYMBOL(memmove)
> +EXPORT_SYMBOL_KASAN(memcpy)
> +EXPORT_SYMBOL_KASAN(memmove)
>
> generic_memcpy:
> srwi. r7,r5,3
> diff --git a/arch/powerpc/lib/mem_64.S b/arch/powerpc/lib/mem_64.S
> index 3c3be02f33b7..7cd6cf6822a2 100644
> --- a/arch/powerpc/lib/mem_64.S
> +++ b/arch/powerpc/lib/mem_64.S
> @@ -12,7 +12,9 @@
> #include <asm/errno.h>
> #include <asm/ppc_asm.h>
> #include <asm/export.h>
> +#include <asm/kasan.h>
>
> +#ifndef CONFIG_KASAN
> _GLOBAL(__memset16)
> rlwimi r4,r4,16,0,15
> /* fall through */
> @@ -29,8 +31,9 @@ _GLOBAL(__memset64)
> EXPORT_SYMBOL(__memset16)
> EXPORT_SYMBOL(__memset32)
> EXPORT_SYMBOL(__memset64)
> +#endif
>
> -_GLOBAL(memset)
> +_GLOBAL_KASAN(memset)
> neg r0,r3
> rlwimi r4,r4,8,16,23
> andi. r0,r0,7 /* # bytes to be 8-byte aligned */
> @@ -95,9 +98,9 @@ _GLOBAL(memset)
> 10: bflr 31
> stb r4,0(r6)
> blr
> -EXPORT_SYMBOL(memset)
> +EXPORT_SYMBOL_KASAN(memset)
>
> -_GLOBAL_TOC(memmove)
> +_GLOBAL_TOC_KASAN(memmove)
> cmplw 0,r3,r4
> bgt backwards_memcpy
> b memcpy
> @@ -138,4 +141,4 @@ _GLOBAL(backwards_memcpy)
> beq 2b
> mtctr r7
> b 1b
> -EXPORT_SYMBOL(memmove)
> +EXPORT_SYMBOL_KASAN(memmove)
> diff --git a/arch/powerpc/lib/memcpy_64.S b/arch/powerpc/lib/memcpy_64.S
> index 273ea67e60a1..862b515b8868 100644
> --- a/arch/powerpc/lib/memcpy_64.S
> +++ b/arch/powerpc/lib/memcpy_64.S
> @@ -11,6 +11,7 @@
> #include <asm/export.h>
> #include <asm/asm-compat.h>
> #include <asm/feature-fixups.h>
> +#include <asm/kasan.h>
>
> #ifndef SELFTEST_CASE
> /* For big-endian, 0 == most CPUs, 1 == POWER6, 2 == Cell */
> @@ -18,7 +19,7 @@
> #endif
>
> .align 7
> -_GLOBAL_TOC(memcpy)
> +_GLOBAL_TOC_KASAN(memcpy)
> BEGIN_FTR_SECTION
> #ifdef __LITTLE_ENDIAN__
> cmpdi cr7,r5,0
> @@ -229,4 +230,4 @@ END_FTR_SECTION_IFCLR(CPU_FTR_UNALIGNED_LD_STD)
> 4: ld r3,-STACKFRAMESIZE+STK_REG(R31)(r1) /* return dest pointer */
> blr
> #endif
> -EXPORT_SYMBOL(memcpy)
> +EXPORT_SYMBOL_KASAN(memcpy)
> --
> 2.13.3
^ permalink raw reply
* Re: [PATCH] powerpc/fsl: Fix the flush of branch predictor.
From: Diana Madalina Craciun @ 2019-02-27 6:47 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <dccb392cd3e2b1825608576ecf2487cc5d84466d.1551205086.git.christophe.leroy@c-s.fr>
Reviewed-by: Diana Craciun <diana.craciun@nxp.com>
Thanks for fixing it!
On 2/26/2019 8:18 PM, Christophe Leroy wrote:
> The commit identified below adds MC_BTB_FLUSH macro only when
> CONFIG_PPC_FSL_BOOK3E is defined. This results in the following error
> on some configs (seen several times with kisskb randconfig_defconfig)
>
> arch/powerpc/kernel/exceptions-64e.S:576: Error: Unrecognized opcode: `mc_btb_flush'
> make[3]: *** [scripts/Makefile.build:367: arch/powerpc/kernel/exceptions-64e.o] Error 1
> make[2]: *** [scripts/Makefile.build:492: arch/powerpc/kernel] Error 2
> make[1]: *** [Makefile:1043: arch/powerpc] Error 2
> make: *** [Makefile:152: sub-make] Error 2
>
> This patch adds a blank definition of MC_BTB_FLUSH for other cases.
>
> Fixes: 10c5e83afd4a ("powerpc/fsl: Flush the branch predictor at each kernel entry (64bit)")
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/exceptions-64e.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 4549ce8d4637..49381f32b374 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -338,6 +338,7 @@ ret_from_mc_except:
> #define GEN_BTB_FLUSH
> #define CRIT_BTB_FLUSH
> #define DBG_BTB_FLUSH
> +#define MC_BTB_FLUSH
> #define GDBELL_BTB_FLUSH
> #endif
>
^ permalink raw reply
* Re: build failure of current mmotm with skiroot_defconfig
From: Christophe Leroy @ 2019-02-27 6:41 UTC (permalink / raw)
To: Paul Mackerras, Mike Rapoport; +Cc: Andrew Morton, linuxppc-dev
In-Reply-To: <20190226222408.GE28015@blackberry>
Le 26/02/2019 à 23:24, Paul Mackerras a écrit :
> On Tue, Feb 26, 2019 at 11:59:08AM +0200, Mike Rapoport wrote:
>> On Tue, Feb 26, 2019 at 10:39:54AM +0100, Christophe Leroy wrote:
>>>
>>>
>>> Le 26/02/2019 à 09:12, Mike Rapoport a écrit :
>>>> Hi,
>>>>
>>>> I've encountered the following error when building skyroot_defconfig with
>>>> current mmotm tree:
>>>>
>>>> make CONFIG_OF_ALL_DTBS=y CONFIG_DTC=y CROSS_COMPILE=/opt/gcc-8.1.0-nolibc/powerpc64-linux/bin/powerpc64-linux- ARCH=powerpc vmlinux
>>>> ...
>>>> CC arch/powerpc/kernel/dbell.o
>>>> In file included from arch/powerpc/kernel/dbell.c:20:
>>>> arch/powerpc/include/asm/kvm_ppc.h: In function 'xics_on_xive':
>>>> arch/powerpc/include/asm/kvm_ppc.h:625:9: error: implicit declaration of function 'xive_enabled'; did you mean 'eeh_enabled'? [-Werror=implicit-function-declaration]
>>>> return xive_enabled() && cpu_has_feature(CPU_FTR_HVMODE);
>>>> ^~~~~~~~~~~~
>>>> eeh_enabled
>>>
>>> I can neither find the above in arch/powerpc/include/asm/kvm_ppc.h in the
>>> powerpc tree, nor a patch removing it.
>>>
>>> Where does that comes from ?
>>
>> It's from current mmotm, probably some merge conflict...
>
> It's not a merge conflict; see http://patchwork.ozlabs.org/patch/1048584/
> for the fix.
>
Why not include asm/xive.h all the time, as it defines xive_enabled()
for when CONFIG_PPC_XIVE is set and for when it is not set ?
Christophe
^ permalink raw reply
* [PATCH V4] ASoC: fsl_esai: fix channel swap issue when stream starts
From: S.j. Wang @ 2019-02-27 6:31 UTC (permalink / raw)
To: timur@kernel.org, nicoleotsuka@gmail.com, Xiubo.Lee@gmail.com,
festevam@gmail.com, broonie@kernel.org,
alsa-devel@alsa-project.org
Cc: linuxppc-dev@lists.ozlabs.org
There is very low possibility ( < 0.1% ) that channel swap happened
in beginning when multi output/input pin is enabled. The issue is
that hardware can't send data to correct pin in the beginning with
the normal enable flow.
This is hardware issue, but there is no errata, the workaround flow
is that: Each time playback/recording, firstly clear the xSMA/xSMB,
then enable TE/RE, then enable xSMB and xSMA (xSMB must be enabled
before xSMA). Which is to use the xSMA as the trigger start register,
previously the xCR_TE or xCR_RE is the bit for starting.
Fixes commit 43d24e76b698 ("ASoC: fsl_esai: Add ESAI CPU DAI driver")
Cc: <stable@vger.kernel.org>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_esai.c | 47 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index afe67c865330..3623aa9a6f2e 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -54,6 +54,8 @@ struct fsl_esai {
u32 fifo_depth;
u32 slot_width;
u32 slots;
+ u32 tx_mask;
+ u32 rx_mask;
u32 hck_rate[2];
u32 sck_rate[2];
bool hck_dir[2];
@@ -361,21 +363,13 @@ static int fsl_esai_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
regmap_update_bits(esai_priv->regmap, REG_ESAI_TCCR,
ESAI_xCCR_xDC_MASK, ESAI_xCCR_xDC(slots));
- regmap_update_bits(esai_priv->regmap, REG_ESAI_TSMA,
- ESAI_xSMA_xS_MASK, ESAI_xSMA_xS(tx_mask));
- regmap_update_bits(esai_priv->regmap, REG_ESAI_TSMB,
- ESAI_xSMB_xS_MASK, ESAI_xSMB_xS(tx_mask));
-
regmap_update_bits(esai_priv->regmap, REG_ESAI_RCCR,
ESAI_xCCR_xDC_MASK, ESAI_xCCR_xDC(slots));
- regmap_update_bits(esai_priv->regmap, REG_ESAI_RSMA,
- ESAI_xSMA_xS_MASK, ESAI_xSMA_xS(rx_mask));
- regmap_update_bits(esai_priv->regmap, REG_ESAI_RSMB,
- ESAI_xSMB_xS_MASK, ESAI_xSMB_xS(rx_mask));
-
esai_priv->slot_width = slot_width;
esai_priv->slots = slots;
+ esai_priv->tx_mask = tx_mask;
+ esai_priv->rx_mask = rx_mask;
return 0;
}
@@ -596,6 +590,7 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd,
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
u8 i, channels = substream->runtime->channels;
u32 pins = DIV_ROUND_UP(channels, esai_priv->slots);
+ u32 mask;
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
@@ -608,15 +603,38 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd,
for (i = 0; tx && i < channels; i++)
regmap_write(esai_priv->regmap, REG_ESAI_ETDR, 0x0);
+ /*
+ * When set the TE/RE in the end of enablement flow, there
+ * will be channel swap issue for multi data line case.
+ * In order to workaround this issue, we switch the bit
+ * enablement sequence to below sequence
+ * 1) clear the xSMB & xSMA: which is done in probe and
+ * stop state.
+ * 2) set TE/RE
+ * 3) set xSMB
+ * 4) set xSMA: xSMA is the last one in this flow, which
+ * will trigger esai to start.
+ */
regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
tx ? ESAI_xCR_TE_MASK : ESAI_xCR_RE_MASK,
tx ? ESAI_xCR_TE(pins) : ESAI_xCR_RE(pins));
+ mask = tx ? esai_priv->tx_mask : esai_priv->rx_mask;
+
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMB(tx),
+ ESAI_xSMB_xS_MASK, ESAI_xSMB_xS(mask));
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMA(tx),
+ ESAI_xSMA_xS_MASK, ESAI_xSMA_xS(mask));
+
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
tx ? ESAI_xCR_TE_MASK : ESAI_xCR_RE_MASK, 0);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMA(tx),
+ ESAI_xSMA_xS_MASK, 0);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMB(tx),
+ ESAI_xSMB_xS_MASK, 0);
/* Disable and reset FIFO */
regmap_update_bits(esai_priv->regmap, REG_ESAI_xFCR(tx),
@@ -906,6 +924,15 @@ static int fsl_esai_probe(struct platform_device *pdev)
return ret;
}
+ esai_priv->tx_mask = 0xFFFFFFFF;
+ esai_priv->rx_mask = 0xFFFFFFFF;
+
+ /* Clear the TSMA, TSMB, RSMA, RSMB */
+ regmap_write(esai_priv->regmap, REG_ESAI_TSMA, 0);
+ regmap_write(esai_priv->regmap, REG_ESAI_TSMB, 0);
+ regmap_write(esai_priv->regmap, REG_ESAI_RSMA, 0);
+ regmap_write(esai_priv->regmap, REG_ESAI_RSMB, 0);
+
ret = devm_snd_soc_register_component(&pdev->dev, &fsl_esai_component,
&fsl_esai_dai, 1);
if (ret) {
--
1.9.1
^ permalink raw reply related
* RE: [PATCH V3] ASoC: fsl_esai: fix channel swap issue when stream starts
From: S.j. Wang @ 2019-02-27 6:30 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190226191057.GA24082@Asurada-Nvidia.nvidia.com>
Ok, will send v4.
Best regards
Wang shengjiu
>
> Hi Shengjiu,
>
> Two more trivial comments inline.
>
> On Tue, Feb 26, 2019 at 03:12:25AM +0000, S.j. Wang wrote:
> > There is very low possibility ( < 0.1% ) that channel swap happened in
> > beginning when multi output/input pin is enabled. The issue is that
> > hardware can't send data to correct pin in the beginning with the
> > normal enable flow.
> >
> > This is hardware issue, but there is no errata, the workaround flow is
> > that: Each time playback/recording, firstly clear the xSMA/xSMB, then
> > enable TE/RE, then enable xSMB and xSMA (xSMB must be enabled
> before
> > xSMA). Which is to use the xSMA as the trigger start register,
> > previously the xCR_TE or xCR_RE is the bit for starting.
> >
> > Fixes commit 43d24e76b698 ("ASoC: fsl_esai: Add ESAI CPU DAI driver")
> > Cc: <stable@vger.kernel.org>
> > Re> @@ -361,21 +363,13 @@ static int fsl_esai_set_dai_tdm_slot(struct
> > Re> snd_soc_dai *dai, u32 tx_mask,
> > esai_priv->slot_width = slot_width;
> > esai_priv->slots = slots;
> > + esai_priv->tx_mask = tx_mask;
> > + esai_priv->rx_mask = rx_mask;
>
> Not necessary to have tabs before "="s.
>
> > @@ -611,12 +606,23 @@ static int fsl_esai_trigger(struct
> snd_pcm_substream *substream, int cmd,
> > regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
> > tx ? ESAI_xCR_TE_MASK :
> ESAI_xCR_RE_MASK,
> > tx ? ESAI_xCR_TE(pins) :
> ESAI_xCR_RE(pins));
> > + mask = tx ? esai_priv->tx_mask : esai_priv->rx_mask;
> > +
>
> Please add a line of brief comments for the reason why we are doing this
> here, something similar to the commit log, but should have:
> 1) Clear xSMA/B 2) Set TE/RE before xSMA/B 3)Set xSMB before xSMA.
>
> Apparently having this mask settings in the trigger() does not look very
> common, so I believe a line of comments would help a lot.
>
> You may add my Acked-by in your next version:
>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> Thanks
>
> > + regmap_update_bits(esai_priv->regmap,
> REG_ESAI_xSMB(tx),
> > + ESAI_xSMB_xS_MASK,
> ESAI_xSMB_xS(mask));
> > + regmap_update_bits(esai_priv->regmap,
> REG_ESAI_xSMA(tx),
> > + ESAI_xSMA_xS_MASK,
> ESAI_xSMA_xS(mask));
> > +
> > break;
> > case SNDRV_PCM_TRIGGER_SUSPEND:
> > case SNDRV_PCM_TRIGGER_STOP:
> > case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
> > tx ? ESAI_xCR_TE_MASK :
> ESAI_xCR_RE_MASK, 0);
> > + regmap_update_bits(esai_priv->regmap,
> REG_ESAI_xSMA(tx),
> > + ESAI_xSMA_xS_MASK, 0);
> > + regmap_update_bits(esai_priv->regmap,
> REG_ESAI_xSMB(tx),
> > + ESAI_xSMB_xS_MASK, 0);
> >
^ permalink raw reply
* [PATCH 4/5] ocxl: Remove superfluous 'extern' from headers
From: Alastair D'Silva @ 2019-02-27 4:57 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The 'extern' keyword adds no value here.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/ocxl_internal.h | 54 +++++++++++++++----------------
include/misc/ocxl.h | 36 ++++++++++-----------
2 files changed, 44 insertions(+), 46 deletions(-)
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index a32f2151029f..321b29e77f45 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -16,7 +16,6 @@
extern struct pci_driver ocxl_pci_driver;
-
struct ocxl_fn {
struct device dev;
int bar_used[3];
@@ -92,41 +91,40 @@ struct ocxl_process_element {
__be32 software_state;
};
+struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu);
+void ocxl_afu_put(struct ocxl_afu *afu);
-extern struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu);
-extern void ocxl_afu_put(struct ocxl_afu *afu);
-
-extern int ocxl_create_cdev(struct ocxl_afu *afu);
-extern void ocxl_destroy_cdev(struct ocxl_afu *afu);
-extern int ocxl_register_afu(struct ocxl_afu *afu);
-extern void ocxl_unregister_afu(struct ocxl_afu *afu);
+int ocxl_create_cdev(struct ocxl_afu *afu);
+void ocxl_destroy_cdev(struct ocxl_afu *afu);
+int ocxl_register_afu(struct ocxl_afu *afu);
+void ocxl_unregister_afu(struct ocxl_afu *afu);
-extern int ocxl_file_init(void);
-extern void ocxl_file_exit(void);
+int ocxl_file_init(void);
+void ocxl_file_exit(void);
-extern int ocxl_pasid_afu_alloc(struct ocxl_fn *fn, u32 size);
-extern void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
-extern int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
-extern void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+int ocxl_pasid_afu_alloc(struct ocxl_fn *fn, u32 size);
+void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
+void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
-extern struct ocxl_context *ocxl_context_alloc(void);
-extern int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
+struct ocxl_context *ocxl_context_alloc(void);
+int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
struct address_space *mapping);
-extern int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
-extern int ocxl_context_mmap(struct ocxl_context *ctx,
+int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
+int ocxl_context_mmap(struct ocxl_context *ctx,
struct vm_area_struct *vma);
-extern int ocxl_context_detach(struct ocxl_context *ctx);
-extern void ocxl_context_detach_all(struct ocxl_afu *afu);
-extern void ocxl_context_free(struct ocxl_context *ctx);
+int ocxl_context_detach(struct ocxl_context *ctx);
+void ocxl_context_detach_all(struct ocxl_afu *afu);
+void ocxl_context_free(struct ocxl_context *ctx);
-extern int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
-extern void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
+int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
+void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
-extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
-extern int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
-extern void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
-extern int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
+int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
+void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
int eventfd);
-extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
#endif /* _OCXL_INTERNAL_H_ */
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 9ff6ddc28e22..4544573cc93c 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -53,7 +53,7 @@ struct ocxl_fn_config {
* Read the configuration space of a function and fill in a
* ocxl_fn_config structure with all the function details
*/
-extern int ocxl_config_read_function(struct pci_dev *dev,
+int ocxl_config_read_function(struct pci_dev *dev,
struct ocxl_fn_config *fn);
/*
@@ -62,14 +62,14 @@ extern int ocxl_config_read_function(struct pci_dev *dev,
* AFU indexes can be sparse, so a driver should check all indexes up
* to the maximum found in the function description
*/
-extern int ocxl_config_check_afu_index(struct pci_dev *dev,
+int ocxl_config_check_afu_index(struct pci_dev *dev,
struct ocxl_fn_config *fn, int afu_idx);
/*
* Read the configuration space of a function for the AFU specified by
* the index 'afu_idx'. Fills in a ocxl_afu_config structure
*/
-extern int ocxl_config_read_afu(struct pci_dev *dev,
+int ocxl_config_read_afu(struct pci_dev *dev,
struct ocxl_fn_config *fn,
struct ocxl_afu_config *afu,
u8 afu_idx);
@@ -77,7 +77,7 @@ extern int ocxl_config_read_afu(struct pci_dev *dev,
/*
* Get the max PASID value that can be used by the function
*/
-extern int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
/*
* Tell an AFU, by writing in the configuration space, the PASIDs that
@@ -87,7 +87,7 @@ extern int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
* 'afu_control_offset' is the offset of the AFU control DVSEC which
* can be found in the function configuration
*/
-extern void ocxl_config_set_afu_pasid(struct pci_dev *dev,
+void ocxl_config_set_afu_pasid(struct pci_dev *dev,
int afu_control_offset,
int pasid_base, u32 pasid_count_log);
@@ -98,7 +98,7 @@ extern void ocxl_config_set_afu_pasid(struct pci_dev *dev,
* 'supported' is the total number of actags desired by all the AFUs
* of the function.
*/
-extern int ocxl_config_get_actag_info(struct pci_dev *dev,
+int ocxl_config_get_actag_info(struct pci_dev *dev,
u16 *base, u16 *enabled, u16 *supported);
/*
@@ -108,7 +108,7 @@ extern int ocxl_config_get_actag_info(struct pci_dev *dev,
* 'func_offset' is the offset of the Function DVSEC that can found in
* the function configuration
*/
-extern void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
+void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
u32 actag_base, u32 actag_count);
/*
@@ -118,7 +118,7 @@ extern void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
* 'afu_control_offset' is the offset of the AFU control DVSEC for the
* desired AFU. It can be found in the AFU configuration
*/
-extern void ocxl_config_set_afu_actag(struct pci_dev *dev,
+void ocxl_config_set_afu_actag(struct pci_dev *dev,
int afu_control_offset,
int actag_base, int actag_count);
@@ -128,7 +128,7 @@ extern void ocxl_config_set_afu_actag(struct pci_dev *dev,
* 'afu_control_offset' is the offset of the AFU control DVSEC for the
* desired AFU. It can be found in the AFU configuration
*/
-extern void ocxl_config_set_afu_state(struct pci_dev *dev,
+void ocxl_config_set_afu_state(struct pci_dev *dev,
int afu_control_offset, int enable);
/*
@@ -139,7 +139,7 @@ extern void ocxl_config_set_afu_state(struct pci_dev *dev,
* between the host and device, and set the Transaction Layer on both
* accordingly.
*/
-extern int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
+int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
/*
* Request an AFU to terminate a PASID.
@@ -152,7 +152,7 @@ extern int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
* 'afu_control_offset' is the offset of the AFU control DVSEC for the
* desired AFU. It can be found in the AFU configuration
*/
-extern int ocxl_config_terminate_pasid(struct pci_dev *dev,
+int ocxl_config_terminate_pasid(struct pci_dev *dev,
int afu_control_offset, int pasid);
/*
@@ -165,13 +165,13 @@ extern int ocxl_config_terminate_pasid(struct pci_dev *dev,
* Returns a 'link handle' that should be used for further calls for
* the link
*/
-extern int ocxl_link_setup(struct pci_dev *dev, int PE_mask,
+int ocxl_link_setup(struct pci_dev *dev, int PE_mask,
void **link_handle);
/*
* Remove the association between the function and its link.
*/
-extern void ocxl_link_release(struct pci_dev *dev, void *link_handle);
+void ocxl_link_release(struct pci_dev *dev, void *link_handle);
/*
* Add a Process Element to the Shared Process Area for a link.
@@ -183,7 +183,7 @@ extern void ocxl_link_release(struct pci_dev *dev, void *link_handle);
* 'xsl_err_data' is an argument passed to the above callback, if
* defined
*/
-extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
+int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
u64 amr, struct mm_struct *mm,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data);
@@ -195,12 +195,12 @@ extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
* pasid: the PASID for the AFU context
* tid: the new thread id for the process element
*/
-extern int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
/*
* Remove a Process Element from the Shared Process Area for a link
*/
-extern int ocxl_link_remove_pe(void *link_handle, int pasid);
+int ocxl_link_remove_pe(void *link_handle, int pasid);
/*
* Allocate an AFU interrupt associated to the link.
@@ -212,12 +212,12 @@ extern int ocxl_link_remove_pe(void *link_handle, int pasid);
* interrupt. It is an MMIO address which needs to be remapped (one
* page).
*/
-extern int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
+int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
u64 *obj_handle);
/*
* Free a previously allocated AFU interrupt
*/
-extern void ocxl_link_free_irq(void *link_handle, int hw_irq);
+void ocxl_link_free_irq(void *link_handle, int hw_irq);
#endif /* _MISC_OCXL_H_ */
--
2.20.1
^ permalink raw reply related
* [PATCH 0/5] ocxl: OpenCAPI Cleanup
From: Alastair D'Silva @ 2019-02-27 4:57 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
From: Alastair D'Silva <alastair@d-silva.org>
Some minor cleanups for the OpenCAPI driver as a prerequisite
for an ocxl driver refactoring to allow the driver core to
be utilised by external drivers.
Alastair D'Silva (5):
ocxl: Rename struct link to ocxl_link
ocxl: Clean up printf formats
ocxl: read_pasid never returns an error, so make it void
ocxl: Remove superfluous 'extern' from headers
ocxl: Remove some unused exported symbols
drivers/misc/ocxl/config.c | 17 ++----
drivers/misc/ocxl/context.c | 2 +-
drivers/misc/ocxl/file.c | 2 +-
drivers/misc/ocxl/link.c | 36 ++++++-------
drivers/misc/ocxl/ocxl_internal.h | 86 +++++++++++++++++++------------
drivers/misc/ocxl/trace.h | 10 ++--
include/misc/ocxl.h | 53 ++++++-------------
7 files changed, 99 insertions(+), 107 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 1/5] ocxl: Rename struct link to ocxl_link
From: Alastair D'Silva @ 2019-02-27 4:57 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The term 'link' is ambiguous (especially when the struct is used for a
list), so rename it for clarity.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
drivers/misc/ocxl/file.c | 2 +-
drivers/misc/ocxl/link.c | 36 ++++++++++++++++++------------------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index e6a607488f8a..16eb8a60d5c7 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
if (status == ATTACHED) {
int rc;
- struct link *link = ctx->afu->fn->link;
+ void *link = ctx->afu->fn->link;
rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
if (rc)
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index d50b861d7e57..8d2690a1a9de 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -76,7 +76,7 @@ struct spa {
* limited number of opencapi slots on a system and lookup is only
* done when the device is probed
*/
-struct link {
+struct ocxl_link {
struct list_head list;
struct kref ref;
int domain;
@@ -179,7 +179,7 @@ static void xsl_fault_handler_bh(struct work_struct *fault_work)
static irqreturn_t xsl_fault_handler(int irq, void *data)
{
- struct link *link = (struct link *) data;
+ struct ocxl_link *link = (struct ocxl_link *) data;
struct spa *spa = link->spa;
u64 dsisr, dar, pe_handle;
struct pe_data *pe_data;
@@ -256,7 +256,7 @@ static int map_irq_registers(struct pci_dev *dev, struct spa *spa)
&spa->reg_tfc, &spa->reg_pe_handle);
}
-static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
+static int setup_xsl_irq(struct pci_dev *dev, struct ocxl_link *link)
{
struct spa *spa = link->spa;
int rc;
@@ -311,7 +311,7 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
return rc;
}
-static void release_xsl_irq(struct link *link)
+static void release_xsl_irq(struct ocxl_link *link)
{
struct spa *spa = link->spa;
@@ -323,7 +323,7 @@ static void release_xsl_irq(struct link *link)
unmap_irq_registers(spa);
}
-static int alloc_spa(struct pci_dev *dev, struct link *link)
+static int alloc_spa(struct pci_dev *dev, struct ocxl_link *link)
{
struct spa *spa;
@@ -350,7 +350,7 @@ static int alloc_spa(struct pci_dev *dev, struct link *link)
return 0;
}
-static void free_spa(struct link *link)
+static void free_spa(struct ocxl_link *link)
{
struct spa *spa = link->spa;
@@ -364,12 +364,12 @@ static void free_spa(struct link *link)
}
}
-static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
+static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_link)
{
- struct link *link;
+ struct ocxl_link *link;
int rc;
- link = kzalloc(sizeof(struct link), GFP_KERNEL);
+ link = kzalloc(sizeof(struct ocxl_link), GFP_KERNEL);
if (!link)
return -ENOMEM;
@@ -405,7 +405,7 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
return rc;
}
-static void free_link(struct link *link)
+static void free_link(struct ocxl_link *link)
{
release_xsl_irq(link);
free_spa(link);
@@ -415,7 +415,7 @@ static void free_link(struct link *link)
int ocxl_link_setup(struct pci_dev *dev, int PE_mask, void **link_handle)
{
int rc = 0;
- struct link *link;
+ struct ocxl_link *link;
mutex_lock(&links_list_lock);
list_for_each_entry(link, &links_list, list) {
@@ -442,7 +442,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_setup);
static void release_xsl(struct kref *ref)
{
- struct link *link = container_of(ref, struct link, ref);
+ struct ocxl_link *link = container_of(ref, struct ocxl_link, ref);
list_del(&link->list);
/* call platform code before releasing data */
@@ -452,7 +452,7 @@ static void release_xsl(struct kref *ref)
void ocxl_link_release(struct pci_dev *dev, void *link_handle)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
mutex_lock(&links_list_lock);
kref_put(&link->ref, release_xsl);
@@ -488,7 +488,7 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
struct spa *spa = link->spa;
struct ocxl_process_element *pe;
int pe_handle, rc = 0;
@@ -558,7 +558,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_add_pe);
int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
struct spa *spa = link->spa;
struct ocxl_process_element *pe;
int pe_handle, rc;
@@ -594,7 +594,7 @@ int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
int ocxl_link_remove_pe(void *link_handle, int pasid)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
struct spa *spa = link->spa;
struct ocxl_process_element *pe;
struct pe_data *pe_data;
@@ -666,7 +666,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_remove_pe);
int ocxl_link_irq_alloc(void *link_handle, int *hw_irq, u64 *trigger_addr)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
int rc, irq;
u64 addr;
@@ -687,7 +687,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_irq_alloc);
void ocxl_link_free_irq(void *link_handle, int hw_irq)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
pnv_ocxl_free_xive_irq(hw_irq);
atomic_inc(&link->irq_available);
--
2.20.1
^ permalink raw reply related
* [PATCH 3/5] ocxl: read_pasid never returns an error, so make it void
From: Alastair D'Silva @ 2019-02-27 4:57 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
No need for a return value in read_pasid as it only returns 0.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
drivers/misc/ocxl/config.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 0ee7856b033d..026ac2ac4f9c 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -68,7 +68,7 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
return 0;
}
-static int read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
+static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
{
u16 val;
int pos;
@@ -89,7 +89,6 @@ static int read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
out:
dev_dbg(&dev->dev, "PASID capability:\n");
dev_dbg(&dev->dev, " Max PASID log = %d\n", fn->max_pasid_log);
- return 0;
}
static int read_dvsec_tl(struct pci_dev *dev, struct ocxl_fn_config *fn)
@@ -205,11 +204,7 @@ int ocxl_config_read_function(struct pci_dev *dev, struct ocxl_fn_config *fn)
{
int rc;
- rc = read_pasid(dev, fn);
- if (rc) {
- dev_err(&dev->dev, "Invalid PASID configuration: %d\n", rc);
- return -ENODEV;
- }
+ read_pasid(dev, fn);
rc = read_dvsec_tl(dev, fn);
if (rc) {
--
2.20.1
^ permalink raw reply related
* [PATCH 2/5] ocxl: Clean up printf formats
From: Alastair D'Silva @ 2019-02-27 4:57 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Use %# instead of using a literal '0x'
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/config.c | 6 +++---
drivers/misc/ocxl/context.c | 2 +-
drivers/misc/ocxl/trace.h | 10 +++++-----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 8f2c5d8bd2ee..0ee7856b033d 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -178,9 +178,9 @@ static int read_dvsec_vendor(struct pci_dev *dev)
pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx);
dev_dbg(&dev->dev, "Vendor specific DVSEC:\n");
- dev_dbg(&dev->dev, " CFG version = 0x%x\n", cfg);
- dev_dbg(&dev->dev, " TLX version = 0x%x\n", tlx);
- dev_dbg(&dev->dev, " DLX version = 0x%x\n", dlx);
+ dev_dbg(&dev->dev, " CFG version = %#x\n", cfg);
+ dev_dbg(&dev->dev, " TLX version = %#x\n", tlx);
+ dev_dbg(&dev->dev, " DLX version = %#x\n", dlx);
return 0;
}
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index c10a940e3b38..3498a0199bde 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -134,7 +134,7 @@ static vm_fault_t ocxl_mmap_fault(struct vm_fault *vmf)
vm_fault_t ret;
offset = vmf->pgoff << PAGE_SHIFT;
- pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
+ pr_debug("%s: pasid %d address %#lx offset %#llx\n", __func__,
ctx->pasid, vmf->address, offset);
if (offset < ctx->afu->irq_base_offset)
diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
index bcb7ff330c1e..8d2f53812edd 100644
--- a/drivers/misc/ocxl/trace.h
+++ b/drivers/misc/ocxl/trace.h
@@ -28,7 +28,7 @@ DECLARE_EVENT_CLASS(ocxl_context,
__entry->tidr = tidr;
),
- TP_printk("linux pid=%d spa=0x%p pasid=0x%x pidr=0x%x tidr=0x%x",
+ TP_printk("linux pid=%d spa=%p pasid=%#x pidr=%#x tidr=%#x",
__entry->pid,
__entry->spa,
__entry->pasid,
@@ -61,7 +61,7 @@ TRACE_EVENT(ocxl_terminate_pasid,
__entry->rc = rc;
),
- TP_printk("pasid=0x%x rc=%d",
+ TP_printk("pasid=%#x rc=%d",
__entry->pasid,
__entry->rc
)
@@ -87,7 +87,7 @@ DECLARE_EVENT_CLASS(ocxl_fault_handler,
__entry->tfc = tfc;
),
- TP_printk("spa=%p pe=0x%llx dsisr=0x%llx dar=0x%llx tfc=0x%llx",
+ TP_printk("spa=%p pe=%#llx dsisr=%#llx dar=%#llx tfc=%#llx",
__entry->spa,
__entry->pe,
__entry->dsisr,
@@ -127,7 +127,7 @@ TRACE_EVENT(ocxl_afu_irq_alloc,
__entry->irq_offset = irq_offset;
),
- TP_printk("pasid=0x%x irq_id=%d virq=%u hw_irq=%d irq_offset=0x%llx",
+ TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d irq_offset=0x%llx",
__entry->pasid,
__entry->irq_id,
__entry->virq,
@@ -150,7 +150,7 @@ TRACE_EVENT(ocxl_afu_irq_free,
__entry->irq_id = irq_id;
),
- TP_printk("pasid=0x%x irq_id=%d",
+ TP_printk("pasid=%#x irq_id=%d",
__entry->pasid,
__entry->irq_id
)
--
2.20.1
^ permalink raw reply related
* [PATCH 5/5] ocxl: Remove some unused exported symbols
From: Alastair D'Silva @ 2019-02-27 4:57 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190227045741.21412-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Remove some unused exported symbols.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/config.c | 2 --
drivers/misc/ocxl/ocxl_internal.h | 26 +++++++++++++++++++++++++-
include/misc/ocxl.h | 23 -----------------------
3 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 026ac2ac4f9c..c90c2e4875bf 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -299,7 +299,6 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
}
return 1;
}
-EXPORT_SYMBOL_GPL(ocxl_config_check_afu_index);
static int read_afu_name(struct pci_dev *dev, struct ocxl_fn_config *fn,
struct ocxl_afu_config *afu)
@@ -535,7 +534,6 @@ int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count)
{
return pnv_ocxl_get_pasid_count(dev, count);
}
-EXPORT_SYMBOL_GPL(ocxl_config_get_pasid_info);
void ocxl_config_set_afu_pasid(struct pci_dev *dev, int pos, int pasid_base,
u32 pasid_count_log)
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 321b29e77f45..cd5a1e3cc950 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -107,10 +107,34 @@ void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+/*
+ * Get the max PASID value that can be used by the function
+ */
+int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+
+int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
+
+/*
+ * Check if an AFU index is valid for the given function.
+ *
+ * AFU indexes can be sparse, so a driver should check all indexes up
+ * to the maximum found in the function description
+ */
+int ocxl_config_check_afu_index(struct pci_dev *dev,
+ struct ocxl_fn_config *fn, int afu_idx);
+
+/**
+ * Update values within a Process Element
+ *
+ * link_handle: the link handle associated with the process element
+ * pasid: the PASID for the AFU context
+ * tid: the new thread id for the process element
+ */
+int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+
struct ocxl_context *ocxl_context_alloc(void);
int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
struct address_space *mapping);
-int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
int ocxl_context_mmap(struct ocxl_context *ctx,
struct vm_area_struct *vma);
int ocxl_context_detach(struct ocxl_context *ctx);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 4544573cc93c..9530d3be1b30 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -56,15 +56,6 @@ struct ocxl_fn_config {
int ocxl_config_read_function(struct pci_dev *dev,
struct ocxl_fn_config *fn);
-/*
- * Check if an AFU index is valid for the given function.
- *
- * AFU indexes can be sparse, so a driver should check all indexes up
- * to the maximum found in the function description
- */
-int ocxl_config_check_afu_index(struct pci_dev *dev,
- struct ocxl_fn_config *fn, int afu_idx);
-
/*
* Read the configuration space of a function for the AFU specified by
* the index 'afu_idx'. Fills in a ocxl_afu_config structure
@@ -74,11 +65,6 @@ int ocxl_config_read_afu(struct pci_dev *dev,
struct ocxl_afu_config *afu,
u8 afu_idx);
-/*
- * Get the max PASID value that can be used by the function
- */
-int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
-
/*
* Tell an AFU, by writing in the configuration space, the PASIDs that
* it can use. Range starts at 'pasid_base' and its size is a multiple
@@ -188,15 +174,6 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data);
-/**
- * Update values within a Process Element
- *
- * link_handle: the link handle associated with the process element
- * pasid: the PASID for the AFU context
- * tid: the new thread id for the process element
- */
-int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
-
/*
* Remove a Process Element from the Shared Process Area for a link
*/
--
2.20.1
^ permalink raw reply related
* [PATCH 6/8] PCI: iproc: fix a leaked reference by adding missing of_node_put
From: Wen Yang @ 2019-02-27 4:40 UTC (permalink / raw)
To: benh
Cc: heiko, hayashi.kunihiko, linux-pci, shawn.lin, minghuan.Lian,
paulus, thomas.petazzoni, Wen Yang, ryder.lee, kishon,
linux-rockchip, bcm-kernel-feedback-list, linux-arm-kernel,
wang.yi59, rjui, lorenzo.pieralisi, linux-mediatek, tyreld,
matthias.bgg, linux-omap, mingkai.hu, roy.zang, sbranden,
yamada.masahiro, linuxppc-dev, linux-kernel, bhelgaas
In-Reply-To: <1551242443-27300-1-git-send-email-wen.yang99@zte.com.cn>
The call to of_parse_phandle returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
iproc_msi_init also calls of_node_get to increase refcount
(proc_msi_init -> iproc_msi_alloc_domains -> pci_msi_create_irq_domain
-> msi_create_irq_domain -> irq_domain_create_linear -> __irq_domain_add),
so irq_domain will not be affected when it is released.
Detected by coccinelle with the following warnings:
./drivers/pci/controller/pcie-iproc.c:1323:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 1299, but without a corresponding object release within this function.
./drivers/pci/controller/pcie-iproc.c:1330:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 1299, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/pci/controller/pcie-iproc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
index c20fd6b..9998c5c 100644
--- a/drivers/pci/controller/pcie-iproc.c
+++ b/drivers/pci/controller/pcie-iproc.c
@@ -1320,14 +1320,18 @@ static int iproc_pcie_msi_enable(struct iproc_pcie *pcie)
if (pcie->need_msi_steer) {
ret = iproc_pcie_msi_steer(pcie, msi_node);
if (ret)
- return ret;
+ goto out_put_node;
}
/*
* If another MSI controller is being used, the call below should fail
* but that is okay
*/
- return iproc_msi_init(pcie, msi_node);
+ ret = iproc_msi_init(pcie, msi_node);
+
+out_put_node:
+ of_node_put(msi_node);
+ return ret;
}
static void iproc_pcie_msi_disable(struct iproc_pcie *pcie)
--
2.9.5
^ permalink raw reply related
* [PATCH 7/8] PCI: mediatek: fix a leaked reference by adding missing of_node_put
From: Wen Yang @ 2019-02-27 4:40 UTC (permalink / raw)
To: benh
Cc: heiko, hayashi.kunihiko, linux-pci, shawn.lin, minghuan.Lian,
paulus, thomas.petazzoni, Wen Yang, ryder.lee, kishon,
linux-rockchip, bcm-kernel-feedback-list, linux-arm-kernel,
wang.yi59, rjui, lorenzo.pieralisi, linux-mediatek, tyreld,
matthias.bgg, linux-omap, mingkai.hu, roy.zang, sbranden,
yamada.masahiro, linuxppc-dev, linux-kernel, bhelgaas
In-Reply-To: <1551242443-27300-1-git-send-email-wen.yang99@zte.com.cn>
The call to of_get_next_child returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
irq_domain_add_linear also calls of_node_get to increase refcount,
so irq_domain will not be affected when it is released.
Detected by coccinelle with the following warnings:
./drivers/pci/controller/pcie-mediatek.c:577:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 567, but without a corresponding object release within this function.
./drivers/pci/controller/pcie-mediatek.c:583:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 567, but without a corresponding object release within this function.
./drivers/pci/controller/pcie-mediatek.c:586:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 567, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Ryder Lee <ryder.lee@mediatek.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-mediatek@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
drivers/pci/controller/pcie-mediatek.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 55e471c..e91716a 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -572,6 +572,7 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
port->irq_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
&intx_domain_ops, port);
+ of_node_put(pcie_intc_node);
if (!port->irq_domain) {
dev_err(dev, "failed to get INTx IRQ domain\n");
return -ENODEV;
--
2.9.5
^ permalink raw reply related
* [PATCH 8/8] PCI: rpadlpar: fix a leaked reference by adding missing of_node_put
From: Wen Yang @ 2019-02-27 4:40 UTC (permalink / raw)
To: benh
Cc: heiko, hayashi.kunihiko, linux-pci, shawn.lin, minghuan.Lian,
paulus, thomas.petazzoni, Wen Yang, ryder.lee, kishon,
linux-rockchip, bcm-kernel-feedback-list, linux-arm-kernel,
wang.yi59, rjui, lorenzo.pieralisi, linux-mediatek, tyreld,
matthias.bgg, linux-omap, mingkai.hu, roy.zang, sbranden,
yamada.masahiro, linuxppc-dev, linux-kernel, bhelgaas
In-Reply-To: <1551242443-27300-1-git-send-email-wen.yang99@zte.com.cn>
The call to of_find_node_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/pci/hotplug/rpadlpar_core.c:55:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 42, but without a corresponding object release within this function
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
---
drivers/pci/hotplug/rpadlpar_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index e2356a9..f3f42ff 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -52,6 +52,7 @@ static struct device_node *find_vio_slot_node(char *drc_name)
break;
}
+ of_node_put(parent);
return dn;
}
--
2.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox