Re: [PATCH v2] firmware: qemu_fw_cfg: Do not hard depend on CONFIG_HAS_IOPORT_MAP

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Nov 03, 2025 at 03:11:05PM +0100, Fabian Vogt wrote:
> Some configs have CONFIG_NO_IOPORT_MAP=y but support qemu_fw_cfg over
> MMIO. Currently the qemu_fw_cfg module can't be built for those as it
> needs HAS_IOPORT_MAP=y for ioport remapping.
> 
> This patch allows to build qemu_fw_cfg in those cases. If
> CONFIG_HAS_IOPORT_MAP=n, qemu_fw_cfg is just built without support for
> ioport based access.
> 
> Signed-off-by: Fabian Vogt <fvogt@suse.de>


Thanks for the patch! yet something to improve:


> ---
> v2: Guard ioport_unmap as well, which makes it work on archs like arm64
> in the !PCI case. Unlike arm, arm64 has no stub for ioport_unmap.
> Reword description.
> 
>  drivers/firmware/Kconfig       | 1 -
>  drivers/firmware/qemu_fw_cfg.c | 7 +++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index bbd2155d8483..91442f85f0f0 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -122,7 +122,6 @@ config RASPBERRYPI_FIRMWARE
>  config FW_CFG_SYSFS
>  	tristate "QEMU fw_cfg device support in sysfs"
>  	depends on SYSFS && (ARM || ARM64 || PARISC || PPC_PMAC || RISCV || SPARC || X86)
> -	depends on HAS_IOPORT_MAP
>  	default n
>  	help
>  	  Say Y or M here to enable the exporting of the QEMU firmware
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 0eebd572f9a5..fa4385339824 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -204,8 +204,10 @@ static void fw_cfg_io_cleanup(void)
>  		iounmap(fw_cfg_dev_base);
>  		release_mem_region(fw_cfg_p_base, fw_cfg_p_size);
>  	} else {
> +#ifdef CONFIG_HAS_IOPORT_MAP
>  		ioport_unmap(fw_cfg_dev_base);
>  		release_region(fw_cfg_p_base, fw_cfg_p_size);
> +#endif
>  	}
>  }
>  
> @@ -258,6 +260,7 @@ static int fw_cfg_do_platform_probe(struct platform_device *pdev)
>  			return -EFAULT;
>  		}
>  	} else {
> +#ifdef CONFIG_HAS_IOPORT_MAP
>  		if (!request_region(fw_cfg_p_base,
>  				    fw_cfg_p_size, "fw_cfg_io"))
>  			return -EBUSY;
> @@ -266,6 +269,10 @@ static int fw_cfg_do_platform_probe(struct platform_device *pdev)
>  			release_region(fw_cfg_p_base, fw_cfg_p_size);
>  			return -EFAULT;
>  		}
> +#else
> +		dev_err(&pdev->dev, "IO region given but CONFIG_HAS_IOPORT_MAP=n");
> +		return -EINVAL;
> +#endif



I do not like ifdefs spread all around the code.

Don't these arches provide stubs so fwcfg can just
have same code, with ioport stuff eliminated by compiler?


If not, stubs can be added to fwcfg as last resort.

-- 
MST