On Wed, Dec 24, 2025 at 5:14 PM Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> Commit 500131154d6 ("exec.c: Add new address_space_ld*/st*
> functions") added a new API to fix a shortcoming of the
> ld/st*_phys() API, which does blind bus access, not reporting
> failure (and it also allow to provide transaction attributes).
>
> Later commit 42874d3a8c6 ("Switch non-CPU callers from ld/st*_phys
> to address_space_ld/st*") automatically converted the legacy uses
> to the new API, not precising transaction attributes
> (MEMTXATTRS_UNSPECIFIED) and ignoring the transation result (passing
> NULL pointer as MemTxResult).
>
> While this is a faithful replacement, without any logical change,
> we later realized better is to not use MEMTXATTRS_UNSPECIFIED or
> NULL MemTxResult, and adapt each call site on a pair basis, looking
> at the device model datasheet to do the correct behavior (which is
> unlikely to ignore transaction failures).
>
> Since this is quite some work, we defer that to device model
> maintainers. Meanwhile we introduce a definition, to allow a
> target which removed all legacy API call to prohibit further
> legacy API uses, named "TARGET_NOT_USING_LEGACY_LDST_PHYS_API".
>

Personally the negation (i.e. TARGET_USING_LEGACY_LDST_PHYS_API) would
make more sense since you're opting-in an API but that must be more
complex to introduce I guess?

In any case:

Reviewed-by: Manos Pitsidianakis <[email protected]>


> Since all targets should be able to check this definition, we
> take care to not poison it.
>
> Suggested-by: Pierrick Bouvier <[email protected]>
> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
> ---
>  include/system/memory.h       | 2 ++
>  scripts/make-config-poison.sh | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/system/memory.h b/include/system/memory.h
> index e69171de05a..d5c248f1794 100644
> --- a/include/system/memory.h
> +++ b/include/system/memory.h
> @@ -2850,10 +2850,12 @@ MemTxResult address_space_write_rom(AddressSpace *as, 
> hwaddr addr,
>  #define ARG1_DECL    AddressSpace *as
>  #include "exec/memory_ldst.h.inc"
>
> +#ifndef TARGET_NOT_USING_LEGACY_LDST_PHYS_API
>  #define SUFFIX
>  #define ARG1         as
>  #define ARG1_DECL    AddressSpace *as
>  #include "exec/memory_ldst_phys.h.inc"
> +#endif
>
>  struct MemoryRegionCache {
>      uint8_t *ptr;
> diff --git a/scripts/make-config-poison.sh b/scripts/make-config-poison.sh
> index 2b36907e239..937357b3531 100755
> --- a/scripts/make-config-poison.sh
> +++ b/scripts/make-config-poison.sh
> @@ -10,6 +10,7 @@ exec sed -n \
>    -e' /CONFIG_TCG/d' \
>    -e '/CONFIG_USER_ONLY/d' \
>    -e '/CONFIG_SOFTMMU/d' \
> +  -e '/TARGET_NOT_USING_LEGACY_LDST_PHYS_API/d' \
>    -e '/^#define / {' \
>    -e    's///' \
>    -e    's/ .*//' \
> --
> 2.52.0
>

Reply via email to