Re: [PULL 02/16] target/alpha: Use explicit little-endian LD/ST API

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> On 1/3/26 15:51, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> > > The Alpha architecture uses little endianness. Directly
> > 
> > Wasn't there one, odd case of the T3E running it big-endian?
> > (I have no idea how that worked in practice).
> 
> Richard said it was not necessary to mention it in the code:
> https://lore.kernel.org/qemu-devel/20260106155755.53646-7-philmd@linaro.org/
> https://lore.kernel.org/qemu-devel/75b13774-93f3-45db-bc1a-5b8687fcb3b9@linaro.org/
> 
> I could have mentioned it here in the commit description
> for clarity but didn't think about it...

Ah right, didn't spot that review.

Thanks for the reply,

Dave

> > 
> > Dave
> > 
> > > use the little-endian LD/ST API.
> > > 
> > > Mechanical change running:
> > > 
> > >    $ for a in uw w l q; do \
> > >        sed -i -e "s/ld${a}_p(/ld${a}_le_p(/" \
> > >          $(git grep -wlE '(ld|st)u?[wlq]_p' target/alpha/);
> > >      done
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > > Message-Id: <20251224160040.88612-2-philmd@linaro.org>
> > > ---
> > >   target/alpha/helper.c | 11 ++++++-----
> > >   1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> > > index a9af52a928f..80542cb0665 100644
> > > --- a/target/alpha/helper.c
> > > +++ b/target/alpha/helper.c
> > > @@ -214,17 +214,18 @@ static int get_physical_address(CPUAlphaState *env, target_ulong addr,
> > >       pt = env->ptbr;
> > > -    /* TODO: rather than using ldq_phys() to read the page table we should
> > > +    /*
> > > +     * TODO: rather than using ldq_phys_le() to read the page table we should
> > >        * use address_space_ldq() so that we can handle the case when
> > >        * the page table read gives a bus fault, rather than ignoring it.
> > > -     * For the existing code the zero data that ldq_phys will return for
> > > +     * For the existing code the zero data that ldq_phys_le will return for
> > >        * an access to invalid memory will result in our treating the page
> > >        * table as invalid, which may even be the right behaviour.
> > >        */
> > >       /* L1 page table read.  */
> > >       index = (addr >> (TARGET_PAGE_BITS + 20)) & 0x3ff;
> > > -    L1pte = ldq_phys(cs->as, pt + index*8);
> > > +    L1pte = ldq_phys_le(cs->as, pt + index * 8);
> > >       if (unlikely((L1pte & PTE_VALID) == 0)) {
> > >           ret = MM_K_TNV;
> > > @@ -237,7 +238,7 @@ static int get_physical_address(CPUAlphaState *env, target_ulong addr,
> > >       /* L2 page table read.  */
> > >       index = (addr >> (TARGET_PAGE_BITS + 10)) & 0x3ff;
> > > -    L2pte = ldq_phys(cs->as, pt + index*8);
> > > +    L2pte = ldq_phys_le(cs->as, pt + index * 8);
> > >       if (unlikely((L2pte & PTE_VALID) == 0)) {
> > >           ret = MM_K_TNV;
> > > @@ -250,7 +251,7 @@ static int get_physical_address(CPUAlphaState *env, target_ulong addr,
> > >       /* L3 page table read.  */
> > >       index = (addr >> TARGET_PAGE_BITS) & 0x3ff;
> > > -    L3pte = ldq_phys(cs->as, pt + index*8);
> > > +    L3pte = ldq_phys_le(cs->as, pt + index * 8);
> > >       phys = L3pte >> 32 << TARGET_PAGE_BITS;
> > >       if (unlikely((L3pte & PTE_VALID) == 0)) {
> > > -- 
> > > 2.52.0
> > > 
> > > 
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/