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

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

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