linux-kernel - Re: [PATCH v4 8/8] fpga: m10bmc-sec: Add support for N6000

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y5gXUMAvN5xBOGpF@yilunxu-OptiPlex-7050>
Date:   Tue, 13 Dec 2022 14:10:24 +0800
From:   Xu Yilun <yilun.xu@...el.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc:     linux-fpga@...r.kernel.org, Wu Hao <hao.wu@...el.com>,
        Tom Rix <trix@...hat.com>, Moritz Fischer <mdf@...nel.org>,
        Lee Jones <lee@...nel.org>,
        Matthew Gerlach <matthew.gerlach@...ux.intel.com>,
        Russ Weight <russell.h.weight@...el.com>,
        Tianfei zhang <tianfei.zhang@...el.com>,
        Mark Brown <broonie@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Marco Pagani <marpagan@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 8/8] fpga: m10bmc-sec: Add support for N6000

On 2022-12-11 at 12:39:13 +0200, Ilpo Järvinen wrote:
> Add support for PMCI-based flash access path and N6000 sec update
> support. Access to flash staging area is different for N6000 from that
> of the SPI interfaced counterparts.
> 
> Introduce intel_m10bmc_flash_bulk_ops to allow interface specific
> differentiations for the flash access path for sec update and make
> m10bmc_sec_read/write() in sec update driver to use the new operations.
> 
> Co-developed-by: Tianfei zhang <tianfei.zhang@...el.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@...el.com>
> Co-developed-by: Russ Weight <russell.h.weight@...el.com>
> Signed-off-by: Russ Weight <russell.h.weight@...el.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---
>  drivers/fpga/intel-m10-bmc-sec-update.c |  65 ++++++++++-
>  drivers/mfd/intel-m10-bmc-pmci.c        | 145 ++++++++++++++++++++++++
>  include/linux/mfd/intel-m10-bmc.h       |  14 +++
>  3 files changed, 223 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
> index 9922027856a4..885e38f13897 100644
> --- a/drivers/fpga/intel-m10-bmc-sec-update.c
> +++ b/drivers/fpga/intel-m10-bmc-sec-update.c
> @@ -14,6 +14,20 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> +#define M10BMC_PMCI_FLASH_MUX_CTRL	0x1d0
> +#define FLASH_MUX_SELECTION		GENMASK(2, 0)
> +#define FLASH_MUX_IDLE			0
> +#define FLASH_MUX_NIOS			1
> +#define FLASH_MUX_HOST			2
> +#define FLASH_MUX_PFL			4
> +#define get_flash_mux(mux)		FIELD_GET(FLASH_MUX_SELECTION, mux)
> +
> +#define FLASH_NIOS_REQUEST		BIT(4)
> +#define FLASH_HOST_REQUEST		BIT(5)
> +
> +#define M10_FLASH_INT_US		1
> +#define M10_FLASH_TIMEOUT_US		10000
> +
>  struct m10bmc_sec {
>  	struct device *dev;
>  	struct intel_m10bmc *m10bmc;
> @@ -21,6 +35,7 @@ struct m10bmc_sec {
>  	char *fw_name;
>  	u32 fw_name_id;
>  	bool cancel_request;
> +	struct mutex flash_mutex;
>  };
>  
>  static DEFINE_XARRAY_ALLOC(fw_upload_xa);
> @@ -31,6 +46,24 @@ static DEFINE_XARRAY_ALLOC(fw_upload_xa);
>  #define REH_MAGIC		GENMASK(15, 0)
>  #define REH_SHA_NUM_BYTES	GENMASK(31, 16)
>  
> +static int m10bmc_sec_set_flash_host_mux(struct intel_m10bmc *m10bmc, bool request)
> +{
> +	u32 ctrl;
> +	int ret;
> +
> +	ret = regmap_update_bits(m10bmc->regmap, M10BMC_PMCI_FLASH_MUX_CTRL,
> +				 FLASH_HOST_REQUEST,
> +				 FIELD_PREP(FLASH_HOST_REQUEST, request));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_read_poll_timeout(m10bmc->regmap,
> +					M10BMC_PMCI_FLASH_MUX_CTRL, ctrl,
> +					request ? (get_flash_mux(ctrl) == FLASH_MUX_HOST) :
> +						  (get_flash_mux(ctrl) != FLASH_MUX_HOST),
> +					M10_FLASH_INT_US, M10_FLASH_TIMEOUT_US);
> +}
> +
>  static int m10bmc_sec_write(struct m10bmc_sec *sec, const u8 *buf, u32 offset, u32 size)
>  {
>  	struct intel_m10bmc *m10bmc = sec->m10bmc;
> @@ -41,6 +74,15 @@ static int m10bmc_sec_write(struct m10bmc_sec *sec, const u8 *buf, u32 offset, u
>  	u32 leftover_tmp = 0;
>  	int ret;
>  
> +	if (sec->m10bmc->flash_bulk_ops) {
> +		mutex_lock(&sec->flash_mutex);
> +		/* On write, firmware manages flash MUX */
> +		ret = sec->m10bmc->flash_bulk_ops->write(m10bmc, buf, offset, size);
> +		mutex_unlock(&sec->flash_mutex);
> +
> +		return ret;
> +	}
> +
>  	if (WARN_ON_ONCE(stride > sizeof(leftover_tmp)))
>  		return -EINVAL;
>  
> @@ -69,7 +111,21 @@ static int m10bmc_sec_read(struct m10bmc_sec *sec, u8 *buf, u32 addr, u32 size)
>  	u32 leftover_offset = read_count * stride;
>  	u32 leftover_size = size - leftover_offset;
>  	u32 leftover_tmp;
> -	int ret;
> +	int ret, ret2;
> +
> +	if (sec->m10bmc->flash_bulk_ops) {
> +		mutex_lock(&sec->flash_mutex);
> +		ret = m10bmc_sec_set_flash_host_mux(m10bmc, true);
> +		if (ret)
> +			goto mux_fail;

If the flash host mux fail, we still need to un-mux it?

Others look good to me.

Thanks,
Yilun


> +		ret = sec->m10bmc->flash_bulk_ops->read(m10bmc, buf, addr, size);
> +mux_fail:
> +		ret2 = m10bmc_sec_set_flash_host_mux(m10bmc, false);
> +		mutex_unlock(&sec->flash_mutex);
> +		if (ret)
> +			return ret;
> +		return ret2;
> +	}
>  
>  	if (WARN_ON_ONCE(stride > sizeof(leftover_tmp)))
>  		return -EINVAL;
> @@ -611,6 +667,8 @@ static int m10bmc_sec_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	mutex_init(&sec->flash_mutex);
> +
>  	len = scnprintf(buf, SEC_UPDATE_LEN_MAX, "secure-update%d",
>  			sec->fw_name_id);
>  	sec->fw_name = kmemdup_nul(buf, len, GFP_KERNEL);
> @@ -633,6 +691,7 @@ static int m10bmc_sec_probe(struct platform_device *pdev)
>  fw_uploader_fail:
>  	kfree(sec->fw_name);
>  fw_name_fail:
> +	mutex_destroy(&sec->flash_mutex);
>  	xa_erase(&fw_upload_xa, sec->fw_name_id);
>  	return ret;
>  }