Re: [PATCH v2 1/2] make ioremap_attr() common

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.04.2026 09:26, Luca Fancellu wrote:
> Hi Jan,
> 
>> On 8 Apr 2026, at 13:07, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> This core backing function is uniform; what varies across architectures
>> are the attributes passed and hence the wrappers around it. Yet of course
>> extra checking or special handling may be needed per arch, so introduce a
>> suitable hook.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Declarations (or inline counterparts) aren't being moved around, to avoid
>> the need to touch source files using the functions. Quite possibly they
>> want to consistently go into xen/io.h and asm/io.h.
>>
>> Of course ioremap.c could also go into lib/.
>>
>> For RISC-V the wrappers likely should become inline functions?
>>
>> PPC doesn't reference any of the functions just yet, so gets only a
>> declaration.
>>
>> For Arm, a TODO item is deliberately retained, yet seeing the use of
>> ioremap_wc() in domain building (which by itself is questionable, see next
>> patch) I wonder if that's even feasible as long as we don't have
>> memremap() or alike.
>> ---
>> v2: Use conditional operator in ioremap_attr()'s final return. Re-base and
>>    leverage that to simplify ioremap_attr() itself.
>>
>> --- a/xen/arch/arm/include/asm/io.h
>> +++ b/xen/arch/arm/include/asm/io.h
>> @@ -1,6 +1,8 @@
>> #ifndef _ASM_IO_H
>> #define _ASM_IO_H
>>
>> +#include <xen/mm-types.h>
>> +
>> #if defined(CONFIG_ARM_32)
>> # include <asm/arm32/io.h>
>> #elif defined(CONFIG_ARM_64)
>> @@ -9,6 +11,16 @@
>> # error "unknown ARM variant"
>> #endif
>>
>> +#ifdef CONFIG_MPU
>> +void __iomem *mpu_ioremap_attr(paddr_t start, size_t len, pte_attr_t flags);
>> +#define arch_ioremap_attr mpu_ioremap_attr
>> +#else
>> +/*
>> + * ioremap_attr() should only be used to remap device address ranges.
>> + * TODO: Add an arch hook to verify this assumption.
>> + */
>> +#endif
> 
> I find a bit strange to have an #else with only a comment, but to be fair I’m 
> not sure where this
> comment can be put otherwise.

That's the issue: I don't want to lose the comment, but I also didn't
see a good place to put it. Having it here means it can be replaced by
a single patch hunk, whenever the TODO is going to be addressed.

> For the Arm and common part, I’ve also tested on Arm64 MMU, Arm32 MMU, Arm64 
> MPU on virtual platforms:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx> # arm, common
> Tested-by: Luca Fancellu <luca.fancellu@xxxxxxx> # arm, common

Thanks.

Jan