Re: [RFC PATCH v1 2/3] early: usb: xhci-dbc: Handle out of bounds xhci-xdbc capability

[Mathias Nyman]

Hi

On 6/4/26 17:41, Umang Jain wrote:
> Currently, the early xhci-dbc assumes that the extended capability
> can be mapped within the fixed boot time mappings dictated by
> NR_FIX_BTMAPS.
>
> This patch iterates over the PCI BAR address size to find and map
> xhci-xdbc capability which could be out-of-bounds otherwise,
> in xdbc_map_pci_mmio(). The iterations map the maximum allowed
> boot time mappings (fixmap size) at a time and search for xhci-xdbc
> capability offset, till the end of the bar address size.

Patch 1/3 can probably be merged into this one.

> Signed-off-by: Umang Jain <uajain@xxxxxxxxxx>
> ---
>   drivers/usb/early/xhci-dbc.c | 47 +++++++++++++++++++++++++++++++++---
>   1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
> index 8ce362a90910..1f6a129d4b5d 100644
> --- a/drivers/usb/early/xhci-dbc.c
> +++ b/drivers/usb/early/xhci-dbc.c
> @@ -35,10 +35,13 @@ static bool early_console_keep;
>   static inline void xdbc_trace(const char *fmt, ...) { }
>   #endif /* XDBC_TRACE */
>   
> +#define XDBC_MAPPING_SIZE 56
> +

I know spec says 56 bytes, but when looking at the Debug capability structure
in xhci section 7.6.8. it looks like 64 bytes.

>   static void __iomem * __init xdbc_map_pci_mmio(u32 bus, u32 dev, u32 func)
>   {
> -	u64 val64, sz64, mask64;
> +	u64 val64, sz64, mask64, fixmap_size, mapped_size;
>   	void __iomem *base;
> +	int offset;
>   	u32 val, sz;
>   	u8 byte;
>   
> @@ -85,8 +88,46 @@ static void __iomem * __init xdbc_map_pci_mmio(u32 bus, u32 dev, u32 func)
>   
>   	xdbc.xhci_start = val64;
>   	xdbc.xhci_length = sz64;
> -	base = early_ioremap(val64, sz64);
> -	xdbc.xhci_base_length = sz64;
> +
> +	fixmap_size = NR_FIX_BTMAPS << PAGE_SHIFT;
> +	if (sz64 < fixmap_size) {
> +		xdbc.xhci_base_length = sz64;
> +		return early_ioremap(val64, sz64);
> +	}
> +
> +	/*
> +	 * Base address size is greater than fixed size boot mappings,
> +	 * hence iterate over the region one fixmap_size at a time.
> +	 */
> +	base = early_ioremap(val64, fixmap_size);
> +	offset = xhci_find_next_ext_cap(base, 0, 0);
> +	mapped_size = fixmap_size;
> +
> +	while (mapped_size <= sz64) {
> +		val = readl(base + offset);
> +		if (XHCI_EXT_CAPS_ID(val) == XHCI_EXT_CAPS_DEBUG) {
> +			if (offset + XDBC_MAPPING_SIZE > fixmap_size) {
> +				early_iounmap(base, fixmap_size);
> +				base = early_ioremap(val64 + offset, XDBC_MAPPING_SIZE);

Took a closer look and it turns out we do sometimes need to touch registers in other
extended capabilities. Mainly BIOS handoff in XHCI_EXT_CAPS_LEGACY and port reset in
XHCI_EXT_CAPS_PROTOCOL

In the case where xHC size is larger than early_ioremap() allows I would just
early_ioremap() maximum allowed size once, starting from xdbc.xhci_start.
Then walk the extended capabilities list ensuring DbC and the other needed capabilities
are inside this maximum allowed size.
early_iounmap() and fail if not.
  
This way we can also access the normal xHC host registers in case we need to reset the
controller, or ensure the 'controller not ready' bit is clear.

Thanks
Mathias