Re: [PATCH v2 04/16] xen/riscv: add ioremap_*() variants using ioremap_attr()

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]


On 5/14/25 4:32 PM, Jan Beulich wrote:
> On 06.05.2025 18:51, Oleksii Kurochko wrote:
>> @@ -583,3 +584,36 @@ void *__init arch_vmap_virt_end(void)
>>   {
>>       return (void *)(VMAP_VIRT_START + VMAP_VIRT_SIZE);
>>   }
>> +
>> +static void *ioremap_attr(paddr_t start, size_t len, pte_attr_t attributes)
>> +{
>> +    mfn_t mfn = _mfn(PFN_DOWN(start));
>> +    unsigned int offs = start & (PAGE_SIZE - 1);
>> +    unsigned int nr = PFN_UP(offs + len);
>> +    void *ptr = __vmap(&mfn, nr, 1, 1, attributes, VMAP_DEFAULT);
>> +
>> +    if ( ptr == NULL )
>> +        return NULL;
>> +
>> +    return ptr + offs;
>> +}
>> +
>> +void __iomem *ioremap_nocache(paddr_t start, size_t len)
>> +{
>> +    return ioremap_attr(start, len, PAGE_HYPERVISOR_NOCACHE);
>> +}
> Why do you need both this and ...
>
>> +void __iomem *ioremap_cache(paddr_t start, size_t len)
>> +{
>> +    return ioremap_attr(start, len, PAGE_HYPERVISOR);
>> +}
>> +
>> +void __iomem *ioremap_wc(paddr_t start, size_t len)
>> +{
>> +    return ioremap_attr(start, len, PAGE_HYPERVISOR_WC);
>> +}
>> +
>> +void *ioremap(paddr_t pa, size_t len)
>> +{
>> +    return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
>> +}
> ... this? And why's the 1st parameter named differently for this last
> one? Can't they all be in sync in this regard?

Originally, I thought|ioremap_nocache()| was needed because it is used in
common code. However, I now realize that the calls to|ioremap_nocache()|
in|xen/drivers/char| are also ARM-specific. I assume all the
UART drivers currently using|ioremap_nocache|() are intended for ARM.

I'll drop|ioremap_nocache()| for RISC-V, and I think it makes sense to
create a separate patch to clean this up for ARM as well.

~ Oleksii

[-- Attachment #2: Type: text/html, Size: 2622 bytes --]