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

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

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

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
> 
> > 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.

One more question - what should be the error handling in this case? At
this stage, if reserving fails, I can still skip giving this range to
the IOMMU driver, which (most likely) will result in IOMMU faults and
in-operational device (xhci console). Since I don't know (theoretically)
what driver requested the range, the error message can only contain an
address and device, so will be a bit less actionable for the user
(although it should be quite easy to notice the BDF being the XHCI one).

Panic surely is safer option, but less user friendly, especially since
(due to the above) I cannot give explicit hint to disable XHCI console.

And kinda independently - I'm tempted to add another field to `struct
extra_reserved_range` (and an argument to
`iommu_add_extra_reserved_device_memory()`) - textual description, for
the error reporting purpose.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]