* [PATCH] powerpc/mm: Add cond_resched() while removing hpte mappings
From: Vaibhav Jain @ 2021-03-10 7:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vaibhav Jain, Aneesh Kumar K . V
While removing large number of mappings from hash page tables for
large memory systems as soft-lockup is reported because of the time
spent inside htap_remove_mapping() like one below:
watchdog: BUG: soft lockup - CPU#8 stuck for 23s!
<snip>
NIP plpar_hcall+0x38/0x58
LR pSeries_lpar_hpte_invalidate+0x68/0xb0
Call Trace:
0x1fffffffffff000 (unreliable)
pSeries_lpar_hpte_removebolted+0x9c/0x230
hash__remove_section_mapping+0xec/0x1c0
remove_section_mapping+0x28/0x3c
arch_remove_memory+0xfc/0x150
devm_memremap_pages_release+0x180/0x2f0
devm_action_release+0x30/0x50
release_nodes+0x28c/0x300
device_release_driver_internal+0x16c/0x280
unbind_store+0x124/0x170
drv_attr_store+0x44/0x60
sysfs_kf_write+0x64/0x90
kernfs_fop_write+0x1b0/0x290
__vfs_write+0x3c/0x70
vfs_write+0xd4/0x270
ksys_write+0xdc/0x130
system_call+0x5c/0x70
Fix this by adding a cond_resched() to the loop in
htap_remove_mapping() that issues hcall to remove hpte mapping. This
should prevent the soft-lockup from being reported.
Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/mm/book3s64/hash_utils.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 581b20a2feaf..ea3945c70b18 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -359,6 +359,8 @@ int htab_remove_mapping(unsigned long vstart, unsigned long vend,
}
if (rc < 0)
return rc;
+
+ cond_resched();
}
return ret;
--
2.29.2
^ permalink raw reply related
* Re: [PATCH 9/9] zsmalloc: remove the zsmalloc file system
From: Minchan Kim @ 2021-03-10 6:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, Michael S. Tsirkin, VMware, Inc.,
David Hildenbrand, linux-kernel, dri-devel, virtualization,
linux-mm, Alex Williamson, Nadav Amit, Al Viro, Daniel Vetter,
linux-fsdevel, Andrew Morton, linuxppc-dev, Nitin Gupta
In-Reply-To: <20210309155348.974875-10-hch@lst.de>
On Tue, Mar 09, 2021 at 04:53:48PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Minchan Kim <minchan@kernel.org>
^ permalink raw reply
* [PATCH] powerpc: fix warning comparing pointer to 0
From: Jiapeng Chong @ 2021-03-10 6:42 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, paulus, linux-kernel, Jiapeng Chong
Fix the following coccicheck warning:
./arch/powerpc/platforms/powermac/pfunc_core.c:688:40-41: WARNING
comparing pointer to 0.
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
arch/powerpc/platforms/powermac/pfunc_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c b/arch/powerpc/platforms/powermac/pfunc_core.c
index 94df0a9..a5aa40f 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -685,7 +685,7 @@ static int pmf_add_functions(struct pmf_device *dev, void *driverdata)
const int plen = strlen(PP_PREFIX);
int count = 0;
- for (pp = dev->node->properties; pp != 0; pp = pp->next) {
+ for (pp = dev->node->properties; pp; pp = pp->next) {
const char *name;
if (strncmp(pp->name, PP_PREFIX, plen) != 0)
continue;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/9] fs: rename alloc_anon_inode to alloc_anon_inode_sb
From: Minchan Kim @ 2021-03-10 6:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, Michael S. Tsirkin, VMware, Inc.,
David Hildenbrand, linux-kernel, dri-devel, virtualization,
linux-mm, Alex Williamson, Nadav Amit, Al Viro, Daniel Vetter,
linux-fsdevel, Andrew Morton, linuxppc-dev, Nitin Gupta
In-Reply-To: <20210309155348.974875-2-hch@lst.de>
On Tue, Mar 09, 2021 at 04:53:40PM +0100, Christoph Hellwig wrote:
> Rename alloc_inode to free the name for a new variant that does not
> need boilerplate to create a super_block first.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/platforms/pseries/cmm.c | 2 +-
> drivers/dma-buf/dma-buf.c | 2 +-
> drivers/gpu/drm/drm_drv.c | 2 +-
> drivers/misc/cxl/api.c | 2 +-
> drivers/misc/vmw_balloon.c | 2 +-
> drivers/scsi/cxlflash/ocxl_hw.c | 2 +-
> drivers/virtio/virtio_balloon.c | 2 +-
> fs/aio.c | 2 +-
> fs/anon_inodes.c | 4 ++--
> fs/libfs.c | 2 +-
> include/linux/fs.h | 2 +-
> kernel/resource.c | 2 +-
> mm/z3fold.c | 2 +-
> mm/zsmalloc.c | 2 +-
> 14 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 45a3a3022a85c9..6d36b858b14df1 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -580,7 +580,7 @@ static int cmm_balloon_compaction_init(void)
> return rc;
> }
>
> - b_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + b_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(b_dev_info.inode)) {
> rc = PTR_ERR(b_dev_info.inode);
> b_dev_info.inode = NULL;
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383eb4..dedcc9483352dc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -445,7 +445,7 @@ static inline int is_dma_buf_file(struct file *file)
> static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(dma_buf_mnt->mnt_sb);
>
> if (IS_ERR(inode))
> return ERR_CAST(inode);
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 20d22e41d7ce74..87e7214a8e3565 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -519,7 +519,7 @@ static struct inode *drm_fs_inode_new(void)
> return ERR_PTR(r);
> }
>
> - inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(drm_fs_mnt->mnt_sb);
> if (IS_ERR(inode))
> simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
>
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index b493de962153ba..2efbf6c98028ef 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -73,7 +73,7 @@ static struct file *cxl_getfile(const char *name,
> goto err_module;
> }
>
> - inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(cxl_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> file = ERR_CAST(inode);
> goto err_fs;
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index b837e7eba5f7dc..5d057a05ddbee8 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -1900,7 +1900,7 @@ static __init int vmballoon_compaction_init(struct vmballoon *b)
> return PTR_ERR(vmballoon_mnt);
>
> b->b_dev_info.migratepage = vmballoon_migratepage;
> - b->b_dev_info.inode = alloc_anon_inode(vmballoon_mnt->mnt_sb);
> + b->b_dev_info.inode = alloc_anon_inode_sb(vmballoon_mnt->mnt_sb);
>
> if (IS_ERR(b->b_dev_info.inode))
> return PTR_ERR(b->b_dev_info.inode);
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 244fc27215dc79..40184ed926b557 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -88,7 +88,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
> goto err2;
> }
>
> - inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
> + inode = alloc_anon_inode_sb(ocxlflash_vfs_mount->mnt_sb);
> if (IS_ERR(inode)) {
> rc = PTR_ERR(inode);
> dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8985fc2cea8615..cae76ee5bdd688 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -916,7 +916,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> }
>
> vb->vb_dev_info.migratepage = virtballoon_migratepage;
> - vb->vb_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
> + vb->vb_dev_info.inode = alloc_anon_inode_sb(balloon_mnt->mnt_sb);
> if (IS_ERR(vb->vb_dev_info.inode)) {
> err = PTR_ERR(vb->vb_dev_info.inode);
> goto out_kern_unmount;
> diff --git a/fs/aio.c b/fs/aio.c
> index 1f32da13d39ee6..d1c2aa7fd6de7c 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -234,7 +234,7 @@ static const struct address_space_operations aio_ctx_aops;
> static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages)
> {
> struct file *file;
> - struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
> + struct inode *inode = alloc_anon_inode_sb(aio_mnt->mnt_sb);
> if (IS_ERR(inode))
> return ERR_CAST(inode);
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index a280156138ed89..4745fc37014332 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -63,7 +63,7 @@ static struct inode *anon_inode_make_secure_inode(
> const struct qstr qname = QSTR_INIT(name, strlen(name));
> int error;
>
> - inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(inode))
> return inode;
> inode->i_flags &= ~S_PRIVATE;
> @@ -231,7 +231,7 @@ static int __init anon_inode_init(void)
> if (IS_ERR(anon_inode_mnt))
> panic("anon_inode_init() kernel mount failed (%ld)\n", PTR_ERR(anon_inode_mnt));
>
> - anon_inode_inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + anon_inode_inode = alloc_anon_inode_sb(anon_inode_mnt->mnt_sb);
> if (IS_ERR(anon_inode_inode))
> panic("anon_inode_init() inode allocation failed (%ld)\n", PTR_ERR(anon_inode_inode));
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index e2de5401abca5a..600bebc1cd847f 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1216,7 +1216,7 @@ static int anon_set_page_dirty(struct page *page)
> return 0;
> };
>
> -struct inode *alloc_anon_inode(struct super_block *s)
> +struct inode *alloc_anon_inode_sb(struct super_block *s)
> {
> static const struct address_space_operations anon_aops = {
> .set_page_dirty = anon_set_page_dirty,
EXPORT_SYMBOL(alloc_anon_inode_sb) ?
^ permalink raw reply
* Re: [PATCH V2] mm/memtest: Add ARCH_USE_MEMTEST
From: Anshuman Khandual @ 2021-03-10 6:03 UTC (permalink / raw)
To: linux-mm
Cc: Chris Zankel, Thomas Bogendoerfer, linux-kernel, linux-xtensa,
Catalin Marinas, linuxppc-dev, Russell King, linux-mips,
Max Filippov, Ingo Molnar, Paul Mackerras, Thomas Gleixner,
Will Deacon, linux-arm-kernel
In-Reply-To: <1614573126-7740-1-git-send-email-anshuman.khandual@arm.com>
On 3/1/21 10:02 AM, Anshuman Khandual wrote:
> early_memtest() does not get called from all architectures. Hence enabling
> CONFIG_MEMTEST and providing a valid memtest=[1..N] kernel command line
> option might not trigger the memory pattern tests as would be expected in
> normal circumstances. This situation is misleading.
>
> The change here prevents the above mentioned problem after introducing a
> new config option ARCH_USE_MEMTEST that should be subscribed on platforms
> that call early_memtest(), in order to enable the config CONFIG_MEMTEST.
> Conversely CONFIG_MEMTEST cannot be enabled on platforms where it would
> not be tested anyway.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-xtensa@linux-xtensa.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> This patch applies on v5.12-rc1 and has been tested on arm64 platform.
> But it has been just build tested on all other platforms.
>
> Changes in V2:
>
> - Added ARCH_USE_MEMTEST in the sorted alphabetical order on platforms
Gentle ping, any updates or objections ?
^ permalink raw reply
* Re: [PATCH 2/6] mm: Generalize SYS_SUPPORTS_HUGETLBFS (rename as ARCH_SUPPORTS_HUGETLBFS)
From: Michael Ellerman @ 2021-03-10 5:20 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas, linux-kernel,
James E.J. Bottomley, Paul Mackerras, linux-riscv, Will Deacon,
linux-s390, Yoshinori Sato, Helge Deller, x86, Russell King,
linux-snps-arc, Albert Ou, Anshuman Khandual, Alexander Viro,
Paul Walmsley, linux-arm-kernel, Thomas Bogendoerfer,
linux-parisc, linux-mips, Palmer Dabbelt, linux-fsdevel,
linuxppc-dev
In-Reply-To: <1615185706-24342-3-git-send-email-anshuman.khandual@arm.com>
Anshuman Khandual <anshuman.khandual@arm.com> writes:
> SYS_SUPPORTS_HUGETLBFS config has duplicate definitions on platforms that
> subscribe it. Instead, just make it a generic option which can be selected
> on applicable platforms. Also rename it as ARCH_SUPPORTS_HUGETLBFS instead.
> This reduces code duplication and makes it cleaner.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-riscv@lists.infradead.org
> Cc: linux-sh@vger.kernel.org
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm/Kconfig | 5 +----
> arch/arm64/Kconfig | 4 +---
> arch/mips/Kconfig | 6 +-----
> arch/parisc/Kconfig | 5 +----
> arch/powerpc/Kconfig | 3 ---
> arch/powerpc/platforms/Kconfig.cputype | 6 +++---
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
* Re: [PATCH v4] powerpc/uprobes: Validation for prefixed instruction
From: Michael Ellerman @ 2021-03-10 5:13 UTC (permalink / raw)
To: Ravi Bangoria, Naveen N. Rao
Cc: Ravi Bangoria, jniethe5, oleg, rostedt, linux-kernel, paulus,
sandipan, naveen.n.rao, linuxppc-dev
In-Reply-To: <1a080cb5-af98-6b6e-352d-772a90cfa902@linux.ibm.com>
Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
> On 3/9/21 4:51 PM, Naveen N. Rao wrote:
>> On 2021/03/09 08:54PM, Michael Ellerman wrote:
>>> Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
>>>> As per ISA 3.1, prefixed instruction should not cross 64-byte
>>>> boundary. So don't allow Uprobe on such prefixed instruction.
>>>>
>>>> There are two ways probed instruction is changed in mapped pages.
>>>> First, when Uprobe is activated, it searches for all the relevant
>>>> pages and replace instruction in them. In this case, if that probe
>>>> is on the 64-byte unaligned prefixed instruction, error out
>>>> directly. Second, when Uprobe is already active and user maps a
>>>> relevant page via mmap(), instruction is replaced via mmap() code
>>>> path. But because Uprobe is invalid, entire mmap() operation can
>>>> not be stopped. In this case just print an error and continue.
>>>>
>>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>>> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>>
>>> Do we have a Fixes: tag for this?
>>
>> Since this is an additional check we are adding, I don't think we should
>> add a Fixes: tag. Nothing is broken per-se -- we're just adding more
>> checks to catch simple mistakes. Also, like Oleg pointed out, there are
>> still many other ways for users to shoot themselves in the foot with
>> uprobes and prefixed instructions, if they so desire.
>>
>> However, if you still think we should add a Fixes: tag, we can perhaps
>> use the below commit since I didn't see any specific commit adding
>> support for prefixed instructions for uprobes:
>>
>> Fixes: 650b55b707fdfa ("powerpc: Add prefixed instructions to
>> instruction data type")
>
> True. IMO, It doesn't really need any Fixes tag.
Yep OK, I'm happy without a Fixes tag based on that explanation.
>>>> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
>>>> index e8a63713e655..4cbfff6e94a3 100644
>>>> --- a/arch/powerpc/kernel/uprobes.c
>>>> +++ b/arch/powerpc/kernel/uprobes.c
>>>> @@ -41,6 +41,13 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
>>>> if (addr & 0x03)
>>>> return -EINVAL;
>>>>
>>>> + if (cpu_has_feature(CPU_FTR_ARCH_31) &&
>>>> + ppc_inst_prefixed(auprobe->insn) &&
>>>> + (addr & (SZ_64 - 4)) == SZ_64 - 4) {
>>>> + pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
>>>> + return -EINVAL;
>>>
>>> I realise we already did the 0x03 check above, but I still think this
>>> would be clearer simply as:
>>>
>>> (addr & 0x3f == 60)
>>
>> Indeed, I like the use of `60' there -- hex is overrated ;)
>
> Sure. Will resend.
Thanks.
cheers
^ permalink raw reply
* Re: PowerPC64 future proof kernel toc, revised for lld
From: Alan Modra @ 2021-03-10 5:07 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: alexey, linuxppc-dev, ellerman
In-Reply-To: <3c92968f-7c61-8d36-4001-91f8630de4b1@linux.ibm.com>
On Wed, Mar 10, 2021 at 03:44:44PM +1100, Alexey Kardashevskiy wrote:
> For my own education, is .got for prom_init.o still generated by ld or gcc?
.got is generated by ld.
> In other words, should "objdump -D -s -j .got" ever dump .got for any .o
> file, like below?
No. "objdump -r prom_init.o | grep GOT" will tell you whether
prom_init.o *may* cause ld to generate .got entries. (Linker
optimisations or --gc-sections might remove the need for those .got
entries.)
> objdump: section '.got' mentioned in a -j option, but not found in any input
> file
Right, expected.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply
* Re: Errant readings on LM81 with T2080 SoC
From: Guenter Roeck @ 2021-03-10 5:06 UTC (permalink / raw)
To: Chris Packham, jdelvare@suse.com
Cc: linux-hwmon@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
In-Reply-To: <1aa0dc23-0706-5902-2f46-0767de0e3ad6@alliedtelesis.co.nz>
On 3/9/21 6:19 PM, Chris Packham wrote:
> On 9/03/21 9:27 am, Chris Packham wrote:
>> On 8/03/21 5:59 pm, Guenter Roeck wrote:
>>> Other than that, the only other real idea I have would be to monitor
>>> the i2c bus.
>> I am in the fortunate position of being able to go into the office and
>> even happen to have the expensive scope at the moment. Now I just need
>> to find a tame HW engineer so I don't burn myself trying to attach the
>> probes.
> One thing I see on the scope is that when there is a CPU load there
> appears to be some clock stretching going on (SCL is held low some
> times). I don't see it without the CPU load. It's hard to correlate a
> clock stretching event with a bad read or error but it is one area where
> the SMBUS spec has a maximum that might cause the device to give up waiting.
>
Do you have CONFIG_PREEMPT enabled in your kernel ? But even without
that it is possible that the hot loops at the beginning and end of
each operation mess up the driver and cause it to sleep longer
than intended. Did you try usleep_range() ?
On a side note, can you send me a register dump for the lm81 ?
It would be useful for my module test code.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Michael Ellerman @ 2021-03-10 5:03 UTC (permalink / raw)
To: Rob Herring, Thiago Jung Bauermann
Cc: kexec, linux-kernel@vger.kernel.org, Mimi Zohar,
Lakshmi Ramasubramanian, linuxppc-dev, Hari Bathini
In-Reply-To: <CAL_JsqJatxTwSbM8a0PB2ievO_fRFeYJ8ZtBN0pRfnEjSXyUkg@mail.gmail.com>
Rob Herring <robh@kernel.org> writes:
> On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
> <bauerman@linux.ibm.com> wrote:
>>
>> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
>> kernel") fixed how elf64_load() estimates the FDT size needed by the
>> crashdump kernel.
>>
>> At the same time, commit 130b2d59cec0 ("powerpc: Use common
>> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
>> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
>> change made the code overestimate it a bit by counting twice the space
>> required for the kernel command line and /chosen properties.
>>
>> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
>> space needed by the kdump kernel, and change the function name so that it
>> better reflects what the function is now doing.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> ---
>> arch/powerpc/include/asm/kexec.h | 2 +-
>> arch/powerpc/kexec/elf_64.c | 2 +-
>> arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>> 3 files changed, 10 insertions(+), 20 deletions(-)
>
> I ended up delaying the referenced series til 5.13, but have applied
> it now. Can I get an ack from the powerpc maintainers on this one?
> I'll fixup the commit log to make sense given the commit id's aren't
> valid.
Thanks for handling it.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH 2/6] mm: Generalize SYS_SUPPORTS_HUGETLBFS (rename as ARCH_SUPPORTS_HUGETLBFS)
From: Palmer Dabbelt @ 2021-03-10 4:39 UTC (permalink / raw)
To: anshuman.khandual
Cc: dalias, linux-sh, linux-kernel, James.Bottomley, linux-mm, paulus,
linux-riscv, will, ysato, deller, linux, catalin.marinas, aou,
anshuman.khandual, viro, Paul Walmsley, linux-arm-kernel,
tsbogend, linux-parisc, linux-mips, linux-fsdevel, linuxppc-dev
In-Reply-To: <1615278790-18053-3-git-send-email-anshuman.khandual@arm.com>
On Tue, 09 Mar 2021 00:33:06 PST (-0800), anshuman.khandual@arm.com wrote:
> SYS_SUPPORTS_HUGETLBFS config has duplicate definitions on platforms that
> subscribe it. Instead, just make it a generic option which can be selected
> on applicable platforms. Also rename it as ARCH_SUPPORTS_HUGETLBFS instead.
> This reduces code duplication and makes it cleaner.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-riscv@lists.infradead.org
> Cc: linux-sh@vger.kernel.org
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm/Kconfig | 5 +----
> arch/arm64/Kconfig | 4 +---
> arch/mips/Kconfig | 6 +-----
> arch/parisc/Kconfig | 5 +----
> arch/powerpc/Kconfig | 3 ---
> arch/powerpc/platforms/Kconfig.cputype | 6 +++---
> arch/riscv/Kconfig | 5 +----
> arch/sh/Kconfig | 5 +----
> fs/Kconfig | 5 ++++-
> 9 files changed, 13 insertions(+), 31 deletions(-)
[...]
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 85d626b8ce5e..69954db3aca9 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -30,6 +30,7 @@ config RISCV
> select ARCH_HAS_STRICT_KERNEL_RWX if MMU
> select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
> select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
> + select ARCH_SUPPORTS_HUGETLBFS if MMU
> select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if 64BIT
> @@ -165,10 +166,6 @@ config ARCH_WANT_GENERAL_HUGETLB
> config ARCH_SUPPORTS_UPROBES
> def_bool y
>
> -config SYS_SUPPORTS_HUGETLBFS
> - depends on MMU
> - def_bool y
> -
> config STACKTRACE_SUPPORT
> def_bool y
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
^ permalink raw reply
* Re: make alloc_anon_inode more useful
From: Matthew Wilcox @ 2021-03-10 4:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, Michael S. Tsirkin, VMware, Inc.,
David Hildenbrand, linux-kernel, dri-devel, virtualization,
linux-mm, Minchan Kim, Alex Williamson, Nadav Amit, Al Viro,
Daniel Vetter, linux-fsdevel, Andrew Morton, linuxppc-dev,
Nitin Gupta
In-Reply-To: <20210309155348.974875-1-hch@lst.de>
On Tue, Mar 09, 2021 at 04:53:39PM +0100, Christoph Hellwig wrote:
> this series first renames the existing alloc_anon_inode to
> alloc_anon_inode_sb to clearly mark it as requiring a superblock.
>
> It then adds a new alloc_anon_inode that works on the anon_inode
> file system super block, thus removing tons of boilerplate code.
>
> The few remainig callers of alloc_anon_inode_sb all use alloc_file_pseudo
> later, but might also be ripe for some cleanup.
On a somewhat related note, could I get you to look at
drivers/video/fbdev/core/fb_defio.c?
As far as I can tell, there's no need for fb_deferred_io_aops to exist.
We could just set file->f_mapping->a_ops to NULL, and set_page_dirty()
would do the exact same thing this code does (except it would get the
return value correct).
But maybe that would make something else go wrong that distinguishes
between page->mapping being NULL and page->mapping->a_ops->foo being NULL?
Completely untested patch ...
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index a591d291b231..441ec31d3e4d 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -151,17 +151,6 @@ static const struct vm_operations_struct fb_deferred_io_vm_ops = {
.page_mkwrite = fb_deferred_io_mkwrite,
};
-static int fb_deferred_io_set_page_dirty(struct page *page)
-{
- if (!PageDirty(page))
- SetPageDirty(page);
- return 0;
-}
-
-static const struct address_space_operations fb_deferred_io_aops = {
- .set_page_dirty = fb_deferred_io_set_page_dirty,
-};
-
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
{
vma->vm_ops = &fb_deferred_io_vm_ops;
@@ -212,14 +201,6 @@ void fb_deferred_io_init(struct fb_info *info)
}
EXPORT_SYMBOL_GPL(fb_deferred_io_init);
-void fb_deferred_io_open(struct fb_info *info,
- struct inode *inode,
- struct file *file)
-{
- file->f_mapping->a_ops = &fb_deferred_io_aops;
-}
-EXPORT_SYMBOL_GPL(fb_deferred_io_open);
-
void fb_deferred_io_cleanup(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 06f5805de2de..c4ba76359f22 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1415,10 +1415,7 @@ __releases(&info->lock)
if (res)
module_put(info->fbops->owner);
}
-#ifdef CONFIG_FB_DEFERRED_IO
- if (info->fbdefio)
- fb_deferred_io_open(info, inode, file);
-#endif
+ file->f_mapping->a_ops = NULL;
out:
unlock_fb_info(info);
if (res)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ecfbcc0553a5..a8dccd23c249 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -659,9 +659,6 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
/* drivers/video/fb_defio.c */
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
extern void fb_deferred_io_init(struct fb_info *info);
-extern void fb_deferred_io_open(struct fb_info *info,
- struct inode *inode,
- struct file *file);
extern void fb_deferred_io_cleanup(struct fb_info *info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);
^ permalink raw reply related
* Re: PowerPC64 future proof kernel toc, revised for lld
From: Alan Modra @ 2021-03-10 3:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alexey, ellerman
In-Reply-To: <20210309045638.GI6042@bubble.grove.modra.org>
This patch future-proofs the kernel against linker changes that might
put the toc pointer at some location other than .got+0x8000, by
replacing __toc_start+0x8000 with .TOC. throughout. If the kernel's
idea of the toc pointer doesn't agree with the linker, bad things
happen.
prom_init.c code relocating its toc is also changed so that a symbolic
__prom_init_toc_start toc-pointer relative address is calculated
rather than assuming that it is always at toc-pointer - 0x8000. The
length calculations loading values from the toc are also avoided.
It's a little incestuous to do that with unreloc_toc picking up
adjusted values (which is fine in practice, they both adjust by the
same amount if all goes well).
I've also changed the way .got is aligned in vmlinux.lds and
zImage.lds, mostly so that dumping out section info by objdump or
readelf plainly shows the alignment is 256. This linker script
feature was added 2005-09-27, available in FSF binutils releases from
2.17 onwards. Should be safe to use in the kernel, I think.
Finally, put *(.got) before the prom_init.o entry which only needs
*(.toc), so that the GOT header goes in the correct place. I don't
believe this makes any difference for the kernel as it would for
dynamic objects being loaded by ld.so. That change is just to stop
lusers who blindly copy kernel scripts being led astray. Of course,
this change needs the prom_init.c changes.
Some notes on .toc and .got.
.toc is a compiler generated section of addresses. .got is a linker
generated section of addresses, generally built when the linker sees
R_*_*GOT* relocations. In the case of powerpc64 ld.bfd, there are
multiple generated .got sections, one per input object file. So you
can somewhat reasonably write in a linker script an input section
statement like *prom_init.o(.got .toc) to mean "the .got and .toc
section for files matching *prom_init.o". On other architectures that
doesn't make sense, because the linker generally has just one .got
section. Even on powerpc64, note well that the GOT entries for
prom_init.o may be merged with GOT entries from other objects. That
means that if prom_init.o references, say, _end via some GOT
relocation, and some other object also references _end via a GOT
relocation, the GOT entry for _end may be in the range
__prom_init_toc_start to __prom_init_toc_end and if the kernel does
something special to GOT/TOC entries in that range then the value of
_end as seen by objects other than prom_init.o will be affected. On
the other hand the GOT entry for _end may not be in the range
__prom_init_toc_start to __prom_init_toc_end. Which way it turns out
is deterministic but a detail of linker operation that should not be
relied on.
A feature of ld.bfd is that input .toc (and .got) sections matching
one linker input section statement may be sorted, to put entries used
by small-model code first, near the toc base. This is why scripts for
powerpc64 normally use *(.got .toc) rather than *(.got) *(.toc), since
the first form allows more freedom to sort.
Another feature of ld.bfd is that indirect addressing sequences using
the GOT/TOC may be edited by the linker to relative addressing. In
many cases relative addressing would be emitted by gcc for
-mcmodel=medium if you appropriately decorate variable declarations
with non-default visibility.
Signed-off-by: Alan Modra <amodra@au1.ibm.com>
diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
index 1d83966f5ef6..e45907fe468f 100644
--- a/arch/powerpc/boot/crt0.S
+++ b/arch/powerpc/boot/crt0.S
@@ -28,7 +28,7 @@ p_etext: .8byte _etext
p_bss_start: .8byte __bss_start
p_end: .8byte _end
-p_toc: .8byte __toc_start + 0x8000 - p_base
+p_toc: .8byte .TOC. - p_base
p_dyn: .8byte __dynamic_start - p_base
p_rela: .8byte __rela_dyn_start - p_base
p_prom: .8byte 0
diff --git a/arch/powerpc/boot/zImage.lds.S b/arch/powerpc/boot/zImage.lds.S
index d6f072865627..d65cd55a6f38 100644
--- a/arch/powerpc/boot/zImage.lds.S
+++ b/arch/powerpc/boot/zImage.lds.S
@@ -36,12 +36,9 @@ SECTIONS
}
#ifdef CONFIG_PPC64_BOOT_WRAPPER
- . = ALIGN(256);
- .got :
+ .got : ALIGN(256)
{
- __toc_start = .;
- *(.got)
- *(.toc)
+ *(.got .toc)
}
#endif
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 324d7b298ec3..e5a1eae11ed5 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -48,14 +48,18 @@ static inline int in_kernel_text(unsigned long addr)
static inline unsigned long kernel_toc_addr(void)
{
- /* Defined by the linker, see vmlinux.lds.S */
- extern unsigned long __toc_start;
-
- /*
- * The TOC register (r2) points 32kB into the TOC, so that 64kB of
- * the TOC can be addressed using a single machine instruction.
- */
- return (unsigned long)(&__toc_start) + 0x8000UL;
+#if 0
+ /* This version is appropriate if the kernel is never compiled
+ -mcmodel=small or the total .toc is always less than 64k. */
+ register unsigned long toc_ptr asm ("r2");
+ return toc_ptr;
+#else
+ /* Otherwise linker automatic multiple toc sections might be
+ required, and in that case r2 may be adjusted by a linker
+ stub. */
+ extern unsigned long __attribute__((visibility("hidden"))) toc asm (".TOC.");
+ return (unsigned long)&toc;
+#endif
}
static inline int overlaps_interrupt_vector_text(unsigned long start,
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index ece7f97bafff..9542d03b2efe 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -899,7 +899,7 @@ _GLOBAL(relative_toc)
blr
.balign 8
-p_toc: .8byte __toc_start + 0x8000 - 0b
+p_toc: .8byte .TOC. - 0b
/*
* This is where the main kernel code starts.
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index ccf77b985c8f..d309a7787652 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -3220,27 +3220,26 @@ static void unreloc_toc(void)
{
}
#else
-static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
+static void __reloc_toc(unsigned long offset)
{
- unsigned long i;
unsigned long *toc_entry;
+ unsigned long *toc_start, *toc_end;
- /* Get the start of the TOC by using r2 directly. */
- asm volatile("addi %0,2,-0x8000" : "=b" (toc_entry));
+ asm("addis %0,2,__prom_init_toc_start@toc@ha\n\t"
+ "addi %0,%0,__prom_init_toc_start@toc@l" : "=b" (toc_start));
+ asm("addis %0,2,__prom_init_toc_end@toc@ha\n\t"
+ "addi %0,%0,__prom_init_toc_end@toc@l" : "=b" (toc_end));
- for (i = 0; i < nr_entries; i++) {
- *toc_entry = *toc_entry + offset;
- toc_entry++;
+ for (toc_entry = toc_start; toc_entry != toc_end; toc_entry++) {
+ *toc_entry += offset;
}
}
static void reloc_toc(void)
{
unsigned long offset = reloc_offset();
- unsigned long nr_entries =
- (__prom_init_toc_end - __prom_init_toc_start) / sizeof(long);
- __reloc_toc(offset, nr_entries);
+ __reloc_toc(offset);
mb();
}
@@ -3248,12 +3247,10 @@ static void reloc_toc(void)
static void unreloc_toc(void)
{
unsigned long offset = reloc_offset();
- unsigned long nr_entries =
- (__prom_init_toc_end - __prom_init_toc_start) / sizeof(long);
mb();
- __reloc_toc(-offset, nr_entries);
+ __reloc_toc(-offset);
}
#endif
#endif
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 72fa3c00229a..3d2e6e2b81b5 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -326,15 +326,13 @@ SECTIONS
__end_opd = .;
}
- . = ALIGN(256);
- .got : AT(ADDR(.got) - LOAD_OFFSET) {
- __toc_start = .;
+ .got : AT(ADDR(.got) - LOAD_OFFSET) ALIGN(256) {
+ *(.got)
#ifndef CONFIG_RELOCATABLE
__prom_init_toc_start = .;
- arch/powerpc/kernel/prom_init.o*(.toc .got)
+ arch/powerpc/kernel/prom_init.o*(.toc)
__prom_init_toc_end = .;
#endif
- *(.got)
*(.toc)
}
#endif
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply related
* Re: Errant readings on LM81 with T2080 SoC
From: Guenter Roeck @ 2021-03-10 3:29 UTC (permalink / raw)
To: Chris Packham, jdelvare@suse.com
Cc: linux-hwmon@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
In-Reply-To: <b1ba3f34-cbcc-4bbd-ea84-aad21f513682@alliedtelesis.co.nz>
On 3/9/21 3:35 PM, Chris Packham wrote:
>
> On 8/03/21 1:31 pm, Guenter Roeck wrote:
>> On 3/7/21 2:52 PM, Chris Packham wrote:
>>> Fundamentally I think this is a problem with the fact that the LM81 is
>>> an SMBus device but the T2080 (and other Freescale SoCs) uses i2c and we
>>> emulate SMBus. I suspect the errant readings are when we don't get round
>>> to completing the read within the timeout specified by the SMBus
>>> specification. Depending on when that happens we either fail the
>>> transfer or interpret the result as all-1s.
>> That is quite unlikely. Many sensor chips are SMBus chips connected to
>> i2c busses. It is much more likely that there is a bug in the T2080 i2c driver,
>> that the chip doesn't like the bulk read command issued through regmap, that
>> the chip has problems with the i2c bus speed, or that the i2c bus is noisy.
> I have noticed that with the switch to regmap we end up using plain i2c
> instead of SMBUS. There appears to be no way of saying use SMBUS
> semantics if the i2c adapter reports I2C_FUNC_I2C.
>
The driver only really supports I2C; SMBUS functions are emulated.
I don't think that makes a real difference.
Guenter
^ permalink raw reply
* Re: [PATCH v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Rob Herring @ 2021-03-10 3:09 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: kexec, linux-kernel@vger.kernel.org, Mimi Zohar,
Thiago Jung Bauermann, linuxppc-dev, Hari Bathini
In-Reply-To: <7d0c6062-ca73-f183-110d-f5b75ae91d10@linux.microsoft.com>
On Tue, Mar 9, 2021 at 7:31 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 3/9/21 6:08 PM, Rob Herring wrote:
>
> Hi Rob,
>
> > On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
> > <bauerman@linux.ibm.com> wrote:
> >>
> >> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> >> kernel") fixed how elf64_load() estimates the FDT size needed by the
> >> crashdump kernel.
> >>
> >> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> >> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> >> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> >> change made the code overestimate it a bit by counting twice the space
> >> required for the kernel command line and /chosen properties.
> >>
> >> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> >> space needed by the kdump kernel, and change the function name so that it
> >> better reflects what the function is now doing.
> >>
> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> >> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> >> ---
> >> arch/powerpc/include/asm/kexec.h | 2 +-
> >> arch/powerpc/kexec/elf_64.c | 2 +-
> >> arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
> >> 3 files changed, 10 insertions(+), 20 deletions(-)
> >
> > I ended up delaying the referenced series til 5.13, but have applied
> > it now. Can I get an ack from the powerpc maintainers on this one?
> > I'll fixup the commit log to make sense given the commit id's aren't
> > valid.
>
> I checked the change applied in linux-next branch and also Device Tree's
> for-next branch - it looks like v1 of Thiago's patch has been applied.
> Could you please pick up the v2 patch?
Huh? This patch (v2) hasn't been applied to any tree AFAICT.
Rob
^ permalink raw reply
* Re: [PATCH v4 3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg cpu dai driver
From: Rob Herring @ 2021-03-10 2:48 UTC (permalink / raw)
To: Shengjiu Wang
Cc: devicetree, alsa-devel, timur, lgirdwood, linuxppc-dev, Xiubo.Lee,
linux-kernel, tiwai, nicoleotsuka, broonie, perex, festevam
In-Reply-To: <1615209750-2357-4-git-send-email-shengjiu.wang@nxp.com>
On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
Bindings describe h/w blocks, not drivers.
> for getting the user's configuration from device tree and configure the
> clocks which is used by Cortex-M core. So in this document define the
> needed property.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> .../devicetree/bindings/sound/fsl,rpmsg.yaml | 118 ++++++++++++++++++
> 1 file changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> new file mode 100644
> index 000000000000..5731c1fbc0a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP Audio RPMSG CPU DAI Controller
> +
> +maintainers:
> + - Shengjiu Wang <shengjiu.wang@nxp.com>
> +
> +description: |
> + fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> + touch hardware. It is mainly used for getting the user's configuration
> + from device tree and configure the clocks which is used by Cortex-M core.
> + So in this document define the needed property.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx7ulp-rpmsg
> + - fsl,imx8mn-rpmsg
> + - fsl,imx8mm-rpmsg
> + - fsl,imx8mp-rpmsg
> +
> + model:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: User specified audio sound card name
> +
> + clocks:
> + items:
> + - description: Peripheral clock for register access
> + - description: Master clock
> + - description: DMA clock for DMA register access
> + - description: Parent clock for multiple of 8kHz sample rates
> + - description: Parent clock for multiple of 11kHz sample rates
> + minItems: 5
If this doesn't touch hardware, what are these clocks for?
You don't need 'minItems' unless it's less than the number of 'items'.
> +
> + clock-names:
> + items:
> + - const: ipg
> + - const: mclk
> + - const: dma
> + - const: pll8k
> + - const: pll11k
> + minItems: 5
> +
> + power-domains:
> + maxItems: 1
> +
> + fsl,audioindex:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1]
> + default: 0
> + description: Instance index for sound card in
> + M core side, which share one rpmsg
> + channel.
We don't do indexes in DT. What's this numbering tied to?
> +
> + fsl,version:
version of what?
This seems odd at best.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2]
You're going to update this with every new firmware version?
> + default: 2
> + description: The version of M core image, which is
> + to make driver compatible with different image.
> +
> + fsl,buffer-size:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: pre allocate dma buffer size
How can you have DMA, this doesn't touch h/w?
> +
> + fsl,enable-lpa:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: enable low power audio path.
> +
> + fsl,rpmsg-out:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + This is a boolean property. If present, the transmitting function
> + will be enabled.
> +
> + fsl,rpmsg-in:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + This is a boolean property. If present, the receiving function
> + will be enabled.
> +
> + fsl,codec-type:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2]
> + default: 0
> + description: Sometimes the codec is registered by
> + driver not by the device tree, this items
> + can be used to distinguish codecs.
How does one decide what value to use?
> +
> + audio-codec:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: The phandle of the audio codec
The codec is controlled from the Linux side?
> +
> + memory-region:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: phandle to the reserved memory nodes
> +
> +required:
> + - compatible
> + - fsl,audioindex
> + - fsl,version
> + - fsl,buffer-size
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + rpmsg_audio: rpmsg_audio {
> + compatible = "fsl,imx8mn-rpmsg";
> + fsl,audioindex = <0> ;
> + fsl,version = <2>;
> + fsl,buffer-size = <0x6000000>;
> + fsl,enable-lpa;
How does this work? Don't you need somewhere to put the 'rpmsg' data?
> + };
> --
> 2.27.0
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Lakshmi Ramasubramanian @ 2021-03-10 2:31 UTC (permalink / raw)
To: Rob Herring, Thiago Jung Bauermann
Cc: kexec, linux-kernel@vger.kernel.org, Mimi Zohar, linuxppc-dev,
Hari Bathini
In-Reply-To: <CAL_JsqJatxTwSbM8a0PB2ievO_fRFeYJ8ZtBN0pRfnEjSXyUkg@mail.gmail.com>
On 3/9/21 6:08 PM, Rob Herring wrote:
Hi Rob,
> On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
> <bauerman@linux.ibm.com> wrote:
>>
>> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
>> kernel") fixed how elf64_load() estimates the FDT size needed by the
>> crashdump kernel.
>>
>> At the same time, commit 130b2d59cec0 ("powerpc: Use common
>> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
>> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
>> change made the code overestimate it a bit by counting twice the space
>> required for the kernel command line and /chosen properties.
>>
>> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
>> space needed by the kdump kernel, and change the function name so that it
>> better reflects what the function is now doing.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> ---
>> arch/powerpc/include/asm/kexec.h | 2 +-
>> arch/powerpc/kexec/elf_64.c | 2 +-
>> arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>> 3 files changed, 10 insertions(+), 20 deletions(-)
>
> I ended up delaying the referenced series til 5.13, but have applied
> it now. Can I get an ack from the powerpc maintainers on this one?
> I'll fixup the commit log to make sense given the commit id's aren't
> valid.
I checked the change applied in linux-next branch and also Device Tree's
for-next branch - it looks like v1 of Thiago's patch has been applied.
Could you please pick up the v2 patch?
thanks,
-lakshmi
^ permalink raw reply
* Re: Errant readings on LM81 with T2080 SoC
From: Chris Packham @ 2021-03-10 2:19 UTC (permalink / raw)
To: Guenter Roeck, jdelvare@suse.com
Cc: linux-hwmon@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
In-Reply-To: <d6074923-ee7e-4499-0e54-383a607d3c41@alliedtelesis.co.nz>
On 9/03/21 9:27 am, Chris Packham wrote:
> On 8/03/21 5:59 pm, Guenter Roeck wrote:
>> Other than that, the only other real idea I have would be to monitor
>> the i2c bus.
> I am in the fortunate position of being able to go into the office and
> even happen to have the expensive scope at the moment. Now I just need
> to find a tame HW engineer so I don't burn myself trying to attach the
> probes.
One thing I see on the scope is that when there is a CPU load there
appears to be some clock stretching going on (SCL is held low some
times). I don't see it without the CPU load. It's hard to correlate a
clock stretching event with a bad read or error but it is one area where
the SMBUS spec has a maximum that might cause the device to give up waiting.
^ permalink raw reply
* Re: [PATCH v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Rob Herring @ 2021-03-10 2:08 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: kexec, linux-kernel@vger.kernel.org, Mimi Zohar,
Lakshmi Ramasubramanian, linuxppc-dev, Hari Bathini
In-Reply-To: <20210220005204.1417200-1-bauerman@linux.ibm.com>
On Fri, Feb 19, 2021 at 6:52 PM Thiago Jung Bauermann
<bauerman@linux.ibm.com> wrote:
>
> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> kernel") fixed how elf64_load() estimates the FDT size needed by the
> crashdump kernel.
>
> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> change made the code overestimate it a bit by counting twice the space
> required for the kernel command line and /chosen properties.
>
> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> space needed by the kdump kernel, and change the function name so that it
> better reflects what the function is now doing.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
> arch/powerpc/include/asm/kexec.h | 2 +-
> arch/powerpc/kexec/elf_64.c | 2 +-
> arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
> 3 files changed, 10 insertions(+), 20 deletions(-)
I ended up delaying the referenced series til 5.13, but have applied
it now. Can I get an ack from the powerpc maintainers on this one?
I'll fixup the commit log to make sense given the commit id's aren't
valid.
Rob
^ permalink raw reply
* Re: [PATCH v2 40/43] powerpc/64s: Make kuap_check_amr() and kuap_get_and_check_amr() generic
From: Nicholas Piggin @ 2021-03-10 1:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7167aef44fb816f6df17f65d540ac07ca98c4af9.1615291474.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of March 9, 2021 10:10 pm:
> In preparation of porting powerpc32 to C syscall entry/exit,
> rename kuap_check_amr() and kuap_get_and_check_amr() as kuap_check()
> and kuap_get_and_check(), and move in the generic asm/kup.h the stub
> for when CONFIG_PPC_KUAP is not selected.
Looks pretty straightforward to me.
While you're renaming things, could kuap_check_amr() be changed to
kuap_assert_locked() or similar? Otherwise,
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/book3s/64/kup.h | 24 ++----------------------
> arch/powerpc/include/asm/kup.h | 10 +++++++++-
> arch/powerpc/kernel/interrupt.c | 12 ++++++------
> arch/powerpc/kernel/irq.c | 2 +-
> 4 files changed, 18 insertions(+), 30 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
> index 8bd905050896..d9b07e9998be 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup.h
> @@ -287,7 +287,7 @@ static inline void kuap_kernel_restore(struct pt_regs *regs,
> */
> }
>
> -static inline unsigned long kuap_get_and_check_amr(void)
> +static inline unsigned long kuap_get_and_check(void)
> {
> if (mmu_has_feature(MMU_FTR_BOOK3S_KUAP)) {
> unsigned long amr = mfspr(SPRN_AMR);
> @@ -298,27 +298,7 @@ static inline unsigned long kuap_get_and_check_amr(void)
> return 0;
> }
>
> -#else /* CONFIG_PPC_PKEY */
> -
> -static inline void kuap_user_restore(struct pt_regs *regs)
> -{
> -}
> -
> -static inline void kuap_kernel_restore(struct pt_regs *regs, unsigned long amr)
> -{
> -}
> -
> -static inline unsigned long kuap_get_and_check_amr(void)
> -{
> - return 0;
> -}
> -
> -#endif /* CONFIG_PPC_PKEY */
> -
> -
> -#ifdef CONFIG_PPC_KUAP
> -
> -static inline void kuap_check_amr(void)
> +static inline void kuap_check(void)
> {
> if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) && mmu_has_feature(MMU_FTR_BOOK3S_KUAP))
> WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
> index 25671f711ec2..b7efa46b3109 100644
> --- a/arch/powerpc/include/asm/kup.h
> +++ b/arch/powerpc/include/asm/kup.h
> @@ -74,7 +74,15 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
> return false;
> }
>
> -static inline void kuap_check_amr(void) { }
> +static inline void kuap_check(void) { }
> +static inline void kuap_save_and_lock(struct pt_regs *regs) { }
> +static inline void kuap_user_restore(struct pt_regs *regs) { }
> +static inline void kuap_kernel_restore(struct pt_regs *regs, unsigned long amr) { }
> +
> +static inline unsigned long kuap_get_and_check(void)
> +{
> + return 0;
> +}
>
> /*
> * book3s/64/kup-radix.h defines these functions for the !KUAP case to flush
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 727b7848c9cc..40ed55064e54 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -76,7 +76,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> } else
> #endif
> #ifdef CONFIG_PPC64
> - kuap_check_amr();
> + kuap_check();
> #endif
>
> booke_restore_dbcr0();
> @@ -254,7 +254,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> #ifdef CONFIG_PPC64
> - kuap_check_amr();
> + kuap_check();
> #endif
>
> regs->result = r3;
> @@ -380,7 +380,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> * AMR can only have been unlocked if we interrupted the kernel.
> */
> #ifdef CONFIG_PPC64
> - kuap_check_amr();
> + kuap_check();
> #endif
>
> local_irq_save(flags);
> @@ -451,7 +451,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> unsigned long flags;
> unsigned long ret = 0;
> #ifdef CONFIG_PPC64
> - unsigned long amr;
> + unsigned long kuap;
> #endif
>
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
> @@ -467,7 +467,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> #ifdef CONFIG_PPC64
> - amr = kuap_get_and_check_amr();
> + kuap = kuap_get_and_check();
> #endif
>
> if (unlikely(current_thread_info()->flags & _TIF_EMULATE_STACK_STORE)) {
> @@ -511,7 +511,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> * value from the check above.
> */
> #ifdef CONFIG_PPC64
> - kuap_kernel_restore(regs, amr);
> + kuap_kernel_restore(regs, kuap);
> #endif
>
> return ret;
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index d71fd10a1dd4..3b18d2b2c702 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -282,7 +282,7 @@ static inline void replay_soft_interrupts_irqrestore(void)
> * and re-locking AMR but we shouldn't get here in the first place,
> * hence the warning.
> */
> - kuap_check_amr();
> + kuap_check();
>
> if (kuap_state != AMR_KUAP_BLOCKED)
> set_kuap(AMR_KUAP_BLOCKED);
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v2 36/43] powerpc/32: Set current->thread.regs in C interrupt entry
From: Nicholas Piggin @ 2021-03-10 1:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <8d523f9ecee1de0515cc31d43030c12ab171a670.1615291474.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of March 9, 2021 10:10 pm:
> No need to do that is assembly, do it in C.
Hmm. No issues with the patch as such, but why does ppc32 need this but
not 64? AFAIKS 64 sets this when a thread is created.
Thanks,
Nick
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/interrupt.h | 4 +++-
> arch/powerpc/kernel/entry_32.S | 3 +--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 861e6eadc98c..e6d71c2e3aa2 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -33,8 +33,10 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
> if (!arch_irq_disabled_regs(regs))
> trace_hardirqs_off();
>
> - if (user_mode(regs))
> + if (user_mode(regs)) {
> + current->thread.regs = regs;
> account_cpu_user_entry();
> + }
> #endif
> /*
> * Book3E reconciles irq soft mask in asm
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 8fe1c3fdfa6e..815a4ff1ba76 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -52,8 +52,7 @@
> prepare_transfer_to_handler:
> andi. r0,r9,MSR_PR
> addi r12, r2, THREAD
> - beq 2f /* if from user, fix up THREAD.regs */
> - stw r3,PT_REGS(r12)
> + beq 2f
> #ifdef CONFIG_PPC_BOOK3S_32
> kuep_lock r11, r12
> #endif
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v2 28/43] powerpc/64e: Call bad_page_fault() from do_page_fault()
From: Nicholas Piggin @ 2021-03-10 1:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b2878184d4c21faa8af55b60e52c83f391272112.1615291473.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of March 9, 2021 10:09 pm:
> book3e/64 is the last one calling __bad_page_fault()
> from assembly.
>
> Save non volatile registers before calling do_page_fault()
> and modify do_page_fault() to call __bad_page_fault()
> for all platforms.
>
> Then it can be refactored by the call of bad_page_fault()
> which avoids the duplication of the exception table search.
This can go in with the 64e change after your series. I think it should
be ready for the next merge window as well.
Thanks,
Nick
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/exceptions-64e.S | 8 +-------
> arch/powerpc/mm/fault.c | 17 ++++-------------
> 2 files changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index e8eb9992a270..b60f89078a3f 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -1010,15 +1010,9 @@ storage_fault_common:
> addi r3,r1,STACK_FRAME_OVERHEAD
> ld r14,PACA_EXGEN+EX_R14(r13)
> ld r15,PACA_EXGEN+EX_R15(r13)
> + bl save_nvgprs
> bl do_page_fault
> - cmpdi r3,0
> - bne- 1f
> b ret_from_except_lite
> -1: bl save_nvgprs
> - mr r4,r3
> - addi r3,r1,STACK_FRAME_OVERHEAD
> - bl __bad_page_fault
> - b ret_from_except
>
> /*
> * Alignment exception doesn't fit entirely in the 0x100 bytes so it
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 2e54bac99a22..7bcff3fca110 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -541,24 +541,15 @@ NOKPROBE_SYMBOL(___do_page_fault);
>
> static long __do_page_fault(struct pt_regs *regs)
> {
> - const struct exception_table_entry *entry;
> long err;
>
> err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> if (likely(!err))
> - return err;
> -
> - entry = search_exception_tables(regs->nip);
> - if (likely(entry)) {
> - instruction_pointer_set(regs, extable_fixup(entry));
> return 0;
> - } else if (!IS_ENABLED(CONFIG_PPC_BOOK3E_64)) {
> - __bad_page_fault(regs, err);
> - return 0;
> - } else {
> - /* 32 and 64e handle the bad page fault in asm */
> - return err;
> - }
> +
> + bad_page_fault(regs, err);
> +
> + return 0;
> }
> NOKPROBE_SYMBOL(__do_page_fault);
>
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v2 02/43] powerpc/traps: Declare unrecoverable_exception() as __noreturn
From: Nicholas Piggin @ 2021-03-10 1:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <f097a1071254e8f6875588f8fb9771467824a569.1615291471.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of March 9, 2021 10:09 pm:
> unrecoverable_exception() is never expected to return, most callers
> have an infiniteloop in case it returns.
>
> Ensure it really never returns by terminating it with a BUG(), and
> declare it __no_return.
>
> It always GCC to really simplify functions calling it. In the exemple
> below, it avoids the stack frame in the likely fast path and avoids
> code duplication for the exit.
>
> With this patch:
[snip]
Nice.
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index a44a30b0688c..d5c9d9ddd186 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -2170,11 +2170,15 @@ DEFINE_INTERRUPT_HANDLER(SPEFloatingPointRoundException)
> * in the MSR is 0. This indicates that SRR0/1 are live, and that
> * we therefore lost state by taking this exception.
> */
> -void unrecoverable_exception(struct pt_regs *regs)
> +void __noreturn unrecoverable_exception(struct pt_regs *regs)
> {
> pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
> regs->trap, regs->nip, regs->msr);
> die("Unrecoverable exception", regs, SIGABRT);
> + /* die() should not return */
> + WARN(true, "die() unexpectedly returned");
> + for (;;)
> + ;
> }
I don't think the WARN should be added because that will cause another
interrupt after something is already badly wrong, so this might just
make it harder to debug.
For example if die() is falling through for some reason, we warn and
cause a program check here, and that might also be unrecoverable so it
might come through here and fall through again and warn again, etc.
Putting the infinite loop is good enough I think (and better than there
was previously).
Otherwise
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 01/43] powerpc/traps: unrecoverable_exception() is not an interrupt handler
From: Nicholas Piggin @ 2021-03-10 1:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <ae96c59fa2cb7f24a8929c58cfa2c909cb8ff1f1.1615291471.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of March 9, 2021 10:09 pm:
> unrecoverable_exception() is called from interrupt handlers or
> after an interrupt handler has failed.
>
> Make it a standard function to avoid doubling the actions
> performed on interrupt entry (e.g.: user time accounting).
>
> Fixes: 3a96570ffceb ("powerpc: convert interrupt handlers to use wrappers")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
This should go in as a fix for this release I think.
> ---
> arch/powerpc/include/asm/interrupt.h | 3 ++-
> arch/powerpc/kernel/interrupt.c | 1 -
> arch/powerpc/kernel/traps.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index aedfba29e43a..e8d09a841373 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -410,7 +410,6 @@ DECLARE_INTERRUPT_HANDLER(altivec_assist_exception);
> DECLARE_INTERRUPT_HANDLER(CacheLockingException);
> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointException);
> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointRoundException);
> -DECLARE_INTERRUPT_HANDLER(unrecoverable_exception);
> DECLARE_INTERRUPT_HANDLER(WatchdogException);
> DECLARE_INTERRUPT_HANDLER(kernel_bad_stack);
>
> @@ -437,6 +436,8 @@ DECLARE_INTERRUPT_HANDLER_NMI(hmi_exception_realmode);
>
> DECLARE_INTERRUPT_HANDLER_ASYNC(TAUException);
>
> +void unrecoverable_exception(struct pt_regs *regs);
> +
> void replay_system_reset(void);
> void replay_soft_interrupts(void);
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 398cd86b6ada..b8e7d25be31b 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -436,7 +436,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> return ret;
> }
>
> -void unrecoverable_exception(struct pt_regs *regs);
> void preempt_schedule_irq(void);
>
> notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 1583fd1c6010..a44a30b0688c 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -2170,7 +2170,7 @@ DEFINE_INTERRUPT_HANDLER(SPEFloatingPointRoundException)
> * in the MSR is 0. This indicates that SRR0/1 are live, and that
> * we therefore lost state by taking this exception.
> */
> -DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)
> +void unrecoverable_exception(struct pt_regs *regs)
> {
> pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
> regs->trap, regs->nip, regs->msr);
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: Errant readings on LM81 with T2080 SoC
From: Chris Packham @ 2021-03-09 23:35 UTC (permalink / raw)
To: Guenter Roeck, jdelvare@suse.com
Cc: linux-hwmon@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
In-Reply-To: <96d660bc-17ab-4e0e-9a94-bce1737a8da1@roeck-us.net>
On 8/03/21 1:31 pm, Guenter Roeck wrote:
> On 3/7/21 2:52 PM, Chris Packham wrote:
>> Fundamentally I think this is a problem with the fact that the LM81 is
>> an SMBus device but the T2080 (and other Freescale SoCs) uses i2c and we
>> emulate SMBus. I suspect the errant readings are when we don't get round
>> to completing the read within the timeout specified by the SMBus
>> specification. Depending on when that happens we either fail the
>> transfer or interpret the result as all-1s.
> That is quite unlikely. Many sensor chips are SMBus chips connected to
> i2c busses. It is much more likely that there is a bug in the T2080 i2c driver,
> that the chip doesn't like the bulk read command issued through regmap, that
> the chip has problems with the i2c bus speed, or that the i2c bus is noisy.
I have noticed that with the switch to regmap we end up using plain i2c
instead of SMBUS. There appears to be no way of saying use SMBUS
semantics if the i2c adapter reports I2C_FUNC_I2C.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox