* [net-next PATCH v3 0/2] octeontx2-af: fix build warnings flagged
@ 2025-03-11 18:26 Sai Krishna
2025-03-11 18:26 ` [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse Sai Krishna
2025-03-11 18:26 ` [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings " Sai Krishna
0 siblings, 2 replies; 8+ messages in thread
From: Sai Krishna @ 2025-03-11 18:26 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, netdev, linux-kernel, sgoutham,
gakula, lcherian, jerinj, hkelam, sbhatta, andrew+netdev,
bbhushan2, nathan, ndesaulniers, morbo, justinstitt, llvm, horms
Cc: Sai Krishna
There are couple of build warnings observed on x86_64 flagged by Sparse.
Some are inconsistent usage of __iomem annotations and other Sparse
warnings are typecasting related. This patch series fixes the same.
Patch #1 This patch corrects the __iomem annotations flagged by Sparse.
Patch #2 This patch fixes rest of compiler warnings flagged by Sparse
which are related to typecasting to required datatype from
_iomem.
Sai Krishna (2):
octeontx2-af: correct __iomem annotations flagged by Sparse
octeontx2-af: fix compiler warnings flagged by Sparse
---
v3 changes:
Addressed review comments given by Simon Horman
1. Fixed compilation warnings reported by kernel test robot.
2. Addressed build warning fixes into separate patches.
v2 changes:
Addressed review comments given by Jakub Kicinski
Corrected Closes tag which was tampered by mail server.
drivers/net/ethernet/marvell/octeontx2/af/common.h | 2 +-
drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 12 +++++++-----
.../net/ethernet/marvell/octeontx2/nic/otx2_common.c | 10 +++++-----
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 9 ++++-----
4 files changed, 17 insertions(+), 16 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse
2025-03-11 18:26 [net-next PATCH v3 0/2] octeontx2-af: fix build warnings flagged Sai Krishna
@ 2025-03-11 18:26 ` Sai Krishna
2025-03-11 21:22 ` Andrew Lunn
2025-03-11 18:26 ` [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings " Sai Krishna
1 sibling, 1 reply; 8+ messages in thread
From: Sai Krishna @ 2025-03-11 18:26 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, netdev, linux-kernel, sgoutham,
gakula, lcherian, jerinj, hkelam, sbhatta, andrew+netdev,
bbhushan2, nathan, ndesaulniers, morbo, justinstitt, llvm, horms
Cc: Sai Krishna, kernel test robot
Sparse flagged a number of inconsistent usage of __iomem annotations.
This patch fixes some of the issues reported by kernel test robot.
These warning messages are address this by proper __iomem
annotations.
Warning messages flagged by Sparse:
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:611:24: sparse:
sparse: incorrect type in assignment (different address spaces) @@
expected void [noderef] __iomem *hwbase @@ got void * @@
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:611:24:
sparse: expected void [noderef] __iomem *hwbase
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:611:24:
sparse: got void *
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:620:56:
sparse: sparse: cast removes address space '__iomem' of expression
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:671:35:
sparse: sparse: incorrect type in argument 1 (different address spaces)
@@ expected void volatile [noderef] __iomem *addr @@ got void *hwbase @@
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:671:35:
sparse: expected void volatile [noderef] __iomem *addr
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:671:35:
sparse: got void *hwbase
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1344:21:
sparse: sparse: incorrect type in assignment (different address spaces)
@@ expected unsigned long long [usertype] *ptr @@
got void [noderef] __iomem * @@
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1344:21:
sparse: expected unsigned long long [usertype] *ptr
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1344:21:
sparse: got void [noderef] __iomem *
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1383:21:
sparse: sparse: incorrect type in assignment (different address spaces)
@@ expected unsigned long long [usertype] *ptr @@
got void [noderef] __iomem * @@
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1383:21:
sparse: expected unsigned long long [usertype] *ptr
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c:1383:21:
sparse: got void [noderef] __iomem *
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c: note:
in included file
(through drivers/net/ethernet/marvell/octeontx2/af/mbox.h,
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h):
Flagged by Sparse on x86_64:
drivers/net/ethernet/marvell/octeontx2/af/common.h:61:26: sparse:
sparse: cast truncates bits from constant value (10000 becomes 0)
To address this increased the size of entry_sz in qmem_alloc(),
otherwise the value will be truncated to 0.
Reported-by: kernel test robot <lkp@intel.com>
Closes:
https://lore.kernel.org/oe-kbuild-all/202410221614.07o9QVjo-lkp@intel.com/
Signed-off-by: Sai Krishna <saikrishnag@marvell.com>
---
drivers/net/ethernet/marvell/octeontx2/af/common.h | 2 +-
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/common.h b/drivers/net/ethernet/marvell/octeontx2/af/common.h
index 406c59100a35..8a08bebf08c2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/common.h
@@ -39,7 +39,7 @@ struct qmem {
void *base;
dma_addr_t iova;
int alloc_sz;
- u16 entry_sz;
+ u32 entry_sz;
u8 align;
u32 qsize;
};
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index e1dde93e8af8..6c23d64e81f8 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -595,8 +595,7 @@ static int otx2_pfvf_mbox_init(struct otx2_nic *pf, int numvfs)
base = pci_resource_start(pf->pdev, PCI_MBOX_BAR_NUM) +
MBOX_SIZE;
else
- base = readq((void __iomem *)((u64)pf->reg_base +
- RVU_PF_VF_BAR4_ADDR));
+ base = readq(pf->reg_base + RVU_PF_VF_BAR4_ADDR);
hwbase = ioremap_wc(base, MBOX_SIZE * pf->total_vfs);
if (!hwbase) {
@@ -645,7 +644,7 @@ static void otx2_pfvf_mbox_destroy(struct otx2_nic *pf)
}
if (mbox->mbox.hwbase)
- iounmap(mbox->mbox.hwbase);
+ iounmap((void __iomem *)mbox->mbox.hwbase);
otx2_mbox_destroy(&mbox->mbox);
}
@@ -1309,7 +1308,7 @@ static irqreturn_t otx2_q_intr_handler(int irq, void *data)
/* CQ */
for (qidx = 0; qidx < pf->qset.cq_cnt; qidx++) {
- ptr = otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT);
+ ptr = (__force u64 *)otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT);
val = otx2_atomic64_add((qidx << 44), ptr);
otx2_write64(pf, NIX_LF_CQ_OP_INT, (qidx << 44) |
@@ -1348,7 +1347,7 @@ static irqreturn_t otx2_q_intr_handler(int irq, void *data)
* these are fatal errors.
*/
- ptr = otx2_get_regaddr(pf, NIX_LF_SQ_OP_INT);
+ ptr = (__force u64 *)otx2_get_regaddr(pf, NIX_LF_SQ_OP_INT);
val = otx2_atomic64_add((qidx << 44), ptr);
otx2_write64(pf, NIX_LF_SQ_OP_INT, (qidx << 44) |
(val & NIX_SQINT_BITS));
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings flagged by Sparse
2025-03-11 18:26 [net-next PATCH v3 0/2] octeontx2-af: fix build warnings flagged Sai Krishna
2025-03-11 18:26 ` [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse Sai Krishna
@ 2025-03-11 18:26 ` Sai Krishna
2025-03-11 21:32 ` Andrew Lunn
1 sibling, 1 reply; 8+ messages in thread
From: Sai Krishna @ 2025-03-11 18:26 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, netdev, linux-kernel, sgoutham,
gakula, lcherian, jerinj, hkelam, sbhatta, andrew+netdev,
bbhushan2, nathan, ndesaulniers, morbo, justinstitt, llvm, horms
Cc: Sai Krishna
Sparse flagged a number of warnings while typecasting iomem
type to required data type.
For example, fwdata is just a shared memory data structure used
between firmware and kernel, thus remapping and typecasting
to required data type may not cause issue.
Signed-off-by: Sai Krishna <saikrishnag@marvell.com>
---
drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 12 +++++++-----
.../net/ethernet/marvell/octeontx2/nic/otx2_common.c | 10 +++++-----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index cd0d7b7774f1..c3a346cff05e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -819,13 +819,14 @@ static int rvu_fwdata_init(struct rvu *rvu)
goto fail;
BUILD_BUG_ON(offsetof(struct rvu_fwdata, cgx_fw_data) > FWDATA_CGX_LMAC_OFFSET);
- rvu->fwdata = ioremap_wc(fwdbase, sizeof(struct rvu_fwdata));
+ rvu->fwdata = (__force struct rvu_fwdata *)
+ ioremap_wc(fwdbase, sizeof(struct rvu_fwdata));
if (!rvu->fwdata)
goto fail;
if (!is_rvu_fwdata_valid(rvu)) {
dev_err(rvu->dev,
"Mismatch in 'fwdata' struct btw kernel and firmware\n");
- iounmap(rvu->fwdata);
+ iounmap((void __iomem *)rvu->fwdata);
rvu->fwdata = NULL;
return -EINVAL;
}
@@ -838,7 +839,7 @@ static int rvu_fwdata_init(struct rvu *rvu)
static void rvu_fwdata_exit(struct rvu *rvu)
{
if (rvu->fwdata)
- iounmap(rvu->fwdata);
+ iounmap((void __iomem *)rvu->fwdata);
}
static int rvu_setup_nix_hw_resource(struct rvu *rvu, int blkaddr)
@@ -2384,7 +2385,8 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
bar4 = rvupf_read64(rvu, RVU_PF_VF_BAR4_ADDR);
bar4 += region * MBOX_SIZE;
}
- mbox_addr[region] = (void *)ioremap_wc(bar4, MBOX_SIZE);
+ mbox_addr[region] = (__force void *)
+ ioremap_wc(bar4, MBOX_SIZE);
if (!mbox_addr[region])
goto error;
}
@@ -2407,7 +2409,7 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
RVU_AF_PF_BAR4_ADDR);
bar4 += region * MBOX_SIZE;
}
- mbox_addr[region] = (void *)ioremap_wc(bar4, MBOX_SIZE);
+ mbox_addr[region] = (__force void *)ioremap_wc(bar4, MBOX_SIZE);
if (!mbox_addr[region])
goto error;
}
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 2b49bfec7869..e0e592fd02f7 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -29,10 +29,10 @@ static void otx2_nix_rq_op_stats(struct queue_stats *stats,
u64 incr = (u64)qidx << 32;
u64 *ptr;
- ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_OCTS);
+ ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_OCTS);
stats->bytes = otx2_atomic64_add(incr, ptr);
- ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_PKTS);
+ ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_PKTS);
stats->pkts = otx2_atomic64_add(incr, ptr);
}
@@ -42,10 +42,10 @@ static void otx2_nix_sq_op_stats(struct queue_stats *stats,
u64 incr = (u64)qidx << 32;
u64 *ptr;
- ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_OCTS);
+ ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_OCTS);
stats->bytes = otx2_atomic64_add(incr, ptr);
- ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_PKTS);
+ ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_PKTS);
stats->pkts = otx2_atomic64_add(incr, ptr);
}
@@ -853,7 +853,7 @@ void otx2_sqb_flush(struct otx2_nic *pfvf)
struct otx2_snd_queue *sq;
u64 incr, *ptr, val;
- ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_STATUS);
+ ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_STATUS);
for (qidx = 0; qidx < otx2_get_total_tx_queues(pfvf); qidx++) {
sq = &pfvf->qset.sq[qidx];
if (!sq->sqb_ptrs)
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse
2025-03-11 18:26 ` [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse Sai Krishna
@ 2025-03-11 21:22 ` Andrew Lunn
2025-04-14 16:38 ` Sai Krishna Gajula
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2025-03-11 21:22 UTC (permalink / raw)
To: Sai Krishna
Cc: davem, edumazet, kuba, pabeni, netdev, linux-kernel, sgoutham,
gakula, lcherian, jerinj, hkelam, sbhatta, andrew+netdev,
bbhushan2, nathan, ndesaulniers, morbo, justinstitt, llvm, horms,
kernel test robot
> if (mbox->mbox.hwbase)
> - iounmap(mbox->mbox.hwbase);
> + iounmap((void __iomem *)mbox->mbox.hwbase);
This looks wrong. If you are unmapping it, you must of mapped it
somewhere. And that mapping would of returned an __iomem value. So
mbox.hwbase should be an __iomem value and you would not need this
cast.
> for (qidx = 0; qidx < pf->qset.cq_cnt; qidx++) {
> - ptr = otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT);
> + ptr = (__force u64 *)otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT);
> val = otx2_atomic64_add((qidx << 44), ptr);
This also looks questionable. You should be removing casts, not adding
them. otx2_get_regaddr() returns an __iomem. So maybe
otx2_atomic64_add() is actually broken here?
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings flagged by Sparse
2025-03-11 18:26 ` [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings " Sai Krishna
@ 2025-03-11 21:32 ` Andrew Lunn
2025-05-26 9:15 ` Subbaraya Sundeep
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2025-03-11 21:32 UTC (permalink / raw)
To: Sai Krishna
Cc: davem, edumazet, kuba, pabeni, netdev, linux-kernel, sgoutham,
gakula, lcherian, jerinj, hkelam, sbhatta, andrew+netdev,
bbhushan2, nathan, ndesaulniers, morbo, justinstitt, llvm, horms
On Tue, Mar 11, 2025 at 11:56:31PM +0530, Sai Krishna wrote:
> Sparse flagged a number of warnings while typecasting iomem
> type to required data type.
> For example, fwdata is just a shared memory data structure used
> between firmware and kernel, thus remapping and typecasting
> to required data type may not cause issue.
This is generally wrong. __iomem is there for a reason. If you are
removing it, it suggests what you do next with the pointer is wrong.
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse
2025-03-11 21:22 ` Andrew Lunn
@ 2025-04-14 16:38 ` Sai Krishna Gajula
2025-04-14 17:17 ` Andrew Lunn
0 siblings, 1 reply; 8+ messages in thread
From: Sai Krishna Gajula @ 2025-04-14 16:38 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Sunil Kovvuri Goutham,
Geethasowjanya Akula, Linu Cherian, Jerin Jacob, Hariprasad Kelam,
Subbaraya Sundeep Bhatta, andrew+netdev@lunn.ch, Bharat Bhushan,
nathan@kernel.org, ndesaulniers@google.com, morbo@google.com,
justinstitt@google.com, llvm@lists.linux.dev, horms@kernel.org,
kernel test robot
> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, March 12, 2025 2:52 AM
> To: Sai Krishna Gajula <saikrishnag@marvell.com>
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
> <gakula@marvell.com>; Linu Cherian <lcherian@marvell.com>; Jerin Jacob
> <jerinj@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Subbaraya
> Sundeep Bhatta <sbhatta@marvell.com>; andrew+netdev@lunn.ch; Bharat
> Bhushan <bbhushan2@marvell.com>; nathan@kernel.org;
> ndesaulniers@google.com; morbo@google.com; justinstitt@google.com;
> llvm@lists.linux.dev; horms@kernel.org; kernel test robot <lkp@intel.com>
> Subject: Re: [net-next PATCH v3 1/2] octeontx2-af: correct
> __iomem annotations flagged by Sparse
>
> > if (mbox->mbox. hwbase) > - iounmap(mbox->mbox. hwbase); > +
> > iounmap((void __iomem *)mbox->mbox. hwbase); This looks wrong. If you
> > are unmapping it, you must of mapped it somewhere. And that mapping
> > would of returned an __iomem
>
> > if (mbox->mbox.hwbase)
> > - iounmap(mbox->mbox.hwbase);
> > + iounmap((void __iomem *)mbox->mbox.hwbase);
>
> This looks wrong. If you are unmapping it, you must of mapped it
> somewhere. And that mapping would of returned an __iomem value. So
> mbox.hwbase should be an __iomem value and you would not need this
> cast.
Yes, mbox->mbox.hwbase is ioremapped with cache (ioremap_wc), while initialization it is declared as __iomem. But this hwbase is actually a DRAM memory mapped to BAR for better accessibility across the system. Since we use large memcpy (64KB and more) to/from this hwbase, we forced it to use as "void */u64" before exiting with unmap. As this is DRAM memory, memory access will not have side effects. Infact the AI applications also recommended similar mechanism. Please suggest if you have any other view and this can be addressed in some other way.
>
> > for (qidx = 0; qidx < pf->qset.cq_cnt; qidx++) {
> > - ptr = otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT);
> > + ptr = (__force u64 *)otx2_get_regaddr(pf,
> NIX_LF_CQ_OP_INT);
> > val = otx2_atomic64_add((qidx << 44), ptr);
>
> This also looks questionable. You should be removing casts, not adding them.
> otx2_get_regaddr() returns an __iomem. So maybe
> otx2_atomic64_add() is actually broken here?
Similar to the above case, otx2_atomic64_add is a special case where it uses assembly code as part of definition, hence we had to use typecasting the "ptr". Please suggest if there is any better way.
static inline u64 otx2_atomic64_add(u64 incr, u64 *ptr)
{
u64 result;
__asm__ volatile(".cpu generic+lse\n"
"ldadd %x[i], %x[r], [%[b]]"
: [r]"=r"(result), "+m"(*ptr)
: [i]"r"(incr), [b]"r"(ptr)
: "memory");
return result;
}
Apologies for delayed in response.
>
> Andrew
>
> ---
> pw-bot: cr
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse
2025-04-14 16:38 ` Sai Krishna Gajula
@ 2025-04-14 17:17 ` Andrew Lunn
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2025-04-14 17:17 UTC (permalink / raw)
To: Sai Krishna Gajula
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Sunil Kovvuri Goutham,
Geethasowjanya Akula, Linu Cherian, Jerin Jacob, Hariprasad Kelam,
Subbaraya Sundeep Bhatta, andrew+netdev@lunn.ch, Bharat Bhushan,
nathan@kernel.org, ndesaulniers@google.com, morbo@google.com,
justinstitt@google.com, llvm@lists.linux.dev, horms@kernel.org,
kernel test robot
On Mon, Apr 14, 2025 at 04:38:53PM +0000, Sai Krishna Gajula wrote:
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Wednesday, March 12, 2025 2:52 AM
> > To: Sai Krishna Gajula <saikrishnag@marvell.com>
> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
> > <gakula@marvell.com>; Linu Cherian <lcherian@marvell.com>; Jerin Jacob
> > <jerinj@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Subbaraya
> > Sundeep Bhatta <sbhatta@marvell.com>; andrew+netdev@lunn.ch; Bharat
> > Bhushan <bbhushan2@marvell.com>; nathan@kernel.org;
> > ndesaulniers@google.com; morbo@google.com; justinstitt@google.com;
> > llvm@lists.linux.dev; horms@kernel.org; kernel test robot <lkp@intel.com>
> > Subject: Re: [net-next PATCH v3 1/2] octeontx2-af: correct
> > __iomem annotations flagged by Sparse
> >
> > > if (mbox->mbox. hwbase) > - iounmap(mbox->mbox. hwbase); > +
> > > iounmap((void __iomem *)mbox->mbox. hwbase); This looks wrong. If you
> > > are unmapping it, you must of mapped it somewhere. And that mapping
> > > would of returned an __iomem
> >
> > > if (mbox->mbox.hwbase)
> > > - iounmap(mbox->mbox.hwbase);
> > > + iounmap((void __iomem *)mbox->mbox.hwbase);
> >
> > This looks wrong. If you are unmapping it, you must of mapped it
> > somewhere. And that mapping would of returned an __iomem value. So
> > mbox.hwbase should be an __iomem value and you would not need this
> > cast.
> Yes, mbox->mbox.hwbase is ioremapped with cache (ioremap_wc), while initialization it is declared as __iomem. But this hwbase is actually a DRAM memory mapped to BAR for better accessibility across the system. Since we use large memcpy (64KB and more) to/from this hwbase, we forced it to use as "void */u64" before exiting with unmap. As this is DRAM memory, memory access will not have side effects. Infact the AI applications also recommended similar mechanism. Please suggest if you have any other view and this can be addressed in some other way.
Please configure your email client to correctly wrap emails.
Did you check the performance of memcpy_fromio()? That is the API you
are supposed to be using with an __iomem. I _think_ memcpy is only
going to work for some architectures and other architectures will give
you problems. That is the whole point of the __iomem, to make sure you
use the correct API which works across architectures.
> >
> > > for (qidx = 0; qidx < pf->qset.cq_cnt; qidx++) {
> > > - ptr = otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT);
> > > + ptr = (__force u64 *)otx2_get_regaddr(pf,
> > NIX_LF_CQ_OP_INT);
> > > val = otx2_atomic64_add((qidx << 44), ptr);
> >
> > This also looks questionable. You should be removing casts, not adding them.
> > otx2_get_regaddr() returns an __iomem. So maybe
> > otx2_atomic64_add() is actually broken here?
> Similar to the above case, otx2_atomic64_add is a special case where it uses assembly code as part of definition, hence we had to use typecasting the "ptr". Please suggest if there is any better way.
>
> static inline u64 otx2_atomic64_add(u64 incr, u64 *ptr)
> {
> u64 result;
>
Teach this function to accept an __iomem. Worst case, you remove the
cast here, before going into assembly.
> __asm__ volatile(".cpu generic+lse\n"
> "ldadd %x[i], %x[r], [%[b]]"
> : [r]"=r"(result), "+m"(*ptr)
> : [i]"r"(incr), [b]"r"(ptr)
> : "memory");
What actually happens if you keep the attribute? My guess is
nothing. These are sparse markups, and gcc/as never seems them.
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings flagged by Sparse
2025-03-11 21:32 ` Andrew Lunn
@ 2025-05-26 9:15 ` Subbaraya Sundeep
0 siblings, 0 replies; 8+ messages in thread
From: Subbaraya Sundeep @ 2025-05-26 9:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: Sai Krishna, davem, edumazet, kuba, pabeni, netdev, linux-kernel,
sgoutham, gakula, lcherian, jerinj, hkelam, andrew+netdev,
bbhushan2, nathan, ndesaulniers, morbo, justinstitt, llvm, horms
On 2025-03-11 at 21:32:12, Andrew Lunn (andrew@lunn.ch) wrote:
> On Tue, Mar 11, 2025 at 11:56:31PM +0530, Sai Krishna wrote:
> > Sparse flagged a number of warnings while typecasting iomem
> > type to required data type.
>
> > For example, fwdata is just a shared memory data structure used
> > between firmware and kernel, thus remapping and typecasting
> > to required data type may not cause issue.
>
> This is generally wrong. __iomem is there for a reason. If you are
> removing it, it suggests what you do next with the pointer is wrong.
>
> Andrew
Hi Andrew,
Sorry for delay in response. To provide some information, firmware sets
aside some DDR memory region for firmware and kernel communication.
Kernel ioremaps that space and typecasts to fwdata structure and uses it.
Agree that ioremap is for io device's csr space but since we know that it
is not really a register space but DDR we are ioremapping, typecasting
and using it. We assumed __force is there for these kind of exceptions.
Please suggest how to proceed with this. memcpy_fromio can done for fwdata
but this fwdata is NOT read only once structure, firmware keeps updating it in
cases like link speed changes and ethtool eeprom info changes. So everytime
we have to ioremap, memcpy_fromio and iounmap which we want to avoid.
Thanks.
Sundeep
>
> ---
> pw-bot: cr
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-05-26 9:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-11 18:26 [net-next PATCH v3 0/2] octeontx2-af: fix build warnings flagged Sai Krishna
2025-03-11 18:26 ` [net-next PATCH v3 1/2] octeontx2-af: correct __iomem annotations flagged by Sparse Sai Krishna
2025-03-11 21:22 ` Andrew Lunn
2025-04-14 16:38 ` Sai Krishna Gajula
2025-04-14 17:17 ` Andrew Lunn
2025-03-11 18:26 ` [net-next PATCH v3 2/2] octeontx2-af: fix compiler warnings " Sai Krishna
2025-03-11 21:32 ` Andrew Lunn
2025-05-26 9:15 ` Subbaraya Sundeep
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox