| Message ID | 20241218182106.78800-3-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/ppc: Remove tswap() calls | expand |
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 {
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 {
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
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 {
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(-)