RE: [EXT] [ppc][dlpar] WARNING: CPU: 2 PID: 1131623 at drivers/scsi/qla2xxx/qla_init.c:498 qla2x00_async_adisc_sp_done+0x294/0x2b0
Abdul, For this test case, the warning can be treated as cometic. During this port disable / driver unload path, driver flush all command but fail to set the proper error code in this path. A recent patch does a warning if the error code is not part of the error code list. Regardless, the same outcome is achieved. Will follow with a patch to suppress the warning. commit 724361921f65a40ae5b80641dc1e92c0ff314d89 Author: Bart Van Assche Date: Thu Aug 8 20:02:14 2019 -0700 scsi: qla2xxx: Report invalid mailbox status codes It is easy to mix up the QLA_* and the MBS_* status codes. Complain loudly if that happens. Regards, Quinn Tran -Original Message- From: Abdul Haleem Sent: Monday, July 12, 2021 11:23 AM To: linux-scsi Cc: linuxppc-dev ; linux-kernel ; lober...@redhat.com; Quinn Tran ; Nilesh Javali ; Martin K. Petersen ; himanshu.madh...@oracle.com; Brian King ; Michael Ellerman Subject: [EXT] [ppc][dlpar] WARNING: CPU: 2 PID: 1131623 at drivers/scsi/qla2xxx/qla_init.c:498 qla2x00_async_adisc_sp_done+0x294/0x2b0 External Email -- Greeting's DLPAR hotunplug of FC adapter results in WARN_ONCE on my powerpc box dmesg logs: qla2xxx [0007:01:00.0]-2072:2: Async-login - 21:00:f4:e9:d4:54:a7:0f hdl=0, loopid=3 portid=0f1100 retries=27. qla2xxx [0007:01:00.0]-b079:2: Removing driver qla2xxx [0007:01:00.0]-00af:2: Performing ISP error recovery - ha=09e61ed1. qla2xxx [0007:01:00.0]-2116:2: unregister localport=4d1de818 qla2xxx [0007:01:00.0]-210f:2: localport delete of 4d1de818 completed. scsi 2:0:0:1: alua: Detached scsi 2:0:1:0: alua: Detached qla2xxx [0007:01:00.1]-5013:1: RSCN database changed -- 000f 0100 . scsi 2:0:0:2: alua: Detached scsi 2:0:1:1: alua: Detached scsi 2:0:0:3: alua: Detached scsi 2:0:0:0: alua: Detached scsi 2:0:1:2: alua: Detached scsi 2:0:0:4: alua: Detached scsi 2:0:1:3: alua: Detached scsi 2:0:0:5: alua: Detached scsi 2:0:1:4: alua: Detached scsi 2:0:1:5: alua: Detached pci 0007:01:00.0: Removing from iommu group 0 qla2xxx [0007:01:00.1]-b079:1: Removing driver qla2xxx [0007:01:00.1]-00af:1: Performing ISP error recovery - ha=c0615263. [ cut here ] mbs: 0x0 WARNING: CPU: 2 PID: 1131623 at drivers/scsi/qla2xxx/qla_init.c:498 qla2x00_async_adisc_sp_done+0x294/0x2b0 [qla2xxx] Modules linked in: isofs cdrom dm_round_robin dm_queue_length vfat fat btrfs blake2b_generic zstd_compress loop xfs raid0 rpadlpar_io rpaphp nfnetlink tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag bonding rfkill sunrpc raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c pseries_rng xts vmx_crypto gf128mul sch_fq_codel binfmt_misc ip_tables ext4 mbcache jbd2 dm_service_time sd_mod sg qla2xxx ibmvfc ibmveth nvme_fc nvme_fabrics nvme_core t10_pi scsi_transport_fc dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse CPU: 2 PID: 1131623 Comm: drmgr Not tainted 5.13.0-rc1-autotest #1 NIP: c0080790857c LR: c00807908578 CTR: 006cdb9c REGS: c0001cdc3380 TRAP: 0700 Not tainted (5.13.0-rc1-autotest) MSR: 8282b033 CR: 48002224 XER: 0009 CFAR: c013fd00 IRQMASK: 1 GPR00: c00807908578 c0001cdc3620 c008079d3000 0008 GPR04: 7fff c0001cdc32e0 0027 c009ff5d7e08 GPR08: 0023 0001 0027 c17e72b8 GPR12: 2000 c009d680 c00aaa9d6000 GPR16: c0002bd19000 c00022a06600 c00aaa9c8890 GPR20: c0003e4b5040 00044000 f003 GPR24: 0001 cccd GPR28: c00aaa9c8890 c008079ab678 c0140a104800 c0002bd19000 NIP [c0080790857c] qla2x00_async_adisc_sp_done+0x294/0x2b0 [qla2xxx] LR [c00807908578] qla2x00_async_adisc_sp_done+0x290/0x2b0 [qla2xxx] Call Trace: [c0001cdc3620] [c00807908578] qla2x00_async_adisc_sp_done+0x290/0x2b0 [qla2xxx] (unreliable) [c0001cdc3710] [c008078f3080] __qla2x00_abort_all_cmds+0x1b8/0x580 [qla2xxx] [c0001cdc3840] [c008078f589c] qla2x00_abort_all_cmds+0x34/0xd0 [qla2xxx] [c0001cdc3880] [c008079153d8] qla2x00_abort_isp_cleanup+0x3f0/0x570 [qla2xxx] [c0001cdc3920] [c008078fb7e8] qla2x00_remove_one+0x3d0/0x480 [qla2xxx] [c0001cdc39b0] [c071c274] pci_device_remove+0x64/0x120 [c0001cdc39f0] [c07fb818] device_release_driver_internal+0x168/0x2a0 [c0001cdc3a30] [c070e304] pci_stop_bus_device+0xb4/0x100 [c0001cdc3a70] [c070e4f0] pci_stop_and_remove_bus_device+0x20/0x40 [c0001cdc3aa0] [c0073940] pci_hp_remove_devices+0x90/0x130 [c0001cdc3b30] [c008070704d0] disable_slot+0x38/0x90 [rpaphp] [c0001cdc3b60] [c073eb4c]
Re: [PATCH v3 2/5] dt-bindings: nintendo-otp: Document the Wii and Wii U OTP support
On Fri, Jul 02, 2021 at 12:57:40AM +0200, Emmanuel Gil Peyrot wrote:
> Both of these consoles use the exact same two registers, even at the
> same address, but the Wii U has eight banks of 128 bytes memory while
> the Wii only has one, hence the two compatible strings.
>
> Signed-off-by: Emmanuel Gil Peyrot
> ---
> .../bindings/nvmem/nintendo-otp.yaml | 44 +++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
>
> diff --git a/Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
> b/Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
> new file mode 100644
> index ..c39bd64b03b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0
GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/nintendo-otp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nintendo Wii and Wii U OTP Device Tree Bindings
> +
> +description: |
> + This binding represents the OTP memory as found on a Nintendo Wii or Wii U,
> + which contains common and per-console keys, signatures and related data
> + required to access peripherals.
> +
> + See https://wiiubrew.org/wiki/Hardware/OTP
> +
> +maintainers:
> + - Emmanuel Gil Peyrot
> +
> +allOf:
> + - $ref: "nvmem.yaml#"
> +
> +properties:
> + compatible:
> +enum:
> + - nintendo,hollywood-otp
> + - nintendo,latte-otp
> +
> + reg:
> +maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +otp@d8001ec {
> +compatible = "nintendo,latte-otp";
> +reg = <0x0d8001ec 0x8>;
> +};
> +
> +...
> --
> 2.32.0
>
>
Re: [PATCH] soc: fsl: qe: convert QE interrupt controller to platform_device
On Mon, Jul 5, 2021 at 6:12 AM Maxim Kochetkov wrote:
>
> Since 5.13 QE's ucc nodes can't get interrupts from devicetree:
>
> ucc@2000 {
> cell-index = <1>;
> reg = <0x2000 0x200>;
> interrupts = <32>;
> interrupt-parent = <>;
> };
>
> Now fw_devlink expects driver to create and probe a struct device
> for interrupt controller.
>
> So lets convert this driver to simple platform_device with probe().
>
> [1] -
> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Fixes: ea718c699055 ("Revert "Revert "driver core: Set fw_devlink=on by
> default""")
> Signed-off-by: Maxim Kochetkov
> ---
> drivers/soc/fsl/qe/qe_ic.c | 38 +++---
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
> index 3f711c1a0996..03d291376895 100644
> --- a/drivers/soc/fsl/qe/qe_ic.c
> +++ b/drivers/soc/fsl/qe/qe_ic.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -404,27 +405,28 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc
> *desc)
> chip->irq_eoi(>irq_data);
> }
>
> -static void __init qe_ic_init(struct device_node *node)
> +static int qe_ic_init(struct platform_device *pdev)
> {
> void (*low_handler)(struct irq_desc *desc);
> void (*high_handler)(struct irq_desc *desc);
> struct qe_ic *qe_ic;
> struct resource res;
> + struct device_node *node = pdev->dev.of_node;
> u32 ret;
>
> ret = of_address_to_resource(node, 0, );
> if (ret)
> - return;
> + return -ENODEV;
>
> qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL);
> if (qe_ic == NULL)
> - return;
> + return -ENOMEM;
>
> qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
>_ic_host_ops, qe_ic);
> if (qe_ic->irqhost == NULL) {
> kfree(qe_ic);
> - return;
> + return -ENODEV;
> }
>
> qe_ic->regs = ioremap(res.start, resource_size());
> @@ -437,7 +439,7 @@ static void __init qe_ic_init(struct device_node *node)
> if (!qe_ic->virq_low) {
> printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
> kfree(qe_ic);
> - return;
> + return -ENODEV;
> }
> if (qe_ic->virq_high != qe_ic->virq_low) {
> low_handler = qe_ic_cascade_low;
> @@ -456,20 +458,26 @@ static void __init qe_ic_init(struct device_node *node)
> irq_set_handler_data(qe_ic->virq_high, qe_ic);
> irq_set_chained_handler(qe_ic->virq_high, high_handler);
> }
> + return 0;
> }
> +static const struct of_device_id qe_ic_ids[] = {
> + { .compatible = "fsl,qe-ic"},
> + { .compatible = "qeic"},
>From the original code, this should be type = "qeic". It is not
defined in current binding but probably needed for backward
compatibility.
It would be great if you can also deal with the comments from Dan too. Thanks.
> + {},
> +};
>
> -static int __init qe_ic_of_init(void)
> +static struct platform_driver qe_ic_driver =
> {
> - struct device_node *np;
> + .driver = {
> + .name = "qe-ic",
> + .of_match_table = qe_ic_ids,
> + },
> + .probe = qe_ic_init,
> +};
>
> - np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> - if (!np) {
> - np = of_find_node_by_type(NULL, "qeic");
> - if (!np)
> - return -ENODEV;
> - }
> - qe_ic_init(np);
> - of_node_put(np);
> +static int __init qe_ic_of_init(void)
> +{
> + platform_driver_register(_ic_driver);
> return 0;
> }
> subsys_initcall(qe_ic_of_init);
> --
> 2.31.1
>
Re: [PATCH v1 2/4] mm/memory_hotplug: remove nid parameter from arch_remove_memory()
> The parameter is unused, let's remove it.
>
> Acked-by: Catalin Marinas
> Acked-by: Michael Ellerman (powerpc)
> Acked-by: Heiko Carstens (s390)
> Cc: Catalin Marinas
> Cc: Will Deacon
> Cc: Michael Ellerman
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Heiko Carstens
> Cc: Vasily Gorbik
> Cc: Christian Borntraeger
> Cc: Yoshinori Sato
> Cc: Rich Felker
> Cc: Dave Hansen
> Cc: Andy Lutomirski
> Cc: Peter Zijlstra
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> Cc: Borislav Petkov
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin"
> Cc: Andrew Morton
> Cc: Anshuman Khandual
> Cc: Ard Biesheuvel
> Cc: Mike Rapoport
> Cc: Nicholas Piggin
> Cc: Pavel Tatashin
> Cc: Baoquan He
> Cc: Laurent Dufour
> Cc: Sergei Trofimovich
> Cc: Kefeng Wang
> Cc: Michel Lespinasse
> Cc: Christophe Leroy
> Cc: "Aneesh Kumar K.V"
> Cc: Thiago Jung Bauermann
> Cc: Joe Perches
> Cc: Pierre Morel
> Cc: Jia He
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-i...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Signed-off-by: David Hildenbrand
> ---
> arch/arm64/mm/mmu.c| 3 +--
> arch/ia64/mm/init.c| 3 +--
> arch/powerpc/mm/mem.c | 3 +--
> arch/s390/mm/init.c| 3 +--
> arch/sh/mm/init.c | 3 +--
> arch/x86/mm/init_32.c | 3 +--
> arch/x86/mm/init_64.c | 3 +--
> include/linux/memory_hotplug.h | 3 +--
> mm/memory_hotplug.c| 4 ++--
> mm/memremap.c | 5 +
> 10 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index d74586508448..af8ab553a268 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1506,8 +1506,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 064a967a7b6e..5c6da8d83c1a 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -484,8 +484,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index ad198b439222..c3c4e31462ec 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -119,8 +119,7 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
> return rc;
> }
>
> -void __ref arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void __ref arch_remove_memory(u64 start, u64 size, struct vmem_altmap
> *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 8ac710de1ab1..d85bd7f5d8dc 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -306,8 +306,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return rc;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index ce26c7f8950a..506784702430 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -414,8 +414,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = PFN_DOWN(start);
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 74b78840182d..bd90b8fe81e4 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -801,8 +801,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return __add_pages(nid, start_pfn, nr_pages, params);
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >>
Re: [PATCH v4 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
On Wed, 2021-07-14 at 18:38 +1000, Alexey Kardashevskiy wrote:
> > for (i = 0; i <
> > > > ARRAY_SIZE(pci->phb->mem_resources);
> > > > i++) {
> > > > + const unsigned long mask =
> > > > IORESOURCE_MEM_64
> > > > > IORESOURCE_MEM;
> > > > +
> > > > + /* Look for MMIO32 */
> > > > + if ((pci->phb->mem_resources[i].flags &
> > > > mask)
> > > > == IORESOURCE_MEM)
> > > > + break;
> > >
> > > What if there is no IORESOURCE_MEM? pci->phb-
> > > >mem_resources[i].start
> > > below will have garbage.
> >
> >
> >
> > Yeah, that makes sense. I will add this lines after 'for':
> >
> > if (i == ARRAY_SIZE(pci->phb->mem_resources)) {
> > iommu_tce_table_put(newtbl);
> > goto out_del_list;
> > }
> >
> > What do you think?
>
>
> Move this and that "for" before iommu_pseries_alloc_table() so you
> won't
> need to free if there is no IORESOURCE_MEM.
Done!
>
>
> >
> >
> > >
> > >
> > > > + }
> > > > +
> > > > + _iommu_table_setparms(newtbl, pci->phb->bus-
> > > > >number,
> > > > create.liobn, win_addr,
> > > > + 1UL << len, page_shift,
> > > > 0,
> > > > _table_lpar_multi_ops);
> > > > + iommu_init_table(newtbl, pci->phb->node, pci-
> > > > >phb-
> > > > > mem_resources[i].start,
> > > > + pci->phb-
> > > > >mem_resources[i].end);
> > > > +
> > > > + if (default_win_removed)
> > > > + iommu_tce_table_put(tbl);
> > >
> > >
> > > iommu_tce_table_put() should have been called when the window was
> > > removed.
> > >
> > > Also after some thinking - what happens if there were 2 devices
> > > in the
> > > PE and one requested 64bit DMA? This will only update
> > > set_iommu_table_base() for the 64bit one but not for the other.
> > >
> > > I think the right thing to do is:
> > >
> > > 1. check if table[0] is in use, if yes => fail (which this does
> > > already)
> > >
> > > 2. remove default dma window but keep the iommu_table struct with
> > > one
> > > change - set it_size to 0 (and free it_map) so the 32bit device
> > > won't
> > > look at a stale structure and think there is some window
> > > (imaginery
> > > situation for phyp but easy to recreate in qemu).
> > >
> > > 3. use table[1] for newly created indirect DDW window.
> > >
> > > 4. change get_iommu_table_base() to return a usable table (or may
> > > be
> > > not
> > > needed?).
> > >
> > > If this sounds reasonable (does it?),
> >
> > Looks ok, I will try your suggestion.
> > I was not aware of how pci->table_group->tables[] worked, so I
> > replaced
> > pci->table_group->tables[0] with the new tbl, while moving the
> > older in
> > pci->table_group->tables[1].
>
>
> pci->table_group->tables[0] is window#0 at @0.
> pci->table_group->tables[1] is window#1 at 0x0800....
> That
> is all :)
>
> pseries does not use tables[1] but powernv does (by VFIO only
> though).
Thanks! This helped a lot!
>
>
> > (4) get_iommu_table_base() does not seem to need update, as it
> > returns
> > the tlb set by set_iommu_table_base() which is already called in
> > the
> > !direct_mapping path in current patch.
>
> Sounds right.
>
> >
> > > the question is now if you have
> > > time to do that and the hardware to test that, or I'll have to
> > > finish
> > > the work :)
> >
> > Sorry, for some reason part of this got lost in Evolution mail
> > client.
> >
> > If possible, I do want to finish this work, and I am talking to IBM
> > Virt people in order to get testing HW.
>
>
> Even I struggle to find a powervm machine :)
>
> > >
> > >
> > > > + else
> > > > + pci->table_group->tables[1] = tbl;
> > >
> > >
> > > What is this for?
> >
> > I was thinking of adding the older table to pci->table_group-
> > >tables[1]
> > while keeping the newer table on pci->table_group->tables[0].
> > This did work, but I think your suggestion may work better.
> >
> > Best regards,
> > Leonardo Bras
> >
> >
>
Thanks a lot for reviewing Alexey!
I will send a v5 soon.
Best regards,
Leonardo Bras
[Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
https://bugzilla.kernel.org/show_bug.cgi?id=210749 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #295621|0 |1 is obsolete|| --- Comment #10 from Erhard F. (erhar...@mailbox.org) --- Created attachment 297869 --> https://bugzilla.kernel.org/attachment.cgi?id=297869=edit kernel .config (kernel 5.14-rc1, Talos II) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
https://bugzilla.kernel.org/show_bug.cgi?id=210749 --- Comment #9 from Erhard F. (erhar...@mailbox.org) --- Created attachment 297867 --> https://bugzilla.kernel.org/attachment.cgi?id=297867=edit dmesg (kernel 5.14-rc1 w. 61f764c307f6, 4e302c3b568e reverted, Talos II) Yes, I can confirm reverting 61f764c307f6 and 4e302c3b568e works. Tested on kernel 5.14-rc1. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v4 07/11] powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new helper
On Wed, 2021-07-14 at 18:32 +1000, Alexey Kardashevskiy wrote: > > > On 13/07/2021 14:47, Leonardo Brás wrote: > > Hello Alexey, > > > > On Fri, 2021-06-18 at 19:26 -0300, Leonardo Brás wrote: > > > > > > > > > + unsigned long liobn, > > > > > unsigned long win_addr, > > > > > + unsigned long > > > > > window_size, > > > > > unsigned long page_shift, > > > > > + unsigned long base, > > > > > struct > > > > > iommu_table_ops *table_ops) > > > > > > > > > > > > iommu_table_setparms() rather than passing 0 around. > > > > > > > > The same comment about "liobn" - set it in > > > > iommu_table_setparms_lpar(). > > > > The reviewer will see what field atters in what situation imho. > > > > > > > > > > The idea here was to keep all tbl parameters setting in > > > _iommu_table_setparms (or iommu_table_setparms_common). > > > > > > I understand the idea that each one of those is optional in the > > > other > > > case, but should we keep whatever value is present in the other > > > variable (not zeroing the other variable), or do someting like: > > > > > > tbl->it_index = 0; > > > tbl->it_base = basep; > > > (in iommu_table_setparms) > > > > > > tbl->it_index = liobn; > > > tbl->it_base = 0; > > > (in iommu_table_setparms_lpar) > > > > > > > This one is supposed to be a question, but I missed the question > > mark. > > Sorry about that. > > Ah ok :) > > > I would like to get your opinion in this :) > > Besides making the "base" parameter a pointer, I really do not have > strong preference, just make it not hurting eyes of a reader, that's > all :) Ok, done :) > imho in general, rather than answering 5 weeks later, it is more > productive to address whatever comments were made, add comments (in > the > code or commit logs) why you are sticking to your initial approach, > rebase and repost the whole thing. Thanks, Thanks for the tip, and for the reviewing :) Best regards, Leonardo Bras
[Bug 213733] Kernel NULL pointer dereference on read (Oops: Kernel access of bad area, sig: 7 [#1]) at systemctl poweroff
https://bugzilla.kernel.org/show_bug.cgi?id=213733 --- Comment #1 from Erhard F. (erhar...@mailbox.org) --- Created attachment 297859 --> https://bugzilla.kernel.org/attachment.cgi?id=297859=edit kernel .config (kernel 5.14-rc1, Talos II) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 213733] New: Kernel NULL pointer dereference on read (Oops: Kernel access of bad area, sig: 7 [#1]) at systemctl poweroff
https://bugzilla.kernel.org/show_bug.cgi?id=213733 Bug ID: 213733 Summary: Kernel NULL pointer dereference on read (Oops: Kernel access of bad area, sig: 7 [#1]) at systemctl poweroff Product: Platform Specific/Hardware Version: 2.5 Kernel Version: 5.14-rc1 Hardware: PPC-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: PPC-64 Assignee: platform_ppc...@kernel-bugs.osdl.org Reporter: erhar...@mailbox.org Regression: No Created attachment 297857 --> https://bugzilla.kernel.org/attachment.cgi?id=297857=edit dmesg (kernel 5.14-rc1, Talos II) My Talos II run fine for a few hours building stuff but at system shutdown (systemctl poweroff) I got this: [...] BUG: Kernel NULL pointer dereference on read at 0x Faulting instruction address: 0xc034396c Oops: Kernel access of bad area, sig: 7 [#1] BE PAGE_SIZE=4K MMU=Radix SMP NR_CPUS=192 DEBUG_PAGEALLOC NUMA PowerNV Modules linked in: auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc rfkill ecb xts ctr cbc aes_generic libaes ibmpowernv evdev radeon snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep ghash_generic snd_hda_core drm_ttm_helper xhci_pci ofpart snd_pcm vmx_crypto ttm gf128mul powernv_flash xhci_hcd mtd i2c_algo_bit snd_timer opal_prd hwmon drm_kms_helper usbcore cfbfillrect cfbcopyarea cfbimgblt sysimgblt snd syscopyarea sysfillrect fb_sys_fops usb_common soundcore at24 regmap_i2c zram zsmalloc powernv_cpufreq drm fuse drm_panel_orientation_quirks backlight configfs CPU: 26 PID: 345930 Comm: kworker/u66:5 Not tainted 5.14.0-rc1-TalosII #2 Workqueue: events_unbound .cleanup_offline_cgwbs_workfn NIP: c034396c LR: c0343850 CTR: REGS: c00020016bf9f7d0 TRAP: 0300 Not tainted (5.14.0-rc1-TalosII) MSR: 90009032 CR: 44002228 XER: 0004 CFAR: c0343864 DAR: DSISR: 0008 IRQMASK: 1 GPR00: c0343848 c00020016bf9fa70 c12d6100 0001 GPR04: c000200183630ac0 90163e29 00409000 GPR08: 0003 c168c488 GPR12: 44002228 c0002007ff7f4c00 c0115e20 c00022950340 GPR16: 0001 c112ef30 c0ea2db8 GPR20: c0ea2d68 c0ea2d98 0001 c11c6352 GPR24: 0001 c000200183630080 c114cf28 c114ced8 GPR28: c00020016bf9faf8 c114cde8 c0002229a000 c0002229a510 NIP [c034396c] .cleanup_offline_cgwbs_workfn+0x3ac/0x410 LR [c0343850] .cleanup_offline_cgwbs_workfn+0x290/0x410 Call Trace: [c00020016bf9fa70] [c0343848] .cleanup_offline_cgwbs_workfn+0x288/0x410 (unreliable) [c00020016bf9fb90] [c010871c] .process_one_work+0x2dc/0x7d0 [c00020016bf9fc70] [c0108ca8] .worker_thread+0x98/0x500 [c00020016bf9fd50] [c0115fa8] .kthread+0x188/0x190 [c00020016bf9fe10] [c000cef8] .ret_from_kernel_thread+0x58/0x60 Instruction dump: 7e85a378 388002b0 7ea3ab78 9ad70002 4be3ff21 6000 e93fff08 712a0003 4182ff0c e95fff10 3900 7c0004ac <7d2050a8> 7c294000 41820018 7d384a14 ---[ end trace d475291d44c4d324 ]--- note: kworker/u66:5[345930] exited with preempt_count 2 watchdog: CPU 16 self-detected hard LOCKUP @ .do_raw_spin_lock+0x90/0x1d0 watchdog: CPU 16 TB:5873709870913, last heartbeat TB:5867694041597 (11749ms ago) Modules linked in: auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc rfkill ecb xts ctr cbc aes_generic libaes ibmpowernv evdev radeon snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep ghash_generic snd_hda_core drm_ttm_helper xhci_pci ofpart snd_pcm vmx_crypto ttm gf128mul powernv_flash xhci_hcd mtd i2c_algo_bit snd_timer opal_prd hwmon drm_kms_helper usbcore cfbfillrect cfbcopyarea cfbimgblt sysimgblt snd syscopyarea sysfillrect fb_sys_fops usb_common soundcore at24 regmap_i2c zram zsmalloc powernv_cpufreq drm fuse drm_panel_orientation_quirks backlight configfs irq event stamp: 3697758 hardirqs last enabled at (3697757): [] .__slab_free+0x3b4/0x5f0 hardirqs last disabled at (3697758): [] ._raw_spin_lock_irq+0x88/0xa0 softirqs last enabled at (3697726): [] .wb_shutdown+0x5c/0x140 softirqs last disabled at (3697724): [] .wb_shutdown+0x20/0x140 CPU: 16 PID: 292187 Comm: kworker/16:0 Tainted: G D 5.14.0-rc1-TalosII #2 Workqueue: cgwb_release .cgwb_release_workfn NIP: c0185e90 LR: c0c7e204 CTR: REGS: c0002007ff667d60 TRAP: 0900 Tainted: G D (5.14.0-rc1-TalosII) MSR: 90009032 CR: 44002228 XER: 2004 CFAR: c0185e9c IRQMASK: 1 GPR00: c0c7e1f8 c0002000e15cf9f0 c12d6100 c114ced8 GPR04: c000267b8a98
Re: [PATCH v2] powerpc/rtas_flash: fix a potential buffer overflow
Yi Zhuang a écrit :
Since snprintf() returns the possible output size instead of the
actual output size, the available flash_msg length returned by
get_validate_flash_msg may exceed the given buffer limit when
simple_read_from_buffer calls copy_to_user
Reported-by: kernel test robot
Fixes: a94a14720eaf5 powerpc/rtas_flash: Fix validate_flash buffer
overflow issue
Signed-off-by: Yi Zhuang
---
arch/powerpc/kernel/rtas_flash.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c
b/arch/powerpc/kernel/rtas_flash.c
index a99179d83538..062f0724c2ff 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -470,9 +470,14 @@ static int get_validate_flash_msg(struct
rtas_validate_flash_t *args_buf,
if (args_buf->status >= VALIDATE_TMP_UPDATE) {
n = sprintf(msg, "%d\n", args_buf->update_results);
if ((args_buf->update_results >= VALIDATE_CUR_UNKNOWN) ||
- (args_buf->update_results == VALIDATE_TMP_UPDATE))
+ (args_buf->update_results == VALIDATE_TMP_UPDATE)) {
n += snprintf(msg + n, msglen - n, "%s\n",
args_buf->buf);
+ if (n >= msglen) {
n cannot be greater than msglen
+ n = msglen;
+ printk(KERN_ERR "FLASH: msg too long.\n");
+ }
+ }
} else {
n = sprintf(msg, "%d\n", args_buf->status);
}
--
2.26.0.106.g9fadedd
[PATCH] powerpc/kexec: blacklist functions called in real mode for kprobe
As kprobe does not handle events happening in real mode, blacklist the
functions that only get called in real mode or in kexec sequence with
MMU turned off.
Signed-off-by: Hari Bathini
---
arch/powerpc/kernel/head_64.S|2 ++
arch/powerpc/kexec/core_64.c |6 --
arch/powerpc/mm/book3s64/hash_native.c |2 +-
arch/powerpc/mm/book3s64/pgtable.c |4 ++--
arch/powerpc/mm/book3s64/radix_pgtable.c |3 ++-
arch/powerpc/platforms/ps3/htab.c|3 ++-
arch/powerpc/platforms/ps3/mm.c |8 ++--
arch/powerpc/platforms/pseries/lpar.c|9 ++---
8 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 79930b0bc781..f17ae2083733 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -712,6 +712,8 @@ _GLOBAL(copy_and_flush)
isync
blr
+_ASM_NOKPROBE_SYMBOL(copy_and_flush); /* Called in real mode */
+
.align 8
copy_to_here:
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 8a449b2d8715..84618d3c8013 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -72,7 +72,8 @@ int default_machine_kexec_prepare(struct kimage *image)
return 0;
}
-static void copy_segments(unsigned long ind)
+/* Called during kexec sequence with MMU off */
+static notrace void copy_segments(unsigned long ind)
{
unsigned long entry;
unsigned long *ptr;
@@ -105,7 +106,8 @@ static void copy_segments(unsigned long ind)
}
}
-void kexec_copy_flush(struct kimage *image)
+/* Called during kexec sequence with MMU off */
+notrace void kexec_copy_flush(struct kimage *image)
{
long i, nr_segments = image->nr_segments;
struct kexec_segment ranges[KEXEC_SEGMENT_MAX];
diff --git a/arch/powerpc/mm/book3s64/hash_native.c
b/arch/powerpc/mm/book3s64/hash_native.c
index 52e170bd95ae..d8279bfe68ea 100644
--- a/arch/powerpc/mm/book3s64/hash_native.c
+++ b/arch/powerpc/mm/book3s64/hash_native.c
@@ -787,7 +787,7 @@ static void hpte_decode(struct hash_pte *hpte, unsigned
long slot,
* TODO: add batching support when enabled. remember, no dynamic memory here,
* although there is the control page available...
*/
-static void native_hpte_clear(void)
+static notrace void native_hpte_clear(void)
{
unsigned long vpn = 0;
unsigned long slot, slots;
diff --git a/arch/powerpc/mm/book3s64/pgtable.c
b/arch/powerpc/mm/book3s64/pgtable.c
index 9ffa65074cb0..300099de553b 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -172,8 +172,8 @@ pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-/* For use by kexec */
-void mmu_cleanup_all(void)
+/* For use by kexec, called with MMU off */
+notrace void mmu_cleanup_all(void)
{
if (radix_enabled())
radix__mmu_cleanup_all();
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index e50ddf129c15..ae20add7954a 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -679,7 +679,8 @@ void radix__early_init_mmu_secondary(void)
mtspr(SPRN_UAMOR, 0);
}
-void radix__mmu_cleanup_all(void)
+/* Called during kexec sequence with MMU off */
+notrace void radix__mmu_cleanup_all(void)
{
unsigned long lpcr;
diff --git a/arch/powerpc/platforms/ps3/htab.c
b/arch/powerpc/platforms/ps3/htab.c
index 7ddc7ec6a7c0..ef710a715903 100644
--- a/arch/powerpc/platforms/ps3/htab.c
+++ b/arch/powerpc/platforms/ps3/htab.c
@@ -169,7 +169,8 @@ static void ps3_hpte_invalidate(unsigned long slot,
unsigned long vpn,
spin_unlock_irqrestore(_htab_lock, flags);
}
-static void ps3_hpte_clear(void)
+/* Called during kexec sequence with MMU off */
+static notrace void ps3_hpte_clear(void)
{
unsigned long hpte_count = (1UL << ppc64_pft_size) >> 4;
u64 i;
diff --git a/arch/powerpc/platforms/ps3/mm.c b/arch/powerpc/platforms/ps3/mm.c
index a81eac35d900..9c44f335c0b9 100644
--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -195,9 +195,11 @@ void __init ps3_mm_vas_create(unsigned long* htab_size)
/**
* ps3_mm_vas_destroy -
+ *
+ * called during kexec sequence with MMU off.
*/
-void ps3_mm_vas_destroy(void)
+notrace void ps3_mm_vas_destroy(void)
{
int result;
@@ -1243,9 +1245,11 @@ void __init ps3_mm_init(void)
/**
* ps3_mm_shutdown - final cleanup of address space
+ *
+ * called during kexec sequence with MMU off.
*/
-void ps3_mm_shutdown(void)
+notrace void ps3_mm_shutdown(void)
{
ps3_mm_region_destroy();
}
diff --git a/arch/powerpc/platforms/pseries/lpar.c
b/arch/powerpc/platforms/pseries/lpar.c
index dab356e3ff87..869ef638698a 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -801,7
Re: [RFC PATCH 10/43] powerpc/64s: Always set PMU control registers to frozen/disabled when not in use
Excerpts from Nicholas Piggin's message of July 12, 2021 12:41 pm:
> Excerpts from Athira Rajeev's message of July 10, 2021 12:50 pm:
>>
>>
>>> On 22-Jun-2021, at 4:27 PM, Nicholas Piggin wrote:
>>>
>>> KVM PMU management code looks for particular frozen/disabled bits in
>>> the PMU registers so it knows whether it must clear them when coming
>>> out of a guest or not. Setting this up helps KVM make these optimisations
>>> without getting confused. Longer term the better approach might be to
>>> move guest/host PMU switching to the perf subsystem.
>>>
>>> Signed-off-by: Nicholas Piggin
>>> ---
>>> arch/powerpc/kernel/cpu_setup_power.c | 4 ++--
>>> arch/powerpc/kernel/dt_cpu_ftrs.c | 6 +++---
>>> arch/powerpc/kvm/book3s_hv.c | 5 +
>>> arch/powerpc/perf/core-book3s.c | 7 +++
>>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/cpu_setup_power.c
>>> b/arch/powerpc/kernel/cpu_setup_power.c
>>> index a29dc8326622..3dc61e203f37 100644
>>> --- a/arch/powerpc/kernel/cpu_setup_power.c
>>> +++ b/arch/powerpc/kernel/cpu_setup_power.c
>>> @@ -109,7 +109,7 @@ static void init_PMU_HV_ISA207(void)
>>> static void init_PMU(void)
>>> {
>>> mtspr(SPRN_MMCRA, 0);
>>> - mtspr(SPRN_MMCR0, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> mtspr(SPRN_MMCR1, 0);
>>> mtspr(SPRN_MMCR2, 0);
>>> }
>>> @@ -123,7 +123,7 @@ static void init_PMU_ISA31(void)
>>> {
>>> mtspr(SPRN_MMCR3, 0);
>>> mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>> - mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>> }
>>>
>>> /*
>>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> index 0a6b36b4bda8..06a089fbeaa7 100644
>>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> @@ -353,7 +353,7 @@ static void init_pmu_power8(void)
>>> }
>>>
>>> mtspr(SPRN_MMCRA, 0);
>>> - mtspr(SPRN_MMCR0, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> mtspr(SPRN_MMCR1, 0);
>>> mtspr(SPRN_MMCR2, 0);
>>> mtspr(SPRN_MMCRS, 0);
>>> @@ -392,7 +392,7 @@ static void init_pmu_power9(void)
>>> mtspr(SPRN_MMCRC, 0);
>>>
>>> mtspr(SPRN_MMCRA, 0);
>>> - mtspr(SPRN_MMCR0, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> mtspr(SPRN_MMCR1, 0);
>>> mtspr(SPRN_MMCR2, 0);
>>> }
>>> @@ -428,7 +428,7 @@ static void init_pmu_power10(void)
>>>
>>> mtspr(SPRN_MMCR3, 0);
>>> mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>> - mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>> }
>>>
>>> static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f)
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index 1f30f98b09d1..f7349d150828 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -2593,6 +2593,11 @@ static int kvmppc_core_vcpu_create_hv(struct
>>> kvm_vcpu *vcpu)
>>> #endif
>>> #endif
>>> vcpu->arch.mmcr[0] = MMCR0_FC;
>>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>> + vcpu->arch.mmcr[0] |= MMCR0_PMCCEXT;
>>> + vcpu->arch.mmcra = MMCRA_BHRB_DISABLE;
>>> + }
>>> +
>>> vcpu->arch.ctrl = CTRL_RUNLATCH;
>>> /* default to host PVR, since we can't spoof it */
>>> kvmppc_set_pvr_hv(vcpu, mfspr(SPRN_PVR));
>>> diff --git a/arch/powerpc/perf/core-book3s.c
>>> b/arch/powerpc/perf/core-book3s.c
>>> index 51622411a7cc..e33b29ec1a65 100644
>>> --- a/arch/powerpc/perf/core-book3s.c
>>> +++ b/arch/powerpc/perf/core-book3s.c
>>> @@ -1361,6 +1361,13 @@ static void power_pmu_enable(struct pmu *pmu)
>>> goto out;
>>>
>>> if (cpuhw->n_events == 0) {
>>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>> + mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>> + } else {
>>> + mtspr(SPRN_MMCRA, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> + }
>>
>>
>> Hi Nick,
>>
>> We are setting these bits in “power_pmu_disable” function. And disable will
>> be called before any event gets deleted/stopped. Can you please help to
>> understand why this is needed in power_pmu_enable path also ?
>
> I'll have to go back and check what I needed it for.
Okay, MMCRA is getting MMCRA_SDAR_MODE_DCACHE set on POWER9, by the looks.
That's not necessarily a problem, but KVM sets MMCRA to 0 to disable
SDAR updates. So KVM and perf don't agree on what the "correct" value
for disabled is. Which could be a problem with POWER10 not setting BHRB
disable before my series.
I'll get rid of this hunk for now, I expect things won't be exactly clean
or consistent until the KVM host PMU code is moved into perf/ though.
Thanks,
Nick
[PATCH v2] powerpc/rtas_flash: fix a potential buffer overflow
Since snprintf() returns the possible output size instead of the
actual output size, the available flash_msg length returned by
get_validate_flash_msg may exceed the given buffer limit when
simple_read_from_buffer calls copy_to_user
Reported-by: kernel test robot
Fixes: a94a14720eaf5 powerpc/rtas_flash: Fix validate_flash buffer overflow
issue
Signed-off-by: Yi Zhuang
---
arch/powerpc/kernel/rtas_flash.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index a99179d83538..062f0724c2ff 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -470,9 +470,14 @@ static int get_validate_flash_msg(struct
rtas_validate_flash_t *args_buf,
if (args_buf->status >= VALIDATE_TMP_UPDATE) {
n = sprintf(msg, "%d\n", args_buf->update_results);
if ((args_buf->update_results >= VALIDATE_CUR_UNKNOWN) ||
- (args_buf->update_results == VALIDATE_TMP_UPDATE))
+ (args_buf->update_results == VALIDATE_TMP_UPDATE)) {
n += snprintf(msg + n, msglen - n, "%s\n",
args_buf->buf);
+ if (n >= msglen) {
+ n = msglen;
+ printk(KERN_ERR "FLASH: msg too long.\n");
+ }
+ }
} else {
n = sprintf(msg, "%d\n", args_buf->status);
}
--
2.26.0.106.g9fadedd
Re: [PATCH v4 5/5] bus: Make remove callback return void
On Tue, Jul 13, 2021 at 09:35:22PM +0200, Uwe Kleine-König wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
>
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
>
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
>
[...]
> diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
> index 784cf0027da3..2682c3df651c 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -116,15 +116,13 @@ static int scmi_dev_probe(struct device *dev)
> return scmi_drv->probe(scmi_dev);
> }
>
> -static int scmi_dev_remove(struct device *dev)
> +static void scmi_dev_remove(struct device *dev)
> {
> struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver);
> struct scmi_device *scmi_dev = to_scmi_dev(dev);
>
> if (scmi_drv->remove)
> scmi_drv->remove(scmi_dev);
> -
> - return 0;
> }
>
> static struct bus_type scmi_bus_type = {
Acked-by: Sudeep Holla
--
Regards,
Sudeep
Re: [PATCH v4 5/5] bus: Make remove callback return void
On Tue, Jul 13, 2021 at 9:35 PM Uwe Kleine-König
wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
>
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
>
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
> drivers/zorro/zorro-driver.c | 3 +--
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v4 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
On 13/07/2021 14:36, Leonardo Brás wrote:
On Tue, 2021-05-11 at 17:57 +1000, Alexey Kardashevskiy wrote:
On 01/05/2021 02:31, Leonardo Bras wrote:
[...]
pmem_present = dn != NULL;
@@ -1218,8 +1224,12 @@ static bool enable_ddw(struct pci_dev *dev,
struct device_node *pdn)
mutex_lock(_window_init_mutex);
- if (find_existing_ddw(pdn, >dev.archdata.dma_offset,
))
- goto out_unlock;
+ if (find_existing_ddw(pdn, >dev.archdata.dma_offset,
)) {
+ direct_mapping = (len >= max_ram_len);
+
+ mutex_unlock(_window_init_mutex);
+ return direct_mapping;
Does not this break the existing case when direct_mapping==true by
skipping setting dev->dev.bus_dma_limit before returning?
Yes, it does. Good catch!
I changed it to use a flag instead of win64 for return, and now I can
use the same success exit path for both the new config and the config
found in list. (out_unlock)
+ }
/*
* If we already went through this for a previous function of
@@ -1298,7 +1308,6 @@ static bool enable_ddw(struct pci_dev *dev,
struct device_node *pdn)
goto out_failed;
}
/* verify the window * number of ptes will map the partition
*/
- /* check largest block * page size > max memory hotplug addr
*/
/*
* The "ibm,pmemory" can appear anywhere in the address
space.
* Assuming it is still backed by page structs, try
MAX_PHYSMEM_BITS
@@ -1320,6 +1329,17 @@ static bool enable_ddw(struct pci_dev *dev,
struct device_node *pdn)
1ULL << len,
query.largest_available_block,
1ULL << page_shift);
+
+ len = order_base_2(query.largest_available_block <<
page_shift);
+ win_name = DMA64_PROPNAME;
[1]
+ } else {
+ direct_mapping = true;
+ win_name = DIRECT64_PROPNAME;
+ }
+
+ /* DDW + IOMMU on single window may fail if there is any
allocation */
+ if (default_win_removed && !direct_mapping &&
iommu_table_in_use(tbl)) {
+ dev_dbg(>dev, "current IOMMU table in use, can't
be replaced.\n");
... remove !direct_mapping and move to [1]?
sure, done!
goto out_failed;
}
@@ -1331,8 +1351,7 @@ static bool enable_ddw(struct pci_dev *dev,
struct device_node *pdn)
create.liobn, dn);
win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
- win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn,
win_addr,
- page_shift, len);
+ win64 = ddw_property_create(win_name, create.liobn, win_addr,
page_shift, len);
if (!win64) {
dev_info(>dev,
"couldn't allocate property, property name,
or value\n");
@@ -1350,12 +1369,47 @@ static bool enable_ddw(struct pci_dev *dev,
struct device_node *pdn)
if (!window)
goto out_del_prop;
- ret = walk_system_ram_range(0, memblock_end_of_DRAM() >>
PAGE_SHIFT,
- win64->value,
tce_setrange_multi_pSeriesLP_walk);
- if (ret) {
- dev_info(>dev, "failed to map direct window for
%pOF: %d\n",
- dn, ret);
- goto out_del_list;
+ if (direct_mapping) {
+ /* DDW maps the whole partition, so enable direct DMA
mapping */
+ ret = walk_system_ram_range(0, memblock_end_of_DRAM()
PAGE_SHIFT,
+ win64->value,
tce_setrange_multi_pSeriesLP_walk);
+ if (ret) {
+ dev_info(>dev, "failed to map direct
window for %pOF: %d\n",
+ dn, ret);
+ goto out_del_list;
+ }
+ } else {
+ struct iommu_table *newtbl;
+ int i;
+
+ /* New table for using DDW instead of the default DMA
window */
+ newtbl = iommu_pseries_alloc_table(pci->phb->node);
+ if (!newtbl) {
+ dev_dbg(>dev, "couldn't create new IOMMU
table\n");
+ goto out_del_list;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(pci->phb->mem_resources);
i++) {
+ const unsigned long mask = IORESOURCE_MEM_64
| IORESOURCE_MEM;
+
+ /* Look for MMIO32 */
+ if ((pci->phb->mem_resources[i].flags & mask)
== IORESOURCE_MEM)
+ break;
What if there is no IORESOURCE_MEM? pci->phb->mem_resources[i].start
below will have garbage.
Yeah, that makes sense. I will add this lines after 'for':
if (i == ARRAY_SIZE(pci->phb->mem_resources)) {
iommu_tce_table_put(newtbl);
goto out_del_list;
}
What do you think?
Move this and that "for" before iommu_pseries_alloc_table() so you
Re: [PATCH v4 07/11] powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new helper
On 13/07/2021 14:47, Leonardo Brás wrote: Hello Alexey, On Fri, 2021-06-18 at 19:26 -0300, Leonardo Brás wrote: + unsigned long liobn, unsigned long win_addr, + unsigned long window_size, unsigned long page_shift, + unsigned long base, struct iommu_table_ops *table_ops) iommu_table_setparms() rather than passing 0 around. The same comment about "liobn" - set it in iommu_table_setparms_lpar(). The reviewer will see what field atters in what situation imho. The idea here was to keep all tbl parameters setting in _iommu_table_setparms (or iommu_table_setparms_common). I understand the idea that each one of those is optional in the other case, but should we keep whatever value is present in the other variable (not zeroing the other variable), or do someting like: tbl->it_index = 0; tbl->it_base = basep; (in iommu_table_setparms) tbl->it_index = liobn; tbl->it_base = 0; (in iommu_table_setparms_lpar) This one is supposed to be a question, but I missed the question mark. Sorry about that. Ah ok :) I would like to get your opinion in this :) Besides making the "base" parameter a pointer, I really do not have strong preference, just make it not hurting eyes of a reader, that's all :) imho in general, rather than answering 5 weeks later, it is more productive to address whatever comments were made, add comments (in the code or commit logs) why you are sticking to your initial approach, rebase and repost the whole thing. Thanks, -- Alexey
