Re: [PATCH] drivers/char: mark XHCI DMA buffers reserved in memory map

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.03.2024 13:02, Marek Marczykowski-Górecki wrote:
> On Tue, Mar 12, 2024 at 11:53:46AM +0100, Jan Beulich wrote:
>> On 12.03.2024 11:24, Roger Pau Monné wrote:
>>>> --- a/xen/arch/x86/setup.c
>>>> +++ b/xen/arch/x86/setup.c
>>>> @@ -1806,6 +1806,9 @@ void asmlinkage __init noreturn __start_xen(unsigned 
>>>> long mbi_p)
>>>>      mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
>>>>                                    RANGESETF_prettyprint_hex);
>>>>  
>>>> +    /* Needs to happen after E820 processing but before IOMMU init */
>>>> +    xhci_dbc_uart_reserve_ram();
>>>
>>> Overall it might be better if some generic solution for all users of
>>> iommu_add_extra_reserved_device_memory() could be implemented,
>>
>> +1
> 
> In that case, is the approach with
> iommu_get_extra_reserved_device_memory() okay (and a comment that it
> will have a side effect...) ?

Counter question: You not having gone that route despite our talking
about it on Matrix must have a reason. When seeing this approach I
concluded something got in the way. What's the deal?

>>> but I'm
>>> unsure whether the intention is for the interface to always be used
>>> against RAM.
>>
>> I think we can work from that assumption for now.
> 
> Ok, I'll add a comment about that. I guess, if needed in the future,
> iommu_add_extra_reserved_device_memory() can gain an extra parameter to
> distinguish RAM from non-RAM mappings.
> 
> BTW should e820_change_range_type() return 1 in case of mapping already
> having the right type? Otherwise, if one wants to use
> iommu_add_extra_reserved_device_memory() on already reserved memory, the
> e820_change_range_type() would fail.

You raised that question on Matrix yesterday, iirc, and I left it
unanswered there because it takes archeology to find the answer (or at
least get closer to one). And, please don't get me wrong, you could as
well do that yourself. (My vague recollection from having dealt with
similar code in Linux is that yes, in the example given the function
ought to indeed have failed back then. Depending on present uses etc
it may well be that we could reconsider, though.)

Jan