On Tue, Mar 12, 2024 at 01:38:53PM +0100, Jan Beulich wrote: > 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?
I added a note about that in the patch (below commit message): > As an alternative implementation I have considered changing > iommu_get_extra_reserved_device_memory() to modify memory map. But that > may hide (or cause) some other issues when this API will gain some more > users in the future. More specifically, if some future use would be on a non-RAM area, or already reserved area. But if we can ignore those cases for now, I'm fine with that approach. > >>> 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.) I sure can do some archaeology there, I was just hoping any of you would know the answer already. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
signature.asc
Description: PGP signature
