* [PATCH v3 0/7] hw/ppc: Remove tswap() calls
@ 2024-12-18 18:20 Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value Philippe Mathieu-Daudé
` (6 more replies)
0 siblings, 7 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:20 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
Remove the tswap() calls on ePAPR, and convert
them to big-endian LD/ST API on sPAPR.
Build-tested only.
Philippe Mathieu-Daudé (7):
meson: Run some compiler checks using -Wno-unused-value
hw/ppc/spapr: Convert HPTE() macro as hpte_get() method
hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method
hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method
hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method
hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method
hw/ppc/epapr: Do not swap ePAPR magic value
meson.build | 11 +++++---
hw/ppc/sam460ex.c | 2 +-
hw/ppc/spapr.c | 63 +++++++++++++++++++++++++++++--------------
hw/ppc/virtex_ml507.c | 2 +-
4 files changed, 52 insertions(+), 26 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-19 0:37 ` Nicholas Piggin
2024-12-18 18:21 ` [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method Philippe Mathieu-Daudé
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
When running Clang static analyzer on macOS I'm getting:
include/qemu/osdep.h:634:8: error: redefinition of 'iovec'
634 | struct iovec {
| ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/_types/_iovec_t.h:31:8: note: previous definition is here
31 | struct iovec {
| ^
1 error generated.
Looking at meson-logs.txt, the analyzer enables -Wunused-value
making meson generated code to fail:
Code:
#include <sys/uio.h>
void bar(void) {
sizeof(struct iovec);
}
-----------
stderr:
meson-private/tmpe8_1b_00/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
3 | sizeof(struct iovec);
| ^~~~~~~~~~~~~~~~~~~~
1 error generated.
-----------
Checking for type "struct iovec" : NO
Code:
#include <utmpx.h>
void bar(void) {
sizeof(struct utmpx);
}
-----------
stderr:
meson-private/tmp3n0u490p/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
3 | sizeof(struct utmpx);
| ^~~~~~~~~~~~~~~~~~~~
1 error generated.
-----------
Checking for type "struct utmpx" : NO
Code:
#include <getopt.h>
int main(void) {
/* If it's not defined as a macro, try to use as a symbol */
#ifndef optreset
optreset;
#endif
return 0;
}
-----------
stderr:
meson-private/tmp1rzob_os/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
6 | optreset;
| ^~~~~~~~
1 error generated.
-----------
Header "getopt.h" has symbol "optreset" : NO
Code:
#include <vmnet/vmnet.h>
int main(void) {
/* If it's not defined as a macro, try to use as a symbol */
#ifndef VMNET_BRIDGED_MODE
VMNET_BRIDGED_MODE;
#endif
return 0;
}
-----------
stderr:
meson-private/tmpl9jgsxpt/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
6 | VMNET_BRIDGED_MODE;
| ^~~~~~~~~~~~~~~~~~
1 error generated.
-----------
Header "vmnet/vmnet.h" has symbol "VMNET_BRIDGED_MODE" with dependency appleframeworks: NO
../meson.build:1174: WARNING: vmnet.framework API is outdated, disabling
Fix by explicitly disabling -Wunused-value from these meson checks.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Probably meson should do that in has_header_symbol() / has_type()?
---
meson.build | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build
index 85f74854735..9d93dcd95d7 100644
--- a/meson.build
+++ b/meson.build
@@ -1189,7 +1189,8 @@ cocoa = dependency('appleframeworks',
vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
'VMNET_BRIDGED_MODE',
- dependencies: vmnet)
+ dependencies: vmnet,
+ args: '-Wno-unused-value')
vmnet = not_found
if get_option('vmnet').enabled()
error('vmnet.framework API is outdated')
@@ -2713,7 +2714,7 @@ config_host_data.set('CONFIG_RTNETLINK',
config_host_data.set('CONFIG_SYSMACROS',
cc.has_header_symbol('sys/sysmacros.h', 'makedev'))
config_host_data.set('HAVE_OPTRESET',
- cc.has_header_symbol('getopt.h', 'optreset'))
+ cc.has_header_symbol('getopt.h', 'optreset', args: '-Wno-unused-value'))
config_host_data.set('HAVE_IPPROTO_MPTCP',
cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
@@ -2731,10 +2732,12 @@ config_host_data.set('HAVE_BLK_ZONE_REP_CAPACITY',
# has_type
config_host_data.set('CONFIG_IOVEC',
cc.has_type('struct iovec',
- prefix: '#include <sys/uio.h>'))
+ prefix: '#include <sys/uio.h>',
+ args: '-Wno-unused-value'))
config_host_data.set('HAVE_UTMPX',
cc.has_type('struct utmpx',
- prefix: '#include <utmpx.h>'))
+ prefix: '#include <utmpx.h>',
+ args: '-Wno-unused-value'))
config_host_data.set('CONFIG_EVENTFD', cc.links('''
#include <sys/eventfd.h>
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-19 0:08 ` Nicholas Piggin
2024-12-19 6:31 ` Harsh Prateek Bora
2024-12-18 18:21 ` [PATCH v3 3/7] hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method Philippe Mathieu-Daudé
` (4 subsequent siblings)
6 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
Convert HPTE() macro as hpte_get() method.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/spapr.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3b022e8da9e..4845bf3244b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1399,7 +1399,13 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
}
}
-#define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2))
+static uint64_t *hpte_get(SpaprMachineState *s, unsigned index)
+{
+ uint64_t *table = s->htab;
+
+ return &table[2 * index];
+}
+
#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID)
#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
@@ -1614,7 +1620,7 @@ int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp)
spapr->htab_shift = shift;
for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
- DIRTY_HPTE(HPTE(spapr->htab, i));
+ DIRTY_HPTE(hpte_get(spapr->htab, i));
}
}
/* We're setting up a hash table, so that means we're not radix */
@@ -2172,7 +2178,7 @@ static void htab_save_chunk(QEMUFile *f, SpaprMachineState *spapr,
qemu_put_be32(f, chunkstart);
qemu_put_be16(f, n_valid);
qemu_put_be16(f, n_invalid);
- qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),
+ qemu_put_buffer(f, (void *)hpte_get(spapr->htab, chunkstart),
HASH_PTE_SIZE_64 * n_valid);
}
@@ -2198,16 +2204,16 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume invalid HPTEs */
while ((index < htabslots)
- && !HPTE_VALID(HPTE(spapr->htab, index))) {
- CLEAN_HPTE(HPTE(spapr->htab, index));
+ && !HPTE_VALID(hpte_get(spapr->htab, index))) {
+ CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
}
/* Consume valid HPTEs */
chunkstart = index;
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
- && HPTE_VALID(HPTE(spapr->htab, index))) {
- CLEAN_HPTE(HPTE(spapr->htab, index));
+ && HPTE_VALID(hpte_get(spapr->htab, index))) {
+ CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
}
@@ -2247,7 +2253,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume non-dirty HPTEs */
while ((index < htabslots)
- && !HPTE_DIRTY(HPTE(spapr->htab, index))) {
+ && !HPTE_DIRTY(hpte_get(spapr->htab, index))) {
index++;
examined++;
}
@@ -2255,9 +2261,9 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
chunkstart = index;
/* Consume valid dirty HPTEs */
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
- && HPTE_DIRTY(HPTE(spapr->htab, index))
- && HPTE_VALID(HPTE(spapr->htab, index))) {
- CLEAN_HPTE(HPTE(spapr->htab, index));
+ && HPTE_DIRTY(hpte_get(spapr->htab, index))
+ && HPTE_VALID(hpte_get(spapr->htab, index))) {
+ CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
examined++;
}
@@ -2265,9 +2271,9 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
invalidstart = index;
/* Consume invalid dirty HPTEs */
while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
- && HPTE_DIRTY(HPTE(spapr->htab, index))
- && !HPTE_VALID(HPTE(spapr->htab, index))) {
- CLEAN_HPTE(HPTE(spapr->htab, index));
+ && HPTE_DIRTY(hpte_get(spapr->htab, index))
+ && !HPTE_VALID(hpte_get(spapr->htab, index))) {
+ CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
examined++;
}
@@ -2449,11 +2455,11 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)
if (spapr->htab) {
if (n_valid) {
- qemu_get_buffer(f, HPTE(spapr->htab, index),
+ qemu_get_buffer(f, (void *)hpte_get(spapr->htab, index),
HASH_PTE_SIZE_64 * n_valid);
}
if (n_invalid) {
- memset(HPTE(spapr->htab, index + n_valid), 0,
+ memset(hpte_get(spapr->htab, index + n_valid), 0,
HASH_PTE_SIZE_64 * n_invalid);
}
} else {
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 3/7] hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-19 0:18 ` Nicholas Piggin
2024-12-18 18:21 ` [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method Philippe Mathieu-Daudé
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
Convert HPTE_VALID() macro as hpte_is_valid() method.
Since sPAPR is in big endian configuration at reset,
use the big endian LD/ST API to access the hash PTEs.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/spapr.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4845bf3244b..b67ab1ee685 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1406,7 +1406,11 @@ static uint64_t *hpte_get(SpaprMachineState *s, unsigned index)
return &table[2 * index];
}
-#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID)
+static bool hpte_is_valid(SpaprMachineState *s, unsigned index)
+{
+ return ldq_be_p(hpte_get(s, index)) & HPTE64_V_VALID;
+}
+
#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
@@ -2204,7 +2208,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume invalid HPTEs */
while ((index < htabslots)
- && !HPTE_VALID(hpte_get(spapr->htab, index))) {
+ && !hpte_is_valid(spapr->htab, index)) {
CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
}
@@ -2212,7 +2216,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume valid HPTEs */
chunkstart = index;
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
- && HPTE_VALID(hpte_get(spapr->htab, index))) {
+ && hpte_is_valid(spapr->htab, index)) {
CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
}
@@ -2262,7 +2266,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume valid dirty HPTEs */
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
&& HPTE_DIRTY(hpte_get(spapr->htab, index))
- && HPTE_VALID(hpte_get(spapr->htab, index))) {
+ && hpte_is_valid(spapr->htab, index)) {
CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
examined++;
@@ -2272,7 +2276,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume invalid dirty HPTEs */
while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
&& HPTE_DIRTY(hpte_get(spapr->htab, index))
- && !HPTE_VALID(hpte_get(spapr->htab, index))) {
+ && !hpte_is_valid(spapr->htab, index)) {
CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
examined++;
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2024-12-18 18:21 ` [PATCH v3 3/7] hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
2024-12-19 6:52 ` Harsh Prateek Bora
2024-12-18 18:21 ` [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method Philippe Mathieu-Daudé
` (2 subsequent siblings)
6 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
Convert HPTE_DIRTY() macro as hpte_is_dirty() method.
Since sPAPR is in big endian configuration at reset,
use the big endian LD/ST API to access the HPTEs.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/spapr.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b67ab1ee685..5bc49598a97 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1411,7 +1411,11 @@ static bool hpte_is_valid(SpaprMachineState *s, unsigned index)
return ldq_be_p(hpte_get(s, index)) & HPTE64_V_VALID;
}
-#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
+static bool hpte_is_dirty(SpaprMachineState *s, unsigned index)
+{
+ return ldq_be_p(hpte_get(s, index)) & HPTE64_V_HPTE_DIRTY;
+}
+
#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
@@ -2257,7 +2261,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume non-dirty HPTEs */
while ((index < htabslots)
- && !HPTE_DIRTY(hpte_get(spapr->htab, index))) {
+ && !hpte_is_dirty(spapr->htab, index)) {
index++;
examined++;
}
@@ -2265,7 +2269,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
chunkstart = index;
/* Consume valid dirty HPTEs */
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
- && HPTE_DIRTY(hpte_get(spapr->htab, index))
+ && hpte_is_dirty(spapr->htab, index)
&& hpte_is_valid(spapr->htab, index)) {
CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
@@ -2275,7 +2279,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
invalidstart = index;
/* Consume invalid dirty HPTEs */
while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
- && HPTE_DIRTY(hpte_get(spapr->htab, index))
+ && hpte_is_dirty(spapr->htab, index)
&& !hpte_is_valid(spapr->htab, index)) {
CLEAN_HPTE(hpte_get(spapr->htab, index));
index++;
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2024-12-18 18:21 ` [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
2024-12-19 6:56 ` Harsh Prateek Bora
2024-12-18 18:21 ` [PATCH v3 6/7] hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value Philippe Mathieu-Daudé
6 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
Convert CLEAN_HPTE() macro as hpte_set_clean() method.
Since sPAPR is in big endian configuration at reset,
use the big endian LD/ST API to access the HPTEs.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/spapr.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5bc49598a97..4e1fe832c29 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1416,7 +1416,12 @@ static bool hpte_is_dirty(SpaprMachineState *s, unsigned index)
return ldq_be_p(hpte_get(s, index)) & HPTE64_V_HPTE_DIRTY;
}
-#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
+static void hpte_set_clean(SpaprMachineState *s, unsigned index)
+{
+ stq_be_p(hpte_get(s, index),
+ ldq_be_p(hpte_get(s, index)) & ~HPTE64_V_HPTE_DIRTY);
+}
+
#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
/*
@@ -2213,7 +2218,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
/* Consume invalid HPTEs */
while ((index < htabslots)
&& !hpte_is_valid(spapr->htab, index)) {
- CLEAN_HPTE(hpte_get(spapr->htab, index));
+ hpte_set_clean(spapr->htab, index);
index++;
}
@@ -2221,7 +2226,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
chunkstart = index;
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
&& hpte_is_valid(spapr->htab, index)) {
- CLEAN_HPTE(hpte_get(spapr->htab, index));
+ hpte_set_clean(spapr->htab, index);
index++;
}
@@ -2271,7 +2276,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
&& hpte_is_dirty(spapr->htab, index)
&& hpte_is_valid(spapr->htab, index)) {
- CLEAN_HPTE(hpte_get(spapr->htab, index));
+ hpte_set_clean(spapr->htab, index);
index++;
examined++;
}
@@ -2281,7 +2286,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
&& hpte_is_dirty(spapr->htab, index)
&& !hpte_is_valid(spapr->htab, index)) {
- CLEAN_HPTE(hpte_get(spapr->htab, index));
+ hpte_set_clean(spapr->htab, index);
index++;
examined++;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 6/7] hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2024-12-18 18:21 ` [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
2024-12-18 18:21 ` [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value Philippe Mathieu-Daudé
6 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
Convert DIRTY_HPTE() macro as hpte_set_dirty() method.
Since sPAPR is in big endian configuration at reset,
use the big endian LD/ST API to access the HPTEs.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/spapr.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4e1fe832c29..dedf6fb2916 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1422,7 +1422,11 @@ static void hpte_set_clean(SpaprMachineState *s, unsigned index)
ldq_be_p(hpte_get(s, index)) & ~HPTE64_V_HPTE_DIRTY);
}
-#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
+static void hpte_set_dirty(SpaprMachineState *s, unsigned index)
+{
+ stq_be_p(hpte_get(s, index),
+ ldq_be_p(hpte_get(s, index)) | HPTE64_V_HPTE_DIRTY);
+}
/*
* Get the fd to access the kernel htab, re-opening it if necessary
@@ -1633,7 +1637,7 @@ int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp)
spapr->htab_shift = shift;
for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
- DIRTY_HPTE(hpte_get(spapr->htab, i));
+ hpte_set_dirty(spapr->htab, i);
}
}
/* We're setting up a hash table, so that means we're not radix */
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2024-12-18 18:21 ` [PATCH v3 6/7] hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method Philippe Mathieu-Daudé
@ 2024-12-18 18:21 ` Philippe Mathieu-Daudé
2024-12-18 19:18 ` BALATON Zoltan
6 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-18 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Harsh Prateek Bora, Nicholas Piggin, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, BALATON Zoltan,
qemu-ppc, Marc-André Lureau, Paolo Bonzini,
Philippe Mathieu-Daudé, Peter Maydell
The ePAPR magic value in $r6 doesn't need to be byte swapped.
See ePAPR-v1.1.pdf chapter 5.4.1 "Boot CPU Initial Register State"
and the following mailing-list thread:
https://lore.kernel.org/qemu-devel/CAFEAcA_NR4XW5DNL4nq7vnH4XRH5UWbhQCxuLyKqYk6_FCBrAA@mail.gmail.com/
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/ppc/sam460ex.c | 2 +-
hw/ppc/virtex_ml507.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 78e2a46e753..db9c8f3fa6e 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -234,7 +234,7 @@ static void main_cpu_reset(void *opaque)
/* Create a mapping for the kernel. */
booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1 << 31);
- env->gpr[6] = tswap32(EPAPR_MAGIC);
+ env->gpr[6] = EPAPR_MAGIC;
env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
} else {
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index f378e5c4a90..6197d31d88f 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -119,7 +119,7 @@ static void main_cpu_reset(void *opaque)
/* Create a mapping spanning the 32bit addr space. */
booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1U << 31);
booke_set_tlb(&env->tlb.tlbe[1], 0x80000000, 0x80000000, 1U << 31);
- env->gpr[6] = tswap32(EPAPR_MAGIC);
+ env->gpr[6] = EPAPR_MAGIC;
env->gpr[7] = bi->ima_size;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value
2024-12-18 18:21 ` [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value Philippe Mathieu-Daudé
@ 2024-12-18 19:18 ` BALATON Zoltan
2024-12-19 0:29 ` Nicholas Piggin
0 siblings, 1 reply; 23+ messages in thread
From: BALATON Zoltan @ 2024-12-18 19:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Harsh Prateek Bora, Nicholas Piggin,
Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, qemu-ppc, Marc-André Lureau,
Paolo Bonzini, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]
On Wed, 18 Dec 2024, Philippe Mathieu-Daudé wrote:
> The ePAPR magic value in $r6 doesn't need to be byte swapped.
>
> See ePAPR-v1.1.pdf chapter 5.4.1 "Boot CPU Initial Register State"
> and the following mailing-list thread:
> https://lore.kernel.org/qemu-devel/CAFEAcA_NR4XW5DNL4nq7vnH4XRH5UWbhQCxuLyKqYk6_FCBrAA@mail.gmail.com/
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/sam460ex.c | 2 +-
> hw/ppc/virtex_ml507.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 78e2a46e753..db9c8f3fa6e 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -234,7 +234,7 @@ static void main_cpu_reset(void *opaque)
>
> /* Create a mapping for the kernel. */
> booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1 << 31);
> - env->gpr[6] = tswap32(EPAPR_MAGIC);
> + env->gpr[6] = EPAPR_MAGIC;
I don't know how to test this (or if anything actually uses it). What I'm
not sure about is what endianness env->gpr is? It's a host array so maybe
it needs to match the host endianness which is little endian most of the
time as opposed to PPC big endian on this machine. So maybe tswap is wrong
but is removing it right? Maybe we need to only swap on LE hosts. I think
it's only used by Linux kernels so maybe trying to boot one could test
this change but I'm not sure.
Regards,
BALATON Zoltan
> env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
>
> } else {
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index f378e5c4a90..6197d31d88f 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -119,7 +119,7 @@ static void main_cpu_reset(void *opaque)
> /* Create a mapping spanning the 32bit addr space. */
> booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1U << 31);
> booke_set_tlb(&env->tlb.tlbe[1], 0x80000000, 0x80000000, 1U << 31);
> - env->gpr[6] = tswap32(EPAPR_MAGIC);
> + env->gpr[6] = EPAPR_MAGIC;
> env->gpr[7] = bi->ima_size;
> }
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method
2024-12-18 18:21 ` [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method Philippe Mathieu-Daudé
@ 2024-12-19 0:08 ` Nicholas Piggin
2024-12-19 6:31 ` Harsh Prateek Bora
1 sibling, 0 replies; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> Convert HPTE() macro as hpte_get() method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Nitpick, could we call this hpte_ptr() or hpte_get_ptr()?
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/ppc/spapr.c | 38 ++++++++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3b022e8da9e..4845bf3244b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1399,7 +1399,13 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
> }
> }
>
> -#define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2))
> +static uint64_t *hpte_get(SpaprMachineState *s, unsigned index)
> +{
> + uint64_t *table = s->htab;
> +
> + return &table[2 * index];
> +}
> +
> #define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID)
> #define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> @@ -1614,7 +1620,7 @@ int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp)
> spapr->htab_shift = shift;
>
> for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
> - DIRTY_HPTE(HPTE(spapr->htab, i));
> + DIRTY_HPTE(hpte_get(spapr->htab, i));
> }
> }
> /* We're setting up a hash table, so that means we're not radix */
> @@ -2172,7 +2178,7 @@ static void htab_save_chunk(QEMUFile *f, SpaprMachineState *spapr,
> qemu_put_be32(f, chunkstart);
> qemu_put_be16(f, n_valid);
> qemu_put_be16(f, n_invalid);
> - qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),
> + qemu_put_buffer(f, (void *)hpte_get(spapr->htab, chunkstart),
> HASH_PTE_SIZE_64 * n_valid);
> }
>
> @@ -2198,16 +2204,16 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume invalid HPTEs */
> while ((index < htabslots)
> - && !HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && !HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> }
>
> /* Consume valid HPTEs */
> chunkstart = index;
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> }
>
> @@ -2247,7 +2253,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume non-dirty HPTEs */
> while ((index < htabslots)
> - && !HPTE_DIRTY(HPTE(spapr->htab, index))) {
> + && !HPTE_DIRTY(hpte_get(spapr->htab, index))) {
> index++;
> examined++;
> }
> @@ -2255,9 +2261,9 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> chunkstart = index;
> /* Consume valid dirty HPTEs */
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_DIRTY(HPTE(spapr->htab, index))
> - && HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> examined++;
> }
> @@ -2265,9 +2271,9 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> invalidstart = index;
> /* Consume invalid dirty HPTEs */
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> - && HPTE_DIRTY(HPTE(spapr->htab, index))
> - && !HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && !HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> examined++;
> }
> @@ -2449,11 +2455,11 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)
>
> if (spapr->htab) {
> if (n_valid) {
> - qemu_get_buffer(f, HPTE(spapr->htab, index),
> + qemu_get_buffer(f, (void *)hpte_get(spapr->htab, index),
> HASH_PTE_SIZE_64 * n_valid);
> }
> if (n_invalid) {
> - memset(HPTE(spapr->htab, index + n_valid), 0,
> + memset(hpte_get(spapr->htab, index + n_valid), 0,
> HASH_PTE_SIZE_64 * n_invalid);
> }
> } else {
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 3/7] hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method
2024-12-18 18:21 ` [PATCH v3 3/7] hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method Philippe Mathieu-Daudé
@ 2024-12-19 0:18 ` Nicholas Piggin
0 siblings, 0 replies; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> Convert HPTE_VALID() macro as hpte_is_valid() method.
> Since sPAPR is in big endian configuration at reset,
> use the big endian LD/ST API to access the hash PTEs.
My knowlege of old platforms isn't great here, but I believe sPAPR
always has big-endian data structures in hardware and hypervisor
interfaces (which encompasses practically everything QEMU should be
concerned with). So patch is good, you could just reword the
changelog to make it clearer.
sPAPR data structures including the hash page table are big-endian
regardless of current CPU endian mode, so use the big-endian LD/ST
API to access the hash PTEs.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/spapr.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4845bf3244b..b67ab1ee685 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1406,7 +1406,11 @@ static uint64_t *hpte_get(SpaprMachineState *s, unsigned index)
> return &table[2 * index];
> }
>
> -#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID)
> +static bool hpte_is_valid(SpaprMachineState *s, unsigned index)
> +{
> + return ldq_be_p(hpte_get(s, index)) & HPTE64_V_VALID;
> +}
With the previous change to hpte_get_ptr(), here we could have a new
uint64_t hpte_get(s, index) helper that calculates the pointer and does
the load?
Otherwise,
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> +
> #define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
> @@ -2204,7 +2208,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume invalid HPTEs */
> while ((index < htabslots)
> - && !HPTE_VALID(hpte_get(spapr->htab, index))) {
> + && !hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> }
> @@ -2212,7 +2216,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
> /* Consume valid HPTEs */
> chunkstart = index;
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_VALID(hpte_get(spapr->htab, index))) {
> + && hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> }
> @@ -2262,7 +2266,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> /* Consume valid dirty HPTEs */
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> && HPTE_DIRTY(hpte_get(spapr->htab, index))
> - && HPTE_VALID(hpte_get(spapr->htab, index))) {
> + && hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> examined++;
> @@ -2272,7 +2276,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> /* Consume invalid dirty HPTEs */
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> && HPTE_DIRTY(hpte_get(spapr->htab, index))
> - && !HPTE_VALID(hpte_get(spapr->htab, index))) {
> + && !hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> examined++;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method
2024-12-18 18:21 ` [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method Philippe Mathieu-Daudé
@ 2024-12-19 0:19 ` Nicholas Piggin
2024-12-19 6:52 ` Harsh Prateek Bora
1 sibling, 0 replies; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> Convert HPTE_DIRTY() macro as hpte_is_dirty() method.
> Since sPAPR is in big endian configuration at reset,
> use the big endian LD/ST API to access the HPTEs.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
All the other helper changes look good +/- suggestions from the
first one.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/ppc/spapr.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b67ab1ee685..5bc49598a97 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1411,7 +1411,11 @@ static bool hpte_is_valid(SpaprMachineState *s, unsigned index)
> return ldq_be_p(hpte_get(s, index)) & HPTE64_V_VALID;
> }
>
> -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
> +static bool hpte_is_dirty(SpaprMachineState *s, unsigned index)
> +{
> + return ldq_be_p(hpte_get(s, index)) & HPTE64_V_HPTE_DIRTY;
> +}
> +
> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
>
> @@ -2257,7 +2261,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume non-dirty HPTEs */
> while ((index < htabslots)
> - && !HPTE_DIRTY(hpte_get(spapr->htab, index))) {
> + && !hpte_is_dirty(spapr->htab, index)) {
> index++;
> examined++;
> }
> @@ -2265,7 +2269,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> chunkstart = index;
> /* Consume valid dirty HPTEs */
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && hpte_is_dirty(spapr->htab, index)
> && hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> @@ -2275,7 +2279,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> invalidstart = index;
> /* Consume invalid dirty HPTEs */
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> - && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && hpte_is_dirty(spapr->htab, index)
> && !hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method
2024-12-18 18:21 ` [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method Philippe Mathieu-Daudé
@ 2024-12-19 0:19 ` Nicholas Piggin
2024-12-19 6:56 ` Harsh Prateek Bora
1 sibling, 0 replies; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> Convert CLEAN_HPTE() macro as hpte_set_clean() method.
> Since sPAPR is in big endian configuration at reset,
> use the big endian LD/ST API to access the HPTEs.
>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/spapr.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5bc49598a97..4e1fe832c29 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1416,7 +1416,12 @@ static bool hpte_is_dirty(SpaprMachineState *s, unsigned index)
> return ldq_be_p(hpte_get(s, index)) & HPTE64_V_HPTE_DIRTY;
> }
>
> -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> +static void hpte_set_clean(SpaprMachineState *s, unsigned index)
> +{
> + stq_be_p(hpte_get(s, index),
> + ldq_be_p(hpte_get(s, index)) & ~HPTE64_V_HPTE_DIRTY);
> +}
> +
> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
>
> /*
> @@ -2213,7 +2218,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
> /* Consume invalid HPTEs */
> while ((index < htabslots)
> && !hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> }
>
> @@ -2221,7 +2226,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
> chunkstart = index;
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> && hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> }
>
> @@ -2271,7 +2276,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> && hpte_is_dirty(spapr->htab, index)
> && hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> examined++;
> }
> @@ -2281,7 +2286,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> && hpte_is_dirty(spapr->htab, index)
> && !hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> examined++;
> }
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/7] hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method
2024-12-18 18:21 ` [PATCH v3 6/7] hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method Philippe Mathieu-Daudé
@ 2024-12-19 0:19 ` Nicholas Piggin
0 siblings, 0 replies; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> Convert DIRTY_HPTE() macro as hpte_set_dirty() method.
> Since sPAPR is in big endian configuration at reset,
> use the big endian LD/ST API to access the HPTEs.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/spapr.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4e1fe832c29..dedf6fb2916 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1422,7 +1422,11 @@ static void hpte_set_clean(SpaprMachineState *s, unsigned index)
> ldq_be_p(hpte_get(s, index)) & ~HPTE64_V_HPTE_DIRTY);
> }
>
> -#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
> +static void hpte_set_dirty(SpaprMachineState *s, unsigned index)
> +{
> + stq_be_p(hpte_get(s, index),
> + ldq_be_p(hpte_get(s, index)) | HPTE64_V_HPTE_DIRTY);
> +}
>
> /*
> * Get the fd to access the kernel htab, re-opening it if necessary
> @@ -1633,7 +1637,7 @@ int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp)
> spapr->htab_shift = shift;
>
> for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
> - DIRTY_HPTE(hpte_get(spapr->htab, i));
> + hpte_set_dirty(spapr->htab, i);
> }
> }
> /* We're setting up a hash table, so that means we're not radix */
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value
2024-12-18 19:18 ` BALATON Zoltan
@ 2024-12-19 0:29 ` Nicholas Piggin
2024-12-19 1:43 ` BALATON Zoltan
0 siblings, 1 reply; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:29 UTC (permalink / raw)
To: BALATON Zoltan, Philippe Mathieu-Daudé
Cc: qemu-devel, Harsh Prateek Bora, Daniel Henrique Barboza,
Edgar E. Iglesias, Daniel P. Berrangé, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 5:18 AM AEST, BALATON Zoltan wrote:
> On Wed, 18 Dec 2024, Philippe Mathieu-Daudé wrote:
> > The ePAPR magic value in $r6 doesn't need to be byte swapped.
> >
> > See ePAPR-v1.1.pdf chapter 5.4.1 "Boot CPU Initial Register State"
> > and the following mailing-list thread:
> > https://lore.kernel.org/qemu-devel/CAFEAcA_NR4XW5DNL4nq7vnH4XRH5UWbhQCxuLyKqYk6_FCBrAA@mail.gmail.com/
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > hw/ppc/sam460ex.c | 2 +-
> > hw/ppc/virtex_ml507.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> > index 78e2a46e753..db9c8f3fa6e 100644
> > --- a/hw/ppc/sam460ex.c
> > +++ b/hw/ppc/sam460ex.c
> > @@ -234,7 +234,7 @@ static void main_cpu_reset(void *opaque)
> >
> > /* Create a mapping for the kernel. */
> > booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1 << 31);
> > - env->gpr[6] = tswap32(EPAPR_MAGIC);
> > + env->gpr[6] = EPAPR_MAGIC;
>
> I don't know how to test this (or if anything actually uses it).
The Linux kernel boot wrapper tests it AFAIKS. Does this mean
they never worked on LE hosts?
> not sure about is what endianness env->gpr is? It's a host array so maybe
> it needs to match the host endianness which is little endian most of the
> time as opposed to PPC big endian on this machine. So maybe tswap is wrong
> but is removing it right? Maybe we need to only swap on LE hosts. I think
> it's only used by Linux kernels so maybe trying to boot one could test
> this change but I'm not sure.
Yes env->gpr is host-endian, and emulated target code will see
the same value that the host does. It can't be correct because
the machine running in emulation can't possibly know what endian
the host is.
I think we should just take it, it looks trivially correct
(always dangerous words when dealing with early boot code, lol).
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Regards,
> BALATON Zoltan
>
> > env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
> >
> > } else {
> > diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> > index f378e5c4a90..6197d31d88f 100644
> > --- a/hw/ppc/virtex_ml507.c
> > +++ b/hw/ppc/virtex_ml507.c
> > @@ -119,7 +119,7 @@ static void main_cpu_reset(void *opaque)
> > /* Create a mapping spanning the 32bit addr space. */
> > booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1U << 31);
> > booke_set_tlb(&env->tlb.tlbe[1], 0x80000000, 0x80000000, 1U << 31);
> > - env->gpr[6] = tswap32(EPAPR_MAGIC);
> > + env->gpr[6] = EPAPR_MAGIC;
> > env->gpr[7] = bi->ima_size;
> > }
> >
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value
2024-12-18 18:21 ` [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value Philippe Mathieu-Daudé
@ 2024-12-19 0:37 ` Nicholas Piggin
2024-12-19 17:39 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 23+ messages in thread
From: Nicholas Piggin @ 2024-12-19 0:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> When running Clang static analyzer on macOS I'm getting:
>
> include/qemu/osdep.h:634:8: error: redefinition of 'iovec'
> 634 | struct iovec {
> | ^
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/_types/_iovec_t.h:31:8: note: previous definition is here
> 31 | struct iovec {
> | ^
> 1 error generated.
>
> Looking at meson-logs.txt, the analyzer enables -Wunused-value
> making meson generated code to fail:
>
> Code:
> #include <sys/uio.h>
> void bar(void) {
> sizeof(struct iovec);
> }
> -----------
> stderr:
> meson-private/tmpe8_1b_00/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
> 3 | sizeof(struct iovec);
> | ^~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> -----------
> Checking for type "struct iovec" : NO
>
> Code:
> #include <utmpx.h>
> void bar(void) {
> sizeof(struct utmpx);
> }
> -----------
> stderr:
> meson-private/tmp3n0u490p/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
> 3 | sizeof(struct utmpx);
> | ^~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> -----------
> Checking for type "struct utmpx" : NO
>
> Code:
>
> #include <getopt.h>
> int main(void) {
> /* If it's not defined as a macro, try to use as a symbol */
> #ifndef optreset
> optreset;
> #endif
> return 0;
> }
> -----------
> stderr:
> meson-private/tmp1rzob_os/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
> 6 | optreset;
> | ^~~~~~~~
> 1 error generated.
> -----------
> Header "getopt.h" has symbol "optreset" : NO
>
> Code:
>
> #include <vmnet/vmnet.h>
> int main(void) {
> /* If it's not defined as a macro, try to use as a symbol */
> #ifndef VMNET_BRIDGED_MODE
> VMNET_BRIDGED_MODE;
> #endif
> return 0;
> }
> -----------
> stderr:
> meson-private/tmpl9jgsxpt/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
> 6 | VMNET_BRIDGED_MODE;
> | ^~~~~~~~~~~~~~~~~~
> 1 error generated.
> -----------
> Header "vmnet/vmnet.h" has symbol "VMNET_BRIDGED_MODE" with dependency appleframeworks: NO
> ../meson.build:1174: WARNING: vmnet.framework API is outdated, disabling
>
> Fix by explicitly disabling -Wunused-value from these meson checks.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Probably meson should do that in has_header_symbol() / has_type()?
I don't know about the build system to answer this, but should we
instead disable -Werror on these tests to be a bit more future-proof?
Compilers often add new warnings or catch more cases of existing
warnings.
Alternative would be to keep -Werror but fail the build if a test
throws a warning, but that seems like a lot more work for little
benefit...
Thanks,
Nick
> ---
> meson.build | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 85f74854735..9d93dcd95d7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1189,7 +1189,8 @@ cocoa = dependency('appleframeworks',
> vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
> if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
> 'VMNET_BRIDGED_MODE',
> - dependencies: vmnet)
> + dependencies: vmnet,
> + args: '-Wno-unused-value')
> vmnet = not_found
> if get_option('vmnet').enabled()
> error('vmnet.framework API is outdated')
> @@ -2713,7 +2714,7 @@ config_host_data.set('CONFIG_RTNETLINK',
> config_host_data.set('CONFIG_SYSMACROS',
> cc.has_header_symbol('sys/sysmacros.h', 'makedev'))
> config_host_data.set('HAVE_OPTRESET',
> - cc.has_header_symbol('getopt.h', 'optreset'))
> + cc.has_header_symbol('getopt.h', 'optreset', args: '-Wno-unused-value'))
> config_host_data.set('HAVE_IPPROTO_MPTCP',
> cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
>
> @@ -2731,10 +2732,12 @@ config_host_data.set('HAVE_BLK_ZONE_REP_CAPACITY',
> # has_type
> config_host_data.set('CONFIG_IOVEC',
> cc.has_type('struct iovec',
> - prefix: '#include <sys/uio.h>'))
> + prefix: '#include <sys/uio.h>',
> + args: '-Wno-unused-value'))
> config_host_data.set('HAVE_UTMPX',
> cc.has_type('struct utmpx',
> - prefix: '#include <utmpx.h>'))
> + prefix: '#include <utmpx.h>',
> + args: '-Wno-unused-value'))
>
> config_host_data.set('CONFIG_EVENTFD', cc.links('''
> #include <sys/eventfd.h>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value
2024-12-19 0:29 ` Nicholas Piggin
@ 2024-12-19 1:43 ` BALATON Zoltan
0 siblings, 0 replies; 23+ messages in thread
From: BALATON Zoltan @ 2024-12-19 1:43 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Philippe Mathieu-Daudé, qemu-devel, Harsh Prateek Bora,
Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, qemu-ppc, Marc-André Lureau,
Paolo Bonzini, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 3020 bytes --]
On Thu, 19 Dec 2024, Nicholas Piggin wrote:
> On Thu Dec 19, 2024 at 5:18 AM AEST, BALATON Zoltan wrote:
>> On Wed, 18 Dec 2024, Philippe Mathieu-Daudé wrote:
>>> The ePAPR magic value in $r6 doesn't need to be byte swapped.
>>>
>>> See ePAPR-v1.1.pdf chapter 5.4.1 "Boot CPU Initial Register State"
>>> and the following mailing-list thread:
>>> https://lore.kernel.org/qemu-devel/CAFEAcA_NR4XW5DNL4nq7vnH4XRH5UWbhQCxuLyKqYk6_FCBrAA@mail.gmail.com/
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/ppc/sam460ex.c | 2 +-
>>> hw/ppc/virtex_ml507.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
>>> index 78e2a46e753..db9c8f3fa6e 100644
>>> --- a/hw/ppc/sam460ex.c
>>> +++ b/hw/ppc/sam460ex.c
>>> @@ -234,7 +234,7 @@ static void main_cpu_reset(void *opaque)
>>>
>>> /* Create a mapping for the kernel. */
>>> booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1 << 31);
>>> - env->gpr[6] = tswap32(EPAPR_MAGIC);
>>> + env->gpr[6] = EPAPR_MAGIC;
>>
>> I don't know how to test this (or if anything actually uses it).
>
> The Linux kernel boot wrapper tests it AFAIKS. Does this mean
> they never worked on LE hosts?
Linux boots on the sam460ex on LE host, I don't know if it works on BE
host as I don't have any so this could be broken on BE host but should
work on LE host. If this breaks Linux boot somebody may report it, I don't
have time to try testing it now.
Regards,
BALATON Zoltan
>> not sure about is what endianness env->gpr is? It's a host array so maybe
>> it needs to match the host endianness which is little endian most of the
>> time as opposed to PPC big endian on this machine. So maybe tswap is wrong
>> but is removing it right? Maybe we need to only swap on LE hosts. I think
>> it's only used by Linux kernels so maybe trying to boot one could test
>> this change but I'm not sure.
>
> Yes env->gpr is host-endian, and emulated target code will see
> the same value that the host does. It can't be correct because
> the machine running in emulation can't possibly know what endian
> the host is.
>
> I think we should just take it, it looks trivially correct
> (always dangerous words when dealing with early boot code, lol).
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
>>
>> Regards,
>> BALATON Zoltan
>>
>>> env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
>>>
>>> } else {
>>> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
>>> index f378e5c4a90..6197d31d88f 100644
>>> --- a/hw/ppc/virtex_ml507.c
>>> +++ b/hw/ppc/virtex_ml507.c
>>> @@ -119,7 +119,7 @@ static void main_cpu_reset(void *opaque)
>>> /* Create a mapping spanning the 32bit addr space. */
>>> booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1U << 31);
>>> booke_set_tlb(&env->tlb.tlbe[1], 0x80000000, 0x80000000, 1U << 31);
>>> - env->gpr[6] = tswap32(EPAPR_MAGIC);
>>> + env->gpr[6] = EPAPR_MAGIC;
>>> env->gpr[7] = bi->ima_size;
>>> }
>>>
>>>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method
2024-12-18 18:21 ` [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method Philippe Mathieu-Daudé
2024-12-19 0:08 ` Nicholas Piggin
@ 2024-12-19 6:31 ` Harsh Prateek Bora
2024-12-20 21:29 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 23+ messages in thread
From: Harsh Prateek Bora @ 2024-12-19 6:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Nicholas Piggin, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
Hi Philippe,
On 12/18/24 23:51, Philippe Mathieu-Daudé wrote:
> Convert HPTE() macro as hpte_get() method.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/spapr.c | 38 ++++++++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3b022e8da9e..4845bf3244b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1399,7 +1399,13 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
> }
> }
>
> -#define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2))
> +static uint64_t *hpte_get(SpaprMachineState *s, unsigned index)
> +{
> + uint64_t *table = s->htab;
> +
> + return &table[2 * index];
> +}
> +
> #define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID)
> #define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> @@ -1614,7 +1620,7 @@ int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp)
> spapr->htab_shift = shift;
>
> for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
> - DIRTY_HPTE(HPTE(spapr->htab, i));
> + DIRTY_HPTE(hpte_get(spapr->htab, i));
Prev HPTE expected _table i.e. spapr->htab as arg, but this new hpte_get
expects SpaprMachineState* i.e. spapr as arg. Shouldn't the arg be
updated accordingly? Wondering it didnt complain during build.
As Nick suggested, hpte_get_ptr or get_hpte_ptr may be better renaming.
regards,
Harsh
> }
> }
> /* We're setting up a hash table, so that means we're not radix */
> @@ -2172,7 +2178,7 @@ static void htab_save_chunk(QEMUFile *f, SpaprMachineState *spapr,
> qemu_put_be32(f, chunkstart);
> qemu_put_be16(f, n_valid);
> qemu_put_be16(f, n_invalid);
> - qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),
> + qemu_put_buffer(f, (void *)hpte_get(spapr->htab, chunkstart),
> HASH_PTE_SIZE_64 * n_valid);
> }
>
> @@ -2198,16 +2204,16 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume invalid HPTEs */
> while ((index < htabslots)
> - && !HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && !HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> }
>
> /* Consume valid HPTEs */
> chunkstart = index;
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> }
>
> @@ -2247,7 +2253,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume non-dirty HPTEs */
> while ((index < htabslots)
> - && !HPTE_DIRTY(HPTE(spapr->htab, index))) {
> + && !HPTE_DIRTY(hpte_get(spapr->htab, index))) {
> index++;
> examined++;
> }
> @@ -2255,9 +2261,9 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> chunkstart = index;
> /* Consume valid dirty HPTEs */
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_DIRTY(HPTE(spapr->htab, index))
> - && HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> examined++;
> }
> @@ -2265,9 +2271,9 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> invalidstart = index;
> /* Consume invalid dirty HPTEs */
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> - && HPTE_DIRTY(HPTE(spapr->htab, index))
> - && !HPTE_VALID(HPTE(spapr->htab, index))) {
> - CLEAN_HPTE(HPTE(spapr->htab, index));
> + && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && !HPTE_VALID(hpte_get(spapr->htab, index))) {
> + CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> examined++;
> }
> @@ -2449,11 +2455,11 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)
>
> if (spapr->htab) {
> if (n_valid) {
> - qemu_get_buffer(f, HPTE(spapr->htab, index),
> + qemu_get_buffer(f, (void *)hpte_get(spapr->htab, index),
> HASH_PTE_SIZE_64 * n_valid);
> }
> if (n_invalid) {
> - memset(HPTE(spapr->htab, index + n_valid), 0,
> + memset(hpte_get(spapr->htab, index + n_valid), 0,
> HASH_PTE_SIZE_64 * n_invalid);
> }
> } else {
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method
2024-12-18 18:21 ` [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
@ 2024-12-19 6:52 ` Harsh Prateek Bora
1 sibling, 0 replies; 23+ messages in thread
From: Harsh Prateek Bora @ 2024-12-19 6:52 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Nicholas Piggin, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
Hi Philippe,
Similar issue here as with patch 2 ..
On 12/18/24 23:51, Philippe Mathieu-Daudé wrote:
> Convert HPTE_DIRTY() macro as hpte_is_dirty() method.
> Since sPAPR is in big endian configuration at reset,
> use the big endian LD/ST API to access the HPTEs.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/spapr.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b67ab1ee685..5bc49598a97 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1411,7 +1411,11 @@ static bool hpte_is_valid(SpaprMachineState *s, unsigned index)
> return ldq_be_p(hpte_get(s, index)) & HPTE64_V_VALID;
> }
>
> -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
> +static bool hpte_is_dirty(SpaprMachineState *s, unsigned index)
> +{
> + return ldq_be_p(hpte_get(s, index)) & HPTE64_V_HPTE_DIRTY;
> +}
> +
> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
>
> @@ -2257,7 +2261,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
>
> /* Consume non-dirty HPTEs */
> while ((index < htabslots)
> - && !HPTE_DIRTY(hpte_get(spapr->htab, index))) {
> + && !hpte_is_dirty(spapr->htab, index)) {
hpte_is_dirty expects SpaprMachineState * as arg, need to update
accordingly.
regards,
Harsh
> index++;
> examined++;
> }
> @@ -2265,7 +2269,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> chunkstart = index;
> /* Consume valid dirty HPTEs */
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> - && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && hpte_is_dirty(spapr->htab, index)
> && hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
> @@ -2275,7 +2279,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> invalidstart = index;
> /* Consume invalid dirty HPTEs */
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> - && HPTE_DIRTY(hpte_get(spapr->htab, index))
> + && hpte_is_dirty(spapr->htab, index)
> && !hpte_is_valid(spapr->htab, index)) {
> CLEAN_HPTE(hpte_get(spapr->htab, index));
> index++;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method
2024-12-18 18:21 ` [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
@ 2024-12-19 6:56 ` Harsh Prateek Bora
1 sibling, 0 replies; 23+ messages in thread
From: Harsh Prateek Bora @ 2024-12-19 6:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Nicholas Piggin, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On 12/18/24 23:51, Philippe Mathieu-Daudé wrote:
> Convert CLEAN_HPTE() macro as hpte_set_clean() method.
> Since sPAPR is in big endian configuration at reset,
> use the big endian LD/ST API to access the HPTEs.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/spapr.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5bc49598a97..4e1fe832c29 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1416,7 +1416,12 @@ static bool hpte_is_dirty(SpaprMachineState *s, unsigned index)
> return ldq_be_p(hpte_get(s, index)) & HPTE64_V_HPTE_DIRTY;
> }
>
> -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
> +static void hpte_set_clean(SpaprMachineState *s, unsigned index)
> +{
> + stq_be_p(hpte_get(s, index),
> + ldq_be_p(hpte_get(s, index)) & ~HPTE64_V_HPTE_DIRTY);
> +}
> +
> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
>
> /*
> @@ -2213,7 +2218,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
> /* Consume invalid HPTEs */
> while ((index < htabslots)
> && !hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
Ditto. I see patch 6 also have similar issue. Please take care.
Thanks
Harsh
> index++;
> }
>
> @@ -2221,7 +2226,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
> chunkstart = index;
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> && hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> }
>
> @@ -2271,7 +2276,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> && hpte_is_dirty(spapr->htab, index)
> && hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> examined++;
> }
> @@ -2281,7 +2286,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
> while ((index < htabslots) && (index - invalidstart < USHRT_MAX)
> && hpte_is_dirty(spapr->htab, index)
> && !hpte_is_valid(spapr->htab, index)) {
> - CLEAN_HPTE(hpte_get(spapr->htab, index));
> + hpte_set_clean(spapr->htab, index);
> index++;
> examined++;
> }
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value
2024-12-19 0:37 ` Nicholas Piggin
@ 2024-12-19 17:39 ` Philippe Mathieu-Daudé
2024-12-19 18:14 ` Richard Henderson
0 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-19 17:39 UTC (permalink / raw)
To: Nicholas Piggin, qemu-devel
Cc: Harsh Prateek Bora, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On 19/12/24 01:37, Nicholas Piggin wrote:
> On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
>> When running Clang static analyzer on macOS I'm getting:
>>
>> include/qemu/osdep.h:634:8: error: redefinition of 'iovec'
>> 634 | struct iovec {
>> | ^
>> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/_types/_iovec_t.h:31:8: note: previous definition is here
>> 31 | struct iovec {
>> | ^
>> 1 error generated.
>>
>> Looking at meson-logs.txt, the analyzer enables -Wunused-value
>> making meson generated code to fail:
>>
>> Code:
>> #include <sys/uio.h>
>> void bar(void) {
>> sizeof(struct iovec);
>> }
>> -----------
>> stderr:
>> meson-private/tmpe8_1b_00/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
>> 3 | sizeof(struct iovec);
>> | ^~~~~~~~~~~~~~~~~~~~
>> 1 error generated.
>> -----------
>> Checking for type "struct iovec" : NO
>>
>> Code:
>> #include <utmpx.h>
>> void bar(void) {
>> sizeof(struct utmpx);
>> }
>> -----------
>> stderr:
>> meson-private/tmp3n0u490p/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
>> 3 | sizeof(struct utmpx);
>> | ^~~~~~~~~~~~~~~~~~~~
>> 1 error generated.
>> -----------
>> Checking for type "struct utmpx" : NO
>>
>> Code:
>>
>> #include <getopt.h>
>> int main(void) {
>> /* If it's not defined as a macro, try to use as a symbol */
>> #ifndef optreset
>> optreset;
>> #endif
>> return 0;
>> }
>> -----------
>> stderr:
>> meson-private/tmp1rzob_os/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>> 6 | optreset;
>> | ^~~~~~~~
>> 1 error generated.
>> -----------
>> Header "getopt.h" has symbol "optreset" : NO
>>
>> Code:
>>
>> #include <vmnet/vmnet.h>
>> int main(void) {
>> /* If it's not defined as a macro, try to use as a symbol */
>> #ifndef VMNET_BRIDGED_MODE
>> VMNET_BRIDGED_MODE;
>> #endif
>> return 0;
>> }
>> -----------
>> stderr:
>> meson-private/tmpl9jgsxpt/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>> 6 | VMNET_BRIDGED_MODE;
>> | ^~~~~~~~~~~~~~~~~~
>> 1 error generated.
>> -----------
>> Header "vmnet/vmnet.h" has symbol "VMNET_BRIDGED_MODE" with dependency appleframeworks: NO
>> ../meson.build:1174: WARNING: vmnet.framework API is outdated, disabling
>>
>> Fix by explicitly disabling -Wunused-value from these meson checks.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: Probably meson should do that in has_header_symbol() / has_type()?
>
> I don't know about the build system to answer this, but should we
> instead disable -Werror on these tests to be a bit more future-proof?
> Compilers often add new warnings or catch more cases of existing
> warnings.
Sorry, I didn't mean to include this patch in this series. I happen
to have my series on top of it and forgot to change the base commit.
> Alternative would be to keep -Werror but fail the build if a test
> throws a warning, but that seems like a lot more work for little
> benefit...
I'm trying to fix it on the meson side with this:
-- >8 --
diff --git a/mesonbuild/compilers/mixins/clike.py
b/mesonbuild/compilers/mixins/clike.py
index d56547b47..9d6957973 100644
--- a/mesonbuild/compilers/mixins/clike.py
+++ b/mesonbuild/compilers/mixins/clike.py
@@ -360,7 +360,7 @@ class CLikeCompiler(Compiler):
int main(void) {{
/* If it's not defined as a macro, try to use as a symbol */
#ifndef {symbol}
- {symbol};
+ (void) {symbol};
#endif
return 0;
}}'''
@@ -885,7 +885,8 @@ class CLikeCompiler(Compiler):
dependencies: T.Optional[T.List['Dependency']] =
None) -> T.Tuple[bool, bool]:
t = f'''{prefix}
void bar(void) {{
- (void) sizeof({typename});
+ size_t foo = sizeof({typename});
+ (void) foo;
}}'''
return self.compiles(t, env, extra_args=extra_args,
dependencies=dependencies)
---
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value
2024-12-19 17:39 ` Philippe Mathieu-Daudé
@ 2024-12-19 18:14 ` Richard Henderson
0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2024-12-19 18:14 UTC (permalink / raw)
To: qemu-devel
On 12/19/24 09:39, Philippe Mathieu-Daudé wrote:
> @@ -885,7 +885,8 @@ class CLikeCompiler(Compiler):
> dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool,
> bool]:
> t = f'''{prefix}
> void bar(void) {{
> - (void) sizeof({typename});
> + size_t foo = sizeof({typename});
> + (void) foo;
> }}'''
As I mentioned elsewhere, sizeof is a compile-time constant.
The function wrapper is getting in the way. This can be just
unsigned long foo = sizeof({typename});
I.e. initialization of a global variable, with no need for <stddef.h> or any other system
header for size_t.
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method
2024-12-19 6:31 ` Harsh Prateek Bora
@ 2024-12-20 21:29 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-12-20 21:29 UTC (permalink / raw)
To: Harsh Prateek Bora, qemu-devel
Cc: Nicholas Piggin, Daniel Henrique Barboza, Edgar E. Iglesias,
Daniel P. Berrangé, BALATON Zoltan, qemu-ppc,
Marc-André Lureau, Paolo Bonzini, Peter Maydell
On 19/12/24 07:31, Harsh Prateek Bora wrote:
> Hi Philippe,
>
> On 12/18/24 23:51, Philippe Mathieu-Daudé wrote:
>> Convert HPTE() macro as hpte_get() method.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/ppc/spapr.c | 38 ++++++++++++++++++++++----------------
>> 1 file changed, 22 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 3b022e8da9e..4845bf3244b 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1399,7 +1399,13 @@ static bool spapr_get_pate(PPCVirtualHypervisor
>> *vhyp, PowerPCCPU *cpu,
>> }
>> }
>> -#define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2))
>> +static uint64_t *hpte_get(SpaprMachineState *s, unsigned index)
>> +{
>> + uint64_t *table = s->htab;
>> +
>> + return &table[2 * index];
>> +}
>> +
>> #define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) &
>> HPTE64_V_VALID)
>> #define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) &
>> HPTE64_V_HPTE_DIRTY)
>> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &=
>> tswap64(~HPTE64_V_HPTE_DIRTY))
>> @@ -1614,7 +1620,7 @@ int spapr_reallocate_hpt(SpaprMachineState
>> *spapr, int shift, Error **errp)
>> spapr->htab_shift = shift;
>> for (i = 0; i < size / HASH_PTE_SIZE_64; i++) {
>> - DIRTY_HPTE(HPTE(spapr->htab, i));
>> + DIRTY_HPTE(hpte_get(spapr->htab, i));
>
> Prev HPTE expected _table i.e. spapr->htab as arg, but this new hpte_get
> expects SpaprMachineState* i.e. spapr as arg. Shouldn't the arg be
> updated accordingly?
Good catch!
> Wondering it didnt complain during build.
Because the macros blindly cast, dropping the type checks.
>
> As Nick suggested, hpte_get_ptr or get_hpte_ptr may be better renaming.
Sure. Thanks!
>
> regards,
> Harsh
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-12-20 21:30 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-18 18:20 [PATCH v3 0/7] hw/ppc: Remove tswap() calls Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 1/7] meson: Run some compiler checks using -Wno-unused-value Philippe Mathieu-Daudé
2024-12-19 0:37 ` Nicholas Piggin
2024-12-19 17:39 ` Philippe Mathieu-Daudé
2024-12-19 18:14 ` Richard Henderson
2024-12-18 18:21 ` [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method Philippe Mathieu-Daudé
2024-12-19 0:08 ` Nicholas Piggin
2024-12-19 6:31 ` Harsh Prateek Bora
2024-12-20 21:29 ` Philippe Mathieu-Daudé
2024-12-18 18:21 ` [PATCH v3 3/7] hw/ppc/spapr: Convert HPTE_VALID() macro as hpte_is_valid() method Philippe Mathieu-Daudé
2024-12-19 0:18 ` Nicholas Piggin
2024-12-18 18:21 ` [PATCH v3 4/7] hw/ppc/spapr: Convert HPTE_DIRTY() macro as hpte_is_dirty() method Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
2024-12-19 6:52 ` Harsh Prateek Bora
2024-12-18 18:21 ` [PATCH v3 5/7] hw/ppc/spapr: Convert CLEAN_HPTE() macro as hpte_set_clean() method Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
2024-12-19 6:56 ` Harsh Prateek Bora
2024-12-18 18:21 ` [PATCH v3 6/7] hw/ppc/spapr: Convert DIRTY_HPTE() macro as hpte_set_dirty() method Philippe Mathieu-Daudé
2024-12-19 0:19 ` Nicholas Piggin
2024-12-18 18:21 ` [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value Philippe Mathieu-Daudé
2024-12-18 19:18 ` BALATON Zoltan
2024-12-19 0:29 ` Nicholas Piggin
2024-12-19 1:43 ` BALATON Zoltan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.