From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: Eric Auger <eric.auger@redhat.com>,
Markus Armbruster <armbru@redhat.com>
Subject: [PATCH 4/4] hw/arm/smmuv3-internal.h: Don't use locals in statement macros
Date: Fri, 22 Sep 2023 16:29:44 +0100 [thread overview]
Message-ID: <20230922152944.3583438-5-peter.maydell@linaro.org> (raw)
In-Reply-To: <20230922152944.3583438-1-peter.maydell@linaro.org>
The STE_CTXPTR() and STE_S2TTB() macros both extract two halves
of an address from fields in the STE and combine them into a
single value to return. The current code for this uses a GCC
statement expression. There are two problems with this:
(1) The type chosen for the variable in the statement expr
is 'unsigned long', which might not be 64 bits
(2) the name chosen for the variable causes -Wshadow warnings
because it's the same as a variable in use at the callsite:
In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmu_get_cd’:
../../hw/arm/smmuv3-internal.h:538:23: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
538 | unsigned long addr; \
| ^~~~
../../hw/arm/smmuv3.c:339:23: note: in expansion of macro ‘STE_CTXPTR’
339 | dma_addr_t addr = STE_CTXPTR(ste);
| ^~~~~~~~~~
../../hw/arm/smmuv3.c:339:16: note: shadowed declaration is here
339 | dma_addr_t addr = STE_CTXPTR(ste);
| ^~~~
Sidestep both of these problems by just using a single
expression rather than a statement expr.
For CMD_ADDR, we got the type of the variable right but still
run into -Wshadow problems:
In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmuv3_range_inval’:
../../hw/arm/smmuv3-internal.h:334:22: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
334 | uint64_t addr = high << 32 | (low << 12); \
| ^~~~
../../hw/arm/smmuv3.c:1104:28: note: in expansion of macro ‘CMD_ADDR’
1104 | dma_addr_t end, addr = CMD_ADDR(cmd);
| ^~~~~~~~
../../hw/arm/smmuv3.c:1104:21: note: shadowed declaration is here
1104 | dma_addr_t end, addr = CMD_ADDR(cmd);
| ^~~~
so convert it too.
CD_TTB has neither problem, but it is the only other macro in
the file that uses this pattern, so we convert it also for
consistency's sake.
We use extract64() rather than extract32() to avoid having
to explicitly cast the result to uint64_t.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3-internal.h | 41 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 6d1c1edab7b..648c2e37a27 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -328,12 +328,9 @@ enum { /* Command completion notification */
#define CMD_TTL(x) extract32((x)->word[2], 8 , 2)
#define CMD_TG(x) extract32((x)->word[2], 10, 2)
#define CMD_STE_RANGE(x) extract32((x)->word[2], 0 , 5)
-#define CMD_ADDR(x) ({ \
- uint64_t high = (uint64_t)(x)->word[3]; \
- uint64_t low = extract32((x)->word[2], 12, 20); \
- uint64_t addr = high << 32 | (low << 12); \
- addr; \
- })
+#define CMD_ADDR(x) \
+ (((uint64_t)((x)->word[3]) << 32) | \
+ ((extract64((x)->word[2], 12, 20)) << 12))
#define SMMU_FEATURE_2LVL_STE (1 << 0)
@@ -533,21 +530,13 @@ typedef struct CD {
#define STE_S2S(x) extract32((x)->word[5], 25, 1)
#define STE_S2R(x) extract32((x)->word[5], 26, 1)
-#define STE_CTXPTR(x) \
- ({ \
- unsigned long addr; \
- addr = (uint64_t)extract32((x)->word[1], 0, 16) << 32; \
- addr |= (uint64_t)((x)->word[0] & 0xffffffc0); \
- addr; \
- })
+#define STE_CTXPTR(x) \
+ ((extract64((x)->word[1], 0, 16) << 32) | \
+ ((x)->word[0] & 0xffffffc0))
-#define STE_S2TTB(x) \
- ({ \
- unsigned long addr; \
- addr = (uint64_t)extract32((x)->word[7], 0, 16) << 32; \
- addr |= (uint64_t)((x)->word[6] & 0xfffffff0); \
- addr; \
- })
+#define STE_S2TTB(x) \
+ ((extract64((x)->word[7], 0, 16) << 32) | \
+ ((x)->word[6] & 0xfffffff0))
static inline int oas2bits(int oas_field)
{
@@ -585,14 +574,10 @@ static inline int pa_range(STE *ste)
#define CD_VALID(x) extract32((x)->word[0], 31, 1)
#define CD_ASID(x) extract32((x)->word[1], 16, 16)
-#define CD_TTB(x, sel) \
- ({ \
- uint64_t hi, lo; \
- hi = extract32((x)->word[(sel) * 2 + 3], 0, 19); \
- hi <<= 32; \
- lo = (x)->word[(sel) * 2 + 2] & ~0xfULL; \
- hi | lo; \
- })
+#define CD_TTB(x, sel) \
+ ((extract64((x)->word[(sel) * 2 + 3], 0, 19) << 32) | \
+ ((x)->word[(sel) * 2 + 2] & ~0xfULL))
+
#define CD_HAD(x, sel) extract32((x)->word[(sel) * 2 + 2], 1, 1)
#define CD_TSZ(x, sel) extract32((x)->word[0], (16 * (sel)) + 0, 6)
--
2.34.1
next prev parent reply other threads:[~2023-09-22 15:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 15:29 [PATCH 0/4] arm: fix some -Wshadow warnings Peter Maydell
2023-09-22 15:29 ` [PATCH 1/4] hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd() Peter Maydell
2023-09-22 15:38 ` Philippe Mathieu-Daudé
2023-09-26 15:06 ` Eric Auger
2023-09-22 15:29 ` [PATCH 2/4] hw/misc/arm_sysctl.c: Avoid shadowing local variable Peter Maydell
2023-09-22 15:38 ` Philippe Mathieu-Daudé
2023-09-26 15:06 ` Eric Auger
2023-09-22 15:29 ` [PATCH 3/4] hw/arm/smmuv3.c: Avoid shadowing variable Peter Maydell
2023-09-22 15:38 ` Philippe Mathieu-Daudé
2023-09-26 15:00 ` Eric Auger
2023-09-22 15:29 ` Peter Maydell [this message]
2023-09-26 15:00 ` [PATCH 4/4] hw/arm/smmuv3-internal.h: Don't use locals in statement macros Eric Auger
2023-09-29 6:16 ` [PATCH 0/4] arm: fix some -Wshadow warnings Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230922152944.3583438-5-peter.maydell@linaro.org \
--to=peter.maydell@linaro.org \
--cc=armbru@redhat.com \
--cc=eric.auger@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).