On 27.01.2022 10:47, Roger Pau Monné wrote:
> On Thu, Jan 27, 2022 at 09:48:16AM +0100, Jan Beulich wrote:
>> On 27.01.2022 09:22, Roger Pau Monne wrote:
>>> --- a/xen/arch/arm/mm.c
>>> +++ b/xen/arch/arm/mm.c
>>> @@ -1625,6 +1625,17 @@ bool is_iomem_page(mfn_t mfn)
>>> return !mfn_valid(mfn);
>>> }
>>>
>>> +bool is_iomem_range(paddr_t start, uint64_t size)
>>> +{
>>> + unsigned int i;
>>> +
>>> + for ( i = 0; i < PFN_UP(size); i++ )
>>> + if ( !is_iomem_page(_mfn(PFN_DOWN(start) + i)) )
>>> + return false;
>>> +
>>> + return true;
>>> +}
>>
>> I'm afraid you can't very well call this non-RFC as long as this doesn't
>> scale. Or if you're building on this still being dead code on Arm, then
>> some kind of "fixme" annotation would be needed here (or the function be
>> omitted altogether, for Arm folks to sort out before they actually start
>> selecting the HAS_PCI Kconfig setting).
>
> I was expecting the lack of 'RFC' tag to attract the attention of the
> maintainers for feedback.
Oh, I see.
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -783,6 +783,23 @@ bool is_iomem_page(mfn_t mfn)
>>> return (page_get_owner(page) == dom_io);
>>> }
>>>
>>> +bool is_iomem_range(paddr_t start, uint64_t size)
>>> +{
>>> + unsigned int i;
>>> +
>>> + for ( i = 0; i < e820.nr_map; i++ )
>>> + {
>>> + const struct e820entry *entry = &e820.map[i];
>>> +
>>> + /* Do not allow overlaps with any memory range. */
>>> + if ( start < entry->addr + entry->size &&
>>> + entry->addr < start + size )
>>> + return false;
>>> + }
>>> +
>>> + return true;
>>> +}
>>
>> I should have realized (and pointed out) earlier that with the type
>> check dropped the function name no longer represents what the
>> function does. It now really is unsuitable for about anything other
>> that the checking of BARs.
>
> is_devmem_range would be more suitable?
I'd make it even more tight and use e.g. is_pci_bar_range(). Or
maybe is_uncovered_range()?
> I will wrap this in an #ifdef HAS_PCI section now.
What would be the point of that, as long as X86 selects HAS_PCI
unconditionally? (Arguably one may want to turn off PCI support in
shim-only builds.)
Jan
