On 13/03/2025 10:04, Luca Fancellu wrote:
>
>
> Hi Michal,
>
> thanks for your review,
>
>>> diff --git a/xen/arch/arm/include/asm/mpu/mm.h
>>> b/xen/arch/arm/include/asm/mpu/mm.h
>>> new file mode 100644
>>> index 000000000000..57f1e558fd44
>>> --- /dev/null
>>> +++ b/xen/arch/arm/include/asm/mpu/mm.h
>>> @@ -0,0 +1,27 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +
>>> +#ifndef __ARM_MPU_MM__
>> Missing _H? Should be: __ARM_MPU_MM_H__
>>
>>> +#define __ARM_MPU_MM__
>>> +
>>> +#include <xen/macros.h>
>> I guess you also need xen/types.h
>>
>>> +
>>> +#define virt_to_maddr(va) ({ \
>>> + (paddr_t)va; \
>>> +})
>> Why multiline? Also, shouldn't we take PA bits into account?
>> I'd imagine:
>> ((paddr_t)((vaddr_t)(va) & PADDR_MASK))
>>
>>> +
>>> +/* On MPU systems there is no translation, ma == va. */
>>> +static inline void *maddr_to_virt(paddr_t ma)
>>> +{
>>> + return _p(ma);
>> Why do we need to cast paddr_t to unsigned long before casting to void?
>> Why not:
>> return (void *)(ma);
>
> So it was pointed out on a previous review that it’s ok to use _p() instead
> of doing straight the cast:
> https://patchwork.kernel.org/project/xen-devel/patch/20230626033443.2943270-29-penny.zh...@arm.com/#25404105
>
> please let me know your thought about it.
I'm ok with that.
~Michal
