* [PATCH v2 5/7] wifi: brcm80211: drop driver owner initialization
From: Krzysztof Kozlowski @ 2024-04-03 14:16 UTC (permalink / raw)
To: Ulf Hansson, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Kalle Valo,
Jeff Johnson, Arend van Spriel, Brian Norris, Francesco Dolcini,
Jérôme Pouiller
Cc: linux-mmc, linux-bluetooth, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-wireless, ath10k, brcm80211,
brcm80211-dev-list.pdl, Krzysztof Kozlowski
In-Reply-To: <20240403-module-owner-sdio-v2-0-ae46d6b955eb@linaro.org>
Core in sdio_register_driver() already sets the .owner, so driver does
not need to.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Depends on the patch in mmc tree.
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 00679a990e3d..13391c2d82aa 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -1238,7 +1238,6 @@ static struct sdio_driver brcmf_sdmmc_driver = {
.name = KBUILD_MODNAME,
.id_table = brcmf_sdmmc_ids,
.drv = {
- .owner = THIS_MODULE,
.pm = pm_sleep_ptr(&brcmf_sdio_pm_ops),
.coredump = brcmf_dev_coredump,
},
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/7] Bluetooth: btmtksdio: drop driver owner initialization
From: Krzysztof Kozlowski @ 2024-04-03 14:16 UTC (permalink / raw)
To: Ulf Hansson, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Kalle Valo,
Jeff Johnson, Arend van Spriel, Brian Norris, Francesco Dolcini,
Jérôme Pouiller
Cc: linux-mmc, linux-bluetooth, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-wireless, ath10k, brcm80211,
brcm80211-dev-list.pdl, Krzysztof Kozlowski
In-Reply-To: <20240403-module-owner-sdio-v2-0-ae46d6b955eb@linaro.org>
Core in sdio_register_driver() already sets the .owner, so driver does
not need to.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Depends on the patch in mmc tree.
---
drivers/bluetooth/btmtksdio.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index ff4868c83cd8..8ded9ef8089a 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1519,7 +1519,6 @@ static struct sdio_driver btmtksdio_driver = {
.remove = btmtksdio_remove,
.id_table = btmtksdio_table,
.drv = {
- .owner = THIS_MODULE,
.pm = BTMTKSDIO_PM_OPS,
}
};
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 1/7] Bluetooth: btmrvl_sdio: drop driver owner initialization
From: Krzysztof Kozlowski @ 2024-04-03 14:16 UTC (permalink / raw)
To: Ulf Hansson, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Kalle Valo,
Jeff Johnson, Arend van Spriel, Brian Norris, Francesco Dolcini,
Jérôme Pouiller
Cc: linux-mmc, linux-bluetooth, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-wireless, ath10k, brcm80211,
brcm80211-dev-list.pdl, Krzysztof Kozlowski
In-Reply-To: <20240403-module-owner-sdio-v2-0-ae46d6b955eb@linaro.org>
Core in sdio_register_driver() already sets the .owner, so driver does
not need to.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Depends on the patch in mmc tree.
---
drivers/bluetooth/btmrvl_sdio.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index d76c799553aa..85b7f2bb4259 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -1736,7 +1736,6 @@ static struct sdio_driver bt_mrvl_sdio = {
.probe = btmrvl_sdio_probe,
.remove = btmrvl_sdio_remove,
.drv = {
- .owner = THIS_MODULE,
.coredump = btmrvl_sdio_coredump,
.pm = &btmrvl_sdio_pm_ops,
}
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 6/7] wifi: mwifiex: drop driver owner initialization
From: Krzysztof Kozlowski @ 2024-04-03 14:16 UTC (permalink / raw)
To: Ulf Hansson, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Kalle Valo,
Jeff Johnson, Arend van Spriel, Brian Norris, Francesco Dolcini,
Jérôme Pouiller
Cc: linux-mmc, linux-bluetooth, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-wireless, ath10k, brcm80211,
brcm80211-dev-list.pdl, Krzysztof Kozlowski, Francesco Dolcini
In-Reply-To: <20240403-module-owner-sdio-v2-0-ae46d6b955eb@linaro.org>
Core in sdio_register_driver() already sets the .owner, so driver does
not need to.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Kalle Valo <kvalo@kernel.org>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
Depends on the patch in mmc tree.
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index f41048b5cd3c..bda9b2b8a1f3 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -979,7 +979,6 @@ static struct sdio_driver mwifiex_sdio = {
.probe = mwifiex_sdio_probe,
.remove = mwifiex_sdio_remove,
.drv = {
- .owner = THIS_MODULE,
.coredump = mwifiex_sdio_coredump,
.pm = &mwifiex_sdio_pm_ops,
}
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 7/7] wifi: wfx: drop driver owner initialization
From: Krzysztof Kozlowski @ 2024-04-03 14:16 UTC (permalink / raw)
To: Ulf Hansson, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Kalle Valo,
Jeff Johnson, Arend van Spriel, Brian Norris, Francesco Dolcini,
Jérôme Pouiller
Cc: linux-mmc, linux-bluetooth, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-wireless, ath10k, brcm80211,
brcm80211-dev-list.pdl, Krzysztof Kozlowski
In-Reply-To: <20240403-module-owner-sdio-v2-0-ae46d6b955eb@linaro.org>
Core in sdio_register_driver() already sets the .owner, so driver does
not need to.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Kalle Valo <kvalo@kernel.org>
---
Depends on the patch in mmc tree.
---
drivers/net/wireless/silabs/wfx/bus_sdio.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/silabs/wfx/bus_sdio.c b/drivers/net/wireless/silabs/wfx/bus_sdio.c
index 909d5f346a01..f290eecde773 100644
--- a/drivers/net/wireless/silabs/wfx/bus_sdio.c
+++ b/drivers/net/wireless/silabs/wfx/bus_sdio.c
@@ -267,7 +267,6 @@ struct sdio_driver wfx_sdio_driver = {
.probe = wfx_sdio_probe,
.remove = wfx_sdio_remove,
.drv = {
- .owner = THIS_MODULE,
.of_match_table = wfx_sdio_of_match,
}
};
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2 04/10] perf/arm_cspmu: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:21 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-5-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:03PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index b9a252272f1e..fd1004251665 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -1322,8 +1322,7 @@ static int arm_cspmu_cpu_online(unsigned int cpu, struct hlist_node *node)
>
> static int arm_cspmu_cpu_teardown(unsigned int cpu, struct hlist_node *node)
> {
> - int dst;
> - struct cpumask online_supported;
> + unsigned int dst;
>
> struct arm_cspmu *cspmu =
> hlist_entry_safe(node, struct arm_cspmu, cpuhp_node);
> @@ -1333,9 +1332,8 @@ static int arm_cspmu_cpu_teardown(unsigned int cpu, struct hlist_node *node)
> return 0;
>
> /* Choose a new CPU to migrate ownership of the PMU to */
> - cpumask_and(&online_supported, &cspmu->associated_cpus,
> - cpu_online_mask);
> - dst = cpumask_any_but(&online_supported, cpu);
> + dst = cpumask_any_and_but(&cspmu->associated_cpus,
> + cpu_online_mask, cpu);
> if (dst >= nr_cpu_ids)
> return 0;
>
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 03/10] perf/arm-cmn: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:23 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-4-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:02PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/arm-cmn.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index 7ef9c7e4836b..6bfb0c4a1287 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -1950,20 +1950,20 @@ static int arm_cmn_pmu_offline_cpu(unsigned int cpu, struct hlist_node *cpuhp_no
> struct arm_cmn *cmn;
> unsigned int target;
> int node;
> - cpumask_t mask;
>
> cmn = hlist_entry_safe(cpuhp_node, struct arm_cmn, cpuhp_node);
> if (cpu != cmn->cpu)
> return 0;
>
> node = dev_to_node(cmn->dev);
> - if (cpumask_and(&mask, cpumask_of_node(node), cpu_online_mask) &&
> - cpumask_andnot(&mask, &mask, cpumask_of(cpu)))
> - target = cpumask_any(&mask);
> - else
> +
> + target = cpumask_any_and_but(cpumask_of_node(node), cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> target = cpumask_any_but(cpu_online_mask, cpu);
> +
> if (target < nr_cpu_ids)
> arm_cmn_migrate(cmn, target);
> +
> return 0;
> }
>
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 7/7] x86: mm: accelerate pagefault when badaccess
From: Suren Baghdasaryan @ 2024-04-03 14:23 UTC (permalink / raw)
To: Kefeng Wang
Cc: akpm, Russell King, Catalin Marinas, Will Deacon,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexander Gordeev,
Gerald Schaefer, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
x86, linux-arm-kernel, linuxppc-dev, linux-riscv, linux-s390
In-Reply-To: <d5177b0f-db4e-4c78-81f1-5761f08f076d@huawei.com>
On Wed, Apr 3, 2024 at 12:58 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
>
>
> On 2024/4/3 13:59, Suren Baghdasaryan wrote:
> > On Tue, Apr 2, 2024 at 12:53 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> >>
> >> The vm_flags of vma already checked under per-VMA lock, if it is a
> >> bad access, directly handle error and return, there is no need to
> >> lock_mm_and_find_vma() and check vm_flags again.
> >>
> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> >
> > Looks safe to me.
> > Using (mm != NULL) to indicate that we are holding mmap_lock is not
> > ideal but I guess that works.
> >
>
> Yes, I will add this part it into change too,
>
> The access_error() of vma already checked under per-VMA lock, if it
> is a bad access, directly handle error, no need to retry with mmap_lock
> again. In order to release the correct lock, pass the mm_struct into
> bad_area_access_error(), if mm is NULL, release vma lock, or release
> mmap_lock. Since the page faut is handled under per-VMA lock, count it
> as a vma lock event with VMA_LOCK_SUCCESS.
The part about passing mm_struct is unnecessary IMHO. It explains "how
you do things" but changelog should describe only "what you do" and
"why you do that". The rest we can see from the code.
>
> Thanks.
>
>
> > Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> >
> >> ---
> >> arch/x86/mm/fault.c | 23 ++++++++++++++---------
> >> 1 file changed, 14 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> >> index a4cc20d0036d..67b18adc75dd 100644
> >> --- a/arch/x86/mm/fault.c
> >> +++ b/arch/x86/mm/fault.c
> >> @@ -866,14 +866,17 @@ bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code,
> >>
> >> static void
> >> __bad_area(struct pt_regs *regs, unsigned long error_code,
> >> - unsigned long address, u32 pkey, int si_code)
> >> + unsigned long address, struct mm_struct *mm,
> >> + struct vm_area_struct *vma, u32 pkey, int si_code)
> >> {
> >> - struct mm_struct *mm = current->mm;
> >> /*
> >> * Something tried to access memory that isn't in our memory map..
> >> * Fix it, but check if it's kernel or user first..
> >> */
> >> - mmap_read_unlock(mm);
> >> + if (mm)
> >> + mmap_read_unlock(mm);
> >> + else
> >> + vma_end_read(vma);
> >>
> >> __bad_area_nosemaphore(regs, error_code, address, pkey, si_code);
> >> }
> >> @@ -897,7 +900,8 @@ static inline bool bad_area_access_from_pkeys(unsigned long error_code,
> >>
> >> static noinline void
> >> bad_area_access_error(struct pt_regs *regs, unsigned long error_code,
> >> - unsigned long address, struct vm_area_struct *vma)
> >> + unsigned long address, struct mm_struct *mm,
> >> + struct vm_area_struct *vma)
> >> {
> >> /*
> >> * This OSPKE check is not strictly necessary at runtime.
> >> @@ -927,9 +931,9 @@ bad_area_access_error(struct pt_regs *regs, unsigned long error_code,
> >> */
> >> u32 pkey = vma_pkey(vma);
> >>
> >> - __bad_area(regs, error_code, address, pkey, SEGV_PKUERR);
> >> + __bad_area(regs, error_code, address, mm, vma, pkey, SEGV_PKUERR);
> >> } else {
> >> - __bad_area(regs, error_code, address, 0, SEGV_ACCERR);
> >> + __bad_area(regs, error_code, address, mm, vma, 0, SEGV_ACCERR);
> >> }
> >> }
> >>
> >> @@ -1357,8 +1361,9 @@ void do_user_addr_fault(struct pt_regs *regs,
> >> goto lock_mmap;
> >>
> >> if (unlikely(access_error(error_code, vma))) {
> >> - vma_end_read(vma);
> >> - goto lock_mmap;
> >> + bad_area_access_error(regs, error_code, address, NULL, vma);
> >> + count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> >> + return;
> >> }
> >> fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK, regs);
> >> if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
> >> @@ -1394,7 +1399,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> >> * we can handle it..
> >> */
> >> if (unlikely(access_error(error_code, vma))) {
> >> - bad_area_access_error(regs, error_code, address, vma);
> >> + bad_area_access_error(regs, error_code, address, mm, vma);
> >> return;
> >> }
> >>
> >> --
> >> 2.27.0
> >>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] firmware: arm_ffa: support running as a guest in a vm
From: Jens Wiklander @ 2024-04-03 14:27 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-kernel, linux-arm-kernel, Marc Bonnici, Olivier Deprez,
Lorenzo Pieralisi, Bertrand Marquis
In-Reply-To: <Zg1TmTcqRbzla3nN@bogus>
On Wed, Apr 3, 2024 at 3:03 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Mar 27, 2024 at 10:23:35AM +0100, Jens Wiklander wrote:
> > On Tue, Mar 26, 2024 at 4:35 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Mon, Mar 25, 2024 at 09:13:35AM +0100, Jens Wiklander wrote:
> > > > Add support for running the driver in a guest to a hypervisor. The main
> > > > difference is introducing notification pending interrupt and that
> > > > FFA_NOTIFICATION_BITMAP_CREATE doesn't need to be called.
> > > >
> > > > The guest may need to use a notification pending interrupt instead of or
> > > > in addition to the schedule receiver interrupt.
> > >
> > > The above statement makes me worry a bit that we are still not on the same
> > > page about NPI vs SRI. NPI need not exist in addition to SRI. And in v1
> > > you did mention you have SRI in the guest as well. Then why do we need
> > > NPI in addition to that. As part of SRI, the callback ffa_self_notif_handle
> > > gets registered and will be called as part of SRI handling. What you
> > > do in notif_pend_irq_handler(), exactly what ffa_self_notif_handle()
> > > already does.
> >
> > That's my understanding of what an NPI handler should do to be able to
> > receive per-vCPU notifications.
> >
> > >
> > > I am still struggling to understand the usecase here. If you just have
> > > NPI and no SRI when running the driver in the VM, then it aligns with
> > > my understanding of possible use-case(not the one you mentioned in v1:
> > > where FF-A driver in VM will have SRI as OPTEE is the secondary scheduler)
> >
> > OP-TEE is not a secondary scheduler. OP-TEE (the SP) is scheduled as
> > usual by the normal world using direct request. OP-TEE doesn't receive
> > FF-A notifications and I'm not sure it will ever be needed.
> >
>
> Sorry for my poor choice of words yet again. I meant VM kernel(running
> as NS virtual endpoint) with OPTEE driver running in it as secondary
> scheduler. IIUC, there will be another instance of OPTEE driver in the
> primary scheduler endpoint(i.e. host kernel) and it will take care of
> running SRI handler ?
Yes, that's what we have in the Xen configuration, except that we
don't use an OP-TEE driver, it's only generic FF-A processing.
The SRI in this case is a physical interrupt, raised by the SPMC.
>
> > >
> > > If we are supporting NPI or SRI, I think we can see if we can further
> > > simplify this change, but I want to get to an agreement with usage model
> > > before we dig into implementation details in this patch.
> >
> > The spec doesn't as far as I know explicitly make NPI and SRI mutually
> > exclusive, it doesn't make sense to use both in all configurations.
> > I'm trying to be as dynamic as possible when configuring the NPI and
> > SRI handlers.
> >
>
> Fair enough
>
> > If the kernel is a physical endpoint, it's easy, it only uses SRI and
> > the SPMC will not give an NPI when asked.
> >
>
> Agreed.
>
> > If the kernel is a virtual endpoint it might be more complicated since
> > a VM may need to act as a secondary scheduler. That's not fully
> > supported in this patch, since it can only schedule itself. SRI is not
> > used in my current configuration. If a hypervisor injects an SRI I
> > expect it to filter what's returned by FFA_NOTIFICATION_INFO_GET for
> > this VM so it doesn't interfere with notifications for other VMs.
> >
>
> OK
>
> > In my current configuration, the hypervisor uses NPI to signal pending
> > notifications to the guest. I do not need a secondary scheduler since
> > OP-TEE doesn't receive notifications. At a later time, we may have SPs
> > that need to be scheduled, but that's not a problem I'm trying to
> > solve here.
>
> Understood. I will take a look at the patch with the above information.
Thanks,
Jens
>
> --
> Regards,
> Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 05/10] perf/arm_dsu: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:32 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-6-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:04PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/arm_dsu_pmu.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index bae3ca37f846..adc0bbb5fafe 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -230,15 +230,6 @@ static const struct attribute_group *dsu_pmu_attr_groups[] = {
> NULL,
> };
>
> -static int dsu_pmu_get_online_cpu_any_but(struct dsu_pmu *dsu_pmu, int cpu)
> -{
> - struct cpumask online_supported;
> -
> - cpumask_and(&online_supported,
> - &dsu_pmu->associated_cpus, cpu_online_mask);
> - return cpumask_any_but(&online_supported, cpu);
> -}
> -
> static inline bool dsu_pmu_counter_valid(struct dsu_pmu *dsu_pmu, u32 idx)
> {
> return (idx < dsu_pmu->num_counters) ||
> @@ -827,14 +818,16 @@ static int dsu_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
>
> static int dsu_pmu_cpu_teardown(unsigned int cpu, struct hlist_node *node)
> {
> - int dst;
> - struct dsu_pmu *dsu_pmu = hlist_entry_safe(node, struct dsu_pmu,
> - cpuhp_node);
> + struct dsu_pmu *dsu_pmu;
> + unsigned int dst;
> +
> + dsu_pmu = hlist_entry_safe(node, struct dsu_pmu, cpuhp_node);
>
> if (!cpumask_test_and_clear_cpu(cpu, &dsu_pmu->active_cpu))
> return 0;
>
> - dst = dsu_pmu_get_online_cpu_any_but(dsu_pmu, cpu);
> + dst = cpumask_any_and_but(&dsu_pmu->associated_cpus,
> + cpu_online_mask, cpu);
> /* If there are no active CPUs in the DSU, leave IRQ disabled */
> if (dst >= nr_cpu_ids)
> return 0;
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 06/10] perf/dwc_pcie: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:33 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-7-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:05PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/dwc_pcie_pmu.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index 957058ad0099..c5e328f23841 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c
> @@ -690,9 +690,8 @@ static int dwc_pcie_pmu_offline_cpu(unsigned int cpu, struct hlist_node *cpuhp_n
> {
> struct dwc_pcie_pmu *pcie_pmu;
> struct pci_dev *pdev;
> - int node;
> - cpumask_t mask;
> unsigned int target;
> + int node;
>
> pcie_pmu = hlist_entry_safe(cpuhp_node, struct dwc_pcie_pmu, cpuhp_node);
> /* Nothing to do if this CPU doesn't own the PMU */
> @@ -702,10 +701,9 @@ static int dwc_pcie_pmu_offline_cpu(unsigned int cpu, struct hlist_node *cpuhp_n
> pcie_pmu->on_cpu = -1;
> pdev = pcie_pmu->pdev;
> node = dev_to_node(&pdev->dev);
> - if (cpumask_and(&mask, cpumask_of_node(node), cpu_online_mask) &&
> - cpumask_andnot(&mask, &mask, cpumask_of(cpu)))
> - target = cpumask_any(&mask);
> - else
> +
> + target = cpumask_any_and_but(cpumask_of_node(node), cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> target = cpumask_any_but(cpu_online_mask, cpu);
>
> if (target >= nr_cpu_ids) {
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 02/15] kunit: bug: Count suppressed warning backtraces
From: Guenter Roeck @ 2024-04-03 13:19 UTC (permalink / raw)
To: linux-kselftest
Cc: David Airlie, Arnd Bergmann, Maíra Canal, Dan Carpenter,
Kees Cook, Daniel Diaz, David Gow, Arthur Grillo, Brendan Higgins,
Naresh Kamboju, Maarten Lankhorst, Andrew Morton, Maxime Ripard,
Ville Syrjälä, Daniel Vetter, Thomas Zimmermann,
dri-devel, kunit-dev, linux-arch, linux-arm-kernel, linux-doc,
linux-kernel, linux-parisc, linuxppc-dev, linux-riscv, linux-s390,
linux-sh, loongarch, netdev, x86, Guenter Roeck,
Linux Kernel Functional Testing
In-Reply-To: <20240403131936.787234-1-linux@roeck-us.net>
Count suppressed warning backtraces to enable code which suppresses
warning backtraces to check if the expected backtrace(s) have been
observed.
Using atomics for the backtrace count resulted in build errors on some
architectures due to include file recursion, so use a plain integer
for now.
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
include/kunit/bug.h | 7 ++++++-
lib/kunit/bug.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
index bd0fe047572b..72e9fb23bbd5 100644
--- a/include/kunit/bug.h
+++ b/include/kunit/bug.h
@@ -20,6 +20,7 @@
struct __suppressed_warning {
struct list_head node;
const char *function;
+ int counter;
};
void __start_suppress_warning(struct __suppressed_warning *warning);
@@ -28,7 +29,7 @@ bool __is_suppressed_warning(const char *function);
#define DEFINE_SUPPRESSED_WARNING(func) \
struct __suppressed_warning __kunit_suppress_##func = \
- { .function = __stringify(func) }
+ { .function = __stringify(func), .counter = 0 }
#define START_SUPPRESSED_WARNING(func) \
__start_suppress_warning(&__kunit_suppress_##func)
@@ -39,12 +40,16 @@ bool __is_suppressed_warning(const char *function);
#define IS_SUPPRESSED_WARNING(func) \
__is_suppressed_warning(func)
+#define SUPPRESSED_WARNING_COUNT(func) \
+ (__kunit_suppress_##func.counter)
+
#else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#define DEFINE_SUPPRESSED_WARNING(func)
#define START_SUPPRESSED_WARNING(func)
#define END_SUPPRESSED_WARNING(func)
#define IS_SUPPRESSED_WARNING(func) (false)
+#define SUPPRESSED_WARNING_COUNT(func) (0)
#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#endif /* __ASSEMBLY__ */
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
index f93544d7a9d1..13b3d896c114 100644
--- a/lib/kunit/bug.c
+++ b/lib/kunit/bug.c
@@ -32,8 +32,10 @@ bool __is_suppressed_warning(const char *function)
return false;
list_for_each_entry(warning, &suppressed_warnings, node) {
- if (!strcmp(function, warning->function))
+ if (!strcmp(function, warning->function)) {
+ warning->counter++;
return true;
+ }
}
return false;
}
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 01/15] bug/kunit: Core support for suppressing warning backtraces
From: Guenter Roeck @ 2024-04-03 13:19 UTC (permalink / raw)
To: linux-kselftest
Cc: David Airlie, Arnd Bergmann, Maíra Canal, Dan Carpenter,
Kees Cook, Daniel Diaz, David Gow, Arthur Grillo, Brendan Higgins,
Naresh Kamboju, Maarten Lankhorst, Andrew Morton, Maxime Ripard,
Ville Syrjälä, Daniel Vetter, Thomas Zimmermann,
dri-devel, kunit-dev, linux-arch, linux-arm-kernel, linux-doc,
linux-kernel, linux-parisc, linuxppc-dev, linux-riscv, linux-s390,
linux-sh, loongarch, netdev, x86, Guenter Roeck,
Linux Kernel Functional Testing
In-Reply-To: <20240403131936.787234-1-linux@roeck-us.net>
Some unit tests intentionally trigger warning backtraces by passing
bad parameters to API functions. Such unit tests typically check the
return value from those calls, not the existence of the warning backtrace.
Such intentionally generated warning backtraces are neither desirable
nor useful for a number of reasons.
- They can result in overlooked real problems.
- A warning that suddenly starts to show up in unit tests needs to be
investigated and has to be marked to be ignored, for example by
adjusting filter scripts. Such filters are ad-hoc because there is
no real standard format for warnings. On top of that, such filter
scripts would require constant maintenance.
One option to address problem would be to add messages such as "expected
warning backtraces start / end here" to the kernel log. However, that
would again require filter scripts, it might result in missing real
problematic warning backtraces triggered while the test is running, and
the irrelevant backtrace(s) would still clog the kernel log.
Solve the problem by providing a means to identify and suppress specific
warning backtraces while executing test code. Since the new functionality
results in an image size increase of about 1% if CONFIG_KUNIT is enabled,
provide configuration option KUNIT_SUPPRESS_BACKTRACE to be able to disable
the new functionality. This option is by default enabled since almost all
systems with CONFIG_KUNIT enabled will want to benefit from it.
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Daniel Diaz <daniel.diaz@linaro.org>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Added CONFIG_KUNIT_SUPPRESS_BACKTRACE configuration option,
enabled by default
v3:
- Rebased to v6.9-rc2
include/asm-generic/bug.h | 16 +++++++++---
include/kunit/bug.h | 51 +++++++++++++++++++++++++++++++++++++++
include/kunit/test.h | 1 +
include/linux/bug.h | 13 ++++++++++
lib/bug.c | 51 ++++++++++++++++++++++++++++++++++++---
lib/kunit/Kconfig | 9 +++++++
lib/kunit/Makefile | 6 +++--
lib/kunit/bug.c | 40 ++++++++++++++++++++++++++++++
8 files changed, 178 insertions(+), 9 deletions(-)
create mode 100644 include/kunit/bug.h
create mode 100644 lib/kunit/bug.c
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..c170b6477689 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -18,6 +18,7 @@
#endif
#ifndef __ASSEMBLY__
+#include <kunit/bug.h>
#include <linux/panic.h>
#include <linux/printk.h>
@@ -39,8 +40,14 @@ struct bug_entry {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char *file;
+#ifdef HAVE_BUG_FUNCTION
+ const char *function;
+#endif
#else
signed int file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ signed int function_disp;
+#endif
#endif
unsigned short line;
#endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
#define __WARN() __WARN_printf(TAINT_WARN, NULL)
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin(); \
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end(); \
} while (0)
#else
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin(); \
- __warn_printk(arg); \
- __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ if (!IS_SUPPRESSED_WARNING(__func__)) { \
+ __warn_printk(arg); \
+ __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ } \
instrumentation_end(); \
} while (0)
#define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index 000000000000..bd0fe047572b
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck <linux@roeck-us.net>
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include <linux/kconfig.h>
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+
+#include <linux/stringify.h>
+#include <linux/types.h>
+
+struct __suppressed_warning {
+ struct list_head node;
+ const char *function;
+};
+
+void __start_suppress_warning(struct __suppressed_warning *warning);
+void __end_suppress_warning(struct __suppressed_warning *warning);
+bool __is_suppressed_warning(const char *function);
+
+#define DEFINE_SUPPRESSED_WARNING(func) \
+ struct __suppressed_warning __kunit_suppress_##func = \
+ { .function = __stringify(func) }
+
+#define START_SUPPRESSED_WARNING(func) \
+ __start_suppress_warning(&__kunit_suppress_##func)
+
+#define END_SUPPRESSED_WARNING(func) \
+ __end_suppress_warning(&__kunit_suppress_##func)
+
+#define IS_SUPPRESSED_WARNING(func) \
+ __is_suppressed_warning(func)
+
+#else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
+#define DEFINE_SUPPRESSED_WARNING(func)
+#define START_SUPPRESSED_WARNING(func)
+#define END_SUPPRESSED_WARNING(func)
+#define IS_SUPPRESSED_WARNING(func) (false)
+
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+#endif /* __ASSEMBLY__ */
+#endif /* _KUNIT_BUG_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 61637ef32302..d0c44594d34c 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -10,6 +10,7 @@
#define _KUNIT_TEST_H
#include <kunit/assert.h>
+#include <kunit/bug.h>
#include <kunit/try-catch.h>
#include <linux/args.h>
diff --git a/include/linux/bug.h b/include/linux/bug.h
index 348acf2558f3..c668762dc76a 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -36,6 +36,9 @@ static inline int is_warning_bug(const struct bug_entry *bug)
return bug->flags & BUGFLAG_WARNING;
}
+void bug_get_file_function_line(struct bug_entry *bug, const char **file,
+ const char **function, unsigned int *line);
+
void bug_get_file_line(struct bug_entry *bug, const char **file,
unsigned int *line);
@@ -62,6 +65,16 @@ static inline enum bug_trap_type report_bug(unsigned long bug_addr,
}
struct bug_entry;
+static inline void bug_get_file_function_line(struct bug_entry *bug,
+ const char **file,
+ const char **function,
+ unsigned int *line)
+{
+ *file = NULL;
+ *function = NULL;
+ *line = 0;
+}
+
static inline void bug_get_file_line(struct bug_entry *bug, const char **file,
unsigned int *line)
{
diff --git a/lib/bug.c b/lib/bug.c
index e0ff21989990..aa8bb12b9809 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -26,6 +26,14 @@
when CONFIG_DEBUG_BUGVERBOSE is not enabled, so you must generate
the values accordingly.
+ 2a.Optionally implement support for the "function" entry in struct
+ bug_entry. This entry must point to the name of the function triggering
+ the warning or bug trap (normally __func__). This is only needed if
+ both CONFIG_DEBUG_BUGVERBOSE and CONFIG_KUNIT_SUPPRESS_BACKTRACE are
+ enabled and if the architecture wants to implement support for suppressing
+ warning backtraces. The architecture must define HAVE_BUG_FUNCTION if it
+ adds pointers to function names to struct bug_entry.
+
3. Implement the trap
- In the illegal instruction trap handler (typically), verify
that the fault was in kernel mode, and call report_bug()
@@ -127,14 +135,21 @@ static inline struct bug_entry *module_find_bug(unsigned long bugaddr)
}
#endif
-void bug_get_file_line(struct bug_entry *bug, const char **file,
- unsigned int *line)
+void bug_get_file_function_line(struct bug_entry *bug, const char **file,
+ const char **function, unsigned int *line)
{
+ *function = NULL;
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
*file = (const char *)&bug->file_disp + bug->file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ *function = (const char *)&bug->function_disp + bug->function_disp;
+#endif
#else
*file = bug->file;
+#ifdef HAVE_BUG_FUNCTION
+ *function = bug->function;
+#endif
#endif
*line = bug->line;
#else
@@ -143,6 +158,13 @@ void bug_get_file_line(struct bug_entry *bug, const char **file,
#endif
}
+void bug_get_file_line(struct bug_entry *bug, const char **file, unsigned int *line)
+{
+ const char *function;
+
+ bug_get_file_function_line(bug, file, &function, line);
+}
+
struct bug_entry *find_bug(unsigned long bugaddr)
{
struct bug_entry *bug;
@@ -157,8 +179,9 @@ struct bug_entry *find_bug(unsigned long bugaddr)
static enum bug_trap_type __report_bug(unsigned long bugaddr, struct pt_regs *regs)
{
struct bug_entry *bug;
- const char *file;
+ const char *file, *function;
unsigned line, warning, once, done;
+ char __maybe_unused sym[KSYM_SYMBOL_LEN];
if (!is_valid_bugaddr(bugaddr))
return BUG_TRAP_TYPE_NONE;
@@ -169,12 +192,32 @@ static enum bug_trap_type __report_bug(unsigned long bugaddr, struct pt_regs *re
disable_trace_on_warning();
- bug_get_file_line(bug, &file, &line);
+ bug_get_file_function_line(bug, &file, &function, &line);
+#if defined(CONFIG_KUNIT_SUPPRESS_BACKTRACE) && defined(CONFIG_KALLSYMS)
+ if (!function) {
+ /*
+ * This will be seen if report_bug is called on an architecture
+ * with no architecture-specific support for suppressing warning
+ * backtraces, if CONFIG_DEBUG_BUGVERBOSE is not enabled, or if
+ * the calling code is from assembler which does not record a
+ * function name. Extracting the function name from the bug
+ * address is less than perfect since compiler optimization may
+ * result in 'bugaddr' pointing to a function which does not
+ * actually trigger the warning, but it is better than no
+ * suppression at all.
+ */
+ sprint_symbol_no_offset(sym, bugaddr);
+ function = sym;
+ }
+#endif /* defined(CONFIG_KUNIT_SUPPRESS_BACKTRACE) && defined(CONFIG_KALLSYMS) */
warning = (bug->flags & BUGFLAG_WARNING) != 0;
once = (bug->flags & BUGFLAG_ONCE) != 0;
done = (bug->flags & BUGFLAG_DONE) != 0;
+ if (warning && IS_SUPPRESSED_WARNING(function))
+ return BUG_TRAP_TYPE_WARN;
+
if (warning && once) {
if (done)
return BUG_TRAP_TYPE_WARN;
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 68a6daec0aef..b1b899265acc 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -15,6 +15,15 @@ menuconfig KUNIT
if KUNIT
+config KUNIT_SUPPRESS_BACKTRACE
+ bool "KUnit - Enable backtrace suppression"
+ default y
+ help
+ Enable backtrace suppression for KUnit. If enabled, backtraces
+ generated intentionally by KUnit tests are suppressed. Disable
+ to reduce kernel image size if image size is more important than
+ suppression of backtraces generated by KUnit tests.
+
config KUNIT_DEBUGFS
bool "KUnit - Enable /sys/kernel/debug/kunit debugfs representation" if !KUNIT_ALL_TESTS
default KUNIT_ALL_TESTS
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 309659a32a78..545b57c3be48 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -14,8 +14,10 @@ ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
kunit-objs += debugfs.o
endif
-# KUnit 'hooks' are built-in even when KUnit is built as a module.
-obj-y += hooks.o
+# KUnit 'hooks' and bug handling are built-in even when KUnit is built
+# as a module.
+obj-y += hooks.o \
+ bug.o
obj-$(CONFIG_KUNIT_TEST) += kunit-test.o
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
new file mode 100644
index 000000000000..f93544d7a9d1
--- /dev/null
+++ b/lib/kunit/bug.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck <linux@roeck-us.net>
+ */
+
+#include <kunit/bug.h>
+#include <linux/export.h>
+#include <linux/list.h>
+#include <linux/string.h>
+
+static LIST_HEAD(suppressed_warnings);
+
+void __start_suppress_warning(struct __suppressed_warning *warning)
+{
+ list_add(&warning->node, &suppressed_warnings);
+}
+EXPORT_SYMBOL_GPL(__start_suppress_warning);
+
+void __end_suppress_warning(struct __suppressed_warning *warning)
+{
+ list_del(&warning->node);
+}
+EXPORT_SYMBOL_GPL(__end_suppress_warning);
+
+bool __is_suppressed_warning(const char *function)
+{
+ struct __suppressed_warning *warning;
+
+ if (!function)
+ return false;
+
+ list_for_each_entry(warning, &suppressed_warnings, node) {
+ if (!strcmp(function, warning->function))
+ return true;
+ }
+ return false;
+}
+EXPORT_SYMBOL_GPL(__is_suppressed_warning);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 08/15] arm64: Add support for suppressing warning backtraces
From: Guenter Roeck @ 2024-04-03 13:19 UTC (permalink / raw)
To: linux-kselftest
Cc: David Airlie, Arnd Bergmann, Maíra Canal, Dan Carpenter,
Kees Cook, Daniel Diaz, David Gow, Arthur Grillo, Brendan Higgins,
Naresh Kamboju, Maarten Lankhorst, Andrew Morton, Maxime Ripard,
Ville Syrjälä, Daniel Vetter, Thomas Zimmermann,
dri-devel, kunit-dev, linux-arch, linux-arm-kernel, linux-doc,
linux-kernel, linux-parisc, linuxppc-dev, linux-riscv, linux-s390,
linux-sh, loongarch, netdev, x86, Guenter Roeck,
Linux Kernel Functional Testing, Catalin Marinas, Will Deacon
In-Reply-To: <20240403131936.787234-1-linux@roeck-us.net>
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/arm64/include/asm/asm-bug.h | 29 +++++++++++++++++++----------
arch/arm64/include/asm/bug.h | 8 +++++++-
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index c762038ba400..c6d22e3cd840 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -8,36 +8,45 @@
#include <asm/brk-imm.h>
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
-#define __BUGVERBOSE_LOCATION(file, line) \
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file, func, line)
+#define __BUGVERBOSE_LOCATION(file, func, line) \
.pushsection .rodata.str,"aMS",@progbits,1; \
14472: .string file; \
.popsection; \
\
.long 14472b - .; \
+ __BUG_FUNC_PTR(func) \
.short line;
#else
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifdef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table,"aw"; \
.align 2; \
14470: .long 14471f - .; \
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
- .short flags; \
+_BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
+ .short flags; \
.popsection; \
14471:
#else
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func) \
brk BUG_BRK_IMM
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
#endif /* __ASM_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 28be048db3f6..044c5e24a17d 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -11,8 +11,14 @@
#include <asm/asm-bug.h>
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC __func__
+#else
+# define __BUG_FUNC NULL
+#endif
+
#define __BUG_FLAGS(flags) \
- asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i" (__BUG_FUNC));
#define BUG() do { \
__BUG_FLAGS(0); \
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 09/15] loongarch: Add support for suppressing warning backtraces
From: Guenter Roeck @ 2024-04-03 13:19 UTC (permalink / raw)
To: linux-kselftest
Cc: David Airlie, Arnd Bergmann, Maíra Canal, Dan Carpenter,
Kees Cook, Daniel Diaz, David Gow, Arthur Grillo, Brendan Higgins,
Naresh Kamboju, Maarten Lankhorst, Andrew Morton, Maxime Ripard,
Ville Syrjälä, Daniel Vetter, Thomas Zimmermann,
dri-devel, kunit-dev, linux-arch, linux-arm-kernel, linux-doc,
linux-kernel, linux-parisc, linuxppc-dev, linux-riscv, linux-s390,
linux-sh, loongarch, netdev, x86, Guenter Roeck,
Linux Kernel Functional Testing, Huacai Chen
In-Reply-To: <20240403131936.787234-1-linux@roeck-us.net>
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Huacai Chen <chenhuacai@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2:
- Rebased to v6.9-rc1; resolved context conflict
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2; resolved context conflict
arch/loongarch/include/asm/bug.h | 38 +++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index 08388876ade4..193f396d81a0 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -3,47 +3,63 @@
#define __ASM_BUG_H
#include <asm/break.h>
+#include <kunit/bug.h>
#include <linux/stringify.h>
#ifndef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#else
-#define __BUGVERBOSE_LOCATION(file, line) \
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
+#define __BUGVERBOSE_LOCATION(file, func, line) \
.pushsection .rodata.str, "aMS", @progbits, 1; \
10002: .string file; \
.popsection; \
\
.long 10002b - .; \
+ __BUG_FUNC_PTR(func) \
.short line;
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifndef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#else
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table, "aw"; \
.align 2; \
10000: .long 10001f - .; \
- _BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
+ _BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
.short flags; \
.popsection; \
10001:
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func) \
break BRK_BUG
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
+
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC __func__
+#else
+# define __BUG_FUNC NULL
+#endif
#define __BUG_FLAGS(flags) \
- asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags, %0)) : : "i" (__BUG_FUNC));
#define __WARN_FLAGS(flags) \
do { \
instrumentation_begin(); \
- __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
annotate_reachable(); \
instrumentation_end(); \
} while (0)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 12/15] sh: Add support for suppressing warning backtraces
From: Guenter Roeck @ 2024-04-03 13:19 UTC (permalink / raw)
To: linux-kselftest
Cc: David Airlie, Arnd Bergmann, Maíra Canal, Dan Carpenter,
Kees Cook, Daniel Diaz, David Gow, Arthur Grillo, Brendan Higgins,
Naresh Kamboju, Maarten Lankhorst, Andrew Morton, Maxime Ripard,
Ville Syrjälä, Daniel Vetter, Thomas Zimmermann,
dri-devel, kunit-dev, linux-arch, linux-arm-kernel, linux-doc,
linux-kernel, linux-parisc, linuxppc-dev, linux-riscv, linux-s390,
linux-sh, loongarch, netdev, x86, Guenter Roeck,
Linux Kernel Functional Testing, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz
In-Reply-To: <20240403131936.787234-1-linux@roeck-us.net>
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/sh/include/asm/bug.h | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
index 05a485c4fabc..470ce6567d20 100644
--- a/arch/sh/include/asm/bug.h
+++ b/arch/sh/include/asm/bug.h
@@ -24,21 +24,36 @@
* The offending file and line are encoded in the __bug_table section.
*/
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR "\t.long %O2\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _EMIT_BUG_ENTRY \
"\t.pushsection __bug_table,\"aw\"\n" \
"2:\t.long 1b, %O1\n" \
- "\t.short %O2, %O3\n" \
- "\t.org 2b+%O4\n" \
+ __BUG_FUNC_PTR \
+ "\t.short %O3, %O4\n" \
+ "\t.org 2b+%O5\n" \
"\t.popsection\n"
#else
#define _EMIT_BUG_ENTRY \
"\t.pushsection __bug_table,\"aw\"\n" \
"2:\t.long 1b\n" \
- "\t.short %O3\n" \
- "\t.org 2b+%O4\n" \
+ "\t.short %O4\n" \
+ "\t.org 2b+%O5\n" \
"\t.popsection\n"
#endif
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC __func__
+#else
+# define __BUG_FUNC NULL
+#endif
+
#define BUG() \
do { \
__asm__ __volatile__ ( \
@@ -47,6 +62,7 @@ do { \
: \
: "n" (TRAPA_BUG_OPCODE), \
"i" (__FILE__), \
+ "i" (__BUG_FUNC), \
"i" (__LINE__), "i" (0), \
"i" (sizeof(struct bug_entry))); \
unreachable(); \
@@ -60,6 +76,7 @@ do { \
: \
: "n" (TRAPA_BUG_OPCODE), \
"i" (__FILE__), \
+ "i" (__BUG_FUNC), \
"i" (__LINE__), \
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry))); \
@@ -85,6 +102,7 @@ do { \
: \
: "n" (TRAPA_BUG_OPCODE), \
"i" (__FILE__), \
+ "i" (__BUG_FUNC), \
"i" (__LINE__), \
"i" (BUGFLAG_UNWINDER), \
"i" (sizeof(struct bug_entry))); \
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2] arm64: dts: ti: k3-{am62p,j722s}: Disable ethernet by default
From: Andrew Davis @ 2024-04-03 14:34 UTC (permalink / raw)
To: Michael Walle, Francesco Dolcini, Nishanth Menon,
Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20240403101545.3932437-1-mwalle@kernel.org>
On 4/3/24 5:15 AM, Michael Walle wrote:
> Device tree best practice is to disable any external interface in the
> dtsi and just enable them if needed in the device tree. Thus, disable
> the ethernet switch and its ports by default and just enable the ones
> used by the EVMs in their device trees.
>
> There is no functional change.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
Acked-by: Andrew Davis <afd@ti.com>
> v2:
> - move the status propert into k3-am62p5-main.dtsi, thus also update
> the k3-am62p5-sk.dts
> - put "status" last
> ---
> arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 3 +++
> arch/arm64/boot/dts/ti/k3-am62p5-sk.dts | 3 +++
> arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> index 7337a9e13535..88bc64111234 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> @@ -673,6 +673,7 @@ cpsw3g: ethernet@8000000 {
> assigned-clock-parents = <&k3_clks 13 11>;
> clock-names = "fck";
> power-domains = <&k3_pds 13 TI_SCI_PD_EXCLUSIVE>;
> + status = "disabled";
>
> dmas = <&main_pktdma 0xc600 15>,
> <&main_pktdma 0xc601 15>,
> @@ -696,6 +697,7 @@ cpsw_port1: port@1 {
> label = "port1";
> phys = <&phy_gmii_sel 1>;
> mac-address = [00 00 00 00 00 00];
> + status = "disabled";
> };
>
> cpsw_port2: port@2 {
> @@ -704,6 +706,7 @@ cpsw_port2: port@2 {
> label = "port2";
> phys = <&phy_gmii_sel 2>;
> mac-address = [00 00 00 00 00 00];
> + status = "disabled";
> };
> };
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
> index 6694087b3665..6a9c99c5fb2a 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
> @@ -431,16 +431,19 @@ &cpsw3g {
> pinctrl-names = "default";
> pinctrl-0 = <&main_rgmii1_pins_default>,
> <&main_rgmii2_pins_default>;
> + status = "okay";
> };
>
> &cpsw_port1 {
> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy0>;
> + status = "okay";
> };
>
> &cpsw_port2 {
> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy1>;
> + status = "okay";
> };
>
> &cpsw3g_mdio {
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> index 6b148da2bcdc..8a38e5ae7d4f 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> @@ -226,10 +226,7 @@ cpsw3g_phy0: ethernet-phy@0 {
> &cpsw_port1 {
> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy0>;
> -};
> -
> -&cpsw_port2 {
> - status = "disabled";
> + status = "okay";
> };
>
> &main_gpio1 {
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 07/10] perf/hisi_pcie: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:35 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-8-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:06PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/hisilicon/hisi_pcie_pmu.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c
> index 5d1f0e9fdb08..06b192cc31d5 100644
> --- a/drivers/perf/hisilicon/hisi_pcie_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c
> @@ -673,7 +673,6 @@ static int hisi_pcie_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> {
> struct hisi_pcie_pmu *pcie_pmu = hlist_entry_safe(node, struct hisi_pcie_pmu, node);
> unsigned int target;
> - cpumask_t mask;
> int numa_node;
>
> /* Nothing to do if this CPU doesn't own the PMU */
> @@ -684,10 +683,10 @@ static int hisi_pcie_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
>
> /* Choose a local CPU from all online cpus. */
> numa_node = dev_to_node(&pcie_pmu->pdev->dev);
> - if (cpumask_and(&mask, cpumask_of_node(numa_node), cpu_online_mask) &&
> - cpumask_andnot(&mask, &mask, cpumask_of(cpu)))
> - target = cpumask_any(&mask);
> - else
> +
> + target = cpumask_any_and_but(cpumask_of_node(numa_node),
> + cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> target = cpumask_any_but(cpu_online_mask, cpu);
>
> if (target >= nr_cpu_ids) {
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 08/10] perf/hisi_uncore: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:35 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-9-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:07PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/hisilicon/hisi_uncore_pmu.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> index 04031450d5fe..ccc9191ad1b6 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> @@ -504,7 +504,6 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> {
> struct hisi_pmu *hisi_pmu = hlist_entry_safe(node, struct hisi_pmu,
> node);
> - cpumask_t pmu_online_cpus;
> unsigned int target;
>
> if (!cpumask_test_and_clear_cpu(cpu, &hisi_pmu->associated_cpus))
> @@ -518,9 +517,8 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> hisi_pmu->on_cpu = -1;
>
> /* Choose a new CPU to migrate ownership of the PMU to */
> - cpumask_and(&pmu_online_cpus, &hisi_pmu->associated_cpus,
> - cpu_online_mask);
> - target = cpumask_any_but(&pmu_online_cpus, cpu);
> + target = cpumask_any_and_but(&hisi_pmu->associated_cpus,
> + cpu_online_mask, cpu);
> if (target >= nr_cpu_ids)
> return 0;
>
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 09/10] perf/qcom_l2: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:36 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-10-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:08PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/qcom_l2_pmu.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/perf/qcom_l2_pmu.c b/drivers/perf/qcom_l2_pmu.c
> index 148df5ae8ef8..b5a44dc1dc3a 100644
> --- a/drivers/perf/qcom_l2_pmu.c
> +++ b/drivers/perf/qcom_l2_pmu.c
> @@ -801,9 +801,8 @@ static int l2cache_pmu_online_cpu(unsigned int cpu, struct hlist_node *node)
>
> static int l2cache_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> {
> - struct cluster_pmu *cluster;
> struct l2cache_pmu *l2cache_pmu;
> - cpumask_t cluster_online_cpus;
> + struct cluster_pmu *cluster;
> unsigned int target;
>
> l2cache_pmu = hlist_entry_safe(node, struct l2cache_pmu, node);
> @@ -820,9 +819,8 @@ static int l2cache_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> cluster->on_cpu = -1;
>
> /* Any other CPU for this cluster which is still online */
> - cpumask_and(&cluster_online_cpus, &cluster->cluster_cpus,
> - cpu_online_mask);
> - target = cpumask_any_but(&cluster_online_cpus, cpu);
> + target = cpumask_any_and_but(&cluster->cluster_cpus,
> + cpu_online_mask, cpu);
> if (target >= nr_cpu_ids) {
> disable_irq(cluster->irq);
> return 0;
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 10/10] perf/thunderx2: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:38 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-11-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:51:09PM +0800, Dawei Li wrote:
> For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> variable on stack is not recommended since it can cause potential stack
> overflow.
>
> Instead, kernel code should always use *cpumask_var API(s) to allocate
> cpumask var in config-neutral way, leaving allocation strategy to
> CONFIG_CPUMASK_OFFSTACK.
>
> But dynamic allocation in cpuhp's teardown callback is somewhat problematic
> for if allocation fails(which is unlikely but still possible):
> - If -ENOMEM is returned to caller, kernel crashes for non-bringup
> teardown;
> - If callback pretends nothing happened and returns 0 to caller, it may
> trap system into an in-consisitent/compromised state;
>
> Use newly-introduced cpumask_any_and_but() to address all issues above.
> It eliminates usage of temporary cpumask var in generic way, no matter how
> the cpumask var is allocated.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
The logic looks good to me, but I'd like the commit message updated the same as
per my comment on patch 2.
With that commit message:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> drivers/perf/thunderx2_pmu.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> index e16d10c763de..b3377b662ffc 100644
> --- a/drivers/perf/thunderx2_pmu.c
> +++ b/drivers/perf/thunderx2_pmu.c
> @@ -932,9 +932,8 @@ static int tx2_uncore_pmu_online_cpu(unsigned int cpu,
> static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
> struct hlist_node *hpnode)
> {
> - int new_cpu;
> struct tx2_uncore_pmu *tx2_pmu;
> - struct cpumask cpu_online_mask_temp;
> + unsigned int new_cpu;
>
> tx2_pmu = hlist_entry_safe(hpnode,
> struct tx2_uncore_pmu, hpnode);
> @@ -945,11 +944,8 @@ static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
> if (tx2_pmu->hrtimer_callback)
> hrtimer_cancel(&tx2_pmu->hrtimer);
>
> - cpumask_copy(&cpu_online_mask_temp, cpu_online_mask);
> - cpumask_clear_cpu(cpu, &cpu_online_mask_temp);
> - new_cpu = cpumask_any_and(
> - cpumask_of_node(tx2_pmu->node),
> - &cpu_online_mask_temp);
> + new_cpu = cpumask_any_and_but(cpumask_of_node(tx2_pmu->node),
> + cpu_online_mask, cpu);
>
> tx2_pmu->cpu = new_cpu;
> if (new_cpu >= nr_cpu_ids)
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESEND PATCH] arm64: defconfig: build ath12k as a module
From: Bartosz Golaszewski @ 2024-04-03 14:38 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Bjorn Andersson,
Krzysztof Kozlowski, Konrad Dybcio, Arnd Bergmann,
Geert Uytterhoeven, Neil Armstrong, Marek Szyprowski,
Nícolas F . R . A . Prado, Udit Kumar, Andrew Halaney,
Maximilian Luz, Alex Elder, Srini Kandagatla
Cc: linux-arm-kernel, linux-kernel, linux-arm-msm,
Bartosz Golaszewski, Jeff Johnson
In-Reply-To: <20240325085756.13843-1-brgl@bgdev.pl>
On Mon, Mar 25, 2024 at 9:58 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Qualcomm sm8550-qrd and sm8650-qrd boards have ath12k modules as part of
> their on-board WCN7850 WLAN/BT packages. Enable the relevant driver in
> defconfig.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 6c45a465a071..937edf09f0aa 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -411,6 +411,7 @@ CONFIG_WCN36XX=m
> CONFIG_ATH11K=m
> CONFIG_ATH11K_AHB=m
> CONFIG_ATH11K_PCI=m
> +CONFIG_ATH12K=m
> CONFIG_BRCMFMAC=m
> CONFIG_MWIFIEX=m
> CONFIG_MWIFIEX_SDIO=m
> --
> 2.40.1
>
Ping.
Bartosz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 00/10] perf: Avoid placing cpumask var on stack
From: Mark Rutland @ 2024-04-03 14:41 UTC (permalink / raw)
To: Dawei Li
Cc: will, yury.norov, linux, xueshuai, renyu.zj, yangyicong,
jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
linux-kernel, linux-arm-msm
In-Reply-To: <20240403125109.2054881-1-dawei.li@shingroup.cn>
On Wed, Apr 03, 2024 at 08:50:59PM +0800, Dawei Li wrote:
> Hi all,
Hi,
> This is v2 of [1] and [2] which basically eliminate cpumask var allocation
> on stack for perf subsystem.
>
> Change since v1:
> - Change from dynamic allocation to a temporary var free helper:
> cpumask_any_and_but(). [Mark]
>
> - Some minor coding style improvements, reverse chrismas tree e.g.
>
> - For cpumask_any_and_but() itself:
> - Moved to cpumask.h, just like other helpers.
> - Return value converted to unsigned int.
> - Remove EXPORT_SYMBOL, for obvious reason.
Thanks for this!
The logic all looks good; if you can spin a v3 with the updated commit messages
I reckon we can queue this up shortly.
Mark.
>
> [1]:
> https://lore.kernel.org/lkml/20240402105610.1695644-1-dawei.li@shingroup.cn/
>
> [2]:
> https://lore.kernel.org/lkml/1486381132-5610-1-git-send-email-mark.rutland@arm.com/
>
> Dawei Li (9):
> perf/alibaba_uncore_drw: Avoid placing cpumask var on stack
> perf/arm-cmn: Avoid placing cpumask var on stack
> perf/arm_cspmu: Avoid placing cpumask var on stack
> perf/arm_dsu: Avoid placing cpumask var on stack
> perf/dwc_pcie: Avoid placing cpumask var on stack
> perf/hisi_pcie: Avoid placing cpumask var on stack
> perf/hisi_uncore: Avoid placing cpumask var on stack
> perf/qcom_l2: Avoid placing cpumask var on stack
> perf/thunderx2: Avoid placing cpumask var on stack
>
> Mark Rutland (1):
> cpumask: add cpumask_any_and_but()
>
> drivers/perf/alibaba_uncore_drw_pmu.c | 10 +++-------
> drivers/perf/arm-cmn.c | 10 +++++-----
> drivers/perf/arm_cspmu/arm_cspmu.c | 8 +++-----
> drivers/perf/arm_dsu_pmu.c | 19 ++++++-------------
> drivers/perf/dwc_pcie_pmu.c | 10 ++++------
> drivers/perf/hisilicon/hisi_pcie_pmu.c | 9 ++++-----
> drivers/perf/hisilicon/hisi_uncore_pmu.c | 6 ++----
> drivers/perf/qcom_l2_pmu.c | 8 +++-----
> drivers/perf/thunderx2_pmu.c | 10 +++-------
> include/linux/cpumask.h | 23 +++++++++++++++++++++++
> 10 files changed, 56 insertions(+), 57 deletions(-)
>
> Thanks,
>
> Dawei
>
> --
> 2.27.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h
From: Andy Shevchenko @ 2024-04-03 14:41 UTC (permalink / raw)
To: Mark Brown
Cc: linux-spi, linux-kernel, linux-arm-kernel, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Russell King, Arnd Bergmann
In-Reply-To: <0af775e1-f5f7-4ad4-b336-78834a9e0342@sirena.org.uk>
On Wed, Apr 03, 2024 at 03:13:48PM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 04:41:40PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote:
>
> > > > All the concerns I have with swnodes just being a more complex and less
> > > > maintainable way of doing things still stand, I'm not clear that this is
> > > > making anything better.
>
> > > As I explained before it's not less maintainable than device tree sources.
> > > The only difference is that we don't have validation tool for in-kernel
> > > tables. And I don't see why we need that. The data describes the platforms
> > > and in the very same way may come to the driver from elsewhere.
> > > How would you validate that? It the same as we trust firmware (boot loader)
> > > or not. If we don't than how should we do at all?
>
> I don't think we should do this at all, all I see from these changes is
> effort in reviewing them - as far as I can tell they are a solution in
> search of a problem. DT has some support for validation of the
> formatting of the data supplied by the board and offers the potential
> for distributing the board description separately to the kernel.
>
> > > Can you point out what the exact aspect is most significant from C language
> > > perspective that we miss after conversion? Type checking? Something else?
>
> You're changing the code from supplying data from the board description
> via a simple C structure where the compiler offers at least some
> validation and where we can just supply directly usable values to an
> abstract data structure that's still encoded in C but has less
> validation and requires a bunch of code to parse at runtime. Platform
> data is unsurprisingly a good solution to the problem of supplying
> platform data.
Linus was long time ago against board files. Yet, we have a few old
(kinda supported) boards left in the tree. The conversion makes the
driver be prepared for the DT conversion when it happens. From maintenance
perspective my patch reduced the code under the maintenance, which reduces
time spent by both contributors and maintainers on this.
AFAIU all what you are moaning about is type checking. Okay, I got
it, but we have a lot of other places with similar approach done,
e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
driver without any platform data being involved and you seems to be fine with
that:
$ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'
I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").
Can you tell how it is different to my proposal?
> > Also note, after hiding the data structures from that file we open
> > the door for the much bigger cleanup, and I have patches already precooked
> > (need a bit of time to test, though).
>
> Perhaps those will motivate a change, as things stand I've not seen what
> you're proposing to do here.
Okay, let me incorporate those into v3.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] PCI: rockchip-ep: Remove wrong mask on subsys_vendor_id
From: Rick Wertenbroek @ 2024-04-03 14:45 UTC (permalink / raw)
To: rick.wertenbroek
Cc: dlemoal, Rick Wertenbroek, stable, Shawn Lin, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Heiko Stuebner, linux-pci, linux-rockchip, linux-arm-kernel,
linux-kernel
Remove wrong mask on subsys_vendor_id. Both the Vendor ID and Subsystem
Vendor ID are u16 variables and are written to a u32 register of the
controller. The Subsystem Vendor ID was always 0 because the u16 value
was masked incorrectly with GENMASK(31,16) resulting in all lower 16
bits being set to 0 prior to the shift.
Remove both masks as they are unnecessary and set the register correctly
i.e., the lower 16-bits are the Vendor ID and the upper 16-bits are the
Subsystem Vendor ID.
This is documented in the RK3399 TRM section 17.6.7.1.17
Fixes: cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip PCIe controller")
Signed-off-by: Rick Wertenbroek <rick.wertenbroek@gmail.com>
Cc: stable@vger.kernel.org
---
drivers/pci/controller/pcie-rockchip-ep.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index c9046e97a1d2..37d4bcb8bd5b 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -98,10 +98,9 @@ static int rockchip_pcie_ep_write_header(struct pci_epc *epc, u8 fn, u8 vfn,
/* All functions share the same vendor ID with function 0 */
if (fn == 0) {
- u32 vid_regs = (hdr->vendorid & GENMASK(15, 0)) |
- (hdr->subsys_vendor_id & GENMASK(31, 16)) << 16;
-
- rockchip_pcie_write(rockchip, vid_regs,
+ rockchip_pcie_write(rockchip,
+ hdr->vendorid |
+ hdr->subsys_vendor_id << 16,
PCIE_CORE_CONFIG_VENDOR);
}
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox