Re: [PATCH 5/8] target/ppc/mmu: Replace legacy ld/st_phys() -> address_space_ld/st()

[Miles Glenn <milesg@linux.ibm.com>]

On Wed, 2026-04-22 at 12:42 +0530, Harsh Prateek Bora wrote:
> 
> On 19/04/26 9:40 pm, Philippe Mathieu-Daudé wrote:
> > On 19/3/26 13:59, BALATON Zoltan wrote:
> > > On Thu, 19 Mar 2026, Philippe Mathieu-Daudé wrote:
> > > > Prefer the address_space_ld/st API over the legacy ld_phys()
> > > > because it allow checking for bus access fault.
> > > > 
> > > > This code however doesn't check for fault, so we simply inline
> > > > the calls (not specifying any memory transaction attribute nor
> > > > expecting transation result) per the definition in
> > > > "system/memory_ldst_phys_endian.h.inc":
> 
> Nit: "include/system/memory_ldst_phys_endian.h.inc"
> 
> > > Recently when trying to remove _nomigrate memory region functions I 
> > > was told if some convenience function has more than 1 use it's 
> > > probably worth to keep it. This looks like similar case even more so 
> > > as the replacement is unnecessarily more complicated. So what's the 
> > > problem with ld_phys in the first place and why do you want to replace 
> > > it with a less convenient function? If there's a reason maybe say that 
> > > in the commit message.
> > 
> > We want to remove the legacy API because it does not "allow checking
> > for bus access fault." The cases where we can safely ignore it are
> > rare, and often ignoring it led to subtle bugs.
> 
> It would be nice to have an ack from BALATON or Glenn as well since rest 
> of the patches are specific to pegasos/ppc32, although no funtional 
> change intended here.
> 
> If we do not have any further comments from them/others, could you
> please post v2 addressing the comment in patch 2/8 reg keeping 
> declaration within !CONFIG_USER_ONLY block? It  could be moved to one of 
> the existing block as well.
> 
> Thanks
> Harsh

Makes sense to me.

Acked-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn