Re: [PATCH] tpm: crb: handle ACPI NVS memory region overlap

[Jarkko Sakkinen <jarkko@kernel.org>]

On Thu, Aug 27, 2026 at 11:55:11AM +0800, microfish wrote:
> On certain Lenovo/AMD systems, the BIOS declares an ACPI NVS region
> that covers a large memory range (e.g. 0xccc40000-0xccd3efff) which
> includes the TPM CRB MMIO area (0xccd35000-0xccd38fff). The E820
> subsystem registers this NVS region with IORESOURCE_BUSY in the
> iomem_resource tree.
> 
> When tpm_crb probes, it calls devm_ioremap_resource() to map the CRB
> registers, which internally calls devm_request_mem_region(). This fails
> with -EBUSY because the NVS region already claims that address range.
> 
> Fix this by adding a crb_try_nvs_fallback() helper that detects the
> ACPI NVS overlap using region_intersects() and falls back to
> devm_memremap() when the resource falls inside ACPI NVS.
> devm_memremap() does not require exclusive resource reservation,
> making it safe to use on NVS-overlapping regions. This approach is
> identical to the one used by the WDAT watchdog driver (wdat_wdt_map_mem).
> 
> The NVS fallback is applied in both code paths of crb_map_res():
> 1. When iores is NULL (no ACPI resource encompasses the target address)
> 2. When iores is non-NULL but devm_ioremap_resource() fails with EBUSY
> 
> Before this patch:
>   tpm_crb MSFT0101:00: error -EBUSY: can't request region for resource
>   [mem 0xccd35000-0xccd38fff]
>   tpm_crb MSFT0101:00: probe with driver tpm_crb failed with error -16
> 
> After this patch:
>   tpm_crb MSFT0101:00: [mem 0xccd35000-0xccd38fff] is inside ACPI NVS,
>   mapping without reservation
>   tpm_crb MSFT0101:00: of device registration complete, result 0: 0
> 
> Signed-off-by: microfish <microfish@kernel.org>
> Signed-off-by: microfish <f13208471983@163.com>

Sorry this took so long.

Mention the hardware that you remarked in the response and provide only
single SOB with your real first and last name.

> ---
>  drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index ceb4100ba..c4e8b925c 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -13,6 +13,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/highmem.h>
> +#include <linux/mm.h>
>  #include <linux/rculist.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> @@ -557,6 +558,39 @@ static int crb_check_resource(struct acpi_resource *ares, void *data)
>  	return 1;
>  }
>  
> +/*
> + * Try to map a resource region. If devm_ioremap_resource() fails with
> + * -EBUSY because the region falls inside an ACPI NVS area (common on
> + * certain Lenovo/AMD firmware), fall back to devm_memremap() which
> + * does not require exclusive resource reservation. This is analogous
> + * to the approach used by the WDAT watchdog driver (wdat_wdt_map_mem).
> + */
> +static void __iomem *crb_try_nvs_fallback(struct device *dev, u64 start,
> +					   u32 size)
> +{
> +	struct resource res = {
> +		.start	= start,
> +		.end	= start + size - 1,
> +		.flags	= IORESOURCE_MEM,
> +	};
> +	void *addr;
> +
> +	if (region_intersects(res.start, resource_size(&res), IORESOURCE_MEM,
> +			     IORES_DESC_ACPI_NV_STORAGE) !=
> +			     REGION_INTERSECTS)
> +		return NULL;
> +
> +	dev_warn(dev,
> +		 "%pR is inside ACPI NVS, mapping without reservation\n",
> +		 &res);
> +
> +	addr = devm_memremap(dev, res.start, resource_size(&res), MEMREMAP_WB);
> +	if (IS_ERR(addr))
> +		return IOMEM_ERR_PTR(PTR_ERR(addr));
> +
> +	return (void __iomem __force *)addr;
> +}
> +
>  static void __iomem *crb_map_res(struct device *dev, struct resource *iores,
>  				 void __iomem **iobase_ptr, u64 start, u32 size)
>  {
> @@ -565,18 +599,30 @@ static void __iomem *crb_map_res(struct device *dev, struct resource *iores,
>  		.end	= start + size - 1,
>  		.flags	= IORESOURCE_MEM,
>  	};
> +	void __iomem *p;
>  
>  	/* Detect a 64 bit address on a 32 bit system */
>  	if (start != new_res.start)
>  		return IOMEM_ERR_PTR(-EINVAL);
>  
> -	if (!iores)
> +	if (!iores) {
> +		p = crb_try_nvs_fallback(dev, start, size);
> +		if (p)
> +			return p;
>  		return devm_ioremap_resource(dev, &new_res);
> +	}
>  
>  	if (!*iobase_ptr) {
>  		*iobase_ptr = devm_ioremap_resource(dev, iores);
> -		if (IS_ERR(*iobase_ptr))
> +		if (IS_ERR(*iobase_ptr)) {
> +			p = crb_try_nvs_fallback(dev, iores->start,
> +						 resource_size(iores));
> +			if (p) {
> +				*iobase_ptr = p;
> +				return p + (new_res.start - iores->start);
> +			}
>  			return *iobase_ptr;
> +		}

Now I recall what I had in mind, and sorry this took so long.

I did what I was unhappy about:

https://lore.kernel.org/linux-integrity/20260901142955.187856-1-jarkko@kernel.org/

It does not make sense to apply this change before my patch is
ack'd because it reduces complexity of this change considerably.

E.g., that helper function will not be needed given no multiple
call sites.

1. You pick my patch to your series. I.e. make this a two patch
   series.
2. We'll hold until my patch is accepted.

>  	}
>  
>  	return *iobase_ptr + (new_res.start - iores->start);
> -- 
> 2.47.3
> 

Br, Jarkko