Re: [PATCH v2 01/12] x86: introduce ioremap_wc()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2021 14:48, Julien Grall wrote:
> On 27/05/2021 13:30, Jan Beulich wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -5881,6 +5881,20 @@ void __iomem *ioremap(paddr_t pa, size_t
>>       return (void __force __iomem *)va;
>>   }
>>   
>> +void __iomem *__init ioremap_wc(paddr_t pa, size_t len)
>> +{
>> +    mfn_t mfn = _mfn(PFN_DOWN(pa));
>> +    unsigned int offs = pa & (PAGE_SIZE - 1);
>> +    unsigned int nr = PFN_UP(offs + len);
>> +    void *va;
>> +
>> +    WARN_ON(page_is_ram_type(mfn_x(mfn), RAM_TYPE_CONVENTIONAL));
>> +
>> +    va = __vmap(&mfn, nr, 1, 1, PAGE_HYPERVISOR_WC, VMAP_DEFAULT);
>> +
>> +    return (void __force __iomem *)(va + offs);
>> +}
> 
> Arm is already providing ioremap_wc() which is a wrapper to 
> ioremap_attr().

I did notice this, yes.

> Can this be moved to the common code to avoid duplication?

If by "this" you mean ioremap_attr(), then I wasn't convinced we want
a function of this name on x86. In particular you may note that
x86'es ioremap() is sort of the equivalent of Arm's ioremap_nocache(),
but is different from the new ioremap_wc() by more than just the
different PTE attributes.

Also I was specifically asked to make ioremap_wc() __init; ioremap()
cannot be, because of at least the use from pci_vtd_quirk().

Plus I'd need to clean up Arm's lack of __iomem if I wanted to fold
things. Or wait - it's declaration and definition which are out of
sync there, i.e. a pre-existing issue.

Bottom line - while I did consider folding, I don't think that's
feasible at this point in time.

Jan