Re: [PATCH v2] early: usb: xhci-dbc: Handle out of bounds xhci-xdbc capability

[Mathias Nyman]

On 9/15/26 10:40, Umang Jain wrote:
> Currently, the early xhci-dbc assumes that the entire PCIe memory IO
> can be entirely mapped  within the fixed boot time mappings
> dictated by NR_FIX_BTMAPS. This patch handles the case where the PCIe
> memory IO size can be larger than the fixed boot time mappings and
> query the xhci debug extended capability in xdbc_map_pci_mmio().
>
> This commit ensures that the xHCI debug capability can still be queried
> when the PCIe memory IO space exceeds the fixmap size. In this scenario,
> the base address is mapped uptil fixmap size and debug capabilities are
> queried thereafter. Iterating over the entire PCIe BAR address size is
> left for future improvement as and when, such a case arises.
>
> Additionally, this brings the need to track the early_ioremap() mapped
> size separately hence, introduce additional struct member xhci_base_length
> in struct xdbc_state.
>
> Signed-off-by: Umang Jain <uajain@xxxxxxxxxx>
> ---
> Changes in v2:
> - Commit message update
> - Changed and tested behaviour for mapping only first fixmap_size
>    of PCIe Base address size instead of iteration over entire BAR size
> - Updated comments
> - Updated XDBC MAPPING SIZE to 64

Thanks, this starts to look good.

Some small fixes and cleanups still needed.

> v1:
> - https://lore.kernel.org/all/20260720191249.1272328-1-uajain@xxxxxxxxxx/
> ---
>   drivers/usb/early/xhci-dbc.c | 78 ++++++++++++++++++++++++++++++++----
>   drivers/usb/early/xhci-dbc.h |  1 +
>   2 files changed, 71 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
> index 41118bba9197..d2e6ae410b0d 100644
> --- a/drivers/usb/early/xhci-dbc.c
> +++ b/drivers/usb/early/xhci-dbc.c
> @@ -35,10 +35,23 @@ static bool early_console_keep;
>   static inline void xdbc_trace(const char *fmt, ...) { }
>   #endif /* XDBC_TRACE */
>   
> +/* Size of xHCI debug capability structure as per section 7.6.8 of xHCI spec. */
> +#define XDBC_MAPPING_SIZE	64
> +
> +enum xdbc_capability_flags {
> +	XDBC_CAP_FLAG_NONE		= 0,
> +	XDBC_CAP_FLAG_LEGACY		= 1 << 0,
> +	XDBC_CAP_FLAG_PROTOCOL		= 1 << 1,
> +	XDBC_CAP_FLAG_DEBUG		= 1 << 2,
> +};
> +
>   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;

mapped_size in no longer needed. Can be removed

> +	enum xdbc_capability_flags cap_flags = XDBC_CAP_FLAG_NONE;
> +	bool found_all_caps = false;
>   	void __iomem *base;
> +	int offset;
>   	u32 val, sz;
>   	u8 byte;
>   
> @@ -85,7 +98,56 @@ 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);
> +
> +	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 time mappings
> +	 * hence, map maximum allowed fixmap size from base address and
> +	 * determine if the required extended capabilities lies within the
> +	 * fixmap.
> +	 */
> +	base = early_ioremap(val64, fixmap_size);

if (!base)
	return NULL;

> +	offset = xhci_find_next_ext_cap(base, 0, 0);
> +	mapped_size = fixmap_size;

mapped_size not used

> +
> +	while (offset < fixmap_size) {
> +		val = readl(base + offset);
> +		switch (XHCI_EXT_CAPS_ID(val)) {
> +		case XHCI_EXT_CAPS_DEBUG:
> +			if (offset + XDBC_MAPPING_SIZE < fixmap_size)
> +				cap_flags |= XDBC_CAP_FLAG_DEBUG;
> +			break;
> +		case XHCI_EXT_CAPS_PROTOCOL:
> +			cap_flags |= XDBC_CAP_FLAG_PROTOCOL;
> +			break;
> +		case XHCI_EXT_CAPS_LEGACY:
> +			cap_flags |= XDBC_CAP_FLAG_LEGACY;
> +			break;
> +		}
> +
> +		if ((cap_flags & XDBC_CAP_FLAG_DEBUG) &&
> +		    (cap_flags & XDBC_CAP_FLAG_PROTOCOL) &&
> +		    (cap_flags & XDBC_CAP_FLAG_LEGACY)) {
> +			found_all_caps = true;
> +			break;
> +		}
> +
> +		offset = xhci_find_next_ext_cap(base, offset, 0);
> +		if (!offset)
> +			break;
> +	}
> +
> +	if (found_all_caps) {
> +		xdbc.xhci_base_length = fixmap_size;
> +	} else {
> +		xdbc.xhci_base_length = 0;
> +		base = NULL;

we need to early_iounmap() in this fail path before returning NULL.

> +	}
>   
>   	return base;
>   }

Thanks
Mathias