* [PATCH] powerpc/smp: Fix OOPS in topology_init()
From: Christophe Leroy @ 2021-08-04 18:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Running an SMP kernel on an UP platform not prepared for it,
I encountered the following OOPS:
BUG: Kernel NULL pointer dereference on read at 0x00000034
Faulting instruction address: 0xc0a04110
Oops: Kernel access of bad area, sig: 11 [#1]
BE PAGE_SIZE=4K SMP NR_CPUS=2 CMPCPRO
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-pmac-00001-g230fedfaad21 #5234
NIP: c0a04110 LR: c0a040d8 CTR: c0a04084
REGS: e100dda0 TRAP: 0300 Not tainted (5.13.0-pmac-00001-g230fedfaad21)
MSR: 00009032 <EE,ME,IR,DR,RI> CR: 84000284 XER: 00000000
DAR: 00000034 DSISR: 20000000
GPR00: c0006bd4 e100de60 c1033320 00000000 00000000 c0942274 00000000 00000000
GPR08: 00000000 00000000 00000001 00000063 00000007 00000000 c0006f30 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000005
GPR24: c0c67d74 c0c67f1c c0c60000 c0c67d70 c0c0c558 1efdf000 c0c00020 00000000
NIP [c0a04110] topology_init+0x8c/0x138
LR [c0a040d8] topology_init+0x54/0x138
Call Trace:
[e100de60] [80808080] 0x80808080 (unreliable)
[e100de90] [c0006bd4] do_one_initcall+0x48/0x1bc
[e100def0] [c0a0150c] kernel_init_freeable+0x1c8/0x278
[e100df20] [c0006f44] kernel_init+0x14/0x10c
[e100df30] [c00190fc] ret_from_kernel_thread+0x14/0x1c
Instruction dump:
7c692e70 7d290194 7c035040 7c7f1b78 5529103a 546706fe 5468103a 39400001
7c641b78 40800054 80c690b4 7fb9402e <81060034> 7fbeea14 2c080000 7fa3eb78
---[ end trace b246ffbc6bbbb6fb ]---
Fix it by checking smp_ops before using it, as already done in
several other places in the arch/powerpc/kernel/smp.c
Fixes: 39f87561454d ("powerpc/smp: Move ppc_md.cpu_die() to smp_ops.cpu_offline_self()")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 5ff0e55d0db1..defecb3b1b15 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -1167,7 +1167,7 @@ static int __init topology_init(void)
* CPU. For instance, the boot cpu might never be valid
* for hotplugging.
*/
- if (smp_ops->cpu_offline_self)
+ if (smp_ops && smp_ops->cpu_offline_self)
c->hotpluggable = 1;
#endif
--
2.25.0
^ permalink raw reply related
* [Bug 213961] Oops while loading radeon driver
From: bugzilla-daemon @ 2021-08-04 19:53 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213961-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213961
--- Comment #5 from Elimar Riesebieter (riesebie@lxtec.de) ---
Configuring KALLSYMS doesn't give the function names
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 213961] Oops while loading radeon driver
From: bugzilla-daemon @ 2021-08-04 19:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213961-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213961
--- Comment #6 from Elimar Riesebieter (riesebie@lxtec.de) ---
Created attachment 298203
--> https://bugzilla.kernel.org/attachment.cgi?id=298203&action=edit
config with KALLSYMS enabled
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 213961] Oops while loading radeon driver
From: bugzilla-daemon @ 2021-08-04 19:55 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213961-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213961
--- Comment #7 from Elimar Riesebieter (riesebie@lxtec.de) ---
Created attachment 298205
--> https://bugzilla.kernel.org/attachment.cgi?id=298205&action=edit
dmesg dump with KALLSYMS enabled
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH] powerpc/configs: Disable legacy ptys on microwatt defconfig
From: Anton Blanchard @ 2021-08-05 1:20 UTC (permalink / raw)
To: Paul Mackerras, Joel Stanley, Michael Ellerman; +Cc: linuxppc-dev
We shouldn't need legacy ptys, and disabling the option improves boot
time by about 0.5 seconds.
Signed-off-by: Anton Blanchard <anton@ozlabs.org>
---
diff --git a/arch/powerpc/configs/microwatt_defconfig b/arch/powerpc/configs/microwatt_defconfig
index a08b739123da..ebc90aefbc0c 100644
--- a/arch/powerpc/configs/microwatt_defconfig
+++ b/arch/powerpc/configs/microwatt_defconfig
@@ -57,6 +57,7 @@ CONFIG_NETDEVICES=y
# CONFIG_INPUT is not set
# CONFIG_SERIO is not set
# CONFIG_VT is not set
+# CONFIG_LEGACY_PTYS is not set
CONFIG_SERIAL_8250=y
# CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
CONFIG_SERIAL_8250_CONSOLE=y
^ permalink raw reply related
* Re: [PATCH] powerpc/32s: Fix napping restore in data storage interrupt (DSI)
From: Finn Thain @ 2021-08-05 1:54 UTC (permalink / raw)
To: Christophe Leroy; +Cc: userm57, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <b04a90a9-9d62-2192-f896-ea99be911604@csgroup.eu>
On Wed, 4 Aug 2021, Christophe Leroy wrote:
>
> This patch is related to the bisect you did that pointed to 4c0104a83fc3
> ("powerpc/32: Dismantle EXC_XFER_STD/LITE/TEMPLATE")
>
> I think maybe the starting point should be to (manually) apply the patch
> on top of that commit in order to check that the bug to leaded to
> pointing that commit as 'first bad commit' is now gone.
>
Stan has now confirmed this. He applied this patch on top of 4c0104a83fc3,
and it did indeed resolve the bug that 'git bisect' isolated [1]. Thanks
Christophe.
[1]
https://lore.kernel.org/lkml/666e3ab4-372-27c2-4621-7cc3933756dd@linux-m68k.org/
^ permalink raw reply
* Re: [PATCH printk v1 03/10] kgdb: delay roundup if holding printk cpulock
From: John Ogness @ 2021-08-05 3:46 UTC (permalink / raw)
To: Daniel Thompson, Petr Mladek
Cc: Gautham R. Shenoy, Douglas Anderson, Srikar Dronamraju,
Peter Zijlstra, linux-kernel, Paul Mackerras, H. Peter Anvin,
Chengyang Fan, Bhaskar Chowdhury, x86, Ingo Molnar,
kgdb-bugreport, Nicholas Piggin, Borislav Petkov, Steven Rostedt,
Thomas Gleixner, Gustavo A. R. Silva, Sergey Senozhatsky,
Jason Wessel, linuxppc-dev, Cédric Le Goater
In-Reply-To: <20210804150431.qtra3wvh2n4m6j64@maple.lan>
On 2021-08-04, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Wed, Aug 04, 2021 at 02:12:22PM +0200, Petr Mladek wrote:
>> On Wed 2021-08-04 12:31:59, Daniel Thompson wrote:
>> > On Tue, Aug 03, 2021 at 05:36:32PM +0206, John Ogness wrote:
>> > > On 2021-08-03, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>> > > > On Tue, Aug 03, 2021 at 03:18:54PM +0206, John Ogness wrote:
>> > > >> kgdb makes use of its own cpulock (@dbg_master_lock, @kgdb_active)
>> > > >> during cpu roundup. This will conflict with the printk cpulock.
>> > > >
>> > > > When the full vision is realized what will be the purpose of the printk
>> > > > cpulock?
>> > > >
>> > > > I'm asking largely because it's current role is actively unhelpful
>> > > > w.r.t. kdb. It is possible that cautious use of in_dbg_master() might
>> > > > be a better (and safer) solution. However it sounds like there is a
>> > > > larger role planned for the printk cpulock...
>> > >
>> > > The printk cpulock is used as a synchronization mechanism for
>> > > implementing atomic consoles, which need to be able to safely interrupt
>> > > the console write() activity at any time and immediately continue with
>> > > their own printing. The ultimate goal is to move all console printing
>> > > into per-console dedicated kthreads, so the primary function of the
>> > > printk cpulock is really to immediately _stop_ the CPU/kthread
>> > > performing write() in order to allow write_atomic() (from any context on
>> > > any CPU) to safely and reliably take over.
>> >
>> > I see.
>> >
>> > Is there any mileage in allowing in_dbg_master() to suppress taking
>> > the console lock?
>> >
>> > There's a couple of reasons to worry about the current approach.
>> >
>> > The first is that we don't want this code to trigger in the case when
>> > kgdb is enabled and kdb is not since it is only kdb (a self-hosted
>> > debugger) than uses the consoles. This case is relatively trivial to
>> > address since we can rename it kdb_roundup_delay() and alter the way it
>> > is conditionally compiled.
Well, _I_ want this code to trigger even without kdb. The printk cpulock
is meant to be the innermost locking for the entire kernel. No code is
allowed to block/spin on any kind of lock if holding the printk
cpulock. This is the only way to guarantee the functionality of the
atomic consoles.
For example, if the kernel were to crash while inside kgdb code, we want
to see the backtrace.
Since kgdb _does_ take locks (spinning on @dbg_slave_lock during roundup
and the master's own cpu lock as a retry loop on @dbg_master_lock),
clearly it is not allowed to hold the printk cpulock. The simplest
solution I could find was just to make sure kgdb_cpu_enter() isn't
called while holding the printk cpulock.
>> > The second is more of a problem however. kdb will only call into the
>> > console code from the debug master. By default this is the CPU that
>> > takes the debug trap so initial prints will work fine. However it is
>> > possible to switch to a different master (so we can read per-CPU
>> > registers and things like that). This will result in one of the CPUs
>> > that did the IPI round up calling into console code and this is unsafe
>> > in that instance.
It is only unsafe if a CPU enters "kgdb/kdb context" while holding the
printk cpulock. That is what I want to prevent.
>> > There are a couple of tricks we could adopt to work around this but
>> > given the slightly odd calling context for kdb (all CPUs quiesced, no
>> > log interleaving possible) it sounds like it would remain safe to
>> > bypass the lock if in_dbg_master() is true.
>> >
>> > Bypassing an inconvenient lock might sound icky but:
>> >
>> > 1. If the lock is not owned by any CPU then what kdb will do is safe.
No. The printk cpulock exists for low-level synchronization. The atomic
consoles need this synchronization. (For example, the 8250 needs this
for correct tracking of its interrupt register, even for
serial8250_put_poll_char().)
>> > 2. If the lock is owned by any CPU then we have quiesced it anyway
>> > and this makes is safe for the owning CPU to share its ownership
>> > (since it isn't much different to recursive acquisition on a single
>> > CPU)
Quiescing the printk cpulock is not permitted.
Just because it is kdb, does not mean that the atomic consoles were
interrupted in a convenient place. The whole purpose of the atomic
consoles is so that we can have guaranteed console output from _any_
context and _any_ line of code in the kernel.
>> I think about the following:
>>
>> void kgdb_roundup_cpus(void)
>> {
>> __printk_cpu_lock();
>> __kgdb_roundup_cpus();
>> }
>>
>> , where __printk_cpu_lock() waits/takes printk_cpu_lock()
>> __kgdb_roundup_cpus() is the original kgdb_roundup_cpus();
>>
>>
>> The idea is that kgdb_roundup_cpus() caller takes the printk_cpu lock.
>> The owner will be well defined.
>>
>> As a result any other CPU will not be able to take the printk_cpu lock
>> as long as it is owned by the kgdb lock. But as you say, kgdb will
>> make sure that everything is serialized at this stage. So that
>> the original raw_printk_cpu_lock_irqsave() might just disable
>> IRQs when called under debugger.
>>
>> Does it make any sense?
>
> Yes but I think it is still has problems.
>
> Primarily is doesn't solve the issue I raised. It would still be unsafe
> to change debug master: we can guarantee the initial master owns the
> lock but if it has been multiply acquired we cannot transfer ownership
> when we want to change master.
>
> Additionally it will delay the round up of cores that do not own the
> lock. The quiescing is never atomic and the operator needs to know
> that but the longer CPUs are allows to execute for the more confusing
> things can become for the operator.
>
> Finally on machines without an NMI this could cause trouble with the
> interrupt disable in raw_printk_cpu_lock_irqsave() (or any outer level
> interrupt disable). If the master get the lock then the other processes
> will become incapable of being rounded up if they are waiting for the
> printk lock).
I am also not happy with such a solution. Aside from Daniel's comments,
it also violates the basic principle of the printk cpulock by allowing
further locking while holding the print cpulock. That is a recipe for
deadlock.
>> I have to say that it is a bit hairy. But it looks slightly better
>> than the delayed/repeated IPI proposed by this patch.
>
> I'd like to reserve judgement for now which one is least worst...
> largely because if the purpose of the lock simply to prevent interleaving
> of console output then the debugger quiescing code should already have
> this covered.
>
> It leaves me wondering if a change like the one below is sufficient
> (based on code without John's patches but hopefully still clear enough).
> I've given the new code it's own branch which it doesn't, strictly
> speaking, need but it is easier to comment this way... and perhaps also
> just a little easier for people who have not set CONFIG_KGDB to
> ignore ;-).
>
> ~~~
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 142a58d124d9..41a7e103bb66 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3599,6 +3599,18 @@ int __printk_cpu_trylock(void)
> /* This CPU is already the owner. */
> atomic_inc(&printk_cpulock_nested);
> return 1;
> + } else if (in_dbg_master()) {
> + /*
> + * If we are executing as the primary CPU and with the debugger
> + * active than all other CPUs in the system are quiesced by
> + * the time kdb winds up calling this function. To execute this
> + * branch then the lock must be owned by one of the quiesced CPUs.
> + * Happily, because it is quiesced and cannot release it, it is
> + * safe for us to allow the lock to be taken from a different CPU!
> + * The lock will be released prior to resuming the real owner.
> + */
> + atomic_inc(&printk_cpulock_nested);
> + return 1;
> }
>
> return 0;
> ~~~
Being in kgdb/kdb context is similar to being in atomic console
context. (Of course, they are both using cpu locks.) But the contexts
are not the same. It is incorrect to handle them as the same.
We need to decide who is inside of who. Either printk is the innermost,
in which case the printk cpulock cannot be held when calling
kgdb_cpu_enter(). Or kgdb is the innermost, meaning that the atomic
consoles are no longer atomic/reliable while in kgdb.
I prefer and am pushing for the first, but am willing to accept the
second (i.e. that kgdb is the innermost function of the kernel).
> PS In the interested of full disclosure there is a special case
> in the debugger to allow it to try to cope if it fails to
> quiesce a CPU and I deliberately omitted this from the long
> comment above. That special case is expected to be unstable
> but since the alternative is likely to be a permanent deadlock
> without any indication of why we choose to take the risk of
> continuing. Personally I don't recommend reasoning about
> console safety based on this emergency case hence omitting the
> comment.
John Ogness
^ permalink raw reply
* Re: [PATCH v4 01/10] net/ps3_gelic: Add gelic_descr structures
From: Christophe Leroy @ 2021-08-05 5:03 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <c95aa8e57aca8b3af6893f13f2e03731f8198184.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> In an effort to make the PS3 gelic driver easier to maintain, create two
> new structures, struct gelic_hw_regs and struct gelic_chain_link, and
> replace the corresponding members of struct gelic_descr with the new
> structures.
>
> struct gelic_hw_regs holds the register variables used by the gelic
> hardware device. struct gelic_chain_link holds variables used to manage
> the driver's linked list of gelic descr structures.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
Running checkpatch script provides the following feedback:
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#164: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:400:
+ descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(ctodev(card),
descr->skb->data,
WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG()
or BUG_ON()
#190: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:497:
+ BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) & GELIC_DESCR_TX_TAIL));
ERROR:SPACING: spaces required around that '?' (ctx:VxE)
#333: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:922:
+ skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size)?
^
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 5c33bdbe1861 ("net/ps3_gelic: Add gelic_descr structures") has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 133 ++++++++++---------
> drivers/net/ethernet/toshiba/ps3_gelic_net.h | 24 ++--
> 2 files changed, 82 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 55e652624bd7..cb45571573d7 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -98,7 +98,7 @@ static void gelic_card_get_ether_port_status(struct gelic_card *card,
> static enum gelic_descr_dma_status
> gelic_descr_get_status(struct gelic_descr *descr)
> {
> - return be32_to_cpu(descr->dmac_cmd_status) & GELIC_DESCR_DMA_STAT_MASK;
> + return be32_to_cpu(descr->hw_regs.dmac_cmd_status) & GELIC_DESCR_DMA_STAT_MASK;
> }
>
> static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
> @@ -154,13 +154,13 @@ static void gelic_card_enable_rxdmac(struct gelic_card *card)
> printk(KERN_ERR "%s: status=%x\n", __func__,
> be32_to_cpu(card->rx_chain.head->dmac_cmd_status));
> printk(KERN_ERR "%s: nextphy=%x\n", __func__,
> - be32_to_cpu(card->rx_chain.head->next_descr_addr));
> + be32_to_cpu(card->rx_chain.head->hw_regs.next_descr_addr));
> printk(KERN_ERR "%s: head=%p\n", __func__,
> card->rx_chain.head);
> }
> #endif
> status = lv1_net_start_rx_dma(bus_id(card), dev_id(card),
> - card->rx_chain.head->bus_addr, 0);
> + card->rx_chain.head->link.cpu_addr, 0);
> if (status)
> dev_info(ctodev(card),
> "lv1_net_start_rx_dma failed, status=%d\n", status);
> @@ -195,8 +195,8 @@ static void gelic_card_disable_rxdmac(struct gelic_card *card)
> static void gelic_descr_set_status(struct gelic_descr *descr,
> enum gelic_descr_dma_status status)
> {
> - descr->dmac_cmd_status = cpu_to_be32(status |
> - (be32_to_cpu(descr->dmac_cmd_status) &
> + descr->hw_regs.dmac_cmd_status = cpu_to_be32(status |
> + (be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> ~GELIC_DESCR_DMA_STAT_MASK));
> /*
> * dma_cmd_status field is used to indicate whether the descriptor
> @@ -224,13 +224,13 @@ static void gelic_card_reset_chain(struct gelic_card *card,
>
> for (descr = start_descr; start_descr != descr->next; descr++) {
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
> - descr->next_descr_addr = cpu_to_be32(descr->next->bus_addr);
> + descr->hw_regs.next_descr_addr = cpu_to_be32(descr->next->link.cpu_addr);
> }
>
> chain->head = start_descr;
> chain->tail = (descr - 1);
>
> - (descr - 1)->next_descr_addr = 0;
> + (descr - 1)->hw_regs.next_descr_addr = 0;
> }
>
> void gelic_card_up(struct gelic_card *card)
> @@ -286,10 +286,10 @@ static void gelic_card_free_chain(struct gelic_card *card,
> {
> struct gelic_descr *descr;
>
> - for (descr = descr_in; descr && descr->bus_addr; descr = descr->next) {
> - dma_unmap_single(ctodev(card), descr->bus_addr,
> - GELIC_DESCR_SIZE, DMA_BIDIRECTIONAL);
> - descr->bus_addr = 0;
> + for (descr = descr_in; descr && descr->link.cpu_addr; descr = descr->next) {
> + dma_unmap_single(ctodev(card), descr->link.cpu_addr,
> + descr->link.size, DMA_BIDIRECTIONAL);
> + descr->link.cpu_addr = 0;
> }
> }
>
> @@ -317,13 +317,14 @@ static int gelic_card_init_chain(struct gelic_card *card,
>
> /* set up the hardware pointers in each descriptor */
> for (i = 0; i < no; i++, descr++) {
> + descr->link.size = sizeof(struct gelic_hw_regs);
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> - descr->bus_addr =
> + descr->link.cpu_addr =
> dma_map_single(ctodev(card), descr,
> - GELIC_DESCR_SIZE,
> + descr->link.size,
> DMA_BIDIRECTIONAL);
>
> - if (!descr->bus_addr)
> + if (!descr->link.cpu_addr)
> goto iommu_error;
>
> descr->next = descr + 1;
> @@ -336,22 +337,22 @@ static int gelic_card_init_chain(struct gelic_card *card,
> /* chain bus addr of hw descriptor */
> descr = start_descr;
> for (i = 0; i < no; i++, descr++) {
> - descr->next_descr_addr = cpu_to_be32(descr->next->bus_addr);
> + descr->hw_regs.next_descr_addr = cpu_to_be32(descr->next->link.cpu_addr);
> }
>
> chain->head = start_descr;
> chain->tail = start_descr;
>
> /* do not chain last hw descriptor */
> - (descr - 1)->next_descr_addr = 0;
> + (descr - 1)->hw_regs.next_descr_addr = 0;
>
> return 0;
>
> iommu_error:
> for (i--, descr--; 0 <= i; i--, descr--)
> - if (descr->bus_addr)
> - dma_unmap_single(ctodev(card), descr->bus_addr,
> - GELIC_DESCR_SIZE,
> + if (descr->link.cpu_addr)
> + dma_unmap_single(ctodev(card), descr->link.cpu_addr,
> + descr->link.size,
> DMA_BIDIRECTIONAL);
> return -ENOMEM;
> }
> @@ -381,25 +382,25 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
> * bit more */
> descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
> if (!descr->skb) {
> - descr->buf_addr = 0; /* tell DMAC don't touch memory */
> + descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
> return -ENOMEM;
> }
> - descr->buf_size = cpu_to_be32(bufsize);
> - descr->dmac_cmd_status = 0;
> - descr->result_size = 0;
> - descr->valid_size = 0;
> - descr->data_error = 0;
> + descr->hw_regs.payload.size = cpu_to_be32(bufsize);
> + descr->hw_regs.dmac_cmd_status = 0;
> + descr->hw_regs.result_size = 0;
> + descr->hw_regs.valid_size = 0;
> + descr->hw_regs.data_error = 0;
>
> offset = ((unsigned long)descr->skb->data) &
> (GELIC_NET_RXBUF_ALIGN - 1);
> if (offset)
> skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
> /* io-mmu-map the skb */
> - descr->buf_addr = cpu_to_be32(dma_map_single(ctodev(card),
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(ctodev(card),
> descr->skb->data,
> GELIC_NET_MAX_MTU,
> DMA_FROM_DEVICE));
> - if (!descr->buf_addr) {
> + if (!descr->hw_regs.payload.dev_addr) {
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> dev_info(ctodev(card),
> @@ -424,10 +425,10 @@ static void gelic_card_release_rx_chain(struct gelic_card *card)
> do {
> if (descr->skb) {
> dma_unmap_single(ctodev(card),
> - be32_to_cpu(descr->buf_addr),
> + be32_to_cpu(descr->hw_regs.payload.dev_addr),
> descr->skb->len,
> DMA_FROM_DEVICE);
> - descr->buf_addr = 0;
> + descr->hw_regs.payload.dev_addr = 0;
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> gelic_descr_set_status(descr,
> @@ -493,19 +494,19 @@ static void gelic_descr_release_tx(struct gelic_card *card,
> {
> struct sk_buff *skb = descr->skb;
>
> - BUG_ON(!(be32_to_cpu(descr->data_status) & GELIC_DESCR_TX_TAIL));
> + BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) & GELIC_DESCR_TX_TAIL));
>
> - dma_unmap_single(ctodev(card), be32_to_cpu(descr->buf_addr), skb->len,
> + dma_unmap_single(ctodev(card), be32_to_cpu(descr->hw_regs.payload.dev_addr), skb->len,
> DMA_TO_DEVICE);
> dev_kfree_skb_any(skb);
>
> - descr->buf_addr = 0;
> - descr->buf_size = 0;
> - descr->next_descr_addr = 0;
> - descr->result_size = 0;
> - descr->valid_size = 0;
> - descr->data_status = 0;
> - descr->data_error = 0;
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> + descr->hw_regs.next_descr_addr = 0;
> + descr->hw_regs.result_size = 0;
> + descr->hw_regs.valid_size = 0;
> + descr->hw_regs.data_status = 0;
> + descr->hw_regs.data_error = 0;
> descr->skb = NULL;
>
> /* set descr status */
> @@ -698,7 +699,7 @@ static void gelic_descr_set_tx_cmdstat(struct gelic_descr *descr,
> struct sk_buff *skb)
> {
> if (skb->ip_summed != CHECKSUM_PARTIAL)
> - descr->dmac_cmd_status =
> + descr->hw_regs.dmac_cmd_status =
> cpu_to_be32(GELIC_DESCR_DMA_CMD_NO_CHKSUM |
> GELIC_DESCR_TX_DMA_FRAME_TAIL);
> else {
> @@ -706,19 +707,19 @@ static void gelic_descr_set_tx_cmdstat(struct gelic_descr *descr,
> * if yes: tcp? udp? */
> if (skb->protocol == htons(ETH_P_IP)) {
> if (ip_hdr(skb)->protocol == IPPROTO_TCP)
> - descr->dmac_cmd_status =
> + descr->hw_regs.dmac_cmd_status =
> cpu_to_be32(GELIC_DESCR_DMA_CMD_TCP_CHKSUM |
> GELIC_DESCR_TX_DMA_FRAME_TAIL);
>
> else if (ip_hdr(skb)->protocol == IPPROTO_UDP)
> - descr->dmac_cmd_status =
> + descr->hw_regs.dmac_cmd_status =
> cpu_to_be32(GELIC_DESCR_DMA_CMD_UDP_CHKSUM |
> GELIC_DESCR_TX_DMA_FRAME_TAIL);
> else /*
> * the stack should checksum non-tcp and non-udp
> * packets on his own: NETIF_F_IP_CSUM
> */
> - descr->dmac_cmd_status =
> + descr->hw_regs.dmac_cmd_status =
> cpu_to_be32(GELIC_DESCR_DMA_CMD_NO_CHKSUM |
> GELIC_DESCR_TX_DMA_FRAME_TAIL);
> }
> @@ -763,7 +764,7 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> struct gelic_descr *descr,
> struct sk_buff *skb)
> {
> - dma_addr_t buf;
> + dma_addr_t cpu_addr;
>
> if (card->vlan_required) {
> struct sk_buff *skb_tmp;
> @@ -777,20 +778,20 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> skb = skb_tmp;
> }
>
> - buf = dma_map_single(ctodev(card), skb->data, skb->len, DMA_TO_DEVICE);
> + cpu_addr = dma_map_single(ctodev(card), skb->data, skb->len, DMA_TO_DEVICE);
>
> - if (!buf) {
> + if (!cpu_addr) {
> dev_err(ctodev(card),
> "dma map 2 failed (%p, %i). Dropping packet\n",
> skb->data, skb->len);
> return -ENOMEM;
> }
>
> - descr->buf_addr = cpu_to_be32(buf);
> - descr->buf_size = cpu_to_be32(skb->len);
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
> + descr->hw_regs.payload.size = cpu_to_be32(skb->len);
> descr->skb = skb;
> - descr->data_status = 0;
> - descr->next_descr_addr = 0; /* terminate hw descr */
> + descr->hw_regs.data_status = 0;
> + descr->hw_regs.next_descr_addr = 0; /* terminate hw descr */
> gelic_descr_set_tx_cmdstat(descr, skb);
>
> /* bump free descriptor pointer */
> @@ -815,7 +816,7 @@ static int gelic_card_kick_txdma(struct gelic_card *card,
> if (gelic_descr_get_status(descr) == GELIC_DESCR_DMA_CARDOWNED) {
> card->tx_dma_progress = 1;
> status = lv1_net_start_tx_dma(bus_id(card), dev_id(card),
> - descr->bus_addr, 0);
> + descr->link.cpu_addr, 0);
> if (status) {
> card->tx_dma_progress = 0;
> dev_info(ctodev(card), "lv1_net_start_txdma failed," \
> @@ -868,7 +869,7 @@ netdev_tx_t gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
> * link this prepared descriptor to previous one
> * to achieve high performance
> */
> - descr->prev->next_descr_addr = cpu_to_be32(descr->bus_addr);
> + descr->prev->hw_regs.next_descr_addr = cpu_to_be32(descr->link.cpu_addr);
> /*
> * as hardware descriptor is modified in the above lines,
> * ensure that the hardware sees it
> @@ -881,12 +882,12 @@ netdev_tx_t gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
> */
> netdev->stats.tx_dropped++;
> /* don't trigger BUG_ON() in gelic_descr_release_tx */
> - descr->data_status = cpu_to_be32(GELIC_DESCR_TX_TAIL);
> + descr->hw_regs.data_status = cpu_to_be32(GELIC_DESCR_TX_TAIL);
> gelic_descr_release_tx(card, descr);
> /* reset head */
> card->tx_chain.head = descr;
> /* reset hw termination */
> - descr->prev->next_descr_addr = 0;
> + descr->prev->hw_regs.next_descr_addr = 0;
> dev_info(ctodev(card), "%s: kick failure\n", __func__);
> }
>
> @@ -911,21 +912,21 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
> struct sk_buff *skb = descr->skb;
> u32 data_status, data_error;
>
> - data_status = be32_to_cpu(descr->data_status);
> - data_error = be32_to_cpu(descr->data_error);
> + data_status = be32_to_cpu(descr->hw_regs.data_status);
> + data_error = be32_to_cpu(descr->hw_regs.data_error);
> /* unmap skb buffer */
> - dma_unmap_single(ctodev(card), be32_to_cpu(descr->buf_addr),
> + dma_unmap_single(ctodev(card), be32_to_cpu(descr->hw_regs.payload.dev_addr),
> GELIC_NET_MAX_MTU,
> DMA_FROM_DEVICE);
>
> - skb_put(skb, be32_to_cpu(descr->valid_size)?
> - be32_to_cpu(descr->valid_size) :
> - be32_to_cpu(descr->result_size));
> - if (!descr->valid_size)
> + skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size)?
> + be32_to_cpu(descr->hw_regs.valid_size) :
> + be32_to_cpu(descr->hw_regs.result_size));
> + if (!descr->hw_regs.valid_size)
> dev_info(ctodev(card), "buffer full %x %x %x\n",
> - be32_to_cpu(descr->result_size),
> - be32_to_cpu(descr->buf_size),
> - be32_to_cpu(descr->dmac_cmd_status));
> + be32_to_cpu(descr->hw_regs.result_size),
> + be32_to_cpu(descr->hw_regs.payload.size),
> + be32_to_cpu(descr->hw_regs.dmac_cmd_status));
>
> descr->skb = NULL;
> /*
> @@ -1036,14 +1037,14 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
>
> /* is the current descriptor terminated with next_descr == NULL? */
> dmac_chain_ended =
> - be32_to_cpu(descr->dmac_cmd_status) &
> + be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> GELIC_DESCR_RX_DMA_CHAIN_END;
> /*
> * So that always DMAC can see the end
> * of the descriptor chain to avoid
> * from unwanted DMAC overrun.
> */
> - descr->next_descr_addr = 0;
> + descr->hw_regs.next_descr_addr = 0;
>
> /* change the descriptor state: */
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> @@ -1060,7 +1061,7 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> /*
> * Set this descriptor the end of the chain.
> */
> - descr->prev->next_descr_addr = cpu_to_be32(descr->bus_addr);
> + descr->prev->hw_regs.next_descr_addr = cpu_to_be32(descr->link.cpu_addr);
>
> /*
> * If dmac chain was met, DMAC stopped.
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.h b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
> index 68f324ed4eaf..569f691021d9 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.h
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.h
> @@ -220,29 +220,35 @@ enum gelic_lv1_phy {
> GELIC_LV1_PHY_ETHERNET_0 = 0x0000000000000002L,
> };
>
> -/* size of hardware part of gelic descriptor */
> -#define GELIC_DESCR_SIZE (32)
> -
> enum gelic_port_type {
> GELIC_PORT_ETHERNET_0 = 0,
> GELIC_PORT_WIRELESS = 1,
> GELIC_PORT_MAX
> };
>
> -struct gelic_descr {
> - /* as defined by the hardware */
> - __be32 buf_addr;
> - __be32 buf_size;
> +/* as defined by the hardware */
> +struct gelic_hw_regs {
> + struct {
> + __be32 dev_addr;
> + __be32 size;
> + } __packed payload;
> __be32 next_descr_addr;
> __be32 dmac_cmd_status;
> __be32 result_size;
> __be32 valid_size; /* all zeroes for tx */
> __be32 data_status;
> __be32 data_error; /* all zeroes for tx */
> +} __packed;
>
> - /* used in the driver */
> +struct gelic_chain_link {
> + dma_addr_t cpu_addr;
> + unsigned int size;
> +};
> +
> +struct gelic_descr {
> + struct gelic_hw_regs hw_regs;
> + struct gelic_chain_link link;
> struct sk_buff *skb;
> - dma_addr_t bus_addr;
> struct gelic_descr *next;
> struct gelic_descr *prev;
> } __attribute__((aligned(32)));
>
^ permalink raw reply
* Re: [PATCH v4 02/10] net/ps3_gelic: Use local dev variable
From: Christophe Leroy @ 2021-08-05 5:05 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <26f56a1c8332d227156cd33586b176a329570117.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> In an effort to make the PS3 gelic driver easier to maintain, add a
> local variable dev to those routines that use the device structure that
> makes the use the device structure more consistent.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
Running checkpatch script provides the following feedback:
WARNING:BRACES: braces {} are not necessary for single statement blocks
#31: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:56:
+ if (status) {
+ dev_err(dev, "%s:%d failed: %d\n", __func__, __LINE__, status);
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#70: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:140:
+ if (status) {
+ dev_err(dev, "lv1_net_stop_tx_dma failed, status=%d\n", status);
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#111: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:191:
+ if (status) {
+ dev_err(dev, "lv1_net_stop_rx_dma failed, %d\n", status);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#170: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:301:
+ dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
+ DMA_BIDIRECTIONAL);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#190: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:335:
+ dma_map_single(dev, descr, descr->link.size,
+ DMA_BIDIRECTIONAL);
WARNING:BRACES: braces {} are not necessary for single statement blocks
#213: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:387:
+ if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) {
+ dev_err(dev, "%s:%d: ERROR status\n", __func__, __LINE__);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#226: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:413:
+ descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(dev,
descr->skb->data,
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#281: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:570:
+ dev_info_ratelimited(dev,
+ "%s:%d: forcing end of tx descriptor with status %x\n",
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#414: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:843:
+ dev_info(dev, "%s:%d: lv1_net_start_txdma failed: %d\n",
+ __func__, __LINE__, status);
WARNING:VSPRINTF_SPECIFIER_PX: Using vsprintf specifier '%px' potentially exposes the kernel memory
layout, if you don't really need the address please consider using '%p'.
#480: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1006:
+ dev_dbg(dev, "%s:%d: dormant descr? %px\n", __func__, __LINE__,
+ descr);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#491: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1024:
+ dev_info(dev, "%s:%d: unknown packet vid=%x\n",
+ __func__, __LINE__, vid);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#502: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1034:
+ dev_info(dev, "%s:%d: dropping RX descriptor with state %x\n",
+ __func__, __LINE__, status);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#687: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1535:
+ dev_info(dev, "%s:%d: %s MAC addr %pxM\n", __func__, __LINE__,
+ netdev->name, netdev->dev_addr);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#797: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1710:
+ dev_info(dev, "%s:%d: gelic_net_alloc_card failed.\n", __func__,
+ __LINE__);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#824: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1737:
+ result = ps3_sb_event_receive_port_setup(sb_dev, PS3_BINDING_CPU_ANY,
&card->irq);
WARNING:VSPRINTF_SPECIFIER_PX: Using vsprintf specifier '%px' potentially exposes the kernel memory
layout, if you don't really need the address please consider using '%p'.
#854: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1773:
+ dev_dbg(dev, "%s:%d: descr rx %px, tx %px, size %#lx, num %#x\n",
+ __func__, __LINE__, card->rx_top, card->tx_top,
+ sizeof(struct gelic_descr), GELIC_NET_RX_DESCRIPTORS);
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 244665a969da ("net/ps3_gelic: Use local dev variable") has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
Christophe
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 340 +++++++++++--------
> 1 file changed, 191 insertions(+), 149 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index cb45571573d7..ba008a98928a 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -48,13 +48,15 @@ MODULE_LICENSE("GPL");
> /* set irq_mask */
> int gelic_card_set_irq_mask(struct gelic_card *card, u64 mask)
> {
> + struct device *dev = ctodev(card);
> int status;
>
> status = lv1_net_set_interrupt_mask(bus_id(card), dev_id(card),
> mask, 0);
> - if (status)
> - dev_info(ctodev(card),
> - "%s failed %d\n", __func__, status);
> + if (status) {
> + dev_err(dev, "%s:%d failed: %d\n", __func__, __LINE__, status);
> + }
> +
> return status;
> }
>
> @@ -103,6 +105,7 @@ gelic_descr_get_status(struct gelic_descr *descr)
>
> static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
> {
> + struct device *dev = ctodev(card);
> int status;
> u64 v1, v2;
>
> @@ -110,8 +113,8 @@ static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
> GELIC_LV1_SET_NEGOTIATION_MODE,
> GELIC_LV1_PHY_ETHERNET_0, mode, 0, &v1, &v2);
> if (status) {
> - pr_info("%s: failed setting negotiation mode %d\n", __func__,
> - status);
> + dev_err(dev, "%s:%d: Failed setting negotiation mode: %d\n",
> + __func__, __LINE__, status);
> return -EBUSY;
> }
>
> @@ -128,13 +131,15 @@ static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
> */
> static void gelic_card_disable_txdmac(struct gelic_card *card)
> {
> + struct device *dev = ctodev(card);
> int status;
>
> /* this hvc blocks until the DMA in progress really stopped */
> status = lv1_net_stop_tx_dma(bus_id(card), dev_id(card));
> - if (status)
> - dev_err(ctodev(card),
> - "lv1_net_stop_tx_dma failed, status=%d\n", status);
> +
> + if (status) {
> + dev_err(dev, "lv1_net_stop_tx_dma failed, status=%d\n", status);
> + }
> }
>
> /**
> @@ -146,6 +151,7 @@ static void gelic_card_disable_txdmac(struct gelic_card *card)
> */
> static void gelic_card_enable_rxdmac(struct gelic_card *card)
> {
> + struct device *dev = ctodev(card);
> int status;
>
> #ifdef DEBUG
> @@ -161,9 +167,10 @@ static void gelic_card_enable_rxdmac(struct gelic_card *card)
> #endif
> status = lv1_net_start_rx_dma(bus_id(card), dev_id(card),
> card->rx_chain.head->link.cpu_addr, 0);
> - if (status)
> - dev_info(ctodev(card),
> - "lv1_net_start_rx_dma failed, status=%d\n", status);
> + if (status) {
> + dev_err(dev, "lv1_net_start_rx_dma failed, status=%d\n",
> + status);
> + }
> }
>
> /**
> @@ -175,13 +182,15 @@ static void gelic_card_enable_rxdmac(struct gelic_card *card)
> */
> static void gelic_card_disable_rxdmac(struct gelic_card *card)
> {
> + struct device *dev = ctodev(card);
> int status;
>
> /* this hvc blocks until the DMA in progress really stopped */
> status = lv1_net_stop_rx_dma(bus_id(card), dev_id(card));
> - if (status)
> - dev_err(ctodev(card),
> - "lv1_net_stop_rx_dma failed, %d\n", status);
> +
> + if (status) {
> + dev_err(dev, "lv1_net_stop_rx_dma failed, %d\n", status);
> + }
> }
>
> /**
> @@ -235,10 +244,11 @@ static void gelic_card_reset_chain(struct gelic_card *card,
>
> void gelic_card_up(struct gelic_card *card)
> {
> - pr_debug("%s: called\n", __func__);
> + struct device *dev = ctodev(card);
> +
> mutex_lock(&card->updown_lock);
> if (atomic_inc_return(&card->users) == 1) {
> - pr_debug("%s: real do\n", __func__);
> + dev_dbg(dev, "%s:%d: Starting...\n", __func__, __LINE__);
> /* enable irq */
> gelic_card_set_irq_mask(card, card->irq_mask);
> /* start rx */
> @@ -247,16 +257,16 @@ void gelic_card_up(struct gelic_card *card)
> napi_enable(&card->napi);
> }
> mutex_unlock(&card->updown_lock);
> - pr_debug("%s: done\n", __func__);
> }
>
> void gelic_card_down(struct gelic_card *card)
> {
> + struct device *dev = ctodev(card);
> u64 mask;
> - pr_debug("%s: called\n", __func__);
> +
> mutex_lock(&card->updown_lock);
> if (atomic_dec_if_positive(&card->users) == 0) {
> - pr_debug("%s: real do\n", __func__);
> + dev_dbg(dev, "%s:%d: Stopping...\n", __func__, __LINE__);
> napi_disable(&card->napi);
> /*
> * Disable irq. Wireless interrupts will
> @@ -273,7 +283,6 @@ void gelic_card_down(struct gelic_card *card)
> gelic_card_disable_txdmac(card);
> }
> mutex_unlock(&card->updown_lock);
> - pr_debug("%s: done\n", __func__);
> }
>
> /**
> @@ -284,11 +293,12 @@ void gelic_card_down(struct gelic_card *card)
> static void gelic_card_free_chain(struct gelic_card *card,
> struct gelic_descr *descr_in)
> {
> + struct device *dev = ctodev(card);
> struct gelic_descr *descr;
>
> for (descr = descr_in; descr && descr->link.cpu_addr; descr = descr->next) {
> - dma_unmap_single(ctodev(card), descr->link.cpu_addr,
> - descr->link.size, DMA_BIDIRECTIONAL);
> + dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
> + DMA_BIDIRECTIONAL);
> descr->link.cpu_addr = 0;
> }
> }
> @@ -311,6 +321,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
> {
> int i;
> struct gelic_descr *descr;
> + struct device *dev = ctodev(card);
>
> descr = start_descr;
> memset(descr, 0, sizeof(*descr) * no);
> @@ -320,9 +331,8 @@ static int gelic_card_init_chain(struct gelic_card *card,
> descr->link.size = sizeof(struct gelic_hw_regs);
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> descr->link.cpu_addr =
> - dma_map_single(ctodev(card), descr,
> - descr->link.size,
> - DMA_BIDIRECTIONAL);
> + dma_map_single(dev, descr, descr->link.size,
> + DMA_BIDIRECTIONAL);
>
> if (!descr->link.cpu_addr)
> goto iommu_error;
> @@ -351,7 +361,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
> iommu_error:
> for (i--, descr--; 0 <= i; i--, descr--)
> if (descr->link.cpu_addr)
> - dma_unmap_single(ctodev(card), descr->link.cpu_addr,
> + dma_unmap_single(dev, descr->link.cpu_addr,
> descr->link.size,
> DMA_BIDIRECTIONAL);
> return -ENOMEM;
> @@ -370,11 +380,14 @@ static int gelic_card_init_chain(struct gelic_card *card,
> static int gelic_descr_prepare_rx(struct gelic_card *card,
> struct gelic_descr *descr)
> {
> + struct device *dev = ctodev(card);
> int offset;
> unsigned int bufsize;
>
> - if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE)
> - dev_info(ctodev(card), "%s: ERROR status\n", __func__);
> + if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) {
> + dev_err(dev, "%s:%d: ERROR status\n", __func__, __LINE__);
> + }
> +
> /* we need to round up the buffer size to a multiple of 128 */
> bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
>
> @@ -396,14 +409,14 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
> if (offset)
> skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
> /* io-mmu-map the skb */
> - descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(ctodev(card),
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(dev,
> descr->skb->data,
> GELIC_NET_MAX_MTU,
> DMA_FROM_DEVICE));
> if (!descr->hw_regs.payload.dev_addr) {
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> - dev_info(ctodev(card),
> + dev_info(dev,
> "%s:Could not iommu-map rx buffer\n", __func__);
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> return -ENOMEM;
> @@ -421,10 +434,11 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
> static void gelic_card_release_rx_chain(struct gelic_card *card)
> {
> struct gelic_descr *descr = card->rx_chain.head;
> + struct device *dev = ctodev(card);
>
> do {
> if (descr->skb) {
> - dma_unmap_single(ctodev(card),
> + dma_unmap_single(dev,
> be32_to_cpu(descr->hw_regs.payload.dev_addr),
> descr->skb->len,
> DMA_FROM_DEVICE);
> @@ -493,10 +507,11 @@ static void gelic_descr_release_tx(struct gelic_card *card,
> struct gelic_descr *descr)
> {
> struct sk_buff *skb = descr->skb;
> + struct device *dev = ctodev(card);
>
> BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) & GELIC_DESCR_TX_TAIL));
>
> - dma_unmap_single(ctodev(card), be32_to_cpu(descr->hw_regs.payload.dev_addr), skb->len,
> + dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr), skb->len,
> DMA_TO_DEVICE);
> dev_kfree_skb_any(skb);
>
> @@ -538,6 +553,7 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> {
> struct gelic_descr_chain *tx_chain;
> enum gelic_descr_dma_status status;
> + struct device *dev = ctodev(card);
> struct net_device *netdev;
> int release = 0;
>
> @@ -550,11 +566,9 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> case GELIC_DESCR_DMA_RESPONSE_ERROR:
> case GELIC_DESCR_DMA_PROTECTION_ERROR:
> case GELIC_DESCR_DMA_FORCE_END:
> - if (printk_ratelimit())
> - dev_info(ctodev(card),
> - "%s: forcing end of tx descriptor " \
> - "with status %x\n",
> - __func__, status);
> + dev_info_ratelimited(dev,
> + "%s:%d: forcing end of tx descriptor with status %x\n",
> + __func__, __LINE__, status);
> netdev->stats.tx_dropped++;
> break;
>
> @@ -592,6 +606,7 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> void gelic_net_set_multi(struct net_device *netdev)
> {
> struct gelic_card *card = netdev_card(netdev);
> + struct device *dev = ctodev(card);
> struct netdev_hw_addr *ha;
> unsigned int i;
> uint8_t *p;
> @@ -601,27 +616,31 @@ void gelic_net_set_multi(struct net_device *netdev)
> /* clear all multicast address */
> status = lv1_net_remove_multicast_address(bus_id(card), dev_id(card),
> 0, 1);
> - if (status)
> - dev_err(ctodev(card),
> - "lv1_net_remove_multicast_address failed %d\n",
> - status);
> + if (status) {
> + dev_err(dev,
> + "%s:%d: lv1_net_remove_multicast_address failed %d\n",
> + __func__, __LINE__, status);
> + }
> +
> /* set broadcast address */
> status = lv1_net_add_multicast_address(bus_id(card), dev_id(card),
> GELIC_NET_BROADCAST_ADDR, 0);
> - if (status)
> - dev_err(ctodev(card),
> - "lv1_net_add_multicast_address failed, %d\n",
> - status);
> + if (status) {
> + dev_err(dev,
> + "%s:%d: lv1_net_add_multicast_address failed, %d\n",
> + __func__, __LINE__, status);
> + }
>
> if ((netdev->flags & IFF_ALLMULTI) ||
> (netdev_mc_count(netdev) > GELIC_NET_MC_COUNT_MAX)) {
> status = lv1_net_add_multicast_address(bus_id(card),
> dev_id(card),
> 0, 1);
> - if (status)
> - dev_err(ctodev(card),
> - "lv1_net_add_multicast_address failed, %d\n",
> - status);
> + if (status) {
> + dev_err(dev,
> + "%s:%d: lv1_net_add_multicast_address failed, %d\n",
> + __func__, __LINE__, status);
> + }
> return;
> }
>
> @@ -636,10 +655,11 @@ void gelic_net_set_multi(struct net_device *netdev)
> status = lv1_net_add_multicast_address(bus_id(card),
> dev_id(card),
> addr, 0);
> - if (status)
> - dev_err(ctodev(card),
> - "lv1_net_add_multicast_address failed, %d\n",
> - status);
> + if (status) {
> + dev_err(dev,
> + "%s:%d: lv1_net_add_multicast_address failed, %d\n",
> + __func__, __LINE__, status);
> + }
> }
> }
>
> @@ -651,17 +671,17 @@ void gelic_net_set_multi(struct net_device *netdev)
> */
> int gelic_net_stop(struct net_device *netdev)
> {
> - struct gelic_card *card;
> + struct gelic_card *card = netdev_card(netdev);
> + struct device *dev = ctodev(card);
>
> - pr_debug("%s: start\n", __func__);
> + dev_dbg(dev, "%s:%d: >\n", __func__, __LINE__);
>
> netif_stop_queue(netdev);
> netif_carrier_off(netdev);
>
> - card = netdev_card(netdev);
> gelic_card_down(card);
>
> - pr_debug("%s: done\n", __func__);
> + dev_dbg(dev, "%s:%d: <\n", __func__, __LINE__);
> return 0;
> }
>
> @@ -764,6 +784,7 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> struct gelic_descr *descr,
> struct sk_buff *skb)
> {
> + struct device *dev = ctodev(card);
> dma_addr_t cpu_addr;
>
> if (card->vlan_required) {
> @@ -778,12 +799,10 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> skb = skb_tmp;
> }
>
> - cpu_addr = dma_map_single(ctodev(card), skb->data, skb->len, DMA_TO_DEVICE);
> + cpu_addr = dma_map_single(dev, skb->data, skb->len, DMA_TO_DEVICE);
>
> if (!cpu_addr) {
> - dev_err(ctodev(card),
> - "dma map 2 failed (%p, %i). Dropping packet\n",
> - skb->data, skb->len);
> + dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
> return -ENOMEM;
> }
>
> @@ -808,6 +827,7 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> static int gelic_card_kick_txdma(struct gelic_card *card,
> struct gelic_descr *descr)
> {
> + struct device *dev = ctodev(card);
> int status = 0;
>
> if (card->tx_dma_progress)
> @@ -819,8 +839,8 @@ static int gelic_card_kick_txdma(struct gelic_card *card,
> descr->link.cpu_addr, 0);
> if (status) {
> card->tx_dma_progress = 0;
> - dev_info(ctodev(card), "lv1_net_start_txdma failed," \
> - "status=%d\n", status);
> + dev_info(dev, "%s:%d: lv1_net_start_txdma failed: %d\n",
> + __func__, __LINE__, status);
> }
> }
> return status;
> @@ -836,6 +856,7 @@ static int gelic_card_kick_txdma(struct gelic_card *card,
> netdev_tx_t gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
> {
> struct gelic_card *card = netdev_card(netdev);
> + struct device *dev = ctodev(card);
> struct gelic_descr *descr;
> int result;
> unsigned long flags;
> @@ -888,7 +909,7 @@ netdev_tx_t gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
> card->tx_chain.head = descr;
> /* reset hw termination */
> descr->prev->hw_regs.next_descr_addr = 0;
> - dev_info(ctodev(card), "%s: kick failure\n", __func__);
> + dev_info(dev, "%s:%d: kick failure\n", __func__, __LINE__);
> }
>
> spin_unlock_irqrestore(&card->tx_lock, flags);
> @@ -909,24 +930,28 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
> struct net_device *netdev)
>
> {
> + struct device *dev = ctodev(card);
> struct sk_buff *skb = descr->skb;
> u32 data_status, data_error;
>
> data_status = be32_to_cpu(descr->hw_regs.data_status);
> data_error = be32_to_cpu(descr->hw_regs.data_error);
> /* unmap skb buffer */
> - dma_unmap_single(ctodev(card), be32_to_cpu(descr->hw_regs.payload.dev_addr),
> + dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> GELIC_NET_MAX_MTU,
> DMA_FROM_DEVICE);
>
> skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size)?
> be32_to_cpu(descr->hw_regs.valid_size) :
> be32_to_cpu(descr->hw_regs.result_size));
> - if (!descr->hw_regs.valid_size)
> - dev_info(ctodev(card), "buffer full %x %x %x\n",
> +
> + if (!descr->hw_regs.valid_size) {
> + dev_err(dev, "%s:%d: buffer full %x %x %x\n", __func__,
> + __LINE__,
> be32_to_cpu(descr->hw_regs.result_size),
> be32_to_cpu(descr->hw_regs.payload.size),
> be32_to_cpu(descr->hw_regs.dmac_cmd_status));
> + }
>
> descr->skb = NULL;
> /*
> @@ -968,6 +993,7 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> enum gelic_descr_dma_status status;
> struct gelic_descr_chain *chain = &card->rx_chain;
> struct gelic_descr *descr = chain->head;
> + struct device *dev = ctodev(card);
> struct net_device *netdev = NULL;
> int dmac_chain_ended;
>
> @@ -977,7 +1003,8 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> return 0;
>
> if (status == GELIC_DESCR_DMA_NOT_IN_USE) {
> - dev_dbg(ctodev(card), "dormant descr? %p\n", descr);
> + dev_dbg(dev, "%s:%d: dormant descr? %px\n", __func__, __LINE__,
> + descr);
> return 0;
> }
>
> @@ -993,7 +1020,8 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> }
> }
> if (GELIC_PORT_MAX <= i) {
> - pr_info("%s: unknown packet vid=%x\n", __func__, vid);
> + dev_info(dev, "%s:%d: unknown packet vid=%x\n",
> + __func__, __LINE__, vid);
> goto refill;
> }
> } else
> @@ -1002,8 +1030,8 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> if ((status == GELIC_DESCR_DMA_RESPONSE_ERROR) ||
> (status == GELIC_DESCR_DMA_PROTECTION_ERROR) ||
> (status == GELIC_DESCR_DMA_FORCE_END)) {
> - dev_info(ctodev(card), "dropping RX descriptor with state %x\n",
> - status);
> + dev_info(dev, "%s:%d: dropping RX descriptor with state %x\n",
> + __func__, __LINE__, status);
> netdev->stats.rx_dropped++;
> goto refill;
> }
> @@ -1018,7 +1046,7 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> * Anyway this frame was longer than the MTU,
> * just drop it.
> */
> - dev_info(ctodev(card), "overlength frame\n");
> + dev_info(dev, "%s:%d: overlength frame\n", __func__, __LINE__);
> goto refill;
> }
> /*
> @@ -1026,8 +1054,8 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> * be treated as error.
> */
> if (status != GELIC_DESCR_DMA_FRAME_END) {
> - dev_dbg(ctodev(card), "RX descriptor with state %x\n",
> - status);
> + dev_dbg(dev, "%s:%d: RX descriptor with state %x\n", __func__,
> + __LINE__, status);
> goto refill;
> }
>
> @@ -1174,14 +1202,11 @@ int gelic_net_open(struct net_device *netdev)
> {
> struct gelic_card *card = netdev_card(netdev);
>
> - dev_dbg(ctodev(card), " -> %s %p\n", __func__, netdev);
> -
> gelic_card_up(card);
>
> netif_start_queue(netdev);
> gelic_card_get_ether_port_status(card, 1);
>
> - dev_dbg(ctodev(card), " <- %s\n", __func__);
> return 0;
> }
>
> @@ -1196,6 +1221,7 @@ static int gelic_ether_get_link_ksettings(struct net_device *netdev,
> struct ethtool_link_ksettings *cmd)
> {
> struct gelic_card *card = netdev_card(netdev);
> + struct device *dev = ctodev(card);
> u32 supported, advertising;
>
> gelic_card_get_ether_port_status(card, 0);
> @@ -1216,7 +1242,7 @@ static int gelic_ether_get_link_ksettings(struct net_device *netdev,
> cmd->base.speed = SPEED_1000;
> break;
> default:
> - pr_info("%s: speed unknown\n", __func__);
> + dev_dbg(dev, "%s:%d: speed unknown\n", __func__, __LINE__);
> cmd->base.speed = SPEED_10;
> break;
> }
> @@ -1247,6 +1273,7 @@ gelic_ether_set_link_ksettings(struct net_device *netdev,
> const struct ethtool_link_ksettings *cmd)
> {
> struct gelic_card *card = netdev_card(netdev);
> + struct device *dev = ctodev(card);
> u64 mode;
> int ret;
>
> @@ -1269,7 +1296,9 @@ gelic_ether_set_link_ksettings(struct net_device *netdev,
> if (cmd->base.duplex == DUPLEX_FULL) {
> mode |= GELIC_LV1_ETHER_FULL_DUPLEX;
> } else if (cmd->base.speed == SPEED_1000) {
> - pr_info("1000 half duplex is not supported.\n");
> + dev_dbg(dev,
> + "%s:%d: 1000 half duplex is not supported.\n",
> + __func__, __LINE__);
> return -EINVAL;
> }
> }
> @@ -1296,8 +1325,9 @@ static void gelic_net_get_wol(struct net_device *netdev,
> static int gelic_net_set_wol(struct net_device *netdev,
> struct ethtool_wolinfo *wol)
> {
> + struct gelic_card *card = netdev_card(netdev);
> + struct device *dev = ctodev(card);
> int status;
> - struct gelic_card *card;
> u64 v1, v2;
>
> if (ps3_compare_firmware_version(2, 2, 0) < 0 ||
> @@ -1307,7 +1337,6 @@ static int gelic_net_set_wol(struct net_device *netdev,
> if (wol->wolopts & ~WAKE_MAGIC)
> return -EINVAL;
>
> - card = netdev_card(netdev);
> if (wol->wolopts & WAKE_MAGIC) {
> status = lv1_net_control(bus_id(card), dev_id(card),
> GELIC_LV1_SET_WOL,
> @@ -1315,8 +1344,8 @@ static int gelic_net_set_wol(struct net_device *netdev,
> 0, GELIC_LV1_WOL_MP_ENABLE,
> &v1, &v2);
> if (status) {
> - pr_info("%s: enabling WOL failed %d\n", __func__,
> - status);
> + dev_dbg(dev, "%s:%d: Enabling WOL failed: %d\n",
> + __func__, __LINE__, status);
> status = -EIO;
> goto done;
> }
> @@ -1328,8 +1357,8 @@ static int gelic_net_set_wol(struct net_device *netdev,
> if (!status)
> ps3_sys_manager_set_wol(1);
> else {
> - pr_info("%s: enabling WOL filter failed %d\n",
> - __func__, status);
> + dev_dbg(dev, "%s:%d: Enabling WOL filter failed: %d\n",
> + __func__, __LINE__, status);
> status = -EIO;
> }
> } else {
> @@ -1339,8 +1368,8 @@ static int gelic_net_set_wol(struct net_device *netdev,
> 0, GELIC_LV1_WOL_MP_DISABLE,
> &v1, &v2);
> if (status) {
> - pr_info("%s: disabling WOL failed %d\n", __func__,
> - status);
> + dev_dbg(dev, "%s:%d: Disabling WOL failed: %d\n",
> + __func__, __LINE__, status);
> status = -EIO;
> goto done;
> }
> @@ -1352,8 +1381,8 @@ static int gelic_net_set_wol(struct net_device *netdev,
> if (!status)
> ps3_sys_manager_set_wol(0);
> else {
> - pr_info("%s: removing WOL filter failed %d\n",
> - __func__, status);
> + dev_dbg(dev, "%s:%d: Removing WOL filter failed: %d\n",
> + __func__, __LINE__, status);
> status = -EIO;
> }
> }
> @@ -1382,8 +1411,9 @@ static void gelic_net_tx_timeout_task(struct work_struct *work)
> struct gelic_card *card =
> container_of(work, struct gelic_card, tx_timeout_task);
> struct net_device *netdev = card->netdev[GELIC_PORT_ETHERNET_0];
> + struct device *dev = ctodev(card);
>
> - dev_info(ctodev(card), "%s:Timed out. Restarting...\n", __func__);
> + dev_info(dev, "%s:%d: Timed out. Restarting...\n", __func__, __LINE__);
>
> if (!(netdev->flags & IFF_UP))
> goto out;
> @@ -1459,6 +1489,7 @@ static void gelic_ether_setup_netdev_ops(struct net_device *netdev,
> **/
> int gelic_net_setup_netdev(struct net_device *netdev, struct gelic_card *card)
> {
> + struct device *dev = ctodev(card);
> int status;
> u64 v1, v2;
>
> @@ -1473,9 +1504,8 @@ int gelic_net_setup_netdev(struct net_device *netdev, struct gelic_card *card)
> 0, 0, 0, &v1, &v2);
> v1 <<= 16;
> if (status || !is_valid_ether_addr((u8 *)&v1)) {
> - dev_info(ctodev(card),
> - "%s:lv1_net_control GET_MAC_ADDR failed %d\n",
> - __func__, status);
> + dev_dbg(dev, "%s:%d: lv1_net_control GET_MAC_ADDR failed: %d\n",
> + __func__, __LINE__, status);
> return -EINVAL;
> }
> memcpy(netdev->dev_addr, &v1, ETH_ALEN);
> @@ -1494,13 +1524,15 @@ int gelic_net_setup_netdev(struct net_device *netdev, struct gelic_card *card)
> netdev->max_mtu = GELIC_NET_MAX_MTU;
>
> status = register_netdev(netdev);
> +
> if (status) {
> - dev_err(ctodev(card), "%s:Couldn't register %s %d\n",
> - __func__, netdev->name, status);
> + dev_err(dev, "%s:%d: Couldn't register %s: %d\n", __func__,
> + __LINE__, netdev->name, status);
> return status;
> }
> - dev_info(ctodev(card), "%s: MAC addr %pM\n",
> - netdev->name, netdev->dev_addr);
> +
> + dev_info(dev, "%s:%d: %s MAC addr %pxM\n", __func__, __LINE__,
> + netdev->name, netdev->dev_addr);
>
> return 0;
> }
> @@ -1566,6 +1598,7 @@ static struct gelic_card *gelic_alloc_card_net(struct net_device **netdev)
>
> static void gelic_card_get_vlan_info(struct gelic_card *card)
> {
> + struct device *dev = ctodev(card);
> u64 v1, v2;
> int status;
> unsigned int i;
> @@ -1590,10 +1623,12 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> vlan_id_ix[i].tx,
> 0, 0, &v1, &v2);
> if (status || !v1) {
> - if (status != LV1_NO_ENTRY)
> - dev_dbg(ctodev(card),
> - "get vlan id for tx(%d) failed(%d)\n",
> - vlan_id_ix[i].tx, status);
> + if (status != LV1_NO_ENTRY) {
> + dev_dbg(dev,
> + "%s:%d: Get vlan id for tx(%d) failed: %d\n",
> + __func__, __LINE__, vlan_id_ix[i].tx,
> + status);
> + }
> card->vlan[i].tx = 0;
> card->vlan[i].rx = 0;
> continue;
> @@ -1606,18 +1641,20 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> vlan_id_ix[i].rx,
> 0, 0, &v1, &v2);
> if (status || !v1) {
> - if (status != LV1_NO_ENTRY)
> - dev_info(ctodev(card),
> - "get vlan id for rx(%d) failed(%d)\n",
> - vlan_id_ix[i].rx, status);
> + if (status != LV1_NO_ENTRY) {
> + dev_dbg(dev,
> + "%s:%d: Get vlan id for rx(%d) failed: %d\n",
> + __func__, __LINE__, vlan_id_ix[i].rx,
> + status);
> + }
> card->vlan[i].tx = 0;
> card->vlan[i].rx = 0;
> continue;
> }
> card->vlan[i].rx = (u16)v1;
>
> - dev_dbg(ctodev(card), "vlan_id[%d] tx=%02x rx=%02x\n",
> - i, card->vlan[i].tx, card->vlan[i].rx);
> + dev_dbg(dev, "%s:%d: vlan_id[%d] tx=%02x rx=%02x\n", __func__,
> + __LINE__, i, card->vlan[i].tx, card->vlan[i].rx);
> }
>
> if (card->vlan[GELIC_PORT_ETHERNET_0].tx) {
> @@ -1632,35 +1669,36 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> card->vlan[GELIC_PORT_WIRELESS].rx = 0;
> }
>
> - dev_info(ctodev(card), "internal vlan %s\n",
> - card->vlan_required? "enabled" : "disabled");
> + dev_dbg(dev, "%s:%d: internal vlan %s\n", __func__, __LINE__,
> + card->vlan_required ? "enabled" : "disabled");
> }
> /*
> * ps3_gelic_driver_probe - add a device to the control of this driver
> */
> -static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> +static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> {
> + struct device *dev = &sb_dev->core;
> struct gelic_card *card;
> struct net_device *netdev;
> int result;
>
> - pr_debug("%s: called\n", __func__);
> + dev_dbg(dev, "%s:%d: >\n", __func__, __LINE__);
>
> udbg_shutdown_ps3gelic();
>
> - result = ps3_open_hv_device(dev);
> + result = ps3_open_hv_device(sb_dev);
>
> if (result) {
> - dev_dbg(&dev->core, "%s:ps3_open_hv_device failed\n",
> - __func__);
> + dev_err(dev, "%s:%d: ps3_open_hv_device failed: %d\n",
> + __func__, __LINE__, result);
> goto fail_open;
> }
>
> - result = ps3_dma_region_create(dev->d_region);
> + result = ps3_dma_region_create(sb_dev->d_region);
>
> if (result) {
> - dev_dbg(&dev->core, "%s:ps3_dma_region_create failed(%d)\n",
> - __func__, result);
> + dev_err(dev, "%s:%d: ps3_dma_region_create failed: %d\n",
> + __func__, __LINE__, result);
> BUG_ON("check region type");
> goto fail_dma_region;
> }
> @@ -1668,13 +1706,13 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> /* alloc card/netdevice */
> card = gelic_alloc_card_net(&netdev);
> if (!card) {
> - dev_info(&dev->core, "%s:gelic_net_alloc_card failed\n",
> - __func__);
> + dev_info(dev, "%s:%d: gelic_net_alloc_card failed.\n", __func__,
> + __LINE__);
> result = -ENOMEM;
> goto fail_alloc_card;
> }
> - ps3_system_bus_set_drvdata(dev, card);
> - card->dev = dev;
> + ps3_system_bus_set_drvdata(sb_dev, card);
> + card->dev = sb_dev;
>
> /* get internal vlan info */
> gelic_card_get_vlan_info(card);
> @@ -1688,20 +1726,19 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> 0);
>
> if (result) {
> - dev_dbg(&dev->core,
> - "%s:set_interrupt_status_indicator failed: %s\n",
> - __func__, ps3_result(result));
> + dev_dbg(dev,
> + "%s:%d: set_interrupt_status_indicator failed: %s\n",
> + __func__, __LINE__, ps3_result(result));
> result = -EIO;
> goto fail_status_indicator;
> }
>
> - result = ps3_sb_event_receive_port_setup(dev, PS3_BINDING_CPU_ANY,
> + result = ps3_sb_event_receive_port_setup(sb_dev, PS3_BINDING_CPU_ANY,
> &card->irq);
>
> if (result) {
> - dev_info(ctodev(card),
> - "%s:gelic_net_open_device failed (%d)\n",
> - __func__, result);
> + dev_dbg(dev, "%s:%d: gelic_net_open_device failed: %d\n",
> + __func__, __LINE__, result);
> result = -EPERM;
> goto fail_alloc_irq;
> }
> @@ -1709,8 +1746,8 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> 0, netdev->name, card);
>
> if (result) {
> - dev_info(ctodev(card), "%s:request_irq failed (%d)\n",
> - __func__, result);
> + dev_dbg(dev, "%s:%d: request_irq failed: %d\n",
> + __func__, __LINE__, result);
> goto fail_request_irq;
> }
>
> @@ -1732,9 +1769,11 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> /* head of chain */
> card->tx_top = card->tx_chain.head;
> card->rx_top = card->rx_chain.head;
> - dev_dbg(ctodev(card), "descr rx %p, tx %p, size %#lx, num %#x\n",
> - card->rx_top, card->tx_top, sizeof(struct gelic_descr),
> - GELIC_NET_RX_DESCRIPTORS);
> +
> + dev_dbg(dev, "%s:%d: descr rx %px, tx %px, size %#lx, num %#x\n",
> + __func__, __LINE__, card->rx_top, card->tx_top,
> + sizeof(struct gelic_descr), GELIC_NET_RX_DESCRIPTORS);
> +
> /* allocate rx skbs */
> result = gelic_card_alloc_rx_skbs(card);
> if (result)
> @@ -1745,23 +1784,23 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
>
> /* setup net_device structure */
> netdev->irq = card->irq;
> - SET_NETDEV_DEV(netdev, &card->dev->core);
> + SET_NETDEV_DEV(netdev, dev);
> gelic_ether_setup_netdev_ops(netdev, &card->napi);
> result = gelic_net_setup_netdev(netdev, card);
> if (result) {
> - dev_dbg(&dev->core, "%s: setup_netdev failed %d\n",
> - __func__, result);
> + dev_err(dev, "%s:%d: setup_netdev failed: %d\n", __func__,
> + __LINE__, result);
> goto fail_setup_netdev;
> }
>
> #ifdef CONFIG_GELIC_WIRELESS
> result = gelic_wl_driver_probe(card);
> if (result) {
> - dev_dbg(&dev->core, "%s: WL init failed\n", __func__);
> + dev_dbg(dev, "%s:%d: WL init failed\n", __func__, __LINE__);
> goto fail_setup_netdev;
> }
> #endif
> - pr_debug("%s: done\n", __func__);
> + dev_dbg(dev, "%s:%d: < OK\n", __func__, __LINE__);
> return 0;
>
> fail_setup_netdev:
> @@ -1773,20 +1812,21 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> free_irq(card->irq, card);
> netdev->irq = 0;
> fail_request_irq:
> - ps3_sb_event_receive_port_destroy(dev, card->irq);
> + ps3_sb_event_receive_port_destroy(sb_dev, card->irq);
> fail_alloc_irq:
> lv1_net_set_interrupt_status_indicator(bus_id(card),
> bus_id(card),
> 0, 0);
> fail_status_indicator:
> - ps3_system_bus_set_drvdata(dev, NULL);
> + ps3_system_bus_set_drvdata(sb_dev, NULL);
> kfree(netdev_card(netdev)->unalign);
> free_netdev(netdev);
> fail_alloc_card:
> - ps3_dma_region_free(dev->d_region);
> + ps3_dma_region_free(sb_dev->d_region);
> fail_dma_region:
> - ps3_close_hv_device(dev);
> + ps3_close_hv_device(sb_dev);
> fail_open:
> + dev_dbg(dev, "%s:%d: < error\n", __func__, __LINE__);
> return result;
> }
>
> @@ -1794,11 +1834,13 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> * ps3_gelic_driver_remove - remove a device from the control of this driver
> */
>
> -static void ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
> +static void ps3_gelic_driver_remove(struct ps3_system_bus_device *sb_dev)
> {
> - struct gelic_card *card = ps3_system_bus_get_drvdata(dev);
> + struct gelic_card *card = ps3_system_bus_get_drvdata(sb_dev);
> + struct device *dev = &sb_dev->core;
> struct net_device *netdev0;
> - pr_debug("%s: called\n", __func__);
> +
> + dev_dbg(dev, "%s:%d: >\n", __func__, __LINE__);
>
> /* set auto-negotiation */
> gelic_card_set_link_mode(card, GELIC_LV1_ETHER_AUTO_NEG);
> @@ -1836,13 +1878,13 @@ static void ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
> kfree(netdev_card(netdev0)->unalign);
> free_netdev(netdev0);
>
> - ps3_system_bus_set_drvdata(dev, NULL);
> + ps3_system_bus_set_drvdata(sb_dev, NULL);
>
> - ps3_dma_region_free(dev->d_region);
> + ps3_dma_region_free(sb_dev->d_region);
>
> - ps3_close_hv_device(dev);
> + ps3_close_hv_device(sb_dev);
>
> - pr_debug("%s: done\n", __func__);
> + dev_dbg(dev, "%s:%d: <\n", __func__, __LINE__);
> }
>
> static struct ps3_system_bus_driver ps3_gelic_driver = {
>
^ permalink raw reply
* Re: [PATCH v4 03/10] net/ps3_gelic: Format cleanups
From: Christophe Leroy @ 2021-08-05 5:06 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <56efff53fcf563a1741904ea0f078d50c378b6cc.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> In an effort to make the PS3 gelic driver easier to maintain, cleanup the
> the driver source file formatting to be more consistent.
WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG()
or BUG_ON()
#268: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:508:
+ BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) &
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#274: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:512:
+ dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
+ skb->len, DMA_TO_DEVICE);
WARNING:BRACES: braces {} are not necessary for single statement blocks
#295: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:580:
+ if (!stop) {
goto out;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#306: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:589:
+ if (!stop && release) {
gelic_card_wake_queues(card);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#405: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:753:
+ pr_debug("%s:%d: hd=%d c=%ud\n", __func__, __LINE__,
+ skb_headroom(skb), c);
WARNING:BRACES: braces {} are not necessary for single statement blocks
#428: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:792:
+ if (!skb_tmp) {
return -ENOMEM;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#515: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1093:
+ if (dmac_chain_ended) {
gelic_card_enable_rxdmac(card);
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#526: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1114:
+ if (!gelic_card_decode_one_descr(card)) {
break;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#546: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1139:
+ if (!status) {
return IRQ_NONE;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#557: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1160:
+ if (status & GELIC_CARD_PORT_STATUS_CHANGED) {
gelic_card_get_ether_port_status(card, 1);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#620: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1273:
+static int gelic_ether_set_link_ksettings(struct net_device *netdev,
+ const struct ethtool_link_ksettings *cmd)
WARNING:BRACES: braces {} are not necessary for single statement blocks
#637: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1309:
+ if (ret) {
return ret;
+ }
WARNING:BRACES: braces {} are not necessary for any arm of this statement
#649: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1319:
+ if (ps3_compare_firmware_version(2, 2, 0) >= 0) {
[...]
- else
[...]
WARNING:BRACES: braces {} are not necessary for single statement blocks
#676: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1342:
+ if (wol->wolopts & ~WAKE_MAGIC) {
return -EINVAL;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#750: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1422:
+ if (!(netdev->flags & IFF_UP)) {
goto out;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#788: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1506:
+ if (GELIC_CARD_RX_CSUM_DEFAULT) {
netdev->features |= NETIF_F_RXCSUM;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#822: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1577:
+ if (!p) {
return NULL;
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#930: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1754:
+ result = request_irq(card->irq, gelic_card_interrupt, 0, netdev->name,
+ card);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#943: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1766:
+ result = gelic_card_init_chain(card, &card->tx_chain, card->descr,
+ GELIC_NET_TX_DESCRIPTORS);
WARNING:BRACES: braces {} are not necessary for single statement blocks
#948: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1768:
+ if (result) {
goto fail_alloc_tx;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#959: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1776:
+ if (result) {
goto fail_alloc_rx;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#975: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1789:
+ if (result) {
goto fail_alloc_skbs;
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#1008: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1831:
+ lv1_net_set_interrupt_status_indicator(bus_id(card), bus_id(card), 0,
+ 0);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#1050: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1883:
+ lv1_net_set_interrupt_status_indicator(bus_id(card), dev_id(card), 0,
+ 0);
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 518e332861e3 ("net/ps3_gelic: Format cleanups") has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 379 ++++++++++---------
> 1 file changed, 193 insertions(+), 186 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index ba008a98928a..ded467d81f36 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -44,8 +44,6 @@ MODULE_AUTHOR("SCE Inc.");
> MODULE_DESCRIPTION("Gelic Network driver");
> MODULE_LICENSE("GPL");
>
> -
> -/* set irq_mask */
> int gelic_card_set_irq_mask(struct gelic_card *card, u64 mask)
> {
> struct device *dev = ctodev(card);
> @@ -65,6 +63,7 @@ static void gelic_card_rx_irq_on(struct gelic_card *card)
> card->irq_mask |= GELIC_CARD_RXINT;
> gelic_card_set_irq_mask(card, card->irq_mask);
> }
> +
> static void gelic_card_rx_irq_off(struct gelic_card *card)
> {
> card->irq_mask &= ~GELIC_CARD_RXINT;
> @@ -72,15 +71,14 @@ static void gelic_card_rx_irq_off(struct gelic_card *card)
> }
>
> static void gelic_card_get_ether_port_status(struct gelic_card *card,
> - int inform)
> + int inform)
> {
> u64 v2;
> struct net_device *ether_netdev;
>
> lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_GET_ETH_PORT_STATUS,
> - GELIC_LV1_VLAN_TX_ETHERNET_0, 0, 0,
> - &card->ether_port_status, &v2);
> + GELIC_LV1_GET_ETH_PORT_STATUS, GELIC_LV1_VLAN_TX_ETHERNET_0, 0,
> + 0, &card->ether_port_status, &v2);
>
> if (inform) {
> ether_netdev = card->netdev[GELIC_PORT_ETHERNET_0];
> @@ -100,7 +98,8 @@ static void gelic_card_get_ether_port_status(struct gelic_card *card,
> static enum gelic_descr_dma_status
> gelic_descr_get_status(struct gelic_descr *descr)
> {
> - return be32_to_cpu(descr->hw_regs.dmac_cmd_status) & GELIC_DESCR_DMA_STAT_MASK;
> + return be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> + GELIC_DESCR_DMA_STAT_MASK;
> }
>
> static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
> @@ -110,8 +109,9 @@ static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
> u64 v1, v2;
>
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_SET_NEGOTIATION_MODE,
> - GELIC_LV1_PHY_ETHERNET_0, mode, 0, &v1, &v2);
> + GELIC_LV1_SET_NEGOTIATION_MODE, GELIC_LV1_PHY_ETHERNET_0, mode,
> + 0, &v1, &v2);
> +
> if (status) {
> dev_err(dev, "%s:%d: Failed setting negotiation mode: %d\n",
> __func__, __LINE__, status);
> @@ -138,7 +138,8 @@ static void gelic_card_disable_txdmac(struct gelic_card *card)
> status = lv1_net_stop_tx_dma(bus_id(card), dev_id(card));
>
> if (status) {
> - dev_err(dev, "lv1_net_stop_tx_dma failed, status=%d\n", status);
> + dev_err(dev, "%s:%d: lv1_net_stop_tx_dma failed: %d\n",
> + __func__, __LINE__, status);
> }
> }
>
> @@ -166,10 +167,11 @@ static void gelic_card_enable_rxdmac(struct gelic_card *card)
> }
> #endif
> status = lv1_net_start_rx_dma(bus_id(card), dev_id(card),
> - card->rx_chain.head->link.cpu_addr, 0);
> + card->rx_chain.head->link.cpu_addr, 0);
> +
> if (status) {
> - dev_err(dev, "lv1_net_start_rx_dma failed, status=%d\n",
> - status);
> + dev_err(dev, "%s:%d: lv1_net_start_rx_dma failed: %d\n",
> + __func__, __LINE__, status);
> }
> }
>
> @@ -189,7 +191,8 @@ static void gelic_card_disable_rxdmac(struct gelic_card *card)
> status = lv1_net_stop_rx_dma(bus_id(card), dev_id(card));
>
> if (status) {
> - dev_err(dev, "lv1_net_stop_rx_dma failed, %d\n", status);
> + dev_err(dev, "%s:%d: lv1_net_stop_rx_dma failed: %d\n",
> + __func__, __LINE__, status);
> }
> }
>
> @@ -202,11 +205,11 @@ static void gelic_card_disable_rxdmac(struct gelic_card *card)
> * in the status
> */
> static void gelic_descr_set_status(struct gelic_descr *descr,
> - enum gelic_descr_dma_status status)
> + enum gelic_descr_dma_status status)
> {
> descr->hw_regs.dmac_cmd_status = cpu_to_be32(status |
> - (be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> - ~GELIC_DESCR_DMA_STAT_MASK));
> + (be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> + ~GELIC_DESCR_DMA_STAT_MASK));
> /*
> * dma_cmd_status field is used to indicate whether the descriptor
> * is valid or not.
> @@ -226,14 +229,14 @@ static void gelic_descr_set_status(struct gelic_descr *descr,
> * and re-initialize the hardware chain for later use
> */
> static void gelic_card_reset_chain(struct gelic_card *card,
> - struct gelic_descr_chain *chain,
> - struct gelic_descr *start_descr)
> + struct gelic_descr_chain *chain, struct gelic_descr *start_descr)
> {
> struct gelic_descr *descr;
>
> for (descr = start_descr; start_descr != descr->next; descr++) {
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
> - descr->hw_regs.next_descr_addr = cpu_to_be32(descr->next->link.cpu_addr);
> + descr->hw_regs.next_descr_addr =
> + cpu_to_be32(descr->next->link.cpu_addr);
> }
>
> chain->head = start_descr;
> @@ -249,11 +252,8 @@ void gelic_card_up(struct gelic_card *card)
> mutex_lock(&card->updown_lock);
> if (atomic_inc_return(&card->users) == 1) {
> dev_dbg(dev, "%s:%d: Starting...\n", __func__, __LINE__);
> - /* enable irq */
> gelic_card_set_irq_mask(card, card->irq_mask);
> - /* start rx */
> gelic_card_enable_rxdmac(card);
> -
> napi_enable(&card->napi);
> }
> mutex_unlock(&card->updown_lock);
> @@ -269,17 +269,14 @@ void gelic_card_down(struct gelic_card *card)
> dev_dbg(dev, "%s:%d: Stopping...\n", __func__, __LINE__);
> napi_disable(&card->napi);
> /*
> - * Disable irq. Wireless interrupts will
> - * be disabled later if any
> + * Disable irq. Wireless interrupts will be disabled later.
> */
> mask = card->irq_mask & (GELIC_CARD_WLAN_EVENT_RECEIVED |
> - GELIC_CARD_WLAN_COMMAND_COMPLETED);
> + GELIC_CARD_WLAN_COMMAND_COMPLETED);
> gelic_card_set_irq_mask(card, mask);
> - /* stop rx */
> gelic_card_disable_rxdmac(card);
> gelic_card_reset_chain(card, &card->rx_chain,
> - card->descr + GELIC_NET_TX_DESCRIPTORS);
> - /* stop tx */
> + card->descr + GELIC_NET_TX_DESCRIPTORS);
> gelic_card_disable_txdmac(card);
> }
> mutex_unlock(&card->updown_lock);
> @@ -291,12 +288,13 @@ void gelic_card_down(struct gelic_card *card)
> * @descr_in: address of desc
> */
> static void gelic_card_free_chain(struct gelic_card *card,
> - struct gelic_descr *descr_in)
> + struct gelic_descr *descr_in)
> {
> struct device *dev = ctodev(card);
> struct gelic_descr *descr;
>
> - for (descr = descr_in; descr && descr->link.cpu_addr; descr = descr->next) {
> + for (descr = descr_in; descr && descr->link.cpu_addr;
> + descr = descr->next) {
> dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
> DMA_BIDIRECTIONAL);
> descr->link.cpu_addr = 0;
> @@ -316,8 +314,8 @@ static void gelic_card_free_chain(struct gelic_card *card,
> * returns 0 on success, <0 on failure
> */
> static int gelic_card_init_chain(struct gelic_card *card,
> - struct gelic_descr_chain *chain,
> - struct gelic_descr *start_descr, int no)
> + struct gelic_descr_chain *chain, struct gelic_descr *start_descr,
> + int no)
> {
> int i;
> struct gelic_descr *descr;
> @@ -326,7 +324,6 @@ static int gelic_card_init_chain(struct gelic_card *card,
> descr = start_descr;
> memset(descr, 0, sizeof(*descr) * no);
>
> - /* set up the hardware pointers in each descriptor */
> for (i = 0; i < no; i++, descr++) {
> descr->link.size = sizeof(struct gelic_hw_regs);
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> @@ -340,14 +337,14 @@ static int gelic_card_init_chain(struct gelic_card *card,
> descr->next = descr + 1;
> descr->prev = descr - 1;
> }
> - /* make them as ring */
> +
> (descr - 1)->next = start_descr;
> start_descr->prev = (descr - 1);
>
> - /* chain bus addr of hw descriptor */
> descr = start_descr;
> for (i = 0; i < no; i++, descr++) {
> - descr->hw_regs.next_descr_addr = cpu_to_be32(descr->next->link.cpu_addr);
> + descr->hw_regs.next_descr_addr =
> + cpu_to_be32(descr->next->link.cpu_addr);
> }
>
> chain->head = start_descr;
> @@ -378,7 +375,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
> * Activate the descriptor state-wise
> */
> static int gelic_descr_prepare_rx(struct gelic_card *card,
> - struct gelic_descr *descr)
> + struct gelic_descr *descr)
> {
> struct device *dev = ctodev(card);
> int offset;
> @@ -439,14 +436,13 @@ static void gelic_card_release_rx_chain(struct gelic_card *card)
> do {
> if (descr->skb) {
> dma_unmap_single(dev,
> - be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - descr->skb->len,
> - DMA_FROM_DEVICE);
> + be32_to_cpu(descr->hw_regs.payload.dev_addr),
> + descr->skb->len, DMA_FROM_DEVICE);
> descr->hw_regs.payload.dev_addr = 0;
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> gelic_descr_set_status(descr,
> - GELIC_DESCR_DMA_NOT_IN_USE);
> + GELIC_DESCR_DMA_NOT_IN_USE);
> }
> descr = descr->next;
> } while (descr != card->rx_chain.head);
> @@ -504,15 +500,16 @@ static int gelic_card_alloc_rx_skbs(struct gelic_card *card)
> * releases a used tx descriptor (unmapping, freeing of skb)
> */
> static void gelic_descr_release_tx(struct gelic_card *card,
> - struct gelic_descr *descr)
> + struct gelic_descr *descr)
> {
> struct sk_buff *skb = descr->skb;
> struct device *dev = ctodev(card);
>
> - BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) & GELIC_DESCR_TX_TAIL));
> + BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) &
> + GELIC_DESCR_TX_TAIL));
>
> - dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr), skb->len,
> - DMA_TO_DEVICE);
> + dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> + skb->len, DMA_TO_DEVICE);
> dev_kfree_skb_any(skb);
>
> descr->hw_regs.payload.dev_addr = 0;
> @@ -524,7 +521,6 @@ static void gelic_descr_release_tx(struct gelic_card *card,
> descr->hw_regs.data_error = 0;
> descr->skb = NULL;
>
> - /* set descr status */
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> }
>
> @@ -580,19 +576,19 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> }
> break;
>
> - case GELIC_DESCR_DMA_CARDOWNED:
> - /* pending tx request */
> default:
> - /* any other value (== GELIC_DESCR_DMA_NOT_IN_USE) */
> - if (!stop)
> + if (!stop) {
> goto out;
> + }
> }
> +
> gelic_descr_release_tx(card, tx_chain->tail);
> - release ++;
> + release++;
> }
> out:
> - if (!stop && release)
> + if (!stop && release) {
> gelic_card_wake_queues(card);
> + }
> }
>
> /**
> @@ -613,18 +609,18 @@ void gelic_net_set_multi(struct net_device *netdev)
> u64 addr;
> int status;
>
> - /* clear all multicast address */
> status = lv1_net_remove_multicast_address(bus_id(card), dev_id(card),
> - 0, 1);
> + 0, 1);
> +
> if (status) {
> dev_err(dev,
> "%s:%d: lv1_net_remove_multicast_address failed %d\n",
> __func__, __LINE__, status);
> }
>
> - /* set broadcast address */
> status = lv1_net_add_multicast_address(bus_id(card), dev_id(card),
> - GELIC_NET_BROADCAST_ADDR, 0);
> + GELIC_NET_BROADCAST_ADDR, 0);
> +
> if (status) {
> dev_err(dev,
> "%s:%d: lv1_net_add_multicast_address failed, %d\n",
> @@ -634,8 +630,8 @@ void gelic_net_set_multi(struct net_device *netdev)
> if ((netdev->flags & IFF_ALLMULTI) ||
> (netdev_mc_count(netdev) > GELIC_NET_MC_COUNT_MAX)) {
> status = lv1_net_add_multicast_address(bus_id(card),
> - dev_id(card),
> - 0, 1);
> + dev_id(card), 0, 1);
> +
> if (status) {
> dev_err(dev,
> "%s:%d: lv1_net_add_multicast_address failed, %d\n",
> @@ -644,7 +640,6 @@ void gelic_net_set_multi(struct net_device *netdev)
> return;
> }
>
> - /* set multicast addresses */
> netdev_for_each_mc_addr(ha, netdev) {
> addr = 0;
> p = ha->addr;
> @@ -653,8 +648,8 @@ void gelic_net_set_multi(struct net_device *netdev)
> addr |= *p++;
> }
> status = lv1_net_add_multicast_address(bus_id(card),
> - dev_id(card),
> - addr, 0);
> + dev_id(card), addr, 0);
> +
> if (status) {
> dev_err(dev,
> "%s:%d: lv1_net_add_multicast_address failed, %d\n",
> @@ -698,8 +693,8 @@ gelic_card_get_next_tx_descr(struct gelic_card *card)
> return NULL;
> /* see if the next descriptor is free */
> if (card->tx_chain.tail != card->tx_chain.head->next &&
> - gelic_descr_get_status(card->tx_chain.head) ==
> - GELIC_DESCR_DMA_NOT_IN_USE)
> + gelic_descr_get_status(card->tx_chain.head) ==
> + GELIC_DESCR_DMA_NOT_IN_USE)
> return card->tx_chain.head;
> else
> return NULL;
> @@ -716,12 +711,12 @@ gelic_card_get_next_tx_descr(struct gelic_card *card)
> * has executed before.
> */
> static void gelic_descr_set_tx_cmdstat(struct gelic_descr *descr,
> - struct sk_buff *skb)
> + struct sk_buff *skb)
> {
> if (skb->ip_summed != CHECKSUM_PARTIAL)
> descr->hw_regs.dmac_cmd_status =
> cpu_to_be32(GELIC_DESCR_DMA_CMD_NO_CHKSUM |
> - GELIC_DESCR_TX_DMA_FRAME_TAIL);
> + GELIC_DESCR_TX_DMA_FRAME_TAIL);
> else {
> /* is packet ip?
> * if yes: tcp? udp? */
> @@ -747,14 +742,15 @@ static void gelic_descr_set_tx_cmdstat(struct gelic_descr *descr,
> }
>
> static struct sk_buff *gelic_put_vlan_tag(struct sk_buff *skb,
> - unsigned short tag)
> + unsigned short tag)
> {
> struct vlan_ethhdr *veth;
> static unsigned int c;
>
> if (skb_headroom(skb) < VLAN_HLEN) {
> struct sk_buff *sk_tmp = skb;
> - pr_debug("%s: hd=%d c=%ud\n", __func__, skb_headroom(skb), c);
> + pr_debug("%s:%d: hd=%d c=%ud\n", __func__, __LINE__,
> + skb_headroom(skb), c);
> skb = skb_realloc_headroom(sk_tmp, VLAN_HLEN);
> if (!skb)
> return NULL;
> @@ -781,8 +777,7 @@ static struct sk_buff *gelic_put_vlan_tag(struct sk_buff *skb,
> *
> */
> static int gelic_descr_prepare_tx(struct gelic_card *card,
> - struct gelic_descr *descr,
> - struct sk_buff *skb)
> + struct gelic_descr *descr, struct sk_buff *skb)
> {
> struct device *dev = ctodev(card);
> dma_addr_t cpu_addr;
> @@ -792,10 +787,11 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> enum gelic_port_type type;
>
> type = netdev_port(skb->dev)->type;
> - skb_tmp = gelic_put_vlan_tag(skb,
> - card->vlan[type].tx);
> - if (!skb_tmp)
> + skb_tmp = gelic_put_vlan_tag(skb, card->vlan[type].tx);
> +
> + if (!skb_tmp) {
> return -ENOMEM;
> + }
> skb = skb_tmp;
> }
>
> @@ -890,7 +886,8 @@ netdev_tx_t gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
> * link this prepared descriptor to previous one
> * to achieve high performance
> */
> - descr->prev->hw_regs.next_descr_addr = cpu_to_be32(descr->link.cpu_addr);
> + descr->prev->hw_regs.next_descr_addr =
> + cpu_to_be32(descr->link.cpu_addr);
> /*
> * as hardware descriptor is modified in the above lines,
> * ensure that the hardware sees it
> @@ -926,9 +923,7 @@ netdev_tx_t gelic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
> * stack. The descriptor state is not changed.
> */
> static void gelic_net_pass_skb_up(struct gelic_descr *descr,
> - struct gelic_card *card,
> - struct net_device *netdev)
> -
> + struct gelic_card *card, struct net_device *netdev)
> {
> struct device *dev = ctodev(card);
> struct sk_buff *skb = descr->skb;
> @@ -938,19 +933,18 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
> data_error = be32_to_cpu(descr->hw_regs.data_error);
> /* unmap skb buffer */
> dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - GELIC_NET_MAX_MTU,
> - DMA_FROM_DEVICE);
> + GELIC_NET_MAX_MTU, DMA_FROM_DEVICE);
>
> - skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size)?
> + skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size) ?
> be32_to_cpu(descr->hw_regs.valid_size) :
> be32_to_cpu(descr->hw_regs.result_size));
>
> if (!descr->hw_regs.valid_size) {
> dev_err(dev, "%s:%d: buffer full %x %x %x\n", __func__,
> __LINE__,
> - be32_to_cpu(descr->hw_regs.result_size),
> - be32_to_cpu(descr->hw_regs.payload.size),
> - be32_to_cpu(descr->hw_regs.dmac_cmd_status));
> + be32_to_cpu(descr->hw_regs.result_size),
> + be32_to_cpu(descr->hw_regs.payload.size),
> + be32_to_cpu(descr->hw_regs.dmac_cmd_status));
> }
>
> descr->skb = NULL;
> @@ -1028,8 +1022,8 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> netdev = card->netdev[GELIC_PORT_ETHERNET_0];
>
> if ((status == GELIC_DESCR_DMA_RESPONSE_ERROR) ||
> - (status == GELIC_DESCR_DMA_PROTECTION_ERROR) ||
> - (status == GELIC_DESCR_DMA_FORCE_END)) {
> + (status == GELIC_DESCR_DMA_PROTECTION_ERROR) ||
> + (status == GELIC_DESCR_DMA_FORCE_END)) {
> dev_info(dev, "%s:%d: dropping RX descriptor with state %x\n",
> __func__, __LINE__, status);
> netdev->stats.rx_dropped++;
> @@ -1064,8 +1058,7 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> refill:
>
> /* is the current descriptor terminated with next_descr == NULL? */
> - dmac_chain_ended =
> - be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> + dmac_chain_ended = be32_to_cpu(descr->hw_regs.dmac_cmd_status) &
> GELIC_DESCR_RX_DMA_CHAIN_END;
> /*
> * So that always DMAC can see the end
> @@ -1089,15 +1082,17 @@ static int gelic_card_decode_one_descr(struct gelic_card *card)
> /*
> * Set this descriptor the end of the chain.
> */
> - descr->prev->hw_regs.next_descr_addr = cpu_to_be32(descr->link.cpu_addr);
> + descr->prev->hw_regs.next_descr_addr =
> + cpu_to_be32(descr->link.cpu_addr);
>
> /*
> * If dmac chain was met, DMAC stopped.
> * thus re-enable it
> */
>
> - if (dmac_chain_ended)
> + if (dmac_chain_ended) {
> gelic_card_enable_rxdmac(card);
> + }
>
> return 1;
> }
> @@ -1116,9 +1111,9 @@ static int gelic_net_poll(struct napi_struct *napi, int budget)
> int packets_done = 0;
>
> while (packets_done < budget) {
> - if (!gelic_card_decode_one_descr(card))
> + if (!gelic_card_decode_one_descr(card)) {
> break;
> -
> + }
> packets_done++;
> }
>
> @@ -1126,6 +1121,7 @@ static int gelic_net_poll(struct napi_struct *napi, int budget)
> napi_complete_done(napi, packets_done);
> gelic_card_rx_irq_on(card);
> }
> +
> return packets_done;
> }
>
> @@ -1140,8 +1136,9 @@ static irqreturn_t gelic_card_interrupt(int irq, void *ptr)
>
> status = card->irq_status;
>
> - if (!status)
> + if (!status) {
> return IRQ_NONE;
> + }
>
> status &= card->irq_mask;
>
> @@ -1160,13 +1157,15 @@ static irqreturn_t gelic_card_interrupt(int irq, void *ptr)
> }
>
> /* ether port status changed */
> - if (status & GELIC_CARD_PORT_STATUS_CHANGED)
> + if (status & GELIC_CARD_PORT_STATUS_CHANGED) {
> gelic_card_get_ether_port_status(card, 1);
> + }
>
> #ifdef CONFIG_GELIC_WIRELESS
> if (status & (GELIC_CARD_WLAN_EVENT_RECEIVED |
> - GELIC_CARD_WLAN_COMMAND_COMPLETED))
> + GELIC_CARD_WLAN_COMMAND_COMPLETED)) {
> gelic_wl_interrupt(card->netdev[GELIC_PORT_WIRELESS], status);
> + }
> #endif
>
> return IRQ_HANDLED;
> @@ -1211,14 +1210,14 @@ int gelic_net_open(struct net_device *netdev)
> }
>
> void gelic_net_get_drvinfo(struct net_device *netdev,
> - struct ethtool_drvinfo *info)
> + struct ethtool_drvinfo *info)
> {
> strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
> strlcpy(info->version, DRV_VERSION, sizeof(info->version));
> }
>
> static int gelic_ether_get_link_ksettings(struct net_device *netdev,
> - struct ethtool_link_ksettings *cmd)
> + struct ethtool_link_ksettings *cmd)
> {
> struct gelic_card *card = netdev_card(netdev);
> struct device *dev = ctodev(card);
> @@ -1248,10 +1247,12 @@ static int gelic_ether_get_link_ksettings(struct net_device *netdev,
> }
>
> supported = SUPPORTED_TP | SUPPORTED_Autoneg |
> - SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full |
> - SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full |
> - SUPPORTED_1000baseT_Full;
> + SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full |
> + SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full |
> + SUPPORTED_1000baseT_Full;
> +
> advertising = supported;
> +
> if (card->link_mode & GELIC_LV1_ETHER_AUTO_NEG) {
> cmd->base.autoneg = AUTONEG_ENABLE;
> } else {
> @@ -1261,16 +1262,15 @@ static int gelic_ether_get_link_ksettings(struct net_device *netdev,
> cmd->base.port = PORT_TP;
>
> ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
> - supported);
> + supported);
> ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
> - advertising);
> + advertising);
>
> return 0;
> }
>
> -static int
> -gelic_ether_set_link_ksettings(struct net_device *netdev,
> - const struct ethtool_link_ksettings *cmd)
> +static int gelic_ether_set_link_ksettings(struct net_device *netdev,
> + const struct ethtool_link_ksettings *cmd)
> {
> struct gelic_card *card = netdev_card(netdev);
> struct device *dev = ctodev(card);
> @@ -1293,6 +1293,7 @@ gelic_ether_set_link_ksettings(struct net_device *netdev,
> default:
> return -EINVAL;
> }
> +
> if (cmd->base.duplex == DUPLEX_FULL) {
> mode |= GELIC_LV1_ETHER_FULL_DUPLEX;
> } else if (cmd->base.speed == SPEED_1000) {
> @@ -1305,25 +1306,28 @@ gelic_ether_set_link_ksettings(struct net_device *netdev,
>
> ret = gelic_card_set_link_mode(card, mode);
>
> - if (ret)
> + if (ret) {
> return ret;
> + }
>
> return 0;
> }
>
> static void gelic_net_get_wol(struct net_device *netdev,
> - struct ethtool_wolinfo *wol)
> + struct ethtool_wolinfo *wol)
> {
> - if (0 <= ps3_compare_firmware_version(2, 2, 0))
> + if (ps3_compare_firmware_version(2, 2, 0) >= 0) {
> wol->supported = WAKE_MAGIC;
> - else
> + } else {
> wol->supported = 0;
> + }
>
> wol->wolopts = ps3_sys_manager_get_wol() ? wol->supported : 0;
> memset(&wol->sopass, 0, sizeof(wol->sopass));
> }
> +
> static int gelic_net_set_wol(struct net_device *netdev,
> - struct ethtool_wolinfo *wol)
> + struct ethtool_wolinfo *wol)
> {
> struct gelic_card *card = netdev_card(netdev);
> struct device *dev = ctodev(card);
> @@ -1331,56 +1335,56 @@ static int gelic_net_set_wol(struct net_device *netdev,
> u64 v1, v2;
>
> if (ps3_compare_firmware_version(2, 2, 0) < 0 ||
> - !capable(CAP_NET_ADMIN))
> + !capable(CAP_NET_ADMIN)) {
> return -EPERM;
> + }
>
> - if (wol->wolopts & ~WAKE_MAGIC)
> + if (wol->wolopts & ~WAKE_MAGIC) {
> return -EINVAL;
> + }
>
> if (wol->wolopts & WAKE_MAGIC) {
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_SET_WOL,
> - GELIC_LV1_WOL_MAGIC_PACKET,
> - 0, GELIC_LV1_WOL_MP_ENABLE,
> - &v1, &v2);
> + GELIC_LV1_SET_WOL, GELIC_LV1_WOL_MAGIC_PACKET, 0,
> + GELIC_LV1_WOL_MP_ENABLE, &v1, &v2);
> +
> if (status) {
> dev_dbg(dev, "%s:%d: Enabling WOL failed: %d\n",
> __func__, __LINE__, status);
> status = -EIO;
> goto done;
> }
> +
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_SET_WOL,
> - GELIC_LV1_WOL_ADD_MATCH_ADDR,
> - 0, GELIC_LV1_WOL_MATCH_ALL,
> - &v1, &v2);
> - if (!status)
> + GELIC_LV1_SET_WOL, GELIC_LV1_WOL_ADD_MATCH_ADDR, 0,
> + GELIC_LV1_WOL_MATCH_ALL, &v1, &v2);
> +
> + if (!status) {
> ps3_sys_manager_set_wol(1);
> - else {
> + } else {
> dev_dbg(dev, "%s:%d: Enabling WOL filter failed: %d\n",
> __func__, __LINE__, status);
> status = -EIO;
> }
> } else {
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_SET_WOL,
> - GELIC_LV1_WOL_MAGIC_PACKET,
> - 0, GELIC_LV1_WOL_MP_DISABLE,
> - &v1, &v2);
> + GELIC_LV1_SET_WOL, GELIC_LV1_WOL_MAGIC_PACKET,
> + 0, GELIC_LV1_WOL_MP_DISABLE, &v1, &v2);
> +
> if (status) {
> dev_dbg(dev, "%s:%d: Disabling WOL failed: %d\n",
> __func__, __LINE__, status);
> status = -EIO;
> goto done;
> }
> +
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_SET_WOL,
> - GELIC_LV1_WOL_DELETE_MATCH_ADDR,
> - 0, GELIC_LV1_WOL_MATCH_ALL,
> - &v1, &v2);
> - if (!status)
> + GELIC_LV1_SET_WOL, GELIC_LV1_WOL_DELETE_MATCH_ADDR,
> + 0, GELIC_LV1_WOL_MATCH_ALL, &v1, &v2);
> +
> + if (!status) {
> ps3_sys_manager_set_wol(0);
> - else {
> + } else {
> dev_dbg(dev, "%s:%d: Removing WOL filter failed: %d\n",
> __func__, __LINE__, status);
> status = -EIO;
> @@ -1415,8 +1419,9 @@ static void gelic_net_tx_timeout_task(struct work_struct *work)
>
> dev_info(dev, "%s:%d: Timed out. Restarting...\n", __func__, __LINE__);
>
> - if (!(netdev->flags & IFF_UP))
> + if (!(netdev->flags & IFF_UP)) {
> goto out;
> + }
>
> netif_device_detach(netdev);
> gelic_net_stop(netdev);
> @@ -1441,10 +1446,12 @@ void gelic_net_tx_timeout(struct net_device *netdev, unsigned int txqueue)
>
> card = netdev_card(netdev);
> atomic_inc(&card->tx_timeout_task_counter);
> - if (netdev->flags & IFF_UP)
> +
> + if (netdev->flags & IFF_UP) {
> schedule_work(&card->tx_timeout_task);
> - else
> + } else {
> atomic_dec(&card->tx_timeout_task_counter);
> + }
> }
>
> static const struct net_device_ops gelic_netdevice_ops = {
> @@ -1468,7 +1475,7 @@ static const struct net_device_ops gelic_netdevice_ops = {
> * fills out function pointers in the net_device structure
> */
> static void gelic_ether_setup_netdev_ops(struct net_device *netdev,
> - struct napi_struct *napi)
> + struct napi_struct *napi)
> {
> netdev->watchdog_timeo = GELIC_NET_WATCHDOG_TIMEOUT;
> /* NAPI */
> @@ -1494,20 +1501,23 @@ int gelic_net_setup_netdev(struct net_device *netdev, struct gelic_card *card)
> u64 v1, v2;
>
> netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
> -
> netdev->features = NETIF_F_IP_CSUM;
> - if (GELIC_CARD_RX_CSUM_DEFAULT)
> +
> + if (GELIC_CARD_RX_CSUM_DEFAULT) {
> netdev->features |= NETIF_F_RXCSUM;
> + }
>
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_GET_MAC_ADDRESS,
> - 0, 0, 0, &v1, &v2);
> + GELIC_LV1_GET_MAC_ADDRESS, 0, 0, 0, &v1, &v2);
> +
> v1 <<= 16;
> +
> if (status || !is_valid_ether_addr((u8 *)&v1)) {
> dev_dbg(dev, "%s:%d: lv1_net_control GET_MAC_ADDR failed: %d\n",
> __func__, __LINE__, status);
> return -EINVAL;
> }
> +
> memcpy(netdev->dev_addr, &v1, ETH_ALEN);
>
> if (card->vlan_required) {
> @@ -1557,34 +1567,32 @@ static struct gelic_card *gelic_alloc_card_net(struct net_device **netdev)
> */
> BUILD_BUG_ON(offsetof(struct gelic_card, irq_status) % 8);
> BUILD_BUG_ON(offsetof(struct gelic_card, descr) % 32);
> - alloc_size =
> - sizeof(struct gelic_card) +
> + alloc_size = sizeof(struct gelic_card) +
> sizeof(struct gelic_descr) * GELIC_NET_RX_DESCRIPTORS +
> sizeof(struct gelic_descr) * GELIC_NET_TX_DESCRIPTORS +
> GELIC_ALIGN - 1;
>
> p = kzalloc(alloc_size, GFP_KERNEL);
> - if (!p)
> +
> + if (!p) {
> return NULL;
> + }
> +
> card = PTR_ALIGN(p, GELIC_ALIGN);
> card->unalign = p;
>
> - /*
> - * alloc netdev
> - */
> *netdev = alloc_etherdev(sizeof(struct gelic_port));
> +
> if (!*netdev) {
> kfree(card->unalign);
> return NULL;
> }
> port = netdev_priv(*netdev);
>
> - /* gelic_port */
> port->netdev = *netdev;
> port->card = card;
> port->type = GELIC_PORT_ETHERNET_0;
>
> - /* gelic_card */
> card->netdev[GELIC_PORT_ETHERNET_0] = *netdev;
>
> INIT_WORK(&card->tx_timeout_task, gelic_net_tx_timeout_task);
> @@ -1619,9 +1627,9 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> for (i = 0; i < ARRAY_SIZE(vlan_id_ix); i++) {
> /* tx tag */
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_GET_VLAN_ID,
> - vlan_id_ix[i].tx,
> - 0, 0, &v1, &v2);
> + GELIC_LV1_GET_VLAN_ID, vlan_id_ix[i].tx, 0, 0, &v1,
> + &v2);
> +
> if (status || !v1) {
> if (status != LV1_NO_ENTRY) {
> dev_dbg(dev,
> @@ -1637,9 +1645,9 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
>
> /* rx tag */
> status = lv1_net_control(bus_id(card), dev_id(card),
> - GELIC_LV1_GET_VLAN_ID,
> - vlan_id_ix[i].rx,
> - 0, 0, &v1, &v2);
> + GELIC_LV1_GET_VLAN_ID, vlan_id_ix[i].rx, 0, 0, &v1,
> + &v2);
> +
> if (status || !v1) {
> if (status != LV1_NO_ENTRY) {
> dev_dbg(dev,
> @@ -1651,6 +1659,7 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> card->vlan[i].rx = 0;
> continue;
> }
> +
> card->vlan[i].rx = (u16)v1;
>
> dev_dbg(dev, "%s:%d: vlan_id[%d] tx=%02x rx=%02x\n", __func__,
> @@ -1672,6 +1681,7 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> dev_dbg(dev, "%s:%d: internal vlan %s\n", __func__, __LINE__,
> card->vlan_required ? "enabled" : "disabled");
> }
> +
> /*
> * ps3_gelic_driver_probe - add a device to the control of this driver
> */
> @@ -1703,27 +1713,24 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> goto fail_dma_region;
> }
>
> - /* alloc card/netdevice */
> card = gelic_alloc_card_net(&netdev);
> +
> if (!card) {
> dev_info(dev, "%s:%d: gelic_net_alloc_card failed.\n", __func__,
> __LINE__);
> result = -ENOMEM;
> goto fail_alloc_card;
> }
> +
> ps3_system_bus_set_drvdata(sb_dev, card);
> card->dev = sb_dev;
>
> - /* get internal vlan info */
> gelic_card_get_vlan_info(card);
>
> card->link_mode = GELIC_LV1_ETHER_AUTO_NEG;
>
> - /* setup interrupt */
> result = lv1_net_set_interrupt_status_indicator(bus_id(card),
> - dev_id(card),
> - ps3_mm_phys_to_lpar(__pa(&card->irq_status)),
> - 0);
> + dev_id(card), ps3_mm_phys_to_lpar(__pa(&card->irq_status)), 0);
>
> if (result) {
> dev_dbg(dev,
> @@ -1742,8 +1749,9 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> result = -EPERM;
> goto fail_alloc_irq;
> }
> - result = request_irq(card->irq, gelic_card_interrupt,
> - 0, netdev->name, card);
> +
> + result = request_irq(card->irq, gelic_card_interrupt, 0, netdev->name,
> + card);
>
> if (result) {
> dev_dbg(dev, "%s:%d: request_irq failed: %d\n",
> @@ -1751,22 +1759,24 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> goto fail_request_irq;
> }
>
> - /* setup card structure */
> card->irq_mask = GELIC_CARD_RXINT | GELIC_CARD_TXINT |
> GELIC_CARD_PORT_STATUS_CHANGED;
>
> + result = gelic_card_init_chain(card, &card->tx_chain, card->descr,
> + GELIC_NET_TX_DESCRIPTORS);
>
> - result = gelic_card_init_chain(card, &card->tx_chain,
> - card->descr, GELIC_NET_TX_DESCRIPTORS);
> - if (result)
> + if (result) {
> goto fail_alloc_tx;
> + }
> +
> result = gelic_card_init_chain(card, &card->rx_chain,
> - card->descr + GELIC_NET_TX_DESCRIPTORS,
> - GELIC_NET_RX_DESCRIPTORS);
> - if (result)
> + card->descr + GELIC_NET_TX_DESCRIPTORS,
> + GELIC_NET_RX_DESCRIPTORS);
> +
> + if (result) {
> goto fail_alloc_rx;
> + }
>
> - /* head of chain */
> card->tx_top = card->tx_chain.head;
> card->rx_top = card->rx_chain.head;
>
> @@ -1774,19 +1784,21 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> __func__, __LINE__, card->rx_top, card->tx_top,
> sizeof(struct gelic_descr), GELIC_NET_RX_DESCRIPTORS);
>
> - /* allocate rx skbs */
> result = gelic_card_alloc_rx_skbs(card);
> - if (result)
> +
> + if (result) {
> goto fail_alloc_skbs;
> + }
>
> spin_lock_init(&card->tx_lock);
> card->tx_dma_progress = 0;
>
> - /* setup net_device structure */
> netdev->irq = card->irq;
> SET_NETDEV_DEV(netdev, dev);
> gelic_ether_setup_netdev_ops(netdev, &card->napi);
> +
> result = gelic_net_setup_netdev(netdev, card);
> +
> if (result) {
> dev_err(dev, "%s:%d: setup_netdev failed: %d\n", __func__,
> __LINE__, result);
> @@ -1795,6 +1807,7 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
>
> #ifdef CONFIG_GELIC_WIRELESS
> result = gelic_wl_driver_probe(card);
> +
> if (result) {
> dev_dbg(dev, "%s:%d: WL init failed\n", __func__, __LINE__);
> goto fail_setup_netdev;
> @@ -1814,9 +1827,8 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> fail_request_irq:
> ps3_sb_event_receive_port_destroy(sb_dev, card->irq);
> fail_alloc_irq:
> - lv1_net_set_interrupt_status_indicator(bus_id(card),
> - bus_id(card),
> - 0, 0);
> + lv1_net_set_interrupt_status_indicator(bus_id(card), bus_id(card), 0,
> + 0);
> fail_status_indicator:
> ps3_system_bus_set_drvdata(sb_dev, NULL);
> kfree(netdev_card(netdev)->unalign);
> @@ -1842,20 +1854,16 @@ static void ps3_gelic_driver_remove(struct ps3_system_bus_device *sb_dev)
>
> dev_dbg(dev, "%s:%d: >\n", __func__, __LINE__);
>
> - /* set auto-negotiation */
> gelic_card_set_link_mode(card, GELIC_LV1_ETHER_AUTO_NEG);
>
> #ifdef CONFIG_GELIC_WIRELESS
> gelic_wl_driver_remove(card);
> #endif
> - /* stop interrupt */
> gelic_card_set_irq_mask(card, 0);
>
> - /* turn off DMA, force end */
> gelic_card_disable_rxdmac(card);
> gelic_card_disable_txdmac(card);
>
> - /* release chains */
> gelic_card_release_tx_chain(card, 1);
> gelic_card_release_rx_chain(card);
>
> @@ -1863,16 +1871,16 @@ static void ps3_gelic_driver_remove(struct ps3_system_bus_device *sb_dev)
> gelic_card_free_chain(card, card->rx_top);
>
> netdev0 = card->netdev[GELIC_PORT_ETHERNET_0];
> - /* disconnect event port */
> +
> free_irq(card->irq, card);
> netdev0->irq = 0;
> ps3_sb_event_receive_port_destroy(card->dev, card->irq);
>
> wait_event(card->waitq,
> - atomic_read(&card->tx_timeout_task_counter) == 0);
> + atomic_read(&card->tx_timeout_task_counter) == 0);
>
> - lv1_net_set_interrupt_status_indicator(bus_id(card), dev_id(card),
> - 0 , 0);
> + lv1_net_set_interrupt_status_indicator(bus_id(card), dev_id(card), 0,
> + 0);
>
> unregister_netdev(netdev0);
> kfree(netdev_card(netdev0)->unalign);
> @@ -1896,14 +1904,14 @@ static struct ps3_system_bus_driver ps3_gelic_driver = {
> .core.owner = THIS_MODULE,
> };
>
> -static int __init ps3_gelic_driver_init (void)
> +static int __init ps3_gelic_driver_init(void)
> {
> return firmware_has_feature(FW_FEATURE_PS3_LV1)
> ? ps3_system_bus_driver_register(&ps3_gelic_driver)
> : -ENODEV;
> }
>
> -static void __exit ps3_gelic_driver_exit (void)
> +static void __exit ps3_gelic_driver_exit(void)
> {
> ps3_system_bus_driver_unregister(&ps3_gelic_driver);
> }
> @@ -1912,4 +1920,3 @@ module_init(ps3_gelic_driver_init);
> module_exit(ps3_gelic_driver_exit);
>
> MODULE_ALIAS(PS3_MODULE_ALIAS_GELIC);
> -
>
^ permalink raw reply
* Re: [PATCH v4 04/10] net/ps3_gelic: Add new macro BUG_ON_DEBUG
From: Christophe Leroy @ 2021-08-05 5:07 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <bc659850d4eec3b2358c1ccb0e00952ceaa6012f.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> Add a new preprocessor macro BUG_ON_DEBUG, that expands to BUG_ON when
> the preprocessor macro DEBUG is defined, or to WARN_ON when DEBUG is not
> defined. Also, replace all occurrences of BUG_ON with BUG_ON_DEBUG.
>
CHECK:MACRO_ARG_REUSE: Macro argument reuse '_cond' - possible side-effects?
#23: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:47:
+#define BUG_ON_DEBUG(_cond) do { \
+ if (__is_defined(DEBUG)) \
+ BUG_ON(_cond); \
+ else \
+ WARN_ON(_cond); \
+} while (0)
WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG()
or BUG_ON()
#25: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:49:
+ BUG_ON(_cond); \
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit e4fbd62abdcd ("net/ps3_gelic: Add new macro BUG_ON_DEBUG") has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index ded467d81f36..946e9bfa071b 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -44,6 +44,13 @@ MODULE_AUTHOR("SCE Inc.");
> MODULE_DESCRIPTION("Gelic Network driver");
> MODULE_LICENSE("GPL");
>
> +#define BUG_ON_DEBUG(_cond) do { \
> + if (__is_defined(DEBUG)) \
> + BUG_ON(_cond); \
> + else \
> + WARN_ON(_cond); \
> +} while (0)
> +
> int gelic_card_set_irq_mask(struct gelic_card *card, u64 mask)
> {
> struct device *dev = ctodev(card);
> @@ -505,7 +512,7 @@ static void gelic_descr_release_tx(struct gelic_card *card,
> struct sk_buff *skb = descr->skb;
> struct device *dev = ctodev(card);
>
> - BUG_ON(!(be32_to_cpu(descr->hw_regs.data_status) &
> + BUG_ON_DEBUG(!(be32_to_cpu(descr->hw_regs.data_status) &
> GELIC_DESCR_TX_TAIL));
>
> dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> @@ -1667,7 +1674,7 @@ static void gelic_card_get_vlan_info(struct gelic_card *card)
> }
>
> if (card->vlan[GELIC_PORT_ETHERNET_0].tx) {
> - BUG_ON(!card->vlan[GELIC_PORT_WIRELESS].tx);
> + BUG_ON_DEBUG(!card->vlan[GELIC_PORT_WIRELESS].tx);
> card->vlan_required = 1;
> } else
> card->vlan_required = 0;
> @@ -1709,7 +1716,7 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *sb_dev)
> if (result) {
> dev_err(dev, "%s:%d: ps3_dma_region_create failed: %d\n",
> __func__, __LINE__, result);
> - BUG_ON("check region type");
> + BUG_ON_DEBUG("check region type");
> goto fail_dma_region;
> }
>
>
^ permalink raw reply
* Re: [PATCH v4 05/10] net/ps3_gelic: Add vlan_id structure
From: Christophe Leroy @ 2021-08-05 5:07 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <1cdd7f718dde93dcaebf7ddd025869901aa30523.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> In an effort to make the PS3 gelic driver easier to maintain, add
> a definition for the vlan_id structure.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
Commit 4298d9fdc87f ("net/ps3_gelic: Add vlan_id structure") has no obvious style problems and is
ready for submission.
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 946e9bfa071b..54e50ad9e629 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -1614,13 +1614,14 @@ static struct gelic_card *gelic_alloc_card_net(struct net_device **netdev)
> static void gelic_card_get_vlan_info(struct gelic_card *card)
> {
> struct device *dev = ctodev(card);
> + unsigned int i;
> u64 v1, v2;
> int status;
> - unsigned int i;
> - struct {
> + struct vlan_id {
> int tx;
> int rx;
> - } vlan_id_ix[2] = {
> + };
> + struct vlan_id vlan_id_ix[2] = {
> [GELIC_PORT_ETHERNET_0] = {
> .tx = GELIC_LV1_VLAN_TX_ETHERNET_0,
> .rx = GELIC_LV1_VLAN_RX_ETHERNET_0
>
^ permalink raw reply
* Re: [PATCH v4 06/10] net/ps3_gelic: Cleanup debug code
From: Christophe Leroy @ 2021-08-05 5:08 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <8421aa2c148d840b11b7115208e5276017999c2a.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> In an effort to make the PS3 gelic driver easier to maintain, change the
> gelic_card_enable_rxdmac routine to use the optimizer to remove
> debug code.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
WARNING:VSPRINTF_SPECIFIER_PX: Using vsprintf specifier '%px' potentially exposes the kernel memory
layout, if you don't really need the address please consider using '%p'.
#38: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:171:
+ dev_err(dev, "%s:%d: head=%px\n", __func__, __LINE__,
+ card->rx_chain.head);
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 65f38d9720ac ("net/ps3_gelic: Cleanup debug code") has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 54e50ad9e629..85fc1915c8be 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -162,17 +162,16 @@ static void gelic_card_enable_rxdmac(struct gelic_card *card)
> struct device *dev = ctodev(card);
> int status;
>
> -#ifdef DEBUG
> - if (gelic_descr_get_status(card->rx_chain.head) !=
> - GELIC_DESCR_DMA_CARDOWNED) {
> - printk(KERN_ERR "%s: status=%x\n", __func__,
> - be32_to_cpu(card->rx_chain.head->dmac_cmd_status));
> - printk(KERN_ERR "%s: nextphy=%x\n", __func__,
> - be32_to_cpu(card->rx_chain.head->hw_regs.next_descr_addr));
> - printk(KERN_ERR "%s: head=%p\n", __func__,
> - card->rx_chain.head);
> + if (__is_defined(DEBUG) && (gelic_descr_get_status(card->rx_chain.head)
> + != GELIC_DESCR_DMA_CARDOWNED)) {
> + dev_err(dev, "%s:%d: status=%x\n", __func__, __LINE__,
> + be32_to_cpu(card->rx_chain.head->hw_regs.dmac_cmd_status));
> + dev_err(dev, "%s:%d: nextphy=%x\n", __func__, __LINE__,
> + be32_to_cpu(card->rx_chain.head->hw_regs.next_descr_addr));
> + dev_err(dev, "%s:%d: head=%px\n", __func__, __LINE__,
> + card->rx_chain.head);
> }
> -#endif
> +
> status = lv1_net_start_rx_dma(bus_id(card), dev_id(card),
> card->rx_chain.head->link.cpu_addr, 0);
>
>
^ permalink raw reply
* Re: [PATCH v4 07/10] net/ps3_gelic: Add new routine gelic_unmap_link
From: Christophe Leroy @ 2021-08-05 5:09 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <024b88e07095f00bc2eabfae2f526851600ee272.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> Put the common code for unmaping a link into its own routine,
> gelic_unmap_link, and add some debugging checks.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#31: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:300:
+ dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
+ DMA_BIDIRECTIONAL);
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit bcb1cb297705 ("net/ps3_gelic: Add new routine gelic_unmap_link") has style problems, please
review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 23 +++++++++++++++-----
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 85fc1915c8be..e55aa9fecfeb 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -288,6 +288,21 @@ void gelic_card_down(struct gelic_card *card)
> mutex_unlock(&card->updown_lock);
> }
>
> +static void gelic_unmap_link(struct device *dev, struct gelic_descr *descr)
> +{
> + BUG_ON_DEBUG(descr->hw_regs.payload.dev_addr);
> + BUG_ON_DEBUG(descr->hw_regs.payload.size);
> +
> + BUG_ON_DEBUG(!descr->link.cpu_addr);
> + BUG_ON_DEBUG(!descr->link.size);
> +
> + dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
> + DMA_BIDIRECTIONAL);
> +
> + descr->link.cpu_addr = 0;
> + descr->link.size = 0;
> +}
> +
> /**
> * gelic_card_free_chain - free descriptor chain
> * @card: card structure
> @@ -301,9 +316,7 @@ static void gelic_card_free_chain(struct gelic_card *card,
>
> for (descr = descr_in; descr && descr->link.cpu_addr;
> descr = descr->next) {
> - dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
> - DMA_BIDIRECTIONAL);
> - descr->link.cpu_addr = 0;
> + gelic_unmap_link(dev, descr);
> }
> }
>
> @@ -364,9 +377,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
> iommu_error:
> for (i--, descr--; 0 <= i; i--, descr--)
> if (descr->link.cpu_addr)
> - dma_unmap_single(dev, descr->link.cpu_addr,
> - descr->link.size,
> - DMA_BIDIRECTIONAL);
> + gelic_unmap_link(dev, descr);
> return -ENOMEM;
> }
>
>
^ permalink raw reply
* Re: [PATCH v4 08/10] net/ps3_gelic: Rename no to descr_count
From: Christophe Leroy @ 2021-08-05 5:09 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <07e42ec30037d514c1d63f33efe4642364d89802.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> In an effort to make the PS3 gelic driver easier to maintain, rename
> the gelic_card_init_chain parameter 'no' to 'descr_count'.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
CHECK:SPACING: spaces preferred around that '*' (ctx:WxV)
#40: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:344:
+ memset(descr, 0, sizeof(*descr) *descr_count);
^
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit fdbd3f08a0b1 ("net/ps3_gelic: Rename no to descr_count") has style problems, please review.
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index e55aa9fecfeb..60fcca5d20dd 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -325,7 +325,7 @@ static void gelic_card_free_chain(struct gelic_card *card,
> * @card: card structure
> * @chain: address of chain
> * @start_descr: address of descriptor array
> - * @no: number of descriptors
> + * @descr_count: number of descriptors
> *
> * we manage a circular list that mirrors the hardware structure,
> * except that the hardware uses bus addresses.
> @@ -334,16 +334,16 @@ static void gelic_card_free_chain(struct gelic_card *card,
> */
> static int gelic_card_init_chain(struct gelic_card *card,
> struct gelic_descr_chain *chain, struct gelic_descr *start_descr,
> - int no)
> + int descr_count)
> {
> int i;
> struct gelic_descr *descr;
> struct device *dev = ctodev(card);
>
> descr = start_descr;
> - memset(descr, 0, sizeof(*descr) * no);
> + memset(descr, 0, sizeof(*descr) *descr_count);
>
> - for (i = 0; i < no; i++, descr++) {
> + for (i = 0; i < descr_count; i++, descr++) {
> descr->link.size = sizeof(struct gelic_hw_regs);
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> descr->link.cpu_addr =
> @@ -361,7 +361,7 @@ static int gelic_card_init_chain(struct gelic_card *card,
> start_descr->prev = (descr - 1);
>
> descr = start_descr;
> - for (i = 0; i < no; i++, descr++) {
> + for (i = 0; i < descr_count; i++, descr++) {
> descr->hw_regs.next_descr_addr =
> cpu_to_be32(descr->next->link.cpu_addr);
> }
>
^ permalink raw reply
* Re: [PATCH v4 09/10] net/ps3_gelic: Add new routine gelic_work_to_card
From: Christophe Leroy @ 2021-08-05 5:10 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <5634f7c76a67345c9735e05b68228ea899a8bf9d.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> Add new helper routine gelic_work_to_card that converts a work_struct
> to a gelic_card.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
Commit 3ffdbef9f86f ("net/ps3_gelic: Add new routine gelic_work_to_card") has no obvious style
problems and is ready for submission.
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 60fcca5d20dd..42f4de9ad5fe 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -1420,6 +1420,11 @@ static const struct ethtool_ops gelic_ether_ethtool_ops = {
> .set_link_ksettings = gelic_ether_set_link_ksettings,
> };
>
> +static struct gelic_card *gelic_work_to_card(struct work_struct *work)
> +{
> + return container_of(work, struct gelic_card, tx_timeout_task);
> +}
> +
> /**
> * gelic_net_tx_timeout_task - task scheduled by the watchdog timeout
> * function (to be called not under interrupt status)
> @@ -1429,8 +1434,7 @@ static const struct ethtool_ops gelic_ether_ethtool_ops = {
> */
> static void gelic_net_tx_timeout_task(struct work_struct *work)
> {
> - struct gelic_card *card =
> - container_of(work, struct gelic_card, tx_timeout_task);
> + struct gelic_card *card = gelic_work_to_card(work);
> struct net_device *netdev = card->netdev[GELIC_PORT_ETHERNET_0];
> struct device *dev = ctodev(card);
>
>
^ permalink raw reply
* Re: [PATCH v4 10/10] net/ps3_gelic: Fix DMA mapping problems
From: Christophe Leroy @ 2021-08-05 5:10 UTC (permalink / raw)
To: Geoff Levand, David S. Miller, Jakub Kicinski; +Cc: netdev, linuxppc-dev
In-Reply-To: <7aa1d9b1b4ffadcbdc6f88e4f8d4a323da307595.1627068552.git.geoff@infradead.org>
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
> Fixes several DMA mapping problems with the PS3's gelic network driver:
>
> * Change from checking the return value of dma_map_single to using the
> dma_mapping_error routine.
> * Use the correct buffer length when mapping the RX skb.
> * Improved error checking and debug logging.
>
> Fixes runtime errors like these, and also other randomly occurring errors:
>
> IP-Config: Complete:
> DMA-API: ps3_gelic_driver sb_05: device driver failed to check map error
> WARNING: CPU: 0 PID: 0 at kernel/dma/debug.c:1027 .check_unmap+0x888/0x8dc
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#55: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:351:
+ descr->link.cpu_addr = dma_map_single(dev, descr,
+ descr->link.size, DMA_BIDIRECTIONAL);
WARNING:BRACES: braces {} are not necessary for single statement blocks
#62: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:358:
+ if (descr->link.cpu_addr) {
+ gelic_unmap_link(dev, descr);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#157: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:440:
+ cpu_addr = dma_map_single(dev, descr->skb->data,
+ descr->hw_regs.payload.size, DMA_FROM_DEVICE);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#262: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:612:
+ dev_info_ratelimited(dev,
+ "%s:%d: forcing end of tx descriptor with status %x\n",
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#323: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:846:
+ cpu_addr = dma_map_single(dev, skb->data, descr->hw_regs.payload.size,
+ DMA_TO_DEVICE);
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit cf6041cd6b17 ("net/ps3_gelic: Fix DMA mapping problems") has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 183 +++++++++++--------
> 1 file changed, 108 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 42f4de9ad5fe..11ddeacb1159 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -336,22 +336,31 @@ static int gelic_card_init_chain(struct gelic_card *card,
> struct gelic_descr_chain *chain, struct gelic_descr *start_descr,
> int descr_count)
> {
> - int i;
> - struct gelic_descr *descr;
> + struct gelic_descr *descr = start_descr;
> struct device *dev = ctodev(card);
> + unsigned int index;
>
> - descr = start_descr;
> - memset(descr, 0, sizeof(*descr) *descr_count);
> + memset(start_descr, 0, descr_count * sizeof(*start_descr));
>
> - for (i = 0; i < descr_count; i++, descr++) {
> - descr->link.size = sizeof(struct gelic_hw_regs);
> + for (index = 0, descr = start_descr; index < descr_count;
> + index++, descr++) {
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> - descr->link.cpu_addr =
> - dma_map_single(dev, descr, descr->link.size,
> - DMA_BIDIRECTIONAL);
>
> - if (!descr->link.cpu_addr)
> - goto iommu_error;
> + descr->link.size = sizeof(struct gelic_hw_regs);
> + descr->link.cpu_addr = dma_map_single(dev, descr,
> + descr->link.size, DMA_BIDIRECTIONAL);
> +
> + if (unlikely(dma_mapping_error(dev, descr->link.cpu_addr))) {
> + dev_err(dev, "%s:%d: dma_mapping_error\n", __func__,
> + __LINE__);
> +
> + for (index--, descr--; index > 0; index--, descr--) {
> + if (descr->link.cpu_addr) {
> + gelic_unmap_link(dev, descr);
> + }
> + }
> + return -ENOMEM;
> + }
>
> descr->next = descr + 1;
> descr->prev = descr - 1;
> @@ -360,8 +369,9 @@ static int gelic_card_init_chain(struct gelic_card *card,
> (descr - 1)->next = start_descr;
> start_descr->prev = (descr - 1);
>
> - descr = start_descr;
> - for (i = 0; i < descr_count; i++, descr++) {
> + /* chain bus addr of hw descriptor */
> + for (index = 0, descr = start_descr; index < descr_count;
> + index++, descr++) {
> descr->hw_regs.next_descr_addr =
> cpu_to_be32(descr->next->link.cpu_addr);
> }
> @@ -373,12 +383,6 @@ static int gelic_card_init_chain(struct gelic_card *card,
> (descr - 1)->hw_regs.next_descr_addr = 0;
>
> return 0;
> -
> -iommu_error:
> - for (i--, descr--; 0 <= i; i--, descr--)
> - if (descr->link.cpu_addr)
> - gelic_unmap_link(dev, descr);
> - return -ENOMEM;
> }
>
> /**
> @@ -395,49 +399,63 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
> struct gelic_descr *descr)
> {
> struct device *dev = ctodev(card);
> - int offset;
> - unsigned int bufsize;
> + struct aligned_buff {
> + unsigned int total_bytes;
> + unsigned int offset;
> + };
> + struct aligned_buff a_buf;
> + dma_addr_t cpu_addr;
>
> if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) {
> dev_err(dev, "%s:%d: ERROR status\n", __func__, __LINE__);
> }
>
> - /* we need to round up the buffer size to a multiple of 128 */
> - bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
> + a_buf.total_bytes = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN)
> + + GELIC_NET_RXBUF_ALIGN;
> +
> + descr->skb = dev_alloc_skb(a_buf.total_bytes);
>
> - /* and we need to have it 128 byte aligned, therefore we allocate a
> - * bit more */
> - descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
> if (!descr->skb) {
> - descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> return -ENOMEM;
> }
> - descr->hw_regs.payload.size = cpu_to_be32(bufsize);
> +
> + a_buf.offset = PTR_ALIGN(descr->skb->data, GELIC_NET_RXBUF_ALIGN)
> + - descr->skb->data;
> +
> + if (a_buf.offset) {
> + dev_dbg(dev, "%s:%d: offset=%u\n", __func__, __LINE__,
> + a_buf.offset);
> + skb_reserve(descr->skb, a_buf.offset);
> + }
> +
> descr->hw_regs.dmac_cmd_status = 0;
> descr->hw_regs.result_size = 0;
> descr->hw_regs.valid_size = 0;
> descr->hw_regs.data_error = 0;
>
> - offset = ((unsigned long)descr->skb->data) &
> - (GELIC_NET_RXBUF_ALIGN - 1);
> - if (offset)
> - skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
> - /* io-mmu-map the skb */
> - descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(dev,
> - descr->skb->data,
> - GELIC_NET_MAX_MTU,
> - DMA_FROM_DEVICE));
> - if (!descr->hw_regs.payload.dev_addr) {
> + descr->hw_regs.payload.size = a_buf.total_bytes - a_buf.offset;
> + cpu_addr = dma_map_single(dev, descr->skb->data,
> + descr->hw_regs.payload.size, DMA_FROM_DEVICE);
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
> +
> + if (unlikely(dma_mapping_error(dev, cpu_addr))) {
> + dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
> +
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> +
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> - dev_info(dev,
> - "%s:Could not iommu-map rx buffer\n", __func__);
> +
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> +
> return -ENOMEM;
> - } else {
> - gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
> - return 0;
> }
> +
> + gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
> + return 0;
> }
>
> /**
> @@ -454,13 +472,18 @@ static void gelic_card_release_rx_chain(struct gelic_card *card)
> if (descr->skb) {
> dma_unmap_single(dev,
> be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - descr->skb->len, DMA_FROM_DEVICE);
> - descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size, DMA_FROM_DEVICE);
> +
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> +
> gelic_descr_set_status(descr,
> GELIC_DESCR_DMA_NOT_IN_USE);
> }
> +
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> +
> descr = descr->next;
> } while (descr != card->rx_chain.head);
> }
> @@ -526,17 +549,19 @@ static void gelic_descr_release_tx(struct gelic_card *card,
> GELIC_DESCR_TX_TAIL));
>
> dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - skb->len, DMA_TO_DEVICE);
> - dev_kfree_skb_any(skb);
> + descr->hw_regs.payload.size, DMA_TO_DEVICE);
>
> descr->hw_regs.payload.dev_addr = 0;
> descr->hw_regs.payload.size = 0;
> +
> + dev_kfree_skb_any(skb);
> + descr->skb = NULL;
> +
> descr->hw_regs.next_descr_addr = 0;
> descr->hw_regs.result_size = 0;
> descr->hw_regs.valid_size = 0;
> descr->hw_regs.data_status = 0;
> descr->hw_regs.data_error = 0;
> - descr->skb = NULL;
>
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> }
> @@ -565,31 +590,34 @@ static void gelic_card_wake_queues(struct gelic_card *card)
> static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> {
> struct gelic_descr_chain *tx_chain;
> - enum gelic_descr_dma_status status;
> struct device *dev = ctodev(card);
> - struct net_device *netdev;
> - int release = 0;
> + int release;
> +
> + for (release = 0, tx_chain = &card->tx_chain;
> + tx_chain->head != tx_chain->tail && tx_chain->tail;
> + tx_chain->tail = tx_chain->tail->next) {
> + enum gelic_descr_dma_status status;
> + struct gelic_descr *descr;
> + struct net_device *netdev;
> +
> + descr = tx_chain->tail;
> + status = gelic_descr_get_status(descr);
> + netdev = descr->skb->dev;
>
> - for (tx_chain = &card->tx_chain;
> - tx_chain->head != tx_chain->tail && tx_chain->tail;
> - tx_chain->tail = tx_chain->tail->next) {
> - status = gelic_descr_get_status(tx_chain->tail);
> - netdev = tx_chain->tail->skb->dev;
> switch (status) {
> case GELIC_DESCR_DMA_RESPONSE_ERROR:
> case GELIC_DESCR_DMA_PROTECTION_ERROR:
> case GELIC_DESCR_DMA_FORCE_END:
> - dev_info_ratelimited(dev,
> - "%s:%d: forcing end of tx descriptor with status %x\n",
> - __func__, __LINE__, status);
> + dev_info_ratelimited(dev,
> + "%s:%d: forcing end of tx descriptor with status %x\n",
> + __func__, __LINE__, status);
> netdev->stats.tx_dropped++;
> break;
>
> case GELIC_DESCR_DMA_COMPLETE:
> - if (tx_chain->tail->skb) {
> + if (descr->skb) {
> netdev->stats.tx_packets++;
> - netdev->stats.tx_bytes +=
> - tx_chain->tail->skb->len;
> + netdev->stats.tx_bytes += descr->skb->len;
> }
> break;
>
> @@ -599,7 +627,7 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> }
> }
>
> - gelic_descr_release_tx(card, tx_chain->tail);
> + gelic_descr_release_tx(card, descr);
> release++;
> }
> out:
> @@ -703,19 +731,19 @@ int gelic_net_stop(struct net_device *netdev)
> *
> * returns the address of the next descriptor, or NULL if not available.
> */
> -static struct gelic_descr *
> -gelic_card_get_next_tx_descr(struct gelic_card *card)
> +static struct gelic_descr *gelic_card_get_next_tx_descr(struct gelic_card *card)
> {
> if (!card->tx_chain.head)
> return NULL;
> +
> /* see if the next descriptor is free */
> if (card->tx_chain.tail != card->tx_chain.head->next &&
> - gelic_descr_get_status(card->tx_chain.head) ==
> - GELIC_DESCR_DMA_NOT_IN_USE)
> + (gelic_descr_get_status(card->tx_chain.head) ==
> + GELIC_DESCR_DMA_NOT_IN_USE)) {
> return card->tx_chain.head;
> - else
> - return NULL;
> + }
>
> + return NULL;
> }
>
> /**
> @@ -809,18 +837,23 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
> if (!skb_tmp) {
> return -ENOMEM;
> }
> +
> skb = skb_tmp;
> }
>
> - cpu_addr = dma_map_single(dev, skb->data, skb->len, DMA_TO_DEVICE);
> + descr->hw_regs.payload.size = skb->len;
> + cpu_addr = dma_map_single(dev, skb->data, descr->hw_regs.payload.size,
> + DMA_TO_DEVICE);
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
>
> - if (!cpu_addr) {
> + if (unlikely(dma_mapping_error(dev, cpu_addr))) {
> dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
> +
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> return -ENOMEM;
> }
>
> - descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
> - descr->hw_regs.payload.size = cpu_to_be32(skb->len);
> descr->skb = skb;
> descr->hw_regs.data_status = 0;
> descr->hw_regs.next_descr_addr = 0; /* terminate hw descr */
> @@ -948,9 +981,9 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
>
> data_status = be32_to_cpu(descr->hw_regs.data_status);
> data_error = be32_to_cpu(descr->hw_regs.data_error);
> - /* unmap skb buffer */
> +
> dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - GELIC_NET_MAX_MTU, DMA_FROM_DEVICE);
> + descr->hw_regs.payload.size, DMA_FROM_DEVICE);
>
> skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size) ?
> be32_to_cpu(descr->hw_regs.valid_size) :
>
^ permalink raw reply
* Re: [PATCH] powerpc/kprobes: Fix kprobe Oops happens in booke
From: Michael Ellerman @ 2021-08-05 6:13 UTC (permalink / raw)
To: Pu Lehui, oleg, benh, paulus, naveen.n.rao, mhiramat,
christophe.leroy, peterz, npiggin, ruscur
Cc: zhangjinhao2, xukuohai, linuxppc-dev, linux-kernel, pulehui
In-Reply-To: <20210804143735.148547-1-pulehui@huawei.com>
Pu Lehui <pulehui@huawei.com> writes:
> When using kprobe on powerpc booke series processor, Oops happens
> as show bellow:
>
> [ 35.861352] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 35.861676] BE PAGE_SIZE=4K SMP NR_CPUS=24 QEMU e500
> [ 35.861905] Modules linked in:
> [ 35.862144] CPU: 0 PID: 76 Comm: sh Not tainted 5.14.0-rc3-00060-g7e96bf476270 #18
> [ 35.862610] NIP: c0b96470 LR: c00107b4 CTR: c0161c80
> [ 35.862805] REGS: c387fe70 TRAP: 0700 Not tainted (5.14.0-rc3-00060-g7e96bf476270)
> [ 35.863198] MSR: 00029002 <CE,EE,ME> CR: 24022824 XER: 20000000
> [ 35.863577]
> [ 35.863577] GPR00: c0015218 c387ff20 c313e300 c387ff50 00000004 40000002 40000000 0a1a2cce
> [ 35.863577] GPR08: 00000000 00000004 00000000 59764000 24022422 102490c2 00000000 00000000
> [ 35.863577] GPR16: 00000000 00000000 00000040 10240000 10240000 10240000 10240000 10220000
> [ 35.863577] GPR24: ffffffff 10240000 00000000 00000000 bfc655e8 00000800 c387ff50 00000000
> [ 35.865367] NIP [c0b96470] schedule+0x0/0x130
> [ 35.865606] LR [c00107b4] interrupt_exit_user_prepare_main+0xf4/0x100
> [ 35.865974] Call Trace:
> [ 35.866142] [c387ff20] [c0053224] irq_exit+0x114/0x120 (unreliable)
> [ 35.866472] [c387ff40] [c0015218] interrupt_return+0x14/0x13c
> [ 35.866728] --- interrupt: 900 at 0x100af3dc
> [ 35.866963] NIP: 100af3dc LR: 100de020 CTR: 00000000
> [ 35.867177] REGS: c387ff50 TRAP: 0900 Not tainted (5.14.0-rc3-00060-g7e96bf476270)
> [ 35.867488] MSR: 0002f902 <CE,EE,PR,FP,ME> CR: 20022422 XER: 20000000
> [ 35.867808]
> [ 35.867808] GPR00: c001509c bfc65570 1024b4d0 00000000 100de020 20022422 bfc655a8 100af3dc
> [ 35.867808] GPR08: 0002f902 00000000 00000000 00000000 72656773 102490c2 00000000 00000000
> [ 35.867808] GPR16: 00000000 00000000 00000040 10240000 10240000 10240000 10240000 10220000
> [ 35.867808] GPR24: ffffffff 10240000 00000000 00000000 bfc655e8 10245910 ffffffff 00000001
> [ 35.869406] NIP [100af3dc] 0x100af3dc
> [ 35.869578] LR [100de020] 0x100de020
> [ 35.869751] --- interrupt: 900
> [ 35.870001] Instruction dump:
> [ 35.870283] 40c20010 815e0518 714a0100 41e2fd04 39200000 913e00c0 3b1e0450 4bfffd80
> [ 35.870666] 0fe00000 92a10024 4bfff1a9 60000000 <7fe00008> 7c0802a6 93e1001c 7c5f1378
> [ 35.871339] ---[ end trace 23ff848139efa9b9 ]---
>
> There is no real mode for booke arch and the MMU translation is
> always on. The corresponding MSR_IS/MSR_DS bit in booke is used
> to switch the address space, but not for real mode judgment.
>
> Fixes: 21f8b2fa3ca5 ("powerpc/kprobes: Ignore traps that happened in real mode")
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
> arch/powerpc/include/asm/ptrace.h | 6 ++++++
> arch/powerpc/kernel/kprobes.c | 5 +----
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index 3e5d470a6155..4aec1a97024b 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -187,6 +187,12 @@ static inline unsigned long frame_pointer(struct pt_regs *regs)
> #define user_mode(regs) (((regs)->msr & MSR_PR) != 0)
> #endif
>
> +#ifdef CONFIG_BOOKE
> +#define real_mode(regs) 0
> +#else
> +#define real_mode(regs) (!((regs)->msr & MSR_IR) || !((regs)->msr & MSR_DR))
> +#endif
I'm not sure about this helper.
Arguably it should only return true if both MSR_IR and MSR_DR are clear.
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index cbc28d1a2e1b..fac9a5974718 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -289,10 +289,7 @@ int kprobe_handler(struct pt_regs *regs)
> unsigned int *addr = (unsigned int *)regs->nip;
> struct kprobe_ctlblk *kcb;
>
> - if (user_mode(regs))
> - return 0;
> -
> - if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
> + if (user_mode(regs) || real_mode(regs))
> return 0;
I think just adding an IS_ENABLED(CONFIG_BOOKE) here might be better.
cheers
^ permalink raw reply
* Re: [PATCH v1 11/55] powerpc/time: add API for KVM to re-arm the host timer/decrementer
From: Christophe Leroy @ 2021-08-05 7:22 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210726035036.739609-12-npiggin@gmail.com>
Le 26/07/2021 à 05:49, Nicholas Piggin a écrit :
> Rather than have KVM look up the host timer and fiddle with the
> irq-work internal details, have the powerpc/time.c code provide a
> function for KVM to re-arm the Linux timer code when exiting a
> guest.
>
> This is implementation has an improvement over existing code of
> marking a decrementer interrupt as soft-pending if a timer has
> expired, rather than setting DEC to a -ve value, which tended to
> cause host timers to take two interrupts (first hdec to exit the
> guest, then the immediate dec).
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/time.h | 16 +++-------
> arch/powerpc/kernel/time.c | 52 +++++++++++++++++++++++++++------
> arch/powerpc/kvm/book3s_hv.c | 7 ++---
> 3 files changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 69b6be617772..924b2157882f 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -99,18 +99,6 @@ extern void div128_by_32(u64 dividend_high, u64 dividend_low,
> extern void secondary_cpu_time_init(void);
> extern void __init time_init(void);
>
> -#ifdef CONFIG_PPC64
> -static inline unsigned long test_irq_work_pending(void)
> -{
> - unsigned long x;
> -
> - asm volatile("lbz %0,%1(13)"
> - : "=r" (x)
> - : "i" (offsetof(struct paca_struct, irq_work_pending)));
> - return x;
> -}
> -#endif
> -
> DECLARE_PER_CPU(u64, decrementers_next_tb);
>
> static inline u64 timer_get_next_tb(void)
> @@ -118,6 +106,10 @@ static inline u64 timer_get_next_tb(void)
> return __this_cpu_read(decrementers_next_tb);
> }
>
> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> +void timer_rearm_host_dec(u64 now);
> +#endif
> +
> /* Convert timebase ticks to nanoseconds */
> unsigned long long tb_to_ns(unsigned long long tb_ticks);
>
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 72d872b49167..016828b7401b 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -499,6 +499,16 @@ EXPORT_SYMBOL(profile_pc);
> * 64-bit uses a byte in the PACA, 32-bit uses a per-cpu variable...
> */
> #ifdef CONFIG_PPC64
> +static inline unsigned long test_irq_work_pending(void)
> +{
> + unsigned long x;
> +
> + asm volatile("lbz %0,%1(13)"
> + : "=r" (x)
> + : "i" (offsetof(struct paca_struct, irq_work_pending)));
Can we just use READ_ONCE() instead of hard coding the read ?
> + return x;
> +}
> +
> static inline void set_irq_work_pending_flag(void)
> {
> asm volatile("stb %0,%1(13)" : :
> @@ -542,13 +552,44 @@ void arch_irq_work_raise(void)
> preempt_enable();
> }
>
> +static void set_dec_or_work(u64 val)
> +{
> + set_dec(val);
> + /* We may have raced with new irq work */
> + if (unlikely(test_irq_work_pending()))
> + set_dec(1);
> +}
> +
> #else /* CONFIG_IRQ_WORK */
>
> #define test_irq_work_pending() 0
> #define clear_irq_work_pending()
>
> +static void set_dec_or_work(u64 val)
> +{
> + set_dec(val);
> +}
> #endif /* CONFIG_IRQ_WORK */
>
> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> +void timer_rearm_host_dec(u64 now)
> +{
> + u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
> +
> + WARN_ON_ONCE(!arch_irqs_disabled());
> + WARN_ON_ONCE(mfmsr() & MSR_EE);
> +
> + if (now >= *next_tb) {
> + local_paca->irq_happened |= PACA_IRQ_DEC;
> + } else {
> + now = *next_tb - now;
> + if (now <= decrementer_max)
> + set_dec_or_work(now);
> + }
> +}
> +EXPORT_SYMBOL_GPL(timer_rearm_host_dec);
> +#endif
> +
> /*
> * timer_interrupt - gets called when the decrementer overflows,
> * with interrupts disabled.
> @@ -609,10 +650,7 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(timer_interrupt)
> } else {
> now = *next_tb - now;
> if (now <= decrementer_max)
> - set_dec(now);
> - /* We may have raced with new irq work */
> - if (test_irq_work_pending())
> - set_dec(1);
> + set_dec_or_work(now);
> __this_cpu_inc(irq_stat.timer_irqs_others);
> }
>
> @@ -854,11 +892,7 @@ static int decrementer_set_next_event(unsigned long evt,
> struct clock_event_device *dev)
> {
> __this_cpu_write(decrementers_next_tb, get_tb() + evt);
> - set_dec(evt);
> -
> - /* We may have raced with new irq work */
> - if (test_irq_work_pending())
> - set_dec(1);
> + set_dec_or_work(evt);
>
> return 0;
> }
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 6e6cfb10e9bb..0cef578930f9 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4018,11 +4018,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> vc->entry_exit_map = 0x101;
> vc->in_guest = 0;
>
> - next_timer = timer_get_next_tb();
> - set_dec(next_timer - tb);
> - /* We may have raced with new irq work */
> - if (test_irq_work_pending())
> - set_dec(1);
> + timer_rearm_host_dec(tb);
> +
> mtspr(SPRN_SPRG_VDSO_WRITE, local_paca->sprg_vdso);
>
> kvmhv_load_host_pmu();
>
^ permalink raw reply
* [RFC PATCH v0 1/5] powerpc: Define Expropriation interrupt bit to VPA byte offset 0xB9
From: Bharata B Rao @ 2021-08-05 7:24 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, kvm, bharata.rao
In-Reply-To: <20210805072439.501481-1-bharata@linux.ibm.com>
VPA byte offset 0xB9 was named as donate_dedicated_cpu as that
was the only used bit. The Expropriation/Subvention support defines
a bit in byte offset 0xB9. Define this bit and rename the field
in VPA to a generic name.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/lppaca.h | 8 +++++++-
drivers/cpuidle/cpuidle-pseries.c | 4 ++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index c390ec377bae..57e432766f3e 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -80,7 +80,7 @@ struct lppaca {
u8 ebb_regs_in_use;
u8 reserved7[6];
u8 dtl_enable_mask; /* Dispatch Trace Log mask */
- u8 donate_dedicated_cpu; /* Donate dedicated CPU cycles */
+ u8 byte_b9; /* Donate dedicated CPU cycles & Expropriation int */
u8 fpregs_in_use;
u8 pmcregs_in_use;
u8 reserved8[28];
@@ -116,6 +116,12 @@ struct lppaca {
#define lppaca_of(cpu) (*paca_ptrs[cpu]->lppaca_ptr)
+/*
+ * Flags for Byte offset 0xB9
+ */
+#define LPPACA_DONATE_DED_CPU_CYCLES 0x1
+#define LPPACA_EXP_INT_ENABLED 0x2
+
/*
* We are using a non architected field to determine if a partition is
* shared or dedicated. This currently works on both KVM and PHYP, but
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index a2b5c6f60cf0..b9d0f41c3f19 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -221,7 +221,7 @@ static int dedicated_cede_loop(struct cpuidle_device *dev,
u8 old_latency_hint;
pseries_idle_prolog();
- get_lppaca()->donate_dedicated_cpu = 1;
+ get_lppaca()->byte_b9 |= LPPACA_DONATE_DED_CPU_CYCLES;
old_latency_hint = get_lppaca()->cede_latency_hint;
get_lppaca()->cede_latency_hint = cede_latency_hint[index];
@@ -229,7 +229,7 @@ static int dedicated_cede_loop(struct cpuidle_device *dev,
check_and_cede_processor();
local_irq_disable();
- get_lppaca()->donate_dedicated_cpu = 0;
+ get_lppaca()->byte_b9 &= ~LPPACA_DONATE_DED_CPU_CYCLES;
get_lppaca()->cede_latency_hint = old_latency_hint;
pseries_idle_epilog();
--
2.31.1
^ permalink raw reply related
* [RFC PATCH v0 0/5] PPC: KVM: pseries: Asynchronous page fault
From: Bharata B Rao @ 2021-08-05 7:24 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, kvm, bharata.rao
Hi,
This series adds asynchronous page fault support for pseries guests
and enables the support for the same in powerpc KVM. This is an
early RFC with details and multiple TODOs listed in patch descriptions.
This patch needs supporting enablement in QEMU too which will be
posted separately.
Bharata B Rao (5):
powerpc: Define Expropriation interrupt bit to VPA byte offset 0xB9
KVM: PPC: Add support for KVM_REQ_ESN_EXIT
KVM: PPC: Book3S: Enable setting SRR1 flags for DSI
KVM: PPC: BOOK3S HV: Async PF support
pseries: Asynchronous page fault support
Documentation/virt/kvm/api.rst | 15 ++
arch/powerpc/include/asm/async-pf.h | 12 ++
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/kvm_book3s_esn.h | 24 +++
arch/powerpc/include/asm/kvm_host.h | 22 +++
arch/powerpc/include/asm/kvm_ppc.h | 4 +-
arch/powerpc/include/asm/lppaca.h | 20 +-
arch/powerpc/include/uapi/asm/kvm.h | 6 +
arch/powerpc/kvm/Kconfig | 2 +
arch/powerpc/kvm/Makefile | 5 +-
arch/powerpc/kvm/book3s.c | 6 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 9 +-
arch/powerpc/kvm/book3s_hv.c | 37 +++-
arch/powerpc/kvm/book3s_hv_esn.c | 189 +++++++++++++++++++
arch/powerpc/kvm/book3s_hv_nested.c | 4 +-
arch/powerpc/kvm/book3s_pr.c | 4 +-
arch/powerpc/mm/fault.c | 7 +-
arch/powerpc/platforms/pseries/Makefile | 2 +-
arch/powerpc/platforms/pseries/async-pf.c | 219 ++++++++++++++++++++++
drivers/cpuidle/cpuidle-pseries.c | 4 +-
include/uapi/linux/kvm.h | 2 +
tools/include/uapi/linux/kvm.h | 1 +
22 files changed, 574 insertions(+), 21 deletions(-)
create mode 100644 arch/powerpc/include/asm/async-pf.h
create mode 100644 arch/powerpc/include/asm/kvm_book3s_esn.h
create mode 100644 arch/powerpc/kvm/book3s_hv_esn.c
create mode 100644 arch/powerpc/platforms/pseries/async-pf.c
--
2.31.1
^ permalink raw reply
* [RFC PATCH v0 2/5] KVM: PPC: Add support for KVM_REQ_ESN_EXIT
From: Bharata B Rao @ 2021-08-05 7:24 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, kvm, bharata.rao
In-Reply-To: <20210805072439.501481-1-bharata@linux.ibm.com>
Add a new KVM exit request KVM_REQ_ESN_EXIT that will be used
to exit to userspace (QEMU) whenever subvention notification
needs to be sent to the guest.
The userspace (QEMU) issues the subvention notification by
injecting an interrupt into the guest.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/kvm/book3s_hv.c | 8 ++++++++
include/uapi/linux/kvm.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 9f52f282b1aa..204dc2d91388 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -52,6 +52,7 @@
#define KVM_REQ_WATCHDOG KVM_ARCH_REQ(0)
#define KVM_REQ_EPR_EXIT KVM_ARCH_REQ(1)
#define KVM_REQ_PENDING_TIMER KVM_ARCH_REQ(2)
+#define KVM_REQ_ESN_EXIT KVM_ARCH_REQ(3)
#include <linux/mmu_notifier.h>
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 085fb8ecbf68..47ccd4a2df54 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2820,6 +2820,14 @@ static void kvmppc_core_vcpu_free_hv(struct kvm_vcpu *vcpu)
static int kvmppc_core_check_requests_hv(struct kvm_vcpu *vcpu)
{
+ /*
+ * If subvention interrupt needs to be injected to the guest
+ * exit to user space.
+ */
+ if (kvm_check_request(KVM_REQ_ESN_EXIT, vcpu)) {
+ vcpu->run->exit_reason = KVM_EXIT_ESN;
+ return 0;
+ }
/* Indicate we want to get back into the guest */
return 1;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d9e4aabcb31a..47be532ed14b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -269,6 +269,7 @@ struct kvm_xen_exit {
#define KVM_EXIT_AP_RESET_HOLD 32
#define KVM_EXIT_X86_BUS_LOCK 33
#define KVM_EXIT_XEN 34
+#define KVM_EXIT_ESN 35
/* For KVM_EXIT_INTERNAL_ERROR */
/* Emulate instruction failed. */
--
2.31.1
^ permalink raw reply related
* [RFC PATCH v0 3/5] KVM: PPC: Book3S: Enable setting SRR1 flags for DSI
From: Bharata B Rao @ 2021-08-05 7:24 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, kvm, bharata.rao
In-Reply-To: <20210805072439.501481-1-bharata@linux.ibm.com>
kvmppc_core_queue_data_storage() doesn't provide an option to
set SRR1 flags when raising DSI. Since kvmppc_inject_interrupt()
allows for such a provision, add an argument to allow the same.
This will be used to raise DSI with SRR1_PROGTRAP set when
expropriation interrupt needs to be injected to the guest.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_ppc.h | 3 ++-
arch/powerpc/kvm/book3s.c | 6 +++---
arch/powerpc/kvm/book3s_64_mmu_radix.c | 6 +++---
arch/powerpc/kvm/book3s_hv.c | 4 ++--
arch/powerpc/kvm/book3s_hv_nested.c | 4 ++--
arch/powerpc/kvm/book3s_pr.c | 4 ++--
6 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 2d88944f9f34..09235bdfd4ac 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -143,7 +143,8 @@ extern void kvmppc_core_queue_dtlb_miss(struct kvm_vcpu *vcpu, ulong dear_flags,
ulong esr_flags);
extern void kvmppc_core_queue_data_storage(struct kvm_vcpu *vcpu,
ulong dear_flags,
- ulong esr_flags);
+ ulong esr_flags,
+ ulong srr1_flags);
extern void kvmppc_core_queue_itlb_miss(struct kvm_vcpu *vcpu);
extern void kvmppc_core_queue_inst_storage(struct kvm_vcpu *vcpu,
ulong esr_flags);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 79833f78d1da..f7f6641a788d 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -284,11 +284,11 @@ void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu)
}
void kvmppc_core_queue_data_storage(struct kvm_vcpu *vcpu, ulong dar,
- ulong flags)
+ ulong dsisr, ulong srr1)
{
kvmppc_set_dar(vcpu, dar);
- kvmppc_set_dsisr(vcpu, flags);
- kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_DATA_STORAGE, 0);
+ kvmppc_set_dsisr(vcpu, dsisr);
+ kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_DATA_STORAGE, srr1);
}
EXPORT_SYMBOL_GPL(kvmppc_core_queue_data_storage);
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index b5905ae4377c..618206a504b0 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -946,7 +946,7 @@ int kvmppc_book3s_radix_page_fault(struct kvm_vcpu *vcpu,
if (dsisr & DSISR_BADACCESS) {
/* Reflect to the guest as DSI */
pr_err("KVM: Got radix HV page fault with DSISR=%lx\n", dsisr);
- kvmppc_core_queue_data_storage(vcpu, ea, dsisr);
+ kvmppc_core_queue_data_storage(vcpu, ea, dsisr, 0);
return RESUME_GUEST;
}
@@ -971,7 +971,7 @@ int kvmppc_book3s_radix_page_fault(struct kvm_vcpu *vcpu,
* Bad address in guest page table tree, or other
* unusual error - reflect it to the guest as DSI.
*/
- kvmppc_core_queue_data_storage(vcpu, ea, dsisr);
+ kvmppc_core_queue_data_storage(vcpu, ea, dsisr, 0);
return RESUME_GUEST;
}
return kvmppc_hv_emulate_mmio(vcpu, gpa, ea, writing);
@@ -981,7 +981,7 @@ int kvmppc_book3s_radix_page_fault(struct kvm_vcpu *vcpu,
if (writing) {
/* give the guest a DSI */
kvmppc_core_queue_data_storage(vcpu, ea, DSISR_ISSTORE |
- DSISR_PROTFAULT);
+ DSISR_PROTFAULT, 0);
return RESUME_GUEST;
}
kvm_ro = true;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 47ccd4a2df54..d07e9065f7c1 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1592,7 +1592,7 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
if (!(vcpu->arch.fault_dsisr & (DSISR_NOHPTE | DSISR_PROTFAULT))) {
kvmppc_core_queue_data_storage(vcpu,
- vcpu->arch.fault_dar, vcpu->arch.fault_dsisr);
+ vcpu->arch.fault_dar, vcpu->arch.fault_dsisr, 0);
r = RESUME_GUEST;
break;
}
@@ -1610,7 +1610,7 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
r = RESUME_PAGE_FAULT;
} else {
kvmppc_core_queue_data_storage(vcpu,
- vcpu->arch.fault_dar, err);
+ vcpu->arch.fault_dar, err, 0);
r = RESUME_GUEST;
}
break;
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 898f942eb198..a10ef0d5f925 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -1556,7 +1556,7 @@ static long int __kvmhv_nested_page_fault(struct kvm_vcpu *vcpu,
if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID)) {
if (dsisr & (DSISR_PRTABLE_FAULT | DSISR_BADACCESS)) {
/* unusual error -> reflect to the guest as a DSI */
- kvmppc_core_queue_data_storage(vcpu, ea, dsisr);
+ kvmppc_core_queue_data_storage(vcpu, ea, dsisr, 0);
return RESUME_GUEST;
}
@@ -1567,7 +1567,7 @@ static long int __kvmhv_nested_page_fault(struct kvm_vcpu *vcpu,
if (writing) {
/* Give the guest a DSI */
kvmppc_core_queue_data_storage(vcpu, ea,
- DSISR_ISSTORE | DSISR_PROTFAULT);
+ DSISR_ISSTORE | DSISR_PROTFAULT, 0);
return RESUME_GUEST;
}
kvm_ro = true;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 6bc9425acb32..f7fc8e01fd8e 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -754,7 +754,7 @@ static int kvmppc_handle_pagefault(struct kvm_vcpu *vcpu,
flags = DSISR_NOHPTE;
if (data) {
flags |= vcpu->arch.fault_dsisr & DSISR_ISSTORE;
- kvmppc_core_queue_data_storage(vcpu, eaddr, flags);
+ kvmppc_core_queue_data_storage(vcpu, eaddr, flags, 0);
} else {
kvmppc_core_queue_inst_storage(vcpu, flags);
}
@@ -1229,7 +1229,7 @@ int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr)
r = kvmppc_handle_pagefault(vcpu, dar, exit_nr);
srcu_read_unlock(&vcpu->kvm->srcu, idx);
} else {
- kvmppc_core_queue_data_storage(vcpu, dar, fault_dsisr);
+ kvmppc_core_queue_data_storage(vcpu, dar, fault_dsisr, 0);
r = RESUME_GUEST;
}
break;
--
2.31.1
^ permalink raw reply related
* [RFC PATCH v0 4/5] KVM: PPC: BOOK3S HV: Async PF support
From: Bharata B Rao @ 2021-08-05 7:24 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, kvm, bharata.rao
In-Reply-To: <20210805072439.501481-1-bharata@linux.ibm.com>
Add asynchronous page fault support for PowerKVM by making
use of the Expropriation/Subvention Notification Option
defined by PAPR specifications.
1. When guest accessed page isn't immediately available in the
host, update the vcpu's VPA with a unique expropriation correlation
number and inject a DSI to the guest with SRR1_PROGTRAP bit set in
SRR1. This informs the guest vcpu to put the process to wait and
schedule a different process.
- Async PF is supported for data pages in this implementation
though PAPR allows it for code pages too.
- Async PF is supported only for user pages here.
- The feature is currently limited only to radix guests.
2. When the page becomes available, update the Subvention Notification
Structure with the corresponding expropriation correlation number and
and inform the guest via subvention interrupt.
- Subvention Notification Structure (SNS) is a region of memory
shared between host and guest via which the communication related
to expropriated and subvened pages happens between guest and host.
- SNS region is registered by the guest via H_REG_SNS hcall which
is implemented in QEMU.
- H_REG_SNS implementation in QEMU needs a new ioctl KVM_PPC_SET_SNS.
This ioctl is used to map and pin the guest page containing SNS
in the host.
- Subvention notification interrupt is raised to the guest by
QEMU in response to the guest exit via KVM_REQ_ESN_EXIT. This
interrupt informs the guest about the availability of the
pages.
TODO:
- H_REG_SNS is implemented in QEMU because this hcall needs to return
the interrupt source number associated with the subvention interrupt.
Claiming of IRQ line and raising an external interrupt seem to be
straightforward from QEMU. Figure out the in-kernel equivalents for
these two so that, we can save on guest exit for each expropriated
page and move the entire hcall implementation into the host kernel.
- The code is pretty much experimental and is barely able to boot a
guest. I do see some requests for expropriated pages not getting
fulfilled by host leading the long delays in guest. This needs some
debugging.
- A few other aspects recommended by PAPR around this feature(like
setting of page state flags) need to be evaluated and incorporated
into the implementation if found appropriate.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
Documentation/virt/kvm/api.rst | 15 ++
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/kvm_book3s_esn.h | 24 +++
arch/powerpc/include/asm/kvm_host.h | 21 +++
arch/powerpc/include/asm/kvm_ppc.h | 1 +
arch/powerpc/include/asm/lppaca.h | 12 +-
arch/powerpc/include/uapi/asm/kvm.h | 6 +
arch/powerpc/kvm/Kconfig | 2 +
arch/powerpc/kvm/Makefile | 5 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 3 +
arch/powerpc/kvm/book3s_hv.c | 25 +++
arch/powerpc/kvm/book3s_hv_esn.c | 189 ++++++++++++++++++++++
include/uapi/linux/kvm.h | 1 +
tools/include/uapi/linux/kvm.h | 1 +
14 files changed, 303 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/include/asm/kvm_book3s_esn.h
create mode 100644 arch/powerpc/kvm/book3s_hv_esn.c
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index dae68e68ca23..512f078b9d02 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5293,6 +5293,21 @@ the trailing ``'\0'``, is indicated by ``name_size`` in the header.
The Stats Data block contains an array of 64-bit values in the same order
as the descriptors in Descriptors block.
+4.134 KVM_PPC_SET_SNS
+---------------------
+
+:Capability: basic
+:Architectures: powerpc
+:Type: vm ioctl
+:Parameters: none
+:Returns: 0 on successful completion,
+
+As part of H_REG_SNS hypercall, this ioctl is used to map and pin
+the guest provided SNS structure in the host.
+
+This is used for providing asynchronous page fault support for
+powerpc pseries KVM guests.
+
5. The kvm_run structure
========================
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 9bcf345cb208..9e33500c1723 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -321,6 +321,7 @@
#define H_SCM_UNBIND_ALL 0x3FC
#define H_SCM_HEALTH 0x400
#define H_SCM_PERFORMANCE_STATS 0x418
+#define H_REG_SNS 0x41C
#define H_RPT_INVALIDATE 0x448
#define H_SCM_FLUSH 0x44C
#define MAX_HCALL_OPCODE H_SCM_FLUSH
diff --git a/arch/powerpc/include/asm/kvm_book3s_esn.h b/arch/powerpc/include/asm/kvm_book3s_esn.h
new file mode 100644
index 000000000000..d79a441ea31d
--- /dev/null
+++ b/arch/powerpc/include/asm/kvm_book3s_esn.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_KVM_BOOK3S_ESN_H__
+#define __ASM_KVM_BOOK3S_ESN_H__
+
+/* SNS buffer EQ state flags */
+#define SNS_EQ_STATE_OPERATIONAL 0X0
+#define SNS_EQ_STATE_OVERFLOW 0x1
+
+/* SNS buffer Notification control bits */
+#define SNS_EQ_CNTRL_TRIGGER 0x1
+
+struct kvmppc_sns {
+ unsigned long gpa;
+ unsigned long len;
+ void *hva;
+ uint16_t exp_corr_nr;
+ uint16_t *eq;
+ uint8_t *eq_cntrl;
+ uint8_t *eq_state;
+ unsigned long next_eq_entry;
+ unsigned long nr_eq_entries;
+};
+
+#endif /* __ASM_KVM_BOOK3S_ESN_H__ */
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 204dc2d91388..8d7f73085ef5 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -25,6 +25,7 @@
#include <asm/cacheflush.h>
#include <asm/hvcall.h>
#include <asm/mce.h>
+#include <asm/kvm_book3s_esn.h>
#define KVM_MAX_VCPUS NR_CPUS
#define KVM_MAX_VCORES NR_CPUS
@@ -325,6 +326,7 @@ struct kvm_arch {
#endif
struct kvmppc_ops *kvm_ops;
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ struct kvmppc_sns sns;
struct mutex uvmem_lock;
struct list_head uvmem_pfns;
struct mutex mmu_setup_lock; /* nests inside vcpu mutexes */
@@ -855,6 +857,25 @@ struct kvm_vcpu_arch {
#define __KVM_HAVE_ARCH_WQP
#define __KVM_HAVE_CREATE_DEVICE
+/* Async pf */
+#define ASYNC_PF_PER_VCPU 64
+struct kvm_arch_async_pf {
+ unsigned long exp_token;
+};
+int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu,
+ unsigned long gpa, unsigned long hva);
+
+void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu,
+ struct kvm_async_pf *work);
+
+bool kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
+ struct kvm_async_pf *work);
+
+void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
+ struct kvm_async_pf *work);
+bool kvm_arch_can_dequeue_async_page_present(struct kvm_vcpu *vcpu);
+static inline void kvm_arch_async_page_present_queued(struct kvm_vcpu *vcpu) {}
+
static inline void kvm_arch_hardware_disable(void) {}
static inline void kvm_arch_hardware_unsetup(void) {}
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 09235bdfd4ac..c14a84041d0e 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -228,6 +228,7 @@ extern long kvm_vm_ioctl_resize_hpt_commit(struct kvm *kvm,
int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq);
extern int kvm_vm_ioctl_rtas_define_token(struct kvm *kvm, void __user *argp);
+long kvm_vm_ioctl_set_sns(struct kvm *kvm, struct kvm_ppc_sns_reg *sns_reg);
extern int kvmppc_rtas_hcall(struct kvm_vcpu *vcpu);
extern void kvmppc_rtas_tokens_free(struct kvm *kvm);
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index 57e432766f3e..17e89c3865e8 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -104,7 +104,17 @@ struct lppaca {
volatile __be32 dispersion_count; /* dispatch changed physical cpu */
volatile __be64 cmo_faults; /* CMO page fault count */
volatile __be64 cmo_fault_time; /* CMO page fault time */
- u8 reserved10[104];
+
+ /*
+ * TODO: Insert this at correct offset
+ * 0x17D - Exp flags (1 byte)
+ * 0x17E - Exp corr number (2 bytes)
+ *
+ * Here I am using only exp corr number at an easy to insert
+ * offset.
+ */
+ __be16 exp_corr_nr; /* Exproppriation correlation number */
+ u8 reserved10[102];
/* cacheline 4-5 */
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 9f18fa090f1f..d72739126ae5 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -470,6 +470,12 @@ struct kvm_ppc_cpu_char {
#define KVM_PPC_CPU_BEHAV_BNDS_CHK_SPEC_BAR (1ULL << 61)
#define KVM_PPC_CPU_BEHAV_FLUSH_COUNT_CACHE (1ull << 58)
+/* For KVM_PPC_SET_SNS */
+struct kvm_ppc_sns_reg {
+ __u64 addr;
+ __u64 len;
+};
+
/* Per-vcpu XICS interrupt controller state */
#define KVM_REG_PPC_ICP_STATE (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8c)
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index e45644657d49..4f552649a4b2 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -85,6 +85,8 @@ config KVM_BOOK3S_64_HV
depends on KVM_BOOK3S_64 && PPC_POWERNV
select KVM_BOOK3S_HV_POSSIBLE
select MMU_NOTIFIER
+ select KVM_ASYNC_PF
+ select KVM_ASYNC_PF_SYNC
select CMA
help
Support running unmodified book3s_64 guest kernels in
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 583c14ef596e..603ab382d021 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -6,7 +6,7 @@
ccflags-y := -Ivirt/kvm -Iarch/powerpc/kvm
KVM := ../../../virt/kvm
-common-objs-y = $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
+common-objs-y = $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o $(KVM)/async_pf.o
common-objs-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o
common-objs-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o
@@ -70,7 +70,8 @@ kvm-hv-y += \
book3s_hv_interrupts.o \
book3s_64_mmu_hv.o \
book3s_64_mmu_radix.o \
- book3s_hv_nested.o
+ book3s_hv_nested.o \
+ book3s_hv_esn.o
kvm-hv-$(CONFIG_PPC_UV) += \
book3s_hv_uvmem.o
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 618206a504b0..1985f84bfebe 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -837,6 +837,9 @@ int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
} else {
unsigned long pfn;
+ if (kvm_arch_setup_async_pf(vcpu, gpa, hva))
+ return RESUME_GUEST;
+
/* Call KVM generic code to do the slow-path check */
pfn = __gfn_to_pfn_memslot(memslot, gfn, false, NULL,
writing, upgrade_p, NULL);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index d07e9065f7c1..5cc564321521 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -77,6 +77,7 @@
#include <asm/ultravisor.h>
#include <asm/dtl.h>
#include <asm/plpar_wrappers.h>
+#include <asm/kvm_book3s_esn.h>
#include "book3s.h"
@@ -4570,6 +4571,11 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
return -EINTR;
}
+ if (kvm_request_pending(vcpu)) {
+ if (!kvmppc_core_check_requests(vcpu))
+ return 0;
+ }
+
kvm = vcpu->kvm;
atomic_inc(&kvm->arch.vcpus_running);
/* Order vcpus_running vs. mmu_ready, see kvmppc_alloc_reset_hpt */
@@ -4591,6 +4597,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
vcpu->arch.state = KVMPPC_VCPU_BUSY_IN_HOST;
do {
+ kvm_check_async_pf_completion(vcpu);
if (cpu_has_feature(CPU_FTR_ARCH_300))
r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
vcpu->arch.vcore->lpcr);
@@ -5257,6 +5264,8 @@ static void kvmppc_free_vcores(struct kvm *kvm)
static void kvmppc_core_destroy_vm_hv(struct kvm *kvm)
{
+ struct kvm_ppc_sns_reg sns_reg;
+
debugfs_remove_recursive(kvm->arch.debugfs_dir);
if (!cpu_has_feature(CPU_FTR_ARCH_300))
@@ -5283,6 +5292,11 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm)
kvmppc_free_lpid(kvm->arch.lpid);
kvmppc_free_pimap(kvm);
+
+ /* Needed for de-registering SNS buffer */
+ sns_reg.addr = -1;
+ sns_reg.len = 0;
+ kvm_vm_ioctl_set_sns(kvm, &sns_reg);
}
/* We don't need to emulate any privileged instructions or dcbz */
@@ -5561,6 +5575,17 @@ static long kvm_arch_vm_ioctl_hv(struct file *filp,
break;
}
+ case KVM_PPC_SET_SNS: {
+ struct kvm_ppc_sns_reg sns_reg;
+
+ r = -EFAULT;
+ if (copy_from_user(&sns_reg, argp, sizeof(sns_reg)))
+ break;
+
+ r = kvm_vm_ioctl_set_sns(kvm, &sns_reg);
+ break;
+ }
+
default:
r = -ENOTTY;
}
diff --git a/arch/powerpc/kvm/book3s_hv_esn.c b/arch/powerpc/kvm/book3s_hv_esn.c
new file mode 100644
index 000000000000..b322a14c1f83
--- /dev/null
+++ b/arch/powerpc/kvm/book3s_hv_esn.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Async page fault support via PAPR Expropriation/Subvention Notification
+ * option(ESN)
+ *
+ * Copyright 2020 Bharata B Rao, IBM Corp. <bharata@linux.ibm.com>
+ */
+
+#include <linux/kvm_host.h>
+#include <asm/kvm_ppc.h>
+#include <asm/kvm_book3s_esn.h>
+
+static DEFINE_SPINLOCK(async_exp_lock); /* for updating exp_corr_nr */
+static DEFINE_SPINLOCK(async_sns_lock); /* SNS buffer updated under this lock */
+
+int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu,
+ unsigned long gpa, unsigned long hva)
+{
+ struct kvm_arch_async_pf arch;
+ struct lppaca *vpa = vcpu->arch.vpa.pinned_addr;
+ u64 msr = kvmppc_get_msr(vcpu);
+ struct kvmppc_sns *sns = &vcpu->kvm->arch.sns;
+
+ /*
+ * If VPA hasn't been registered yet, can't support
+ * async pf.
+ */
+ if (!vpa)
+ return 0;
+
+ /*
+ * If SNS memory area hasn't been registered yet,
+ * can't support async pf.
+ */
+ if (!vcpu->kvm->arch.sns.eq)
+ return 0;
+
+ /*
+ * If guest hasn't enabled expropriation interrupt,
+ * don't try async pf.
+ */
+ if (!(vpa->byte_b9 & LPPACA_EXP_INT_ENABLED))
+ return 0;
+
+ /*
+ * If the fault is in the guest kernel, don,t
+ * try async pf.
+ */
+ if (!(msr & MSR_PR) && !(msr & MSR_HV))
+ return 0;
+
+ spin_lock(&async_sns_lock);
+ /*
+ * Check if subvention event queue can
+ * overflow, if so, don't try async pf.
+ */
+ if (*(sns->eq + sns->next_eq_entry)) {
+ pr_err("%s: SNS buffer overflow\n", __func__);
+ spin_unlock(&async_sns_lock);
+ return 0;
+ }
+ spin_unlock(&async_sns_lock);
+
+ /*
+ * TODO:
+ *
+ * 1. Update exp flags bit 7 to 1
+ * ("The Subvened page data will be restored")
+ *
+ * 2. Check if request to this page has been
+ * notified to guest earlier, if so send back
+ * the same exp corr number.
+ *
+ * 3. exp_corr_nr could be a random but non-zero
+ * number. Not taking care of wrapping here. Fix
+ * it.
+ */
+ spin_lock(&async_exp_lock);
+ vpa->exp_corr_nr = cpu_to_be16(vcpu->kvm->arch.sns.exp_corr_nr);
+ arch.exp_token = vcpu->kvm->arch.sns.exp_corr_nr++;
+ spin_unlock(&async_exp_lock);
+
+ return kvm_setup_async_pf(vcpu, gpa, hva, &arch);
+}
+
+bool kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
+ struct kvm_async_pf *work)
+{
+ /* Inject DSI to guest with srr1 bit 46 set */
+ kvmppc_core_queue_data_storage(vcpu, kvmppc_get_dar(vcpu), DSISR_NOHPTE, SRR1_PROGTRAP);
+ return true;
+}
+
+void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
+ struct kvm_async_pf *work)
+{
+ struct kvmppc_sns *sns = &vcpu->kvm->arch.sns;
+
+ spin_lock(&async_sns_lock);
+ if (*sns->eq_cntrl != SNS_EQ_CNTRL_TRIGGER) {
+ pr_err("%s: SNS Notification Trigger not set by guest\n", __func__);
+ spin_unlock(&async_sns_lock);
+ /* TODO: Terminate the guest? */
+ return;
+ }
+
+ if (arch_cmpxchg(sns->eq + sns->next_eq_entry, 0,
+ work->arch.exp_token)) {
+ *sns->eq_state |= SNS_EQ_STATE_OVERFLOW;
+ pr_err("%s: SNS buffer overflow\n", __func__);
+ spin_unlock(&async_sns_lock);
+ /* TODO: Terminate the guest? */
+ return;
+ }
+
+ sns->next_eq_entry = (sns->next_eq_entry + 1) % sns->nr_eq_entries;
+ spin_unlock(&async_sns_lock);
+
+ /*
+ * Request a guest exit so that ESN virtual interrupt can
+ * be injected by QEMU.
+ */
+ kvm_make_request(KVM_REQ_ESN_EXIT, vcpu);
+}
+
+void kvm_arch_async_page_ready(struct kvm_vcpu *vcpu, struct kvm_async_pf *work)
+{
+ /* We will inject the page directly */
+}
+
+bool kvm_arch_can_dequeue_async_page_present(struct kvm_vcpu *vcpu)
+{
+ /*
+ * PowerPC will always inject the page directly,
+ * but we still want check_async_completion to cleanup
+ */
+ return true;
+}
+
+long kvm_vm_ioctl_set_sns(struct kvm *kvm, struct kvm_ppc_sns_reg *sns_reg)
+{
+ unsigned long nb;
+
+ /* Deregister */
+ if (sns_reg->addr == -1) {
+ if (!kvm->arch.sns.hva)
+ return 0;
+
+ pr_info("%s: Deregistering SNS buffer for LPID %d\n",
+ __func__, kvm->arch.lpid);
+ kvmppc_unpin_guest_page(kvm, kvm->arch.sns.hva, kvm->arch.sns.gpa, false);
+ kvm->arch.sns.gpa = -1;
+ kvm->arch.sns.hva = 0;
+ return 0;
+ }
+
+ /*
+ * Already registered with the same address?
+ */
+ if (sns_reg->addr == kvm->arch.sns.gpa)
+ return 0;
+
+ /* If previous registration exists, free it */
+ if (kvm->arch.sns.hva) {
+ pr_info("%s: Deregistering Previous SNS buffer for LPID %d\n",
+ __func__, kvm->arch.lpid);
+ kvmppc_unpin_guest_page(kvm, kvm->arch.sns.hva, kvm->arch.sns.gpa, false);
+ kvm->arch.sns.gpa = -1;
+ kvm->arch.sns.hva = 0;
+ }
+
+ kvm->arch.sns.gpa = sns_reg->addr;
+ kvm->arch.sns.hva = kvmppc_pin_guest_page(kvm, kvm->arch.sns.gpa, &nb);
+ kvm->arch.sns.len = sns_reg->len;
+ kvm->arch.sns.nr_eq_entries = (kvm->arch.sns.len - 2) / sizeof(uint16_t);
+ kvm->arch.sns.next_eq_entry = 0;
+ kvm->arch.sns.eq = kvm->arch.sns.hva + 2;
+ kvm->arch.sns.eq_cntrl = kvm->arch.sns.hva;
+ kvm->arch.sns.eq_state = kvm->arch.sns.hva + 1;
+ kvm->arch.sns.exp_corr_nr = 1; /* Should be non-zero */
+
+ *(kvm->arch.sns.eq_state) = SNS_EQ_STATE_OPERATIONAL;
+
+ pr_info("%s: Registering SNS buffer for LPID %d sns_addr %llx eq %lx\n",
+ __func__, kvm->arch.lpid, sns_reg->addr,
+ (unsigned long)kvm->arch.sns.eq);
+
+ return 0;
+}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 47be532ed14b..dbe65e8d68d8 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1459,6 +1459,7 @@ struct kvm_s390_ucas_mapping {
#define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter)
#define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3)
#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags)
+#define KVM_PPC_SET_SNS _IOR(KVMIO, 0xb5, struct kvm_ppc_sns_reg)
/* ioctl for vm fd */
#define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index d9e4aabcb31a..e9dea164498f 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1458,6 +1458,7 @@ struct kvm_s390_ucas_mapping {
#define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter)
#define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3)
#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags)
+#define KVM_PPC_SET_SNS _IOR(KVMIO, 0xb5, struct kvm_ppc_sns_reg)
/* ioctl for vm fd */
#define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
--
2.31.1
^ permalink raw reply related
* [RFC PATCH v0 5/5] pseries: Asynchronous page fault support
From: Bharata B Rao @ 2021-08-05 7:24 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, kvm, bharata.rao
In-Reply-To: <20210805072439.501481-1-bharata@linux.ibm.com>
Add asynchronous page fault support for pseries guests.
1. Setup the guest to handle async-pf
- Issue H_REG_SNS hcall to register the SNS region.
- Setup the subvention interrupt irq.
- Enable async-pf by updating the byte_b9 of VPA for each
CPU.
2. Check if the page fault is an expropriation notification
(SRR1_PROGTRAP set in SRR1) and if so put the task on
wait queue based on the expropriation correlation number
read from the VPA.
3. Handle subvention interrupt to wake any waiting tasks.
The wait and wakeup mechanism from x86 async-pf implementation
is being reused here.
TODO:
- Check how to keep this feature together with other CMO features.
- The async-pf check in the page fault handler path is limited to
guest with an #ifdef. This isn't sufficient and hence needs to
be replaced by an appropriate check.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/async-pf.h | 12 ++
arch/powerpc/mm/fault.c | 7 +-
arch/powerpc/platforms/pseries/Makefile | 2 +-
arch/powerpc/platforms/pseries/async-pf.c | 219 ++++++++++++++++++++++
4 files changed, 238 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/async-pf.h
create mode 100644 arch/powerpc/platforms/pseries/async-pf.c
diff --git a/arch/powerpc/include/asm/async-pf.h b/arch/powerpc/include/asm/async-pf.h
new file mode 100644
index 000000000000..95d6c3da9f50
--- /dev/null
+++ b/arch/powerpc/include/asm/async-pf.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Async page fault support via PAPR Expropriation/Subvention Notification
+ * option(ESN)
+ *
+ * Copyright 2020 Bharata B Rao, IBM Corp. <bharata@linux.ibm.com>
+ */
+
+#ifndef _ASM_POWERPC_ASYNC_PF_H
+int handle_async_page_fault(struct pt_regs *regs, unsigned long addr);
+#define _ASM_POWERPC_ASYNC_PF_H
+#endif
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index a8d0ce85d39a..bbdc61605885 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -44,7 +44,7 @@
#include <asm/debug.h>
#include <asm/kup.h>
#include <asm/inst.h>
-
+#include <asm/async-pf.h>
/*
* do_page_fault error handling helpers
@@ -395,6 +395,11 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
vm_fault_t fault, major = 0;
bool kprobe_fault = kprobe_page_fault(regs, 11);
+#ifdef CONFIG_PPC_PSERIES
+ if (handle_async_page_fault(regs, address))
+ return 0;
+#endif
+
if (unlikely(debugger_fault_handler(regs) || kprobe_fault))
return 0;
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 4cda0ef87be0..e0ada605ef20 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -6,7 +6,7 @@ obj-y := lpar.o hvCall.o nvram.o reconfig.o \
of_helpers.o \
setup.o iommu.o event_sources.o ras.o \
firmware.o power.o dlpar.o mobility.o rng.o \
- pci.o pci_dlpar.o eeh_pseries.o msi.o
+ pci.o pci_dlpar.o eeh_pseries.o msi.o async-pf.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_SCANLOG) += scanlog.o
obj-$(CONFIG_KEXEC_CORE) += kexec.o
diff --git a/arch/powerpc/platforms/pseries/async-pf.c b/arch/powerpc/platforms/pseries/async-pf.c
new file mode 100644
index 000000000000..c2f3bbc0d674
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/async-pf.c
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Async page fault support via PAPR Expropriation/Subvention Notification
+ * option(ESN)
+ *
+ * Copyright 2020 Bharata B Rao, IBM Corp. <bharata@linux.ibm.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/swait.h>
+#include <linux/irqdomain.h>
+#include <asm/machdep.h>
+#include <asm/hvcall.h>
+#include <asm/paca.h>
+
+static char sns_buffer[PAGE_SIZE] __aligned(4096);
+static uint16_t *esn_q = (uint16_t *)sns_buffer + 1;
+static unsigned long next_eq_entry, nr_eq_entries;
+
+#define ASYNC_PF_SLEEP_HASHBITS 8
+#define ASYNC_PF_SLEEP_HASHSIZE (1<<ASYNC_PF_SLEEP_HASHBITS)
+
+/* Controls access to SNS buffer */
+static DEFINE_RAW_SPINLOCK(async_sns_guest_lock);
+
+/* Wait queue handling is from x86 asyn-pf implementation */
+struct async_pf_sleep_node {
+ struct hlist_node link;
+ struct swait_queue_head wq;
+ u64 token;
+ int cpu;
+};
+
+static struct async_pf_sleep_head {
+ raw_spinlock_t lock;
+ struct hlist_head list;
+} async_pf_sleepers[ASYNC_PF_SLEEP_HASHSIZE];
+
+static struct async_pf_sleep_node *_find_apf_task(struct async_pf_sleep_head *b,
+ u64 token)
+{
+ struct hlist_node *p;
+
+ hlist_for_each(p, &b->list) {
+ struct async_pf_sleep_node *n =
+ hlist_entry(p, typeof(*n), link);
+ if (n->token == token)
+ return n;
+ }
+
+ return NULL;
+}
+static int async_pf_queue_task(u64 token, struct async_pf_sleep_node *n)
+{
+ u64 key = hash_64(token, ASYNC_PF_SLEEP_HASHBITS);
+ struct async_pf_sleep_head *b = &async_pf_sleepers[key];
+ struct async_pf_sleep_node *e;
+
+ raw_spin_lock(&b->lock);
+ e = _find_apf_task(b, token);
+ if (e) {
+ /* dummy entry exist -> wake up was delivered ahead of PF */
+ hlist_del(&e->link);
+ raw_spin_unlock(&b->lock);
+ kfree(e);
+ return false;
+ }
+
+ n->token = token;
+ n->cpu = smp_processor_id();
+ init_swait_queue_head(&n->wq);
+ hlist_add_head(&n->link, &b->list);
+ raw_spin_unlock(&b->lock);
+ return true;
+}
+
+/*
+ * Handle Expropriation notification.
+ */
+int handle_async_page_fault(struct pt_regs *regs, unsigned long addr)
+{
+ struct async_pf_sleep_node n;
+ DECLARE_SWAITQUEUE(wait);
+ unsigned long exp_corr_nr;
+
+ /* Is this Expropriation notification? */
+ if (!(mfspr(SPRN_SRR1) & SRR1_PROGTRAP))
+ return 0;
+
+ if (unlikely(!user_mode(regs)))
+ panic("Host injected async PF in kernel mode\n");
+
+ exp_corr_nr = be16_to_cpu(get_lppaca()->exp_corr_nr);
+ if (!async_pf_queue_task(exp_corr_nr, &n))
+ return 0;
+
+ for (;;) {
+ prepare_to_swait_exclusive(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
+ if (hlist_unhashed(&n.link))
+ break;
+
+ local_irq_enable();
+ schedule();
+ local_irq_disable();
+ }
+
+ finish_swait(&n.wq, &wait);
+ return 1;
+}
+
+static void apf_task_wake_one(struct async_pf_sleep_node *n)
+{
+ hlist_del_init(&n->link);
+ if (swq_has_sleeper(&n->wq))
+ swake_up_one(&n->wq);
+}
+
+static void async_pf_wake_task(u64 token)
+{
+ u64 key = hash_64(token, ASYNC_PF_SLEEP_HASHBITS);
+ struct async_pf_sleep_head *b = &async_pf_sleepers[key];
+ struct async_pf_sleep_node *n;
+
+again:
+ raw_spin_lock(&b->lock);
+ n = _find_apf_task(b, token);
+ if (!n) {
+ /*
+ * async PF was not yet handled.
+ * Add dummy entry for the token.
+ */
+ n = kzalloc(sizeof(*n), GFP_ATOMIC);
+ if (!n) {
+ /*
+ * Allocation failed! Busy wait while other cpu
+ * handles async PF.
+ */
+ raw_spin_unlock(&b->lock);
+ cpu_relax();
+ goto again;
+ }
+ n->token = token;
+ n->cpu = smp_processor_id();
+ init_swait_queue_head(&n->wq);
+ hlist_add_head(&n->link, &b->list);
+ } else {
+ apf_task_wake_one(n);
+ }
+ raw_spin_unlock(&b->lock);
+}
+
+/*
+ * Handle Subvention notification.
+ */
+static irqreturn_t async_pf_handler(int irq, void *dev_id)
+{
+ uint16_t exp_token, old;
+
+ raw_spin_lock(&async_sns_guest_lock);
+ do {
+ exp_token = *(esn_q + next_eq_entry);
+ if (!exp_token)
+ break;
+
+ old = arch_cmpxchg(esn_q + next_eq_entry, exp_token, 0);
+ BUG_ON(old != exp_token);
+
+ async_pf_wake_task(exp_token);
+ next_eq_entry = (next_eq_entry + 1) % nr_eq_entries;
+ } while (1);
+ raw_spin_unlock(&async_sns_guest_lock);
+ return IRQ_HANDLED;
+}
+
+static int __init pseries_async_pf_init(void)
+{
+ long rc;
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ unsigned int irq, cpu;
+ int i;
+
+ /* Register buffer via H_REG_SNS */
+ rc = plpar_hcall(H_REG_SNS, ret, __pa(sns_buffer), PAGE_SIZE);
+ if (rc != H_SUCCESS)
+ return -1;
+
+ nr_eq_entries = (PAGE_SIZE - 2) / sizeof(uint16_t);
+
+ /* Register irq handler */
+ irq = irq_create_mapping(NULL, ret[1]);
+ if (!irq) {
+ plpar_hcall(H_REG_SNS, ret, -1, PAGE_SIZE);
+ return -1;
+ }
+
+ rc = request_irq(irq, async_pf_handler, 0, "sns-interrupt", NULL);
+ if (rc < 0) {
+ plpar_hcall(H_REG_SNS, ret, -1, PAGE_SIZE);
+ return -1;
+ }
+
+ for (i = 0; i < ASYNC_PF_SLEEP_HASHSIZE; i++)
+ raw_spin_lock_init(&async_pf_sleepers[i].lock);
+
+ /*
+ * Enable subvention notifications from the hypervisor
+ * by setting bit 0, byte 0 of SNS buffer
+ */
+ *sns_buffer |= 0x1;
+
+ /* Enable LPPACA_EXP_INT_ENABLED in VPA */
+ for_each_possible_cpu(cpu)
+ lppaca_of(cpu).byte_b9 |= LPPACA_EXP_INT_ENABLED;
+
+ pr_err("%s: Enabled Async PF\n", __func__);
+ return 0;
+}
+
+machine_arch_initcall(pseries, pseries_async_pf_init);
--
2.31.1
^ 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