Re: [PATCH v3 2/7] hw/ppc/spapr: Convert HPTE() macro as hpte_get() method

[Philippe Mathieu-Daudé]

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