* Re: [PATCH v6 1/7] kvmppc: Driver to manage pages of secure guest
From: Christoph Hellwig @ 2019-08-10 10:58 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, linuxppc-dev, hch
In-Reply-To: <20190809084108.30343-2-bharata@linux.ibm.com>
> +#ifdef CONFIG_PPC_UV
> +extern unsigned long kvmppc_h_svm_page_in(struct kvm *kvm,
> + unsigned long gra,
> + unsigned long flags,
> + unsigned long page_shift);
> +extern unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
> + unsigned long gra,
> + unsigned long flags,
> + unsigned long page_shift);
No need for externs on function declarations.
> +struct kvmppc_devm_device {
> + struct device dev;
> + dev_t devt;
> + struct dev_pagemap pagemap;
> + unsigned long pfn_first, pfn_last;
> + unsigned long *pfn_bitmap;
> +};
We shouldn't really need this conaining structucture given that there
is only a single global instance of it anyway.
> +struct kvmppc_devm_copy_args {
> + unsigned long *rmap;
> + unsigned int lpid;
> + unsigned long gpa;
> + unsigned long page_shift;
> +};
Do we really need this args structure? It is just used in a single
function call where passing the arguments might be cleaner.
> +static void kvmppc_devm_put_page(struct page *page)
> +{
> + unsigned long pfn = page_to_pfn(page);
> + unsigned long flags;
> + struct kvmppc_devm_page_pvt *pvt;
> +
> + spin_lock_irqsave(&kvmppc_devm_lock, flags);
> + pvt = (struct kvmppc_devm_page_pvt *)page->zone_device_data;
No need for the cast.
> + page->zone_device_data = 0;
This should be NULL.
> +
> + bitmap_clear(kvmppc_devm.pfn_bitmap,
> + pfn - kvmppc_devm.pfn_first, 1);
> + *(pvt->rmap) = 0;
No need for the braces.
> + dpage = alloc_page_vma(GFP_HIGHUSER, mig->vma, mig->start);
> + if (!dpage)
> + return -EINVAL;
> + lock_page(dpage);
> + pvt = (struct kvmppc_devm_page_pvt *)spage->zone_device_data;
No need for the cast here.
> +static void kvmppc_devm_page_free(struct page *page)
> +{
> + kvmppc_devm_put_page(page);
> +}
This seems to be the only caller of kvmppc_devm_put_page, any reason
not to just merge the two functions?
> +static int kvmppc_devm_pages_init(void)
> +{
> + unsigned long nr_pfns = kvmppc_devm.pfn_last -
> + kvmppc_devm.pfn_first;
> +
> + kvmppc_devm.pfn_bitmap = kcalloc(BITS_TO_LONGS(nr_pfns),
> + sizeof(unsigned long), GFP_KERNEL);
> + if (!kvmppc_devm.pfn_bitmap)
> + return -ENOMEM;
> +
> + spin_lock_init(&kvmppc_devm_lock);
Just initialize the spinlock using DEFINE_SPINLOCK() at compile time.
The rest of the function is so trivial that it can be inlined into the
caller.
Also is kvmppc_devm_lock such a good name? This mostly just protects
the allocation bitmap, so reflecting that in the name might be a good
idea.
> +int kvmppc_devm_init(void)
> +{
> + int ret = 0;
> + unsigned long size;
> + struct resource *res;
> + void *addr;
> +
> + size = kvmppc_get_secmem_size();
> + if (!size) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + ret = alloc_chrdev_region(&kvmppc_devm.devt, 0, 1,
> + "kvmppc-devm");
> + if (ret)
> + goto out;
> +
> + dev_set_name(&kvmppc_devm.dev, "kvmppc_devm_device%d", 0);
> + kvmppc_devm.dev.release = kvmppc_devm_release;
> + device_initialize(&kvmppc_devm.dev);
> + res = devm_request_free_mem_region(&kvmppc_devm.dev,
> + &iomem_resource, size);
> + if (IS_ERR(res)) {
> + ret = PTR_ERR(res);
> + goto out_unregister;
> + }
> +
> + kvmppc_devm.pagemap.type = MEMORY_DEVICE_PRIVATE;
> + kvmppc_devm.pagemap.res = *res;
> + kvmppc_devm.pagemap.ops = &kvmppc_devm_ops;
> + addr = devm_memremap_pages(&kvmppc_devm.dev, &kvmppc_devm.pagemap);
> + if (IS_ERR(addr)) {
> + ret = PTR_ERR(addr);
> + goto out_unregister;
> + }
It seems a little silly to allocate a struct device just so that we can
pass it to devm_request_free_mem_region and devm_memremap_pages. I think
we should just create non-dev_ versions of those as well.
> +
> + kvmppc_devm.pfn_first = res->start >> PAGE_SHIFT;
> + kvmppc_devm.pfn_last = kvmppc_devm.pfn_first +
> + (resource_size(res) >> PAGE_SHIFT);
pfn_last is only used to calculat a size. Also I think we could
just use kvmppc_devm.pagemap.res directly instead of copying these
values out. the ">> PAGE_SHIFT" is cheap enough.
^ permalink raw reply
* Re: [PATCH v6 1/7] kvmppc: Driver to manage pages of secure guest
From: Bharata B Rao @ 2019-08-10 14:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, sukadev, linuxppc-dev
In-Reply-To: <20190810105819.GA26030@lst.de>
On Sat, Aug 10, 2019 at 12:58:19PM +0200, Christoph Hellwig wrote:
>
> > +int kvmppc_devm_init(void)
> > +{
> > + int ret = 0;
> > + unsigned long size;
> > + struct resource *res;
> > + void *addr;
> > +
> > + size = kvmppc_get_secmem_size();
> > + if (!size) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + ret = alloc_chrdev_region(&kvmppc_devm.devt, 0, 1,
> > + "kvmppc-devm");
> > + if (ret)
> > + goto out;
> > +
> > + dev_set_name(&kvmppc_devm.dev, "kvmppc_devm_device%d", 0);
> > + kvmppc_devm.dev.release = kvmppc_devm_release;
> > + device_initialize(&kvmppc_devm.dev);
> > + res = devm_request_free_mem_region(&kvmppc_devm.dev,
> > + &iomem_resource, size);
> > + if (IS_ERR(res)) {
> > + ret = PTR_ERR(res);
> > + goto out_unregister;
> > + }
> > +
> > + kvmppc_devm.pagemap.type = MEMORY_DEVICE_PRIVATE;
> > + kvmppc_devm.pagemap.res = *res;
> > + kvmppc_devm.pagemap.ops = &kvmppc_devm_ops;
> > + addr = devm_memremap_pages(&kvmppc_devm.dev, &kvmppc_devm.pagemap);
> > + if (IS_ERR(addr)) {
> > + ret = PTR_ERR(addr);
> > + goto out_unregister;
> > + }
>
> It seems a little silly to allocate a struct device just so that we can
> pass it to devm_request_free_mem_region and devm_memremap_pages. I think
> we should just create non-dev_ versions of those as well.
There is no reason for us to create a device really. If non-dev versions
of the above two routines are present, I can switch.
I will take care of the rest of your comments. Thanks for the review.
Regards,
Bharata.
^ permalink raw reply
* [Bug 204479] KASAN hit at modprobe zram
From: bugzilla-daemon @ 2019-08-10 14:42 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-204479-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=204479
--- Comment #16 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 284309
--> https://bugzilla.kernel.org/attachment.cgi?id=284309&action=edit
dmesg (kernel 5.3-rc3 + debug patch + shadow patch + parallel patch, PowerMac
G4 DP)
Also tested your powerpc-kasan-fix-parallele-loading-of-modules.diff now which
seems to work fine! dmesg from the G4 DP with CONFIG_SMP back on is almost
identical to non-smp kernel dmesg.
raid6 pq reliably oopses. Probably the 1st issue revealed by ppc32 KASAN. ;)
Loading the radeon module at boot still freezes the G4. modprobing it later on
works, without any special dmesg output, switching display over from Offb to
radeonfb.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.3-4 tag
From: Linus Torvalds @ 2019-08-10 17:21 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Linux List Kernel Mailing
In-Reply-To: <87imr5s522.fsf@concordia.ellerman.id.au>
On Sat, Aug 10, 2019 at 3:11 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Just one fix, a revert of a commit that was meant to be a minor improvement to
> some inline asm, but ended up having no real benefit with GCC and broke booting
> 32-bit machines when using Clang.
Pulled, but whenever there are possible subtle compiler issues I get
nervous, and wonder if the problem was reported to the clang guys?
In particular, if the kernel change was technically correct, maybe
somebody else comes along in a few years and tries the same, and then
it's another odd "why doesn't this work for person X when it works
just fine for me"..
Linus
^ permalink raw reply
* [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Sourabh Jain @ 2019-08-10 17:59 UTC (permalink / raw)
To: mpe
Cc: corbet, mahesh, linux-doc, linux-kernel, Sourabh Jain,
linuxppc-dev, hbathini
Add a sys interface to allow querying the memory reserved by
fadump for saving the crash dump.
Add an ABI doc entry for new sysfs interface.
- /sys/kernel/fadump_mem_reserved
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
Changelog:
v1 -> v2:
- Added ABI doc for new sysfs interface.
v2 -> v3:
- Updated the ABI documentation.
---
Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
Documentation/powerpc/firmware-assisted-dump.rst | 5 +++++
arch/powerpc/kernel/fadump.c | 14 ++++++++++++++
3 files changed, 25 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-kernel-fadump
diff --git a/Documentation/ABI/testing/sysfs-kernel-fadump b/Documentation/ABI/testing/sysfs-kernel-fadump
new file mode 100644
index 000000000000..ec034939475b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-kernel-fadump
@@ -0,0 +1,6 @@
+What: /sys/kernel/fadump_mem_reserved
+Date: August 2019
+Contact: linuxppc-dev@lists.ozlabs.org
+Description: read only
+ Provide information about the amount of memory
+ reserved by fadump to save the crash dump.
diff --git a/Documentation/powerpc/firmware-assisted-dump.rst b/Documentation/powerpc/firmware-assisted-dump.rst
index 9ca12830a48e..a5dfb20d4dc3 100644
--- a/Documentation/powerpc/firmware-assisted-dump.rst
+++ b/Documentation/powerpc/firmware-assisted-dump.rst
@@ -222,6 +222,11 @@ Here is the list of files under kernel sysfs:
be handled and vmcore will not be captured. This interface can be
easily integrated with kdump service start/stop.
+ /sys/kernel/fadump_mem_reserved
+
+ This is used to display the memory reserved by fadump for saving the
+ crash dump.
+
/sys/kernel/fadump_release_mem
This file is available only when fadump is active during
second kernel. This is used to release the reserved memory
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 4eab97292cc2..cd373d1d4b82 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1514,6 +1514,13 @@ static ssize_t fadump_enabled_show(struct kobject *kobj,
return sprintf(buf, "%d\n", fw_dump.fadump_enabled);
}
+static ssize_t fadump_mem_reserved_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%ld\n", fw_dump.reserve_dump_area_size);
+}
+
static ssize_t fadump_register_show(struct kobject *kobj,
struct kobj_attribute *attr,
char *buf)
@@ -1632,6 +1639,9 @@ static struct kobj_attribute fadump_attr = __ATTR(fadump_enabled,
static struct kobj_attribute fadump_register_attr = __ATTR(fadump_registered,
0644, fadump_register_show,
fadump_register_store);
+static struct kobj_attribute fadump_mem_reserved_attr =
+ __ATTR(fadump_mem_reserved, 0444,
+ fadump_mem_reserved_show, NULL);
DEFINE_SHOW_ATTRIBUTE(fadump_region);
@@ -1663,6 +1673,10 @@ static void fadump_init_files(void)
printk(KERN_ERR "fadump: unable to create sysfs file"
" fadump_release_mem (%d)\n", rc);
}
+ rc = sysfs_create_file(kernel_kobj, &fadump_mem_reserved_attr.attr);
+ if (rc)
+ pr_err("unable to create sysfs file fadump_mem_reserved (%d)\n",
+ rc);
return;
}
--
2.17.2
^ permalink raw reply related
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.3-4 tag
From: pr-tracker-bot @ 2019-08-10 19:30 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Linus Torvalds, linux-kernel
In-Reply-To: <87imr5s522.fsf@concordia.ellerman.id.au>
The pull request you sent on Sat, 10 Aug 2019 20:11:49 +1000:
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.3-4
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/23df57afe8eebff6ece05a815934f2f70a851e0a
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.3-4 tag
From: Nathan Chancellor @ 2019-08-10 22:42 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev, Linux List Kernel Mailing, clang-built-linux
In-Reply-To: <CAHk-=whnEp5+EM53MaT-3ep1xjhrUqCdcfBfTF9YxByGsmDMRw@mail.gmail.com>
On Sat, Aug 10, 2019 at 10:21:01AM -0700, Linus Torvalds wrote:
> On Sat, Aug 10, 2019 at 3:11 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > Just one fix, a revert of a commit that was meant to be a minor improvement to
> > some inline asm, but ended up having no real benefit with GCC and broke booting
> > 32-bit machines when using Clang.
>
> Pulled, but whenever there are possible subtle compiler issues I get
> nervous, and wonder if the problem was reported to the clang guys?
>
> In particular, if the kernel change was technically correct, maybe
> somebody else comes along in a few years and tries the same, and then
> it's another odd "why doesn't this work for person X when it works
> just fine for me"..
>
> Linus
It was.
https://github.com/ClangBuiltLinux/linux/issues/593
https://bugs.llvm.org/show_bug.cgi?id=42762
We're still waiting for input from the PowerPC backend maintainers as
that is most likely where this issue originates from.
Cheers,
Nathan
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.3-4 tag
From: Michael Ellerman @ 2019-08-10 22:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: arnd, Nick Desaulniers, Linux List Kernel Mailing,
clang-built-linux, Nathan Chancellor, linuxppc-dev
In-Reply-To: <CAHk-=whnEp5+EM53MaT-3ep1xjhrUqCdcfBfTF9YxByGsmDMRw@mail.gmail.com>
[ expanded Cc ]
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sat, Aug 10, 2019 at 3:11 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Just one fix, a revert of a commit that was meant to be a minor improvement to
>> some inline asm, but ended up having no real benefit with GCC and broke booting
>> 32-bit machines when using Clang.
>
> Pulled, but whenever there are possible subtle compiler issues I get
> nervous, and wonder if the problem was reported to the clang guys?
Yes, sorry I should have included more context. It was actually the
Clang Linux folks who noticed it and reported it to us:
https://github.com/ClangBuiltLinux/linux/issues/593
There's an LLVM bug filed:
https://bugs.llvm.org/show_bug.cgi?id=42762
And I think there's now agreement that the Clang behaviour is not
correct, Nick actually sent a revert as well but I already had one
queued:
https://patchwork.ozlabs.org/patch/1144980/
Arnd identified some work arounds, which we may end up using, but for
this cycle we thought it was preferable to just revert this change as it
didn't actually change code generation with GCC anyway.
cheers
^ permalink raw reply
* Re: [PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages
From: Santosh Sivaraj @ 2019-08-11 1:32 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar, linuxppc-dev, Linux Kernel
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <9b3e144e-8f12-de24-ff6a-ea599dc6e021@linux.vnet.ibm.com>
Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> On 8/7/19 8:26 PM, Santosh Sivaraj wrote:
>> From: Balbir Singh <bsingharora@gmail.com>
>>
>> The current code would fail on huge pages addresses, since the shift would
>> be incorrect. Use the correct page shift value returned by
>> __find_linux_pte() to get the correct physical address. The code is more
>> generic and can handle both regular and compound pages.
>>
>> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
>> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
>> [arbab@linux.ibm.com: Fixup pseries_do_memory_failure()]
>> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
>> Co-developed-by: Santosh Sivaraj <santosh@fossix.org>
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>> arch/powerpc/include/asm/mce.h | 2 +-
>> arch/powerpc/kernel/mce_power.c | 50 ++++++++++++++--------------
>> arch/powerpc/platforms/pseries/ras.c | 9 ++---
>> 3 files changed, 29 insertions(+), 32 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>> index a4c6a74ad2fb..f3a6036b6bc0 100644
>> --- a/arch/powerpc/include/asm/mce.h
>> +++ b/arch/powerpc/include/asm/mce.h
>> @@ -209,7 +209,7 @@ extern void release_mce_event(void);
>> extern void machine_check_queue_event(void);
>> extern void machine_check_print_event_info(struct machine_check_event *evt,
>> bool user_mode, bool in_guest);
>> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
>> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr);
>> #ifdef CONFIG_PPC_BOOK3S_64
>> void flush_and_reload_slb(void);
>> #endif /* CONFIG_PPC_BOOK3S_64 */
>> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
>> index a814d2dfb5b0..bed38a8e2e50 100644
>> --- a/arch/powerpc/kernel/mce_power.c
>> +++ b/arch/powerpc/kernel/mce_power.c
>> @@ -20,13 +20,14 @@
>> #include <asm/exception-64s.h>
>>
>> /*
>> - * Convert an address related to an mm to a PFN. NOTE: we are in real
>> - * mode, we could potentially race with page table updates.
>> + * Convert an address related to an mm to a physical address.
>> + * NOTE: we are in real mode, we could potentially race with page table updates.
>> */
>> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr)
>> {
>> - pte_t *ptep;
>> - unsigned long flags;
>> + pte_t *ptep, pte;
>> + unsigned int shift;
>> + unsigned long flags, phys_addr;
>> struct mm_struct *mm;
>>
>> if (user_mode(regs))
>> @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>> mm = &init_mm;
>>
>> local_irq_save(flags);
>> - if (mm == current->mm)
>> - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
>> - else
>> - ptep = find_init_mm_pte(addr, NULL);
>> + ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
>> local_irq_restore(flags);
>> +
>> if (!ptep || pte_special(*ptep))
>> return ULONG_MAX;
>> - return pte_pfn(*ptep);
>> +
>> + pte = *ptep;
>> + if (shift > PAGE_SHIFT) {
>> + unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
>> +
>> + pte = __pte(pte_val(pte) | (addr & rpnmask));
>> + }
>> + phys_addr = pte_pfn(pte) << PAGE_SHIFT;
>> +
>> + return phys_addr;
>> }
>>
>> /* flush SLBs and reload */
>> @@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
>
> Now that we have addr_to_phys() can we change this function name as well
> to mce_find_instr_ea_and_phys() ?
Makes sense, will avoid confusions.
>
> Tested-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> This should go to stable tree. Can you move this patch to 2nd position ?
Thanks for testing. Sure. Will reorder and mark stable as well.
Thanks,
Santosh
>
> Thanks,
> -Mahesh.
>
^ permalink raw reply
* Re: [PATCH v8 7/7] powerpc: add machine check safe copy_to_user
From: Santosh Sivaraj @ 2019-08-11 1:35 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, Linux Kernel
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <87lfw1s6u0.fsf@concordia.ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> Santosh Sivaraj <santosh@fossix.org> writes:
>> Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe()
>>
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/include/asm/uaccess.h | 14 ++++++++++++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 77f6ebf97113..4316e36095a2 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -137,6 +137,7 @@ config PPC
>> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
>> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>> select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
>> + select ARCH_HAS_UACCESS_MCSAFE if PPC64
>> select ARCH_HAS_UBSAN_SANITIZE_ALL
>> select ARCH_HAVE_NMI_SAFE_CMPXCHG
>> select ARCH_KEEP_MEMBLOCK
>> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
>> index 8b03eb44e876..15002b51ff18 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -387,6 +387,20 @@ static inline unsigned long raw_copy_to_user(void __user *to,
>> return ret;
>> }
>>
>> +static __always_inline unsigned long __must_check
>> +copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
>> +{
>> + if (likely(check_copy_size(from, n, true))) {
>> + if (access_ok(to, n)) {
>> + allow_write_to_user(to, n);
>> + n = memcpy_mcsafe((void *)to, from, n);
>> + prevent_write_to_user(to, n);
>> + }
>> + }
>> +
>> + return n;
>> +}
>
> This looks OK to me.
>
> It would be nice though if copy_to_user_mcsafe() followed the pattern of
> the other copy_to_user() etc. routines where the arch code is only
> responsible for the actual arch details, and all the checks are done in
> the generic code. That would be a good cleanup to do after this has gone
> in, as the 2nd implementation of the API.
Sure, will do that.
>
> cheers
^ permalink raw reply
* [PATCH 3/6] usb: add a HCD_DMA flag instead of guestimating DMA capabilities
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190811080520.21712-1-hch@lst.de>
The usb core is the only major place in the kernel that checks for
a non-NULL device dma_mask to see if a device is DMA capable. This
is generally a bad idea, as all major busses always set up a DMA mask,
even if the device is not DMA capable - in fact bus layers like PCI
can't even know if a device is DMA capable at enumeration time. This
leads to lots of workaround in HCD drivers, and also prevented us from
setting up a DMA mask for platform devices by default last time we
tried.
Replace this guess with an explicit HCD_DMA that is set by drivers that
appear to have DMA support.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
drivers/usb/core/hcd.c | 1 -
drivers/usb/dwc2/hcd.c | 6 +++---
drivers/usb/host/ehci-grlib.c | 2 +-
drivers/usb/host/ehci-hcd.c | 2 +-
drivers/usb/host/ehci-pmcmsp.c | 2 +-
drivers/usb/host/ehci-ppc-of.c | 2 +-
drivers/usb/host/ehci-ps3.c | 2 +-
drivers/usb/host/ehci-sh.c | 2 +-
drivers/usb/host/ehci-xilinx-of.c | 2 +-
drivers/usb/host/fhci-hcd.c | 2 +-
drivers/usb/host/fotg210-hcd.c | 2 +-
drivers/usb/host/imx21-hcd.c | 2 +-
drivers/usb/host/isp116x-hcd.c | 6 ------
drivers/usb/host/isp1362-hcd.c | 5 -----
drivers/usb/host/ohci-hcd.c | 2 +-
drivers/usb/host/ohci-ppc-of.c | 2 +-
drivers/usb/host/ohci-ps3.c | 2 +-
drivers/usb/host/ohci-sa1111.c | 2 +-
drivers/usb/host/ohci-sm501.c | 2 +-
drivers/usb/host/ohci-tmio.c | 2 +-
drivers/usb/host/oxu210hp-hcd.c | 3 ---
drivers/usb/host/r8a66597-hcd.c | 6 ------
drivers/usb/host/sl811-hcd.c | 6 ------
drivers/usb/host/u132-hcd.c | 2 --
drivers/usb/host/uhci-grlib.c | 2 +-
drivers/usb/host/uhci-pci.c | 2 +-
drivers/usb/host/uhci-platform.c | 2 +-
drivers/usb/host/xhci.c | 2 +-
drivers/usb/isp1760/isp1760-core.c | 3 ---
drivers/usb/isp1760/isp1760-if.c | 1 -
drivers/usb/musb/musb_host.c | 2 +-
drivers/usb/renesas_usbhs/mod_host.c | 2 +-
include/linux/usb.h | 1 -
include/linux/usb/hcd.h | 7 +++++--
35 files changed, 31 insertions(+), 62 deletions(-)
diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
index cd2b777073c4..a5321cc692c5 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -3512,7 +3512,7 @@ static const struct hc_driver octeon_hc_driver = {
.product_desc = "Octeon Host Controller",
.hcd_priv_size = sizeof(struct octeon_hcd),
.irq = octeon_usb_irq,
- .flags = HCD_MEMORY | HCD_USB2,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2,
.start = octeon_usb_start,
.stop = octeon_usb_stop,
.urb_enqueue = octeon_usb_urb_enqueue,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 8592c0344fe8..add2af4af766 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2454,7 +2454,6 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
hcd->self.controller = dev;
hcd->self.sysdev = sysdev;
hcd->self.bus_name = bus_name;
- hcd->self.uses_dma = (sysdev->dma_mask != NULL);
timer_setup(&hcd->rh_timer, rh_timer_func, 0);
#ifdef CONFIG_PM
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 111787a137ee..81afe553aa66 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5062,13 +5062,13 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
dwc2_hc_driver.reset_device = dwc2_reset_device;
}
+ if (hsotg->params.host_dma)
+ dwc2_hc_driver.flags |= HCD_DMA;
+
hcd = usb_create_hcd(&dwc2_hc_driver, hsotg->dev, dev_name(hsotg->dev));
if (!hcd)
goto error1;
- if (!hsotg->params.host_dma)
- hcd->self.uses_dma = 0;
-
hcd->has_tt = 1;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/usb/host/ehci-grlib.c b/drivers/usb/host/ehci-grlib.c
index 656b8c08efc8..a2c3b4ec8a8b 100644
--- a/drivers/usb/host/ehci-grlib.c
+++ b/drivers/usb/host/ehci-grlib.c
@@ -30,7 +30,7 @@ static const struct hc_driver ehci_grlib_hc_driver = {
* generic hardware linkage
*/
.irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 9da7e22848c9..cf2b7ae93b7e 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1193,7 +1193,7 @@ static const struct hc_driver ehci_hc_driver = {
* generic hardware linkage
*/
.irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-pmcmsp.c b/drivers/usb/host/ehci-pmcmsp.c
index 46e160370d6e..a2b610dbedfc 100644
--- a/drivers/usb/host/ehci-pmcmsp.c
+++ b/drivers/usb/host/ehci-pmcmsp.c
@@ -250,7 +250,7 @@ static const struct hc_driver ehci_msp_hc_driver = {
* generic hardware linkage
*/
.irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-ppc-of.c b/drivers/usb/host/ehci-ppc-of.c
index 576f7d79ad4e..9d17e0695e35 100644
--- a/drivers/usb/host/ehci-ppc-of.c
+++ b/drivers/usb/host/ehci-ppc-of.c
@@ -31,7 +31,7 @@ static const struct hc_driver ehci_ppc_of_hc_driver = {
* generic hardware linkage
*/
.irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HC_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-ps3.c b/drivers/usb/host/ehci-ps3.c
index 454d8c624a3f..fb52133c3557 100644
--- a/drivers/usb/host/ehci-ps3.c
+++ b/drivers/usb/host/ehci-ps3.c
@@ -59,7 +59,7 @@ static const struct hc_driver ps3_ehci_hc_driver = {
.product_desc = "PS3 EHCI Host Controller",
.hcd_priv_size = sizeof(struct ehci_hcd),
.irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
.reset = ps3_ehci_hc_reset,
.start = ehci_run,
.stop = ehci_stop,
diff --git a/drivers/usb/host/ehci-sh.c b/drivers/usb/host/ehci-sh.c
index a9ee767952c1..6a28fb93b9f1 100644
--- a/drivers/usb/host/ehci-sh.c
+++ b/drivers/usb/host/ehci-sh.c
@@ -33,7 +33,7 @@ static const struct hc_driver ehci_sh_hc_driver = {
* generic hardware linkage
*/
.irq = ehci_irq,
- .flags = HCD_USB2 | HCD_MEMORY | HCD_BH,
+ .flags = HCD_USB2 | HCD_DMA | HCD_MEMORY | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ehci-xilinx-of.c b/drivers/usb/host/ehci-xilinx-of.c
index d2a27578e440..67a6ee8cb5d8 100644
--- a/drivers/usb/host/ehci-xilinx-of.c
+++ b/drivers/usb/host/ehci-xilinx-of.c
@@ -66,7 +66,7 @@ static const struct hc_driver ehci_xilinx_of_hc_driver = {
* generic hardware linkage
*/
.irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2 | HCD_BH,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2 | HCD_BH,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/fhci-hcd.c b/drivers/usb/host/fhci-hcd.c
index 48fe9e6c2465..04733876c9c6 100644
--- a/drivers/usb/host/fhci-hcd.c
+++ b/drivers/usb/host/fhci-hcd.c
@@ -538,7 +538,7 @@ static const struct hc_driver fhci_driver = {
/* generic hardware linkage */
.irq = fhci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
+ .flags = HCD_DMA | HCD_USB11 | HCD_MEMORY,
/* basic lifecycle operation */
.start = fhci_start,
diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 77cc36efae95..8d7ccd032d47 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -5504,7 +5504,7 @@ static const struct hc_driver fotg210_fotg210_hc_driver = {
* generic hardware linkage
*/
.irq = fotg210_irq,
- .flags = HCD_MEMORY | HCD_USB2,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB2,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/imx21-hcd.c b/drivers/usb/host/imx21-hcd.c
index 6e3dad19d369..bd5fcc935e09 100644
--- a/drivers/usb/host/imx21-hcd.c
+++ b/drivers/usb/host/imx21-hcd.c
@@ -1771,7 +1771,7 @@ static const struct hc_driver imx21_hc_driver = {
.product_desc = "IMX21 USB Host Controller",
.hcd_priv_size = sizeof(struct imx21),
- .flags = HCD_USB11,
+ .flags = HCD_DMA | HCD_USB11,
.irq = imx21_irq,
.reset = imx21_hc_reset,
diff --git a/drivers/usb/host/isp116x-hcd.c b/drivers/usb/host/isp116x-hcd.c
index 74da136d322a..a87c0b26279e 100644
--- a/drivers/usb/host/isp116x-hcd.c
+++ b/drivers/usb/host/isp116x-hcd.c
@@ -1581,12 +1581,6 @@ static int isp116x_probe(struct platform_device *pdev)
irq = ires->start;
irqflags = ires->flags & IRQF_TRIGGER_MASK;
- if (pdev->dev.dma_mask) {
- DBG("DMA not supported\n");
- ret = -EINVAL;
- goto err1;
- }
-
if (!request_mem_region(addr->start, 2, hcd_name)) {
ret = -EBUSY;
goto err1;
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 28bf8bfb091e..96f8daa11f25 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -2645,11 +2645,6 @@ static int isp1362_probe(struct platform_device *pdev)
if (pdev->num_resources < 3)
return -ENODEV;
- if (pdev->dev.dma_mask) {
- DBG(1, "won't do DMA");
- return -ENODEV;
- }
-
irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!irq_res)
return -ENODEV;
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index b457fdaff297..1eb8d17e19db 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1178,7 +1178,7 @@ static const struct hc_driver ohci_hc_driver = {
* generic hardware linkage
*/
.irq = ohci_irq,
- .flags = HCD_MEMORY | HCD_USB11,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB11,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ohci-ppc-of.c b/drivers/usb/host/ohci-ppc-of.c
index 76a9b40b08f1..45f7cceb6df3 100644
--- a/drivers/usb/host/ohci-ppc-of.c
+++ b/drivers/usb/host/ohci-ppc-of.c
@@ -50,7 +50,7 @@ static const struct hc_driver ohci_ppc_of_hc_driver = {
* generic hardware linkage
*/
.irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
+ .flags = HCD_USB11 | HCD_DMA | HCD_MEMORY,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ohci-ps3.c b/drivers/usb/host/ohci-ps3.c
index 395f9d3bc849..f77cd6af0ccf 100644
--- a/drivers/usb/host/ohci-ps3.c
+++ b/drivers/usb/host/ohci-ps3.c
@@ -46,7 +46,7 @@ static const struct hc_driver ps3_ohci_hc_driver = {
.product_desc = "PS3 OHCI Host Controller",
.hcd_priv_size = sizeof(struct ohci_hcd),
.irq = ohci_irq,
- .flags = HCD_MEMORY | HCD_USB11,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB11,
.reset = ps3_ohci_hc_reset,
.start = ps3_ohci_hc_start,
.stop = ohci_stop,
diff --git a/drivers/usb/host/ohci-sa1111.c b/drivers/usb/host/ohci-sa1111.c
index ebec9a7699e3..8e19a5eb5b62 100644
--- a/drivers/usb/host/ohci-sa1111.c
+++ b/drivers/usb/host/ohci-sa1111.c
@@ -84,7 +84,7 @@ static const struct hc_driver ohci_sa1111_hc_driver = {
* generic hardware linkage
*/
.irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
+ .flags = HCD_USB11 | HCD_DMA | HCD_MEMORY,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c
index c158cda9e4b9..0b2aea6e28d4 100644
--- a/drivers/usb/host/ohci-sm501.c
+++ b/drivers/usb/host/ohci-sm501.c
@@ -49,7 +49,7 @@ static const struct hc_driver ohci_sm501_hc_driver = {
* generic hardware linkage
*/
.irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
+ .flags = HCD_USB11 | HCD_DMA | HCD_MEMORY,
/*
* basic lifecycle operations
diff --git a/drivers/usb/host/ohci-tmio.c b/drivers/usb/host/ohci-tmio.c
index d5a293a707b6..8edbacd3eb17 100644
--- a/drivers/usb/host/ohci-tmio.c
+++ b/drivers/usb/host/ohci-tmio.c
@@ -153,7 +153,7 @@ static const struct hc_driver ohci_tmio_hc_driver = {
/* generic hardware linkage */
.irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
+ .flags = HCD_USB11 | HCD_DMA | HCD_MEMORY,
/* basic lifecycle operations */
.start = ohci_tmio_start,
diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 47c5515a9ce4..29a49cc8a1ed 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -2649,9 +2649,6 @@ static int oxu_reset(struct usb_hcd *hcd)
INIT_LIST_HEAD(&oxu->urb_list);
oxu->urb_len = 0;
- /* FIMXE */
- hcd->self.controller->dma_mask = NULL;
-
if (oxu->is_otg) {
oxu->caps = hcd->regs + OXU_OTG_CAP_OFFSET;
oxu->regs = hcd->regs + OXU_OTG_CAP_OFFSET + \
diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index 42668aeca57c..0c03ac6b0213 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2411,12 +2411,6 @@ static int r8a66597_probe(struct platform_device *pdev)
if (usb_disabled())
return -ENODEV;
- if (pdev->dev.dma_mask) {
- ret = -EINVAL;
- dev_err(&pdev->dev, "dma not supported\n");
- goto clean_up;
- }
-
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
ret = -ENODEV;
diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index 5b061e599948..72a34a1eb618 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1632,12 +1632,6 @@ sl811h_probe(struct platform_device *dev)
irq = ires->start;
irqflags = ires->flags & IRQF_TRIGGER_MASK;
- /* refuse to confuse usbcore */
- if (dev->dev.dma_mask) {
- dev_dbg(&dev->dev, "no we won't dma\n");
- return -EINVAL;
- }
-
/* the chip may be wired for either kind of addressing */
addr = platform_get_resource(dev, IORESOURCE_MEM, 0);
data = platform_get_resource(dev, IORESOURCE_MEM, 1);
diff --git a/drivers/usb/host/u132-hcd.c b/drivers/usb/host/u132-hcd.c
index 400c40bc43a6..4efee34f154f 100644
--- a/drivers/usb/host/u132-hcd.c
+++ b/drivers/usb/host/u132-hcd.c
@@ -3077,8 +3077,6 @@ static int u132_probe(struct platform_device *pdev)
retval = ftdi_read_pcimem(pdev, roothub.a, &rh_a);
if (retval)
return retval;
- if (pdev->dev.dma_mask)
- return -EINVAL;
hcd = usb_create_hcd(&u132_hc_driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd) {
diff --git a/drivers/usb/host/uhci-grlib.c b/drivers/usb/host/uhci-grlib.c
index 2103b1ed0f8f..0a201a73b196 100644
--- a/drivers/usb/host/uhci-grlib.c
+++ b/drivers/usb/host/uhci-grlib.c
@@ -63,7 +63,7 @@ static const struct hc_driver uhci_grlib_hc_driver = {
/* Generic hardware linkage */
.irq = uhci_irq,
- .flags = HCD_MEMORY | HCD_USB11,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB11,
/* Basic lifecycle operations */
.reset = uhci_grlib_init,
diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
index 0dd944277c99..0fa3d72bae26 100644
--- a/drivers/usb/host/uhci-pci.c
+++ b/drivers/usb/host/uhci-pci.c
@@ -261,7 +261,7 @@ static const struct hc_driver uhci_driver = {
/* Generic hardware linkage */
.irq = uhci_irq,
- .flags = HCD_USB11,
+ .flags = HCD_DMA | HCD_USB11,
/* Basic lifecycle operations */
.reset = uhci_pci_init,
diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
index 89700e26fb29..70dbd95c3f06 100644
--- a/drivers/usb/host/uhci-platform.c
+++ b/drivers/usb/host/uhci-platform.c
@@ -41,7 +41,7 @@ static const struct hc_driver uhci_platform_hc_driver = {
/* Generic hardware linkage */
.irq = uhci_irq,
- .flags = HCD_MEMORY | HCD_USB11,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB11,
/* Basic lifecycle operations */
.reset = uhci_platform_init,
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 03d1e552769b..e315c0158e90 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5217,7 +5217,7 @@ static const struct hc_driver xhci_hc_driver = {
* generic hardware linkage
*/
.irq = xhci_irq,
- .flags = HCD_MEMORY | HCD_USB3 | HCD_SHARED,
+ .flags = HCD_MEMORY | HCD_DMA | HCD_USB3 | HCD_SHARED,
/*
* basic lifecycle operations
diff --git a/drivers/usb/isp1760/isp1760-core.c b/drivers/usb/isp1760/isp1760-core.c
index 55b94fd10331..fdeb4cf97cc5 100644
--- a/drivers/usb/isp1760/isp1760-core.c
+++ b/drivers/usb/isp1760/isp1760-core.c
@@ -120,9 +120,6 @@ int isp1760_register(struct resource *mem, int irq, unsigned long irqflags,
(!IS_ENABLED(CONFIG_USB_ISP1761_UDC) || udc_disabled))
return -ENODEV;
- /* prevent usb-core allocating DMA pages */
- dev->dma_mask = NULL;
-
isp = devm_kzalloc(dev, sizeof(*isp), GFP_KERNEL);
if (!isp)
return -ENOMEM;
diff --git a/drivers/usb/isp1760/isp1760-if.c b/drivers/usb/isp1760/isp1760-if.c
index 241a00d75027..07cc82ff327c 100644
--- a/drivers/usb/isp1760/isp1760-if.c
+++ b/drivers/usb/isp1760/isp1760-if.c
@@ -139,7 +139,6 @@ static int isp1761_pci_probe(struct pci_dev *dev,
pci_set_master(dev);
- dev->dev.dma_mask = NULL;
ret = isp1760_register(&dev->resource[3], dev->irq, 0, &dev->dev,
devflags);
if (ret < 0)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index eb308ec35c66..5a44b70372d9 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2689,7 +2689,7 @@ static const struct hc_driver musb_hc_driver = {
.description = "musb-hcd",
.product_desc = "MUSB HDRC host driver",
.hcd_priv_size = sizeof(struct musb *),
- .flags = HCD_USB2 | HCD_MEMORY,
+ .flags = HCD_USB2 | HCD_DMA | HCD_MEMORY,
/* not using irq handler or reset hooks from usbcore, since
* those must be shared with peripheral code for OTG configs
diff --git a/drivers/usb/renesas_usbhs/mod_host.c b/drivers/usb/renesas_usbhs/mod_host.c
index ddd3be48f948..ae54221011c3 100644
--- a/drivers/usb/renesas_usbhs/mod_host.c
+++ b/drivers/usb/renesas_usbhs/mod_host.c
@@ -1283,7 +1283,7 @@ static const struct hc_driver usbhsh_driver = {
/*
* generic hardware linkage
*/
- .flags = HCD_USB2,
+ .flags = HCD_DMA | HCD_USB2,
.start = usbhsh_host_start,
.stop = usbhsh_host_stop,
diff --git a/include/linux/usb.h b/include/linux/usb.h
index e87826e23d59..85a8865f9e83 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -426,7 +426,6 @@ struct usb_bus {
struct device *sysdev; /* as seen from firmware or bus */
int busnum; /* Bus number (in order of reg) */
const char *bus_name; /* stable id (PCI slot_name etc) */
- u8 uses_dma; /* Does the host controller use DMA? */
u8 uses_pio_for_control; /*
* Does the host controller use PIO
* for control transfers?
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index a20e7815d814..8d3869c7de85 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -256,6 +256,7 @@ struct hc_driver {
int flags;
#define HCD_MEMORY 0x0001 /* HC regs use memory (else I/O) */
+#define HCD_DMA 0x0002 /* HC uses DMA */
#define HCD_SHARED 0x0004 /* Two (or more) usb_hcds share HW */
#define HCD_USB11 0x0010 /* USB 1.1 */
#define HCD_USB2 0x0020 /* USB 2.0 */
@@ -422,8 +423,10 @@ static inline bool hcd_periodic_completion_in_progress(struct usb_hcd *hcd,
return hcd->high_prio_bh.completing_ep == ep;
}
-#define hcd_uses_dma(hcd) \
- (IS_ENABLED(CONFIG_HAS_DMA) && (hcd)->self.uses_dma)
+static inline bool hcd_uses_dma(struct usb_hcd *hcd)
+{
+ return IS_ENABLED(CONFIG_HAS_DMA) && (hcd->driver->flags & HCD_DMA);
+}
extern int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb);
extern int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
--
2.20.1
^ permalink raw reply related
* [PATCH 1/6] usb: don't create dma pools for HCDs with a localmem_pool
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190811080520.21712-1-hch@lst.de>
If the HCD provides a localmem pool we will never use the DMA pools, so
don't create them.
Fixes: b0310c2f09bb ("USB: use genalloc for USB HCs with local memory")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/usb/core/buffer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 1359b78a624e..1a5b3dcae930 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -66,9 +66,9 @@ int hcd_buffer_create(struct usb_hcd *hcd)
char name[16];
int i, size;
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- (!is_device_dma_capable(hcd->self.sysdev) &&
- !hcd->localmem_pool))
+ if (hcd->localmem_pool ||
+ !IS_ENABLED(CONFIG_HAS_DMA) ||
+ !is_device_dma_capable(hcd->self.sysdev))
return 0;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
--
2.20.1
^ permalink raw reply related
* [PATCH 6/6] driver core: initialize a default DMA mask for platform device
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190811080520.21712-1-hch@lst.de>
We still treat devices without a DMA mask as defaulting to 32-bits for
both mask, but a few releases ago we've started warning about such
cases, as they require special cases to work around this sloppyness.
Add a dma_mask field to struct platform_object so that we can initialize
the dma_mask pointer in struct device and initialize both masks to
32-bits by default. Architectures can still override this in
arch_setup_pdev_archdata if needed.
Note that the code looks a little odd with the various conditionals
because we have to support platform_device structures that are
statically allocated.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/base/platform.c | 15 +++++++++++++--
include/linux/platform_device.h | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ec974ba9c0c4..b216fcb0a8af 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -264,6 +264,17 @@ struct platform_object {
char name[];
};
+static void setup_pdev_archdata(struct platform_device *pdev)
+{
+ if (!pdev->dev.coherent_dma_mask)
+ pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+ if (!pdev->dma_mask)
+ pdev->dma_mask = DMA_BIT_MASK(32);
+ if (!pdev->dev.dma_mask)
+ pdev->dev.dma_mask = &pdev->dma_mask;
+ arch_setup_pdev_archdata(pdev);
+};
+
/**
* platform_device_put - destroy a platform device
* @pdev: platform device to free
@@ -310,7 +321,7 @@ struct platform_device *platform_device_alloc(const char *name, int id)
pa->pdev.id = id;
device_initialize(&pa->pdev.dev);
pa->pdev.dev.release = platform_device_release;
- arch_setup_pdev_archdata(&pa->pdev);
+ setup_pdev_archdata(&pa->pdev);
}
return pa ? &pa->pdev : NULL;
@@ -512,7 +523,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
int platform_device_register(struct platform_device *pdev)
{
device_initialize(&pdev->dev);
- arch_setup_pdev_archdata(pdev);
+ setup_pdev_archdata(pdev);
return platform_device_add(pdev);
}
EXPORT_SYMBOL_GPL(platform_device_register);
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 9bc36b589827..a2abde2aef25 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -24,6 +24,7 @@ struct platform_device {
int id;
bool id_auto;
struct device dev;
+ u64 dma_mask;
u32 num_resources;
struct resource *resource;
--
2.20.1
^ permalink raw reply related
* [PATCH 5/6] dma-mapping: remove is_device_dma_capable
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190811080520.21712-1-hch@lst.de>
No users left.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/dma-mapping.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea32c78..14702e2d6fa8 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -149,11 +149,6 @@ static inline int valid_dma_direction(int dma_direction)
(dma_direction == DMA_FROM_DEVICE));
}
-static inline int is_device_dma_capable(struct device *dev)
-{
- return dev->dma_mask != NULL && *dev->dma_mask != DMA_MASK_NONE;
-}
-
#ifdef CONFIG_DMA_DECLARE_COHERENT
/*
* These three functions are only for dma allocator.
--
2.20.1
^ permalink raw reply related
* next take at setting up a dma mask by default for platform devices
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
Hi all,
this is another attempt to make sure the dma_mask pointer is always
initialized for platform devices. Not doing so lead to lots of
boilerplate code, and makes platform devices different from all our
major busses like PCI where we always set up a dma_mask. In the long
run this should also help to eventually make dma_mask a scalar value
instead of a pointer and remove even more cruft.
The bigger blocker for this last time was the fact that the usb
subsystem uses the presence or lack of a dma_mask to check if the core
should do dma mapping for the driver, which is highly unusual. So we
fix this first. Note that this has some overlap with the pending
desire to use the proper dma_mmap_coherent helper for mapping usb
buffers. The first two patches from this series should probably
go into 5.3 and then uses as the basis for the decision to use
dma_mmap_coherent.
^ permalink raw reply
* [PATCH 4/6] usb/max3421: remove the dummy {un, }map_urb_for_dma methods
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190811080520.21712-1-hch@lst.de>
Now that we have an explicit HCD_DMA flag, there is not need to override
these methods.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/usb/host/max3421-hcd.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index afa321ab55fc..8819f502b6a6 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1800,21 +1800,6 @@ max3421_bus_resume(struct usb_hcd *hcd)
return -1;
}
-/*
- * The SPI driver already takes care of DMA-mapping/unmapping, so no
- * reason to do it twice.
- */
-static int
-max3421_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
-{
- return 0;
-}
-
-static void
-max3421_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
-{
-}
-
static const struct hc_driver max3421_hcd_desc = {
.description = "max3421",
.product_desc = DRIVER_DESC,
@@ -1826,8 +1811,6 @@ static const struct hc_driver max3421_hcd_desc = {
.get_frame_number = max3421_get_frame_number,
.urb_enqueue = max3421_urb_enqueue,
.urb_dequeue = max3421_urb_dequeue,
- .map_urb_for_dma = max3421_map_urb_for_dma,
- .unmap_urb_for_dma = max3421_unmap_urb_for_dma,
.endpoint_disable = max3421_endpoint_disable,
.hub_status_data = max3421_hub_status_data,
.hub_control = max3421_hub_control,
--
2.20.1
^ permalink raw reply related
* [PATCH 2/6] usb: add a hcd_uses_dma helper
From: Christoph Hellwig @ 2019-08-11 8:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Maxime Chevallier
Cc: linux-arch, Olav Kongas, Gavin Li, linuxppc-dev, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, linux-kernel, Tony Prisk, iommu, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190811080520.21712-1-hch@lst.de>
The USB buffer allocation code is the only place in the usb core (and in
fact the whole kernel) that uses is_device_dma_capable, while the URB
mapping code uses the uses_dma flag in struct usb_bus. Switch the buffer
allocation to use the uses_dma flag used by the rest of the USB code,
and create a helper in hcd.h that checks this flag as well as the
CONFIG_HAS_DMA to simplify the caller a bit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/usb/core/buffer.c | 10 +++-------
drivers/usb/core/hcd.c | 4 ++--
drivers/usb/dwc2/hcd.c | 2 +-
include/linux/usb.h | 2 +-
include/linux/usb/hcd.h | 3 +++
5 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 1a5b3dcae930..6cf22c27f2d2 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -66,9 +66,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
char name[16];
int i, size;
- if (hcd->localmem_pool ||
- !IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(hcd->self.sysdev))
+ if (hcd->localmem_pool || !hcd_uses_dma(hcd))
return 0;
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -129,8 +127,7 @@ void *hcd_buffer_alloc(
return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);
/* some USB hosts just use PIO */
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(bus->sysdev)) {
+ if (!hcd_uses_dma(hcd)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
@@ -160,8 +157,7 @@ void hcd_buffer_free(
return;
}
- if (!IS_ENABLED(CONFIG_HAS_DMA) ||
- !is_device_dma_capable(bus->sysdev)) {
+ if (!hcd_uses_dma(hcd)) {
kfree(addr);
return;
}
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2ccbc2f83570..8592c0344fe8 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1412,7 +1412,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
if (usb_endpoint_xfer_control(&urb->ep->desc)) {
if (hcd->self.uses_pio_for_control)
return ret;
- if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (is_vmalloc_addr(urb->setup_packet)) {
WARN_ONCE(1, "setup packet is not dma capable\n");
return -EAGAIN;
@@ -1446,7 +1446,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
if (urb->transfer_buffer_length != 0
&& !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
- if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (urb->num_sgs) {
int n;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index ee144ff8af5b..111787a137ee 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4608,7 +4608,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
buf = urb->transfer_buffer;
- if (hcd->self.uses_dma) {
+ if (hcd_uses_dma(hcd)) {
if (!buf && (urb->transfer_dma & 3)) {
dev_err(hsotg->dev,
"%s: unaligned transfer with no transfer_buffer",
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 83d35d993e8c..e87826e23d59 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1457,7 +1457,7 @@ typedef void (*usb_complete_t)(struct urb *);
* field rather than determining a dma address themselves.
*
* Note that transfer_buffer must still be set if the controller
- * does not support DMA (as indicated by bus.uses_dma) and when talking
+ * does not support DMA (as indicated by hcd_uses_dma()) and when talking
* to root hub. If you have to trasfer between highmem zone and the device
* on such controller, create a bounce buffer or bail out with an error.
* If transfer_buffer cannot be set (is in highmem) and the controller is DMA
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index bab27ccc8ff5..a20e7815d814 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -422,6 +422,9 @@ static inline bool hcd_periodic_completion_in_progress(struct usb_hcd *hcd,
return hcd->high_prio_bh.completing_ep == ep;
}
+#define hcd_uses_dma(hcd) \
+ (IS_ENABLED(CONFIG_HAS_DMA) && (hcd)->self.uses_dma)
+
extern int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb);
extern int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
int status);
--
2.20.1
^ permalink raw reply related
* [Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-11 20:03 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=204371
--- Comment #12 from Erhard F. (erhard_f@mailbox.org) ---
On Fri, 09 Aug 2019 12:31:26 +0000
bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=204371
>
Tried a few LTS kernels on the G4 DP. Looks like 4.19.x is affected (tested
4.19.66) whereas 4.14.x (tested 4.14.138) is not.
Also found a way to trigger the bug without the need of a btrfs root partition:
btrfs still built into the kernel. Mount another btrfs partition via /etc/fstab
at boot, e.g.
LABEL="tmp" /var/tmp/portage btrfs compress=lzo,noatime
0 1
Mounting /var/tmp/portage in my case works without problems. But I reliably get
the BUG kmalloc-4k at unmounting /var/tmp/portage.
I'll try to bisect the next few days and report back.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-11 21:18 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=204371
--- Comment #13 from Erhard F. (erhard_f@mailbox.org) ---
On Fri, 09 Aug 2019 12:31:26 +0000
bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=204371
>
[...]
[ 22.809365]
=============================================================================
[ 22.809700] BUG kmalloc-4096 (Tainted: G W ): Redzone
overwritten
[ 22.809971]
-----------------------------------------------------------------------------
[ 22.810286] INFO: 0xbe1a5921-0xfbfc06cd. First byte 0x0 instead of 0xcc
[ 22.810866] INFO: Allocated in __load_free_space_cache+0x588/0x780 [btrfs]
age=22 cpu=0 pid=224
[ 22.811193] __slab_alloc.constprop.26+0x44/0x70
[ 22.811345] kmem_cache_alloc_trace+0xf0/0x2ec
[ 22.811588] __load_free_space_cache+0x588/0x780 [btrfs]
[ 22.811848] load_free_space_cache+0xf4/0x1b0 [btrfs]
[ 22.812090] cache_block_group+0x1d0/0x3d0 [btrfs]
[ 22.812321] find_free_extent+0x680/0x12a4 [btrfs]
[ 22.812549] btrfs_reserve_extent+0xec/0x220 [btrfs]
[ 22.812785] btrfs_alloc_tree_block+0x178/0x5f4 [btrfs]
[ 22.813032] __btrfs_cow_block+0x150/0x5d4 [btrfs]
[ 22.813262] btrfs_cow_block+0x194/0x298 [btrfs]
[ 22.813484] commit_cowonly_roots+0x44/0x294 [btrfs]
[ 22.813718] btrfs_commit_transaction+0x63c/0xc0c [btrfs]
[ 22.813973] close_ctree+0xf8/0x2a4 [btrfs]
[ 22.814107] generic_shutdown_super+0x80/0x110
[ 22.814250] kill_anon_super+0x18/0x30
[ 22.814437] btrfs_kill_super+0x18/0x90 [btrfs]
[ 22.814590] INFO: Freed in proc_cgroup_show+0xc0/0x248 age=41 cpu=0 pid=83
[ 22.814841] proc_cgroup_show+0xc0/0x248
[ 22.814967] proc_single_show+0x54/0x98
[ 22.815086] seq_read+0x278/0x45c
[ 22.815190] __vfs_read+0x28/0x17c
[ 22.815289] vfs_read+0xa8/0x14c
[ 22.815381] ksys_read+0x50/0x94
[ 22.815475] ret_from_syscall+0x0/0x38
[ 22.815593] INFO: Slab 0x6b5768ec objects=7 used=7 fp=0x (null)
flags=0x8101
[ 22.815854] INFO: Object 0x6eefea7d @offset=17128 fp=0x (null)
[ 22.816063] Redzone be1a5921: 00 00 00 00 00 00 00 00
........
[ 22.816354] Object 6eefea7d: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
[...]
[ 23.715311] Object ea0b92e7: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
[ 23.718376] Redzone a1d8f890: cc cc cc cc
....
[ 23.721607] Padding d4007128: 5a 5a 5a 5a 5a 5a 5a 5a
ZZZZZZZZ
[ 23.724958] CPU: 0 PID: 224 Comm: umount Tainted: G B W 4.19.0
#1
[ 23.728433] Call Trace:
[ 23.731847] [ec525cc0] [c053ca68] dump_stack+0xa4/0x100 (unreliable)
[ 23.735595] [ec525ce0] [c019b21c] check_bytes_and_report+0xc8/0xf0
[ 23.739445] [ec525d10] [c019bf44] check_object+0x50/0x278
[ 23.743339] [ec525d30] [c019e4c4] free_debug_processing+0x200/0x318
[ 23.747341] [ec525d70] [c019e7b4] __slab_free+0x1d8/0x440
[ 23.751591] [ec525df0] [f3c34854] free_bitmap+0x24/0x68 [btrfs]
[ 23.755906] [ec525e00] [f3c35a28]
__btrfs_remove_free_space_cache_locked+0x68/0x6c [btrfs]
[ 23.760481] [ec525e20] [f3c38de8] btrfs_remove_free_space_cache+0x38/0x84
[btrfs]
[ 23.765173] [ec525e40] [f3bc7408] btrfs_free_block_groups+0x218/0x2f0
[btrfs]
[ 23.769993] [ec525e70] [f3bde164] close_ctree+0x200/0x2a4 [btrfs]
[ 23.774824] [ec525eb0] [c01b6534] generic_shutdown_super+0x80/0x110
[ 23.779750] [ec525ec0] [c01b678c] kill_anon_super+0x18/0x30
[ 23.784852] [ec525ed0] [f3baec88] btrfs_kill_super+0x18/0x90 [btrfs]
[ 23.790012] [ec525ee0] [c01b6cd8] deactivate_locked_super+0x54/0xa4
[ 23.795258] [ec525ef0] [c01d5db8] cleanup_mnt+0x50/0x78
[ 23.800575] [ec525f00] [c0055cac] task_work_run+0xa4/0xc4
[ 23.805994] [ec525f30] [c000b658] do_notify_resume+0xcc/0x108
[ 23.811478] [ec525f40] [c00146bc] do_user_signal+0x2c/0x34
[ 23.817049] --- interrupt: c00 at 0x8d43d4
LR = 0x8d43b8
[ 23.828287] FIX kmalloc-4096: Restoring 0xbe1a5921-0xfbfc06cd=0xcc
[ 23.840295] FIX kmalloc-4096: Object at 0x6eefea7d not freed
[ 23.846788]
=============================================================================
[ 23.852638] BUG kmalloc-4096 (Tainted: G B W ): Redzone
overwritten
[ 23.858590]
-----------------------------------------------------------------------------
[ 23.870891] INFO: 0xad3f3ec9-0x8e4e748e. First byte 0x0 instead of 0xcc
[ 23.877502] INFO: Allocated in __load_free_space_cache+0x588/0x780 [btrfs]
age=333 cpu=0 pid=224
[ 23.884297] __slab_alloc.constprop.26+0x44/0x70
[ 23.891119] kmem_cache_alloc_trace+0xf0/0x2ec
[ 23.898100] __load_free_space_cache+0x588/0x780 [btrfs]
[ 23.905235] load_free_space_cache+0xf4/0x1b0 [btrfs]
[ 23.912417] cache_block_group+0x1d0/0x3d0 [btrfs]
[ 23.919721] find_free_extent+0x680/0x12a4 [btrfs]
[ 23.927070] btrfs_reserve_extent+0xec/0x220 [btrfs]
[ 23.934474] btrfs_alloc_tree_block+0x178/0x5f4 [btrfs]
[ 23.942024] __btrfs_cow_block+0x150/0x5d4 [btrfs]
[ 23.949627] btrfs_cow_block+0x194/0x298 [btrfs]
[ 23.957351] commit_cowonly_roots+0x44/0x294 [btrfs]
[ 23.965154] btrfs_commit_transaction+0x63c/0xc0c [btrfs]
[ 23.973073] close_ctree+0xf8/0x2a4 [btrfs]
[ 23.980977] generic_shutdown_super+0x80/0x110
[ 23.988999] kill_anon_super+0x18/0x30
[ 23.997063] btrfs_kill_super+0x18/0x90 [btrfs]
[ 24.005191] INFO: Freed in seq_release+0x1c/0x38 age=352 cpu=1 pid=1
[ 24.013500] seq_release+0x1c/0x38
[ 24.021894] kernfs_fop_release+0x74/0x90
[ 24.030337] __fput+0x104/0x1e4
[ 24.038822] task_work_run+0xa4/0xc4
[ 24.047320] do_notify_resume+0xcc/0x108
[ 24.055936] do_user_signal+0x2c/0x34
[ 24.064520] INFO: Slab 0x7ec9c2e3 objects=7 used=6 fp=0xbc375e23
flags=0x8101
[ 24.073478] INFO: Object 0x8564a246 @offset=17128 fp=0x (null)
[ 24.091483] Redzone ad3f3ec9: 00 00 00 00 00 00 00 00
........
[ 24.100772] Object 8564a246: f0 00 00 00 ff ff ff ff 00 00 00 00 00 00 00 00
................
[...]
[ 25.242900] Object 5560df93: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
................
[ 25.245595] Redzone 4cfc344b: cc cc cc cc
....
[ 25.248446] Padding 399de3f9: 5a 5a 5a 5a 5a 5a 5a 5a
ZZZZZZZZ
[ 25.251412] CPU: 0 PID: 224 Comm: umount Tainted: G B W 4.19.0
#1
[ 25.254501] Call Trace:
[ 25.257513] [ec525cc0] [c053ca68] dump_stack+0xa4/0x100 (unreliable)
[ 25.260807] [ec525ce0] [c019b21c] check_bytes_and_report+0xc8/0xf0
[ 25.264180] [ec525d10] [c019bf44] check_object+0x50/0x278
[ 25.267620] [ec525d30] [c019e4c4] free_debug_processing+0x200/0x318
[ 25.271174] [ec525d70] [c019e7b4] __slab_free+0x1d8/0x440
[ 25.274931] [ec525df0] [f3c34854] free_bitmap+0x24/0x68 [btrfs]
[ 25.278720] [ec525e00] [f3c35a28]
__btrfs_remove_free_space_cache_locked+0x68/0x6c [btrfs]
[ 25.282776] [ec525e20] [f3c38de8] btrfs_remove_free_space_cache+0x38/0x84
[btrfs]
[ 25.286969] [ec525e40] [f3bc7408] btrfs_free_block_groups+0x218/0x2f0
[btrfs]
[ 25.291230] [ec525e70] [f3bde164] close_ctree+0x200/0x2a4 [btrfs]
[ 25.295473] [ec525eb0] [c01b6534] generic_shutdown_super+0x80/0x110
[ 25.299835] [ec525ec0] [c01b678c] kill_anon_super+0x18/0x30
[ 25.304360] [ec525ed0] [f3baec88] btrfs_kill_super+0x18/0x90 [btrfs]
[ 25.308936] [ec525ee0] [c01b6cd8] deactivate_locked_super+0x54/0xa4
[ 25.313590] [ec525ef0] [c01d5db8] cleanup_mnt+0x50/0x78
[ 25.318277] [ec525f00] [c0055cac] task_work_run+0xa4/0xc4
[ 25.323064] [ec525f30] [c000b658] do_notify_resume+0xcc/0x108
[ 25.327903] [ec525f40] [c00146bc] do_user_signal+0x2c/0x34
[ 25.332836] --- interrupt: c00 at 0x8d43d4
LR = 0x8d43b8
[ 25.342792] FIX kmalloc-4096: Restoring 0xad3f3ec9-0x8e4e748e=0xcc
[ 25.353647] FIX kmalloc-4096: Object at 0x8564a246 not freed
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [PATCH] dmaengine: fsldma: Mark expected switch fall-through
From: Gustavo A. R. Silva @ 2019-08-12 0:22 UTC (permalink / raw)
To: Li Yang, Zhang Wei, Vinod Koul, Dan Williams
Cc: dmaengine, linuxppc-dev, linux-kernel, Gustavo A. R. Silva
Mark switch cases where we are expecting to fall through.
Fix the following warning (Building: powerpc-ppa8548_defconfig powerpc):
drivers/dma/fsldma.c: In function ‘fsl_dma_chan_probe’:
drivers/dma/fsldma.c:1165:26: warning: this statement may fall through [-Wimplicit-fallthrough=]
chan->toggle_ext_pause = fsl_chan_toggle_ext_pause;
~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/dma/fsldma.c:1166:2: note: here
case FSL_DMA_IP_83XX:
^~~~
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/dma/fsldma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 23e0a356f167..ad72b3f42ffa 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1163,6 +1163,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
switch (chan->feature & FSL_DMA_IP_MASK) {
case FSL_DMA_IP_85XX:
chan->toggle_ext_pause = fsl_chan_toggle_ext_pause;
+ /* Fall through */
case FSL_DMA_IP_83XX:
chan->toggle_ext_start = fsl_chan_toggle_ext_start;
chan->set_src_loop_size = fsl_chan_set_src_loop_size;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc: Allow flush_icache_range to work across ranges >4GB
From: Alastair D'Silva @ 2019-08-12 1:19 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, stable, Paul Mackerras, Greg Kroah-Hartman,
Thomas Gleixner, linuxppc-dev
In-Reply-To: <a9bcc457-9f9b-7010-6796-fb263135f8bc@c-s.fr>
On Fri, 2019-08-09 at 10:59 +0200, Christophe Leroy wrote:
>
> Le 09/08/2019 à 02:45, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > When calling flush_icache_range with a size >4GB, we were masking
> > off the upper 32 bits, so we would incorrectly flush a range
> > smaller
> > than intended.
> >
> > This patch replaces the 32 bit shifts with 64 bit ones, so that
> > the full size is accounted for.
> >
> > Heads-up for backporters: the old version of flush_dcache_range is
> > subject to a similar bug (this has since been replaced with a C
> > implementation).
>
> Can you submit a patch to stable, explaining this ?
>
This patch was sent to stable too - or did you mean send another patch
for the stable asm version of flush_dcache_range?
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 1/3] powerpc/spinlocks: Refactor SHARED_PROCESSOR
From: kbuild test robot @ 2019-08-12 1:52 UTC (permalink / raw)
To: Christopher M. Riedl; +Cc: linuxppc-dev, kbuild-all, Andrew Donnellan
In-Reply-To: <20190806030112.15232-2-cmr@informatik.wtf>
[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]
Hi "Christopher,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc4 next-20190809]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Christopher-M-Riedl/Fix-oops-in-shared-processor-spinlocks/20190806-204502
config: powerpc-powernv_defconfig (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/spinlock.h:89:0,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/compat.h:10,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/spinlock.h: In function 'is_shared_processor':
>> arch/powerpc/include/asm/spinlock.h:119:34: error: 'struct paca_struct' has no member named 'lppaca_ptr'; did you mean 'slb_cache_ptr'?
lppaca_shared_proc(local_paca->lppaca_ptr));
^~~~~~~~~~
slb_cache_ptr
make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
7 real 4 user 3 sys 110.24% cpu make prepare
vim +119 arch/powerpc/include/asm/spinlock.h
110
111 static inline bool is_shared_processor(void)
112 {
113 /*
114 * LPPACA is only available on BOOK3S so guard anything LPPACA related to
115 * allow other platforms (which include this common header) to compile.
116 */
117 #ifdef CONFIG_PPC_BOOK3S
118 return (IS_ENABLED(CONFIG_PPC_SPLPAR) &&
> 119 lppaca_shared_proc(local_paca->lppaca_ptr));
120 #else
121 return false;
122 #endif
123 }
124
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23318 bytes --]
^ permalink raw reply
* [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
From: Nathan Chancellor @ 2019-08-12 2:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nick Desaulniers
Cc: clang-built-linux, Nathan Chancellor, linuxppc-dev, linux-kernel,
stable
Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.
r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.
In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
^
2 errors generated.
Take the same approach as the above commit by disabling the warning for
the same reason, we provide our own longjmp/setjmp function.
Cc: stable@vger.kernel.org # 4.19+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
instead as it makes it clear to clang that we are not using the builtin
longjmp and setjmp functions, which I think is why these warnings are
appearing (at least according to the commit that introduced this waring).
Sample patch:
https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
However, this is the most conservative approach, as I have already had
someone notice this error when building LLVM with PGO on tip of tree
LLVM.
arch/powerpc/kernel/Makefile | 5 +++--
arch/powerpc/xmon/Makefile | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ea0c69236789..44e340ed4722 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,9 @@
CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
+CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header) \
+ $(call cc-disable-warning, incomplete-setjmp-declaration)
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..53f341391210 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of setjmp.h and declaration of jmp_buf type)
+subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header) \
+ $(call cc-disable-warning, incomplete-setjmp-declaration)
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
--
2.23.0.rc2
^ permalink raw reply related
* [PATCH 1/2] KVM: PPC: Book3S HV: Fix race in re-enabling XIVE escalation interrupts
From: Paul Mackerras @ 2019-08-12 5:07 UTC (permalink / raw)
To: linuxppc-dev, kvm; +Cc: kvm-ppc, David Gibson
In-Reply-To: <20190812050623.ltla46gh5futsqv4@oak.ozlabs.ibm.com>
Escalation interrupts are interrupts sent to the host by the XIVE
hardware when it has an interrupt to deliver to a guest VCPU but that
VCPU is not running anywhere in the system. Hence we disable the
escalation interrupt for the VCPU being run when we enter the guest
and re-enable it when the guest does an H_CEDE hypercall indicating
it is idle.
It is possible that an escalation interrupt gets generated just as we
are entering the guest. In that case the escalation interrupt may be
using a queue entry in one of the interrupt queues, and that queue
entry may not have been processed when the guest exits with an H_CEDE.
The existing entry code detects this situation and does not clear the
vcpu->arch.xive_esc_on flag as an indication that there is a pending
queue entry (if the queue entry gets processed, xive_esc_irq() will
clear the flag). There is a comment in the code saying that if the
flag is still set on H_CEDE, we have to abort the cede rather than
re-enabling the escalation interrupt, lest we end up with two
occurrences of the escalation interrupt in the interrupt queue.
However, the exit code doesn't do that; it aborts the cede in the sense
that vcpu->arch.ceded gets cleared, but it still enables the escalation
interrupt by setting the source's PQ bits to 00. Instead we need to
set the PQ bits to 10, indicating that an interrupt has been triggered.
We also need to avoid setting vcpu->arch.xive_esc_on in this case
(i.e. vcpu->arch.xive_esc_on seen to be set on H_CEDE) because
xive_esc_irq() will run at some point and clear it, and if we race with
that we may end up with an incorrect result (i.e. xive_esc_on set when
the escalation interrupt has just been handled).
It is extremely unlikely that having two queue entries would cause
observable problems; theoretically it could cause queue overflow, but
the CPU would have to have thousands of interrupts targetted to it for
that to be possible. However, this fix will also make it possible to
determine accurately whether there is an unhandled escalation
interrupt in the queue, which will be needed by the following patch.
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 337e64468d78..0e70d63b16b3 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2831,29 +2831,38 @@ kvm_cede_prodded:
kvm_cede_exit:
ld r9, HSTATE_KVM_VCPU(r13)
#ifdef CONFIG_KVM_XICS
- /* Abort if we still have a pending escalation */
+ li r6, XIVE_ESB_SET_PQ_00
+ /*
+ * If we still have a pending escalation, abort the cede,
+ * and we must set PQ to 10 rather than 00 so that we don't
+ * potentially end up with two entries for the escalation
+ * interrupt in the XIVE interrupt queue. In that case
+ * we also don't want to set xive_esc_on to 1 here in
+ * case we race with xive_esc_irq().
+ */
lbz r5, VCPU_XIVE_ESC_ON(r9)
cmpwi r5, 0
- beq 1f
+ beq 4f
li r0, 0
stb r0, VCPU_CEDED(r9)
+ li r6, XIVE_ESB_SET_PQ_10
+ b 1f
+4: li r0, 1
+ stb r0, VCPU_XIVE_ESC_ON(r9)
1: /* Enable XIVE escalation */
- li r5, XIVE_ESB_SET_PQ_00
mfmsr r0
andi. r0, r0, MSR_DR /* in real mode? */
beq 1f
ld r10, VCPU_XIVE_ESC_VADDR(r9)
cmpdi r10, 0
beq 3f
- ldx r0, r10, r5
+ ldx r0, r10, r6
b 2f
1: ld r10, VCPU_XIVE_ESC_RADDR(r9)
cmpdi r10, 0
beq 3f
- ldcix r0, r10, r5
+ ldcix r0, r10, r6
2: sync
- li r0, 1
- stb r0, VCPU_XIVE_ESC_ON(r9)
#endif /* CONFIG_KVM_XICS */
3: b guest_exit_cont
--
2.11.0
^ permalink raw reply related
* [PATCH 2/2] powerpc/xive: Implement get_irqchip_state method for XIVE to fix shutdown race
From: Paul Mackerras @ 2019-08-12 5:07 UTC (permalink / raw)
To: linuxppc-dev, kvm; +Cc: kvm-ppc, David Gibson
In-Reply-To: <20190812050623.ltla46gh5futsqv4@oak.ozlabs.ibm.com>
Testing has revealed the existence of a race condition where a XIVE
interrupt being shut down can be in one of the XIVE interrupt queues
(of which there are up to 8 per CPU, one for each priority) at the
point where free_irq() is called. If this happens, can return an
interrupt number which has been shut down. This can lead to various
symptoms:
- irq_to_desc(irq) can be NULL. In this case, no end-of-interrupt
function gets called, resulting in the CPU's elevated interrupt
priority (numerically lowered CPPR) never gets reset. That then
means that the CPU stops processing interrupts, causing device
timeouts and other errors in various device drivers.
- The irq descriptor or related data structures can be in the process
of being freed as the interrupt code is using them. This typically
leads to crashes due to bad pointer dereferences.
This race is basically what commit 62e0468650c3 ("genirq: Add optional
hardware synchronization for shutdown", 2019-06-28) is intended to
fix, given a get_irqchip_state() method for the interrupt controller
being used. It works by polling the interrupt controller when an
interrupt is being freed until the controller says it is not pending.
With XIVE, the PQ bits of the interrupt source indicate the state of
the interrupt source, and in particular the P bit goes from 0 to 1 at
the point where the hardware writes an entry into the interrupt queue
that this interrupt is directed towards. Normally, the code will then
process the interrupt and do an end-of-interrupt (EOI) operation which
will reset PQ to 00 (assuming another interrupt hasn't been generated
in the meantime). However, there are situations where the code resets
P even though a queue entry exists (for example, by setting PQ to 01,
which disables the interrupt source), and also situations where the
code leaves P at 1 after removing the queue entry (for example, this
is done for escalation interrupts so they cannot fire again until
they are explicitly re-enabled).
The code already has a 'saved_p' flag for the interrupt source which
indicates that a queue entry exists, although it isn't maintained
consistently. This patch adds a 'stale_p' flag to indicate that
P has been left at 1 after processing a queue entry, and adds code
to set and clear saved_p and stale_p as necessary to maintain a
consistent indication of whether a queue entry may or may not exist.
With this, we can implement xive_get_irqchip_state() by looking at
stale_p, saved_p and the ESB PQ bits for the interrupt.
There is some additional code to handle escalation interrupts properly;
because they are enabled and disabled in KVM assembly code, which does
not have access to the xive_irq_data struct for the escalation
interrupt. Hence, stale_p may be incorrect when the escalation
interrupt is freed in kvmppc_xive_cleanup_vcpu(). Fortunately, we
can fix it up by looking at vcpu->arch.xive_esc_on, with some
careful attention to barriers in order to ensure the correct result
if xive_esc_irq() races with kvmppc_xive_cleanup_vcpu().
Finally, this adds code to make noise on the console (pr_crit and
WARN_ON(1)) if we find an interrupt queue entry for an interrupt
which does not have a descriptor. While this won't catch the race
reliably, if it does get triggered it will be an indication that
the race is occurring and needs to be debugged.
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
---
arch/powerpc/include/asm/xive.h | 8 ++++
arch/powerpc/kvm/book3s_xive.c | 31 ++++++++++++++
arch/powerpc/sysdev/xive/common.c | 87 ++++++++++++++++++++++++++++-----------
3 files changed, 103 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index e4016985764e..efb0e597b272 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -46,7 +46,15 @@ struct xive_irq_data {
/* Setup/used by frontend */
int target;
+ /*
+ * saved_p means that there is a queue entry for this interrupt
+ * in some CPU's queue (not including guest vcpu queues), even
+ * if P is not set in the source ESB.
+ * stale_p means that there is no queue entry for this interrupt
+ * in some CPU's queue, even if P is set in the source ESB.
+ */
bool saved_p;
+ bool stale_p;
};
#define XIVE_IRQ_FLAG_STORE_EOI 0x01
#define XIVE_IRQ_FLAG_LSI 0x02
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 09f838aa3138..74eea009c095 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -160,6 +160,9 @@ static irqreturn_t xive_esc_irq(int irq, void *data)
*/
vcpu->arch.xive_esc_on = false;
+ /* This orders xive_esc_on = false vs. subsequent stale_p = true */
+ smp_wmb(); /* goes with smp_mb() in cleanup_single_escalation */
+
return IRQ_HANDLED;
}
@@ -1113,6 +1116,31 @@ void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
vcpu->arch.xive_esc_raddr = 0;
}
+/*
+ * In single escalation mode, the escalation interrupt is marked so
+ * that EOI doesn't re-enable it, but just sets the stale_p flag to
+ * indicate that the P bit has already been dealt with. However, the
+ * assembly code that enters the guest sets PQ to 00 without clearing
+ * stale_p (because it has no easy way to address it). Hence we have
+ * to adjust stale_p before shutting down the interrupt.
+ */
+static void cleanup_single_escalation(struct kvm_vcpu *vcpu,
+ struct kvmppc_xive_vcpu *xc, int irq)
+{
+ struct irq_data *d = irq_get_irq_data(irq);
+ struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
+
+ /*
+ * This slightly odd sequence gives the right result
+ * (i.e. stale_p set if xive_esc_on is false) even if
+ * we race with xive_esc_irq() and xive_irq_eoi().
+ */
+ xd->stale_p = false;
+ smp_mb(); /* paired with smb_wmb in xive_esc_irq */
+ if (!vcpu->arch.xive_esc_on)
+ xd->stale_p = true;
+}
+
void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)
{
struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
@@ -1137,6 +1165,9 @@ void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)
/* Free escalations */
for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
if (xc->esc_virq[i]) {
+ if (xc->xive->single_escalation)
+ cleanup_single_escalation(vcpu, xc,
+ xc->esc_virq[i]);
free_irq(xc->esc_virq[i], vcpu);
irq_dispose_mapping(xc->esc_virq[i]);
kfree(xc->esc_virq_names[i]);
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 1cdb39575eae..be86fce1a84e 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -135,7 +135,7 @@ static u32 xive_read_eq(struct xive_q *q, bool just_peek)
static u32 xive_scan_interrupts(struct xive_cpu *xc, bool just_peek)
{
u32 irq = 0;
- u8 prio;
+ u8 prio = 0;
/* Find highest pending priority */
while (xc->pending_prio != 0) {
@@ -148,8 +148,19 @@ static u32 xive_scan_interrupts(struct xive_cpu *xc, bool just_peek)
irq = xive_read_eq(&xc->queue[prio], just_peek);
/* Found something ? That's it */
- if (irq)
- break;
+ if (irq) {
+ if (just_peek || irq_to_desc(irq))
+ break;
+ /*
+ * We should never get here; if we do then we must
+ * have failed to synchronize the interrupt properly
+ * when shutting it down.
+ */
+ pr_crit("xive: got interrupt %d without descriptor, dropping\n",
+ irq);
+ WARN_ON(1);
+ continue;
+ }
/* Clear pending bits */
xc->pending_prio &= ~(1 << prio);
@@ -307,6 +318,7 @@ static void xive_do_queue_eoi(struct xive_cpu *xc)
*/
static void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd)
{
+ xd->stale_p = false;
/* If the XIVE supports the new "store EOI facility, use it */
if (xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
xive_esb_write(xd, XIVE_ESB_STORE_EOI, 0);
@@ -350,7 +362,7 @@ static void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd)
}
}
-/* irq_chip eoi callback */
+/* irq_chip eoi callback, called with irq descriptor lock held */
static void xive_irq_eoi(struct irq_data *d)
{
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
@@ -366,6 +378,8 @@ static void xive_irq_eoi(struct irq_data *d)
if (!irqd_irq_disabled(d) && !irqd_is_forwarded_to_vcpu(d) &&
!(xd->flags & XIVE_IRQ_NO_EOI))
xive_do_source_eoi(irqd_to_hwirq(d), xd);
+ else
+ xd->stale_p = true;
/*
* Clear saved_p to indicate that it's no longer occupying
@@ -397,11 +411,16 @@ static void xive_do_source_set_mask(struct xive_irq_data *xd,
*/
if (mask) {
val = xive_esb_read(xd, XIVE_ESB_SET_PQ_01);
- xd->saved_p = !!(val & XIVE_ESB_VAL_P);
- } else if (xd->saved_p)
+ if (!xd->stale_p && !!(val & XIVE_ESB_VAL_P))
+ xd->saved_p = true;
+ xd->stale_p = false;
+ } else if (xd->saved_p) {
xive_esb_read(xd, XIVE_ESB_SET_PQ_10);
- else
+ xd->saved_p = false;
+ } else {
xive_esb_read(xd, XIVE_ESB_SET_PQ_00);
+ xd->stale_p = false;
+ }
}
/*
@@ -541,6 +560,8 @@ static unsigned int xive_irq_startup(struct irq_data *d)
unsigned int hw_irq = (unsigned int)irqd_to_hwirq(d);
int target, rc;
+ xd->saved_p = false;
+ xd->stale_p = false;
pr_devel("xive_irq_startup: irq %d [0x%x] data @%p\n",
d->irq, hw_irq, d);
@@ -587,6 +608,7 @@ static unsigned int xive_irq_startup(struct irq_data *d)
return 0;
}
+/* called with irq descriptor lock held */
static void xive_irq_shutdown(struct irq_data *d)
{
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
@@ -602,16 +624,6 @@ static void xive_irq_shutdown(struct irq_data *d)
xive_do_source_set_mask(xd, true);
/*
- * The above may have set saved_p. We clear it otherwise it
- * will prevent re-enabling later on. It is ok to forget the
- * fact that the interrupt might be in a queue because we are
- * accounting that already in xive_dec_target_count() and will
- * be re-routing it to a new queue with proper accounting when
- * it's started up again
- */
- xd->saved_p = false;
-
- /*
* Mask the interrupt in HW in the IVT/EAS and set the number
* to be the "bad" IRQ number
*/
@@ -797,6 +809,10 @@ static int xive_irq_retrigger(struct irq_data *d)
return 1;
}
+/*
+ * Caller holds the irq descriptor lock, so this won't be called
+ * concurrently with xive_get_irqchip_state on the same interrupt.
+ */
static int xive_irq_set_vcpu_affinity(struct irq_data *d, void *state)
{
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
@@ -820,6 +836,10 @@ static int xive_irq_set_vcpu_affinity(struct irq_data *d, void *state)
/* Set it to PQ=10 state to prevent further sends */
pq = xive_esb_read(xd, XIVE_ESB_SET_PQ_10);
+ if (!xd->stale_p) {
+ xd->saved_p = !!(pq & XIVE_ESB_VAL_P);
+ xd->stale_p = !xd->saved_p;
+ }
/* No target ? nothing to do */
if (xd->target == XIVE_INVALID_TARGET) {
@@ -827,7 +847,7 @@ static int xive_irq_set_vcpu_affinity(struct irq_data *d, void *state)
* An untargetted interrupt should have been
* also masked at the source
*/
- WARN_ON(pq & 2);
+ WARN_ON(xd->saved_p);
return 0;
}
@@ -847,9 +867,8 @@ static int xive_irq_set_vcpu_affinity(struct irq_data *d, void *state)
* This saved_p is cleared by the host EOI, when we know
* for sure the queue slot is no longer in use.
*/
- if (pq & 2) {
- pq = xive_esb_read(xd, XIVE_ESB_SET_PQ_11);
- xd->saved_p = true;
+ if (xd->saved_p) {
+ xive_esb_read(xd, XIVE_ESB_SET_PQ_11);
/*
* Sync the XIVE source HW to ensure the interrupt
@@ -862,8 +881,7 @@ static int xive_irq_set_vcpu_affinity(struct irq_data *d, void *state)
*/
if (xive_ops->sync_source)
xive_ops->sync_source(hw_irq);
- } else
- xd->saved_p = false;
+ }
} else {
irqd_clr_forwarded_to_vcpu(d);
@@ -914,6 +932,23 @@ static int xive_irq_set_vcpu_affinity(struct irq_data *d, void *state)
return 0;
}
+/* Called with irq descriptor lock held. */
+static int xive_get_irqchip_state(struct irq_data *data,
+ enum irqchip_irq_state which, bool *state)
+{
+ struct xive_irq_data *xd = irq_data_get_irq_handler_data(data);
+
+ switch (which) {
+ case IRQCHIP_STATE_ACTIVE:
+ *state = !xd->stale_p &&
+ (xd->saved_p ||
+ !!(xive_esb_read(xd, XIVE_ESB_GET) & XIVE_ESB_VAL_P));
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static struct irq_chip xive_irq_chip = {
.name = "XIVE-IRQ",
.irq_startup = xive_irq_startup,
@@ -925,6 +960,7 @@ static struct irq_chip xive_irq_chip = {
.irq_set_type = xive_irq_set_type,
.irq_retrigger = xive_irq_retrigger,
.irq_set_vcpu_affinity = xive_irq_set_vcpu_affinity,
+ .irq_get_irqchip_state = xive_get_irqchip_state,
};
bool is_xive_irq(struct irq_chip *chip)
@@ -1338,6 +1374,11 @@ static void xive_flush_cpu_queue(unsigned int cpu, struct xive_cpu *xc)
xd = irq_desc_get_handler_data(desc);
/*
+ * Clear saved_p to indicate that it's no longer pending
+ */
+ xd->saved_p = false;
+
+ /*
* For LSIs, we EOI, this will cause a resend if it's
* still asserted. Otherwise do an MSI retrigger.
*/
--
2.11.0
^ 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