[RFC] fpga: add PolarFire SoC IAP support

Conor Dooley conor.dooley at microchip.com
Tue Nov 22 01:30:50 PST 2022


On Tue, Nov 22, 2022 at 11:19:40AM +0800, Xu Yilun wrote:
> On 2022-11-21 at 22:57:49 +0000, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley at microchip.com>
> > 
> > Add support for "IAP" reprogramming of the FPGA fabric on PolarFire SoC.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley at microchip.com>
> > ---
> > 
> > Hey all,
> > RFC yadda yadda, but not asking people to look at the code per se -
> > really what I am sending this RFC to achieve is a bit of feedback on
> > what the re-programming "flow" looks like.
> > 
> > PolarFire and the SoC variants are flash FPGAs. All well and good for
> > PolarFire, but for PolarFire SoC in IAP mode it is re-programmed from
> > the on-board SoC. The spi-slave stuff that Ivan upstreamed recently
> > works too, but that's only when you have an external programmer.
> > 
> > IAP, or In Application Programming, a programming running on the cpu
> > cores writes the FPGA bitstream, or I suppose "firmware" in Linux land,
> > out to a flash chip which the system then will use to reprogram the
> > FPGA. When IAP is initiated, the system controller will take the FPGA
> > down completely & do the reprogramming. This means, once Linux (or some
> 
> Do you mean the Linux OS, FPGA mgr driver & reprograming Application are
> all running on the SoC?

Aye. What we have is 4 RISC-V cores running Linux & then the system
controller. The system controller is accessed via a mailbox, but is part
of the SoC. The fpga manager driver is, as you might imagine, running on
the RISC-V cores.
I suppose the "reprogramming Application" is part of the system
controller functionality & yea, runs on the SoC.

> > other program) kicks off the re-programming (in write_complete() below)
> > the system will *immediately* power cycle, whether the reprogramming
> > passes or not. You can see in my write_complete() implementation, that I
> > do not expect the function to return, unless we catch an error case early.
> 
> I don't think an FPGA driver could have an interface to power cycle the
> whole system.

Right. That was kinda my assumption too.

> > From my cursory looking around, there doesnt appear to me to be all that
> > much info in-tree about what each FPGA does when you reprogram it, but
> > it doesn't appear that for other FPGAs the CPUs get shut down during
> > this kind of self-reprogramming?
> > 
> > The alternative approach would be to use the "auto update" mode, which
> > just installs an image that will not be used until the FPGA is rebooted
> > in the regular way. Then, on reboot, the system controller would pluck
> 
> This seems to be a better way from OS perspective.

Again, yup. I just was not sure if there was an expectation that the
device would be reprogrammed once write_complete() terminated.

> > the image from its flash chip and program the FPGA with it. From the
> > last time I looked at the documentation (and the implementations) it
> > seemed that people had custom (and out of tree) ways to initiate/handle
> > the programming, so perhaps I have the freedom to do the "auto update"
> > approach, even if it may be a little different to other implementations?
> 
> FPGA manager framework takes care of the runtime FPGA reprograming, a
> main concern is the removal and re-enumeration of the sub devices when
> an FPGA region is being reprogramed. Otherwise, OS is not aware of the
> change of the devices and may just crash.

Right. I don't think I need to worry about this since initiating a
reprogramming from within Linux would take take the CPUs down too!

> For your "auto update" mode, I assume it doesn't affect the running
> devices at all, just do flash read/write. So maybe leverage MTD is just
> fine?

Aye, it would just write the image to the flash, set up the relevant
bits to make sure auto-update would trigger & validate the bitstream
written to the flash. Then nothing would happen until the user reboots
their system.

Thanks for your help,
Conor.

> > If I've missed something, please let me know. I hope I have!
> > 
> > I've only recently got a board capable of testing this & have not yet
> > tested my code here in anger etc, it's just the high level thoughts
> > about how to approach the re-programming in an FPGA manager friendly way
> > that I'm looking for comments on.
> > 
> > @LKP folks, if you happen to see this - you can disable building this
> > patch, it won't without some dependencies ;)
> > 
> > Thanks,
> > Conor.
> > ---
> >  drivers/fpga/Kconfig         |   6 +
> >  drivers/fpga/Makefile        |   1 +
> >  drivers/fpga/microchip-iap.c | 350 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 357 insertions(+)
> >  create mode 100644 drivers/fpga/microchip-iap.c
> > 
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index 6c416955da53..525d753a28a4 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -255,6 +255,12 @@ config FPGA_M10_BMC_SEC_UPDATE
> >  	  (BMC) and provides support for secure updates for the BMC image,
> >  	  the FPGA image, the Root Entry Hashes, etc.
> >  
> > +config FPGA_MGR_MICROCHIP_IAP
> > +	tristate "Microchip Polarfire IAP FPGA manager"
> > +	depends on POLARFIRE_SOC_SYS_CTRL
> > +	help
> > +	  NOP
> > +
> >  config FPGA_MGR_MICROCHIP_SPI
> >  	tristate "Microchip Polarfire SPI FPGA manager"
> >  	depends on SPI
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 42ae8b58abce..df1dfb54046b 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
> >  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
> >  obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
> >  obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)	+= versal-fpga.o
> > +obj-$(CONFIG_FPGA_MGR_MICROCHIP_IAP)	+= microchip-iap.o
> >  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> >  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
> >  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
> > diff --git a/drivers/fpga/microchip-iap.c b/drivers/fpga/microchip-iap.c
> > new file mode 100644
> > index 000000000000..10f25ec64d32
> > --- /dev/null
> > +++ b/drivers/fpga/microchip-iap.c
> > @@ -0,0 +1,350 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Microchip Polarfire SoC "IAP" FPGA reprogramming.
> > + *
> > + * Copyright (c) 2022 Microchip Corporation. All rights reserved.
> > + *
> > + * Author: Conor Dooley <conor.dooley at microchip.com>
> > + */
> > +#include <linux/module.h>
> > +#include <linux/mtd/mtd.h>
> > +#include <linux/of_device.h>
> > +#include <linux/fpga/fpga-mgr.h>
> > +#include <soc/microchip/mpfs.h>
> > +
> > +#define IAP_DEFAULT_MBOX_OFFSET		0u
> > +#define IAP_DEFAULT_RESP_OFFSET		0u
> > +
> > +#define IAP_FEATURE_CMD_OPCODE		0x05
> > +#define IAP_FEATURE_CMD_DATA_SIZE	0u
> > +#define IAP_FEATURE_RESP_SIZE		33u
> > +#define IAP_FEATURE_CMD_DATA		NULL
> > +#define IAP_FEATURE_ENABLED		BIT(5)
> > +
> > +#define IAP_VERIFY_CMD_OPCODE		0x22
> > +#define IAP_VERIFY_CMD_DATA_SIZE	0u
> > +#define IAP_VERIFY_RESP_SIZE		1u
> > +#define IAP_VERIFY_CMD_DATA		NULL
> > +
> > +#define IAP_PROGRAM_CMD_OPCODE		0x42
> > +#define IAP_PROGRAM_CMD_DATA_SIZE	0u
> > +#define IAP_PROGRAM_RESP_SIZE		1u
> > +#define IAP_PROGRAM_CMD_DATA		NULL
> > +
> > +#define IAP_DIRECTORY_WIDTH		4u
> > +#define IAP_UPGRADE_INDEX		1u
> > +#define IAP_UPGRADE_DIRECTORY		(IAP_UPGRADE_INDEX * 0x4)
> > +#define IAP_IMAGE_INDEX			2u
> > +#define IAP_IMAGE_DIRECTORY		(IAP_IMAGE_INDEX * 0x4)
> > +#define IAP_IMAGE_ADDRESS		(IAP_IMAGE_INDEX * 0xA00000)
> > +
> > +struct mpf_iap_config {
> > +	u8 feature_response_size;
> > +};
> > +
> > +struct mpf_iap_priv {
> > +	struct mpfs_sys_controller *sys_controller;
> > +	struct device *dev;
> > +	struct fpga_region *region;
> > +	struct mpf_iap_config *config;
> > +	struct mtd_info *flash;
> > +};
> > +
> > +static enum fpga_mgr_states mpf_iap_state(struct fpga_manager *mgr)
> > +{
> > +	struct mpf_iap_priv *priv = mgr->priv;
> > +	struct mpfs_mss_response *response;
> > +	struct mpfs_mss_msg *message;
> > +	u32 *response_msg;
> > +	int ret;
> > +	enum fpga_mgr_states rc = FPGA_MGR_STATE_WRITE_INIT_ERR;
> > +
> > +	response_msg = devm_kzalloc(priv->dev,
> > +				    IAP_FEATURE_RESP_SIZE * sizeof(response_msg),
> > +				    GFP_KERNEL);
> > +	if (!response_msg)
> > +		return FPGA_MGR_STATE_WRITE_INIT_ERR;
> > +
> > +	response = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_response), GFP_KERNEL);
> > +	if (!response) {
> > +		devm_kfree(priv->dev, response_msg);
> > +		return FPGA_MGR_STATE_WRITE_INIT_ERR;
> > +	}
> > +
> > +	message = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_msg), GFP_KERNEL);
> > +	if (!response) {
> > +		devm_kfree(priv->dev, response_msg);
> > +		devm_kfree(priv->dev, response);
> > +		return FPGA_MGR_STATE_WRITE_INIT_ERR;
> > +	}
> > +
> > +	/*
> > +	 * To verify that IAP is possible, the "Query Security Service Request"
> > +	 * is performed. Bit 5 of byte 1 is "UL_IAP" & if it is set, IAP is not
> > +	 * possible.
> > +	 * This service has no command data & does not overload mbox_offset.
> > +	 * The size of the response varies between PolarFire & PolarFire SoC.
> > +	 */
> > +	response->resp_msg = response_msg;
> > +	response->resp_size = IAP_FEATURE_RESP_SIZE;
> > +	message->cmd_opcode = IAP_FEATURE_CMD_OPCODE;
> > +	message->cmd_data_size = IAP_FEATURE_CMD_DATA_SIZE;
> > +	message->response = response;
> > +	message->cmd_data = IAP_FEATURE_CMD_DATA;
> > +	message->mbox_offset = IAP_DEFAULT_MBOX_OFFSET;
> > +	message->resp_offset = IAP_DEFAULT_RESP_OFFSET;
> > +
> > +	ret = mpfs_blocking_transaction(priv->sys_controller, message);
> > +	if (ret | response->resp_status) {
> > +		rc = FPGA_MGR_STATE_UNKNOWN;
> > +		goto out;
> > +	}
> > +
> > +	if (!(response_msg[1] & IAP_FEATURE_ENABLED))
> > +		rc = FPGA_MGR_STATE_OPERATING;
> > +
> > +out:
> > +	devm_kfree(priv->dev, response_msg);
> > +	devm_kfree(priv->dev, response);
> > +	devm_kfree(priv->dev, message);
> > +
> > +	return rc;
> > +}
> > +
> > +static int mpf_iap_write_init(struct fpga_manager *mgr, struct fpga_image_info *info,
> > +			      const char *buf, size_t count)
> > +{
> > +	struct mpf_iap_priv *priv = mgr->priv;
> > +	size_t *bytes_read;
> > +	u32 upgrade_address;
> > +
> > +	priv->flash = mpfs_sys_controller_get_flash(priv->sys_controller);
> > +	if (!priv->flash)
> > +		return PTR_ERR(priv->flash);
> > +
> > +	/*
> > +	 * We need to read the "SPI DIRECTORY" in the first 1 KiB, to see if
> > +	 * the index 1 has an address in it. If it is non zero, IAP will fail.
> > +	 * As the system controller will initiate upgrade mode instead.
> > +	 */
> > +	int ret = mtd_read(priv->flash, IAP_UPGRADE_DIRECTORY, IAP_DIRECTORY_WIDTH,
> > +			   bytes_read, (u_char *) &upgrade_address);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (*bytes_read != IAP_DIRECTORY_WIDTH || upgrade_address)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mpf_iap_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > +{
> > +	/*
> > +	 * No parsing etc of the bitstream is required. The system controller
> > +	 * will do all of that itself - including verifying that the bitstream
> > +	 * is valid.
> > +	 */
> > +	struct mpf_iap_priv *priv = mgr->priv;
> > +	size_t *bytes_written;
> > +	u32 image_address = IAP_IMAGE_ADDRESS;
> > +
> > +	/*
> > +	 * We need to write the "SPI DIRECTORY" to the first 1 KiB, telling
> > +	 * the system controller where to find the actual bitstream.
> > +	 */
> > +	int ret = mtd_write(priv->flash, IAP_IMAGE_DIRECTORY, IAP_DIRECTORY_WIDTH,
> > +			    bytes_written, (u_char *) &image_address);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (*bytes_written != IAP_DIRECTORY_WIDTH)
> > +		return -EIO;
> > +
> > +	ret = mtd_write(priv->flash, (loff_t) image_address, count, bytes_written, (u_char *) buf);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (*bytes_written != count)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mpf_iap_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info)
> > +{
> > +	struct mpf_iap_priv *priv = mgr->priv;
> > +	struct mpfs_mss_response *response;
> > +	struct mpfs_mss_msg *message;
> > +	u32 *response_msg;
> > +	int ret = 0;
> > +
> > +	response_msg = devm_kzalloc(priv->dev,
> > +				    IAP_FEATURE_RESP_SIZE * sizeof(response_msg),
> > +				    GFP_KERNEL);
> > +	if (!response_msg)
> > +		return -ENOMEM;
> > +
> > +	response = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_response), GFP_KERNEL);
> > +	if (!response) {
> > +		devm_kfree(priv->dev, response_msg);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	message = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_msg), GFP_KERNEL);
> > +	if (!response) {
> > +		devm_kfree(priv->dev, response_msg);
> > +		devm_kfree(priv->dev, response);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	/*
> > +	 * The system controller can verify that an image in the flash is valid.
> > +	 * Rather than duplicate the check in this driver, call the relevant
> > +	 * service from the system controller instead.
> > +	 * This service has no command data and no response data. It overloads
> > +	 * mbox_offset with the image index in the flash's SPI directory where
> > +	 * the bitstream is located.
> > +	 */
> > +	response->resp_msg = response_msg;
> > +	response->resp_size = IAP_VERIFY_RESP_SIZE;
> > +	message->cmd_opcode = IAP_VERIFY_CMD_OPCODE;
> > +	message->cmd_data_size = IAP_VERIFY_CMD_DATA_SIZE;
> > +	message->response = response;
> > +	message->cmd_data = IAP_VERIFY_CMD_DATA;
> > +	message->mbox_offset = IAP_IMAGE_INDEX;
> > +	message->resp_offset = IAP_DEFAULT_RESP_OFFSET;
> > +
> > +	pr_info("ran IAP_VERIFY_RESP_SIZE\n");
> > +	ret = mpfs_blocking_transaction(priv->sys_controller, message);
> > +	if (ret | response->resp_status) {
> > +		ret = ret ? ret : -EBADMSG;
> > +		goto out;
> > +	}
> > +
> > +	/*
> > +	 * If the validation has passed, initiate IAP.
> > +	 * This service has no command data and no response data. It overloads
> > +	 * mbox_offset with the image index in the flash's SPI directory where
> > +	 * the bitstream is located.
> > +	 * Once we attempt IAP either:
> > +	 * - it passes and the board reboots
> > +	 * - it fails and the board reboots to recover
> > +	 * - the system controller aborts and we exit "gracefully"
> > +	 * This function will never return 0.
> > +	 */
> > +	response->resp_msg = response_msg;
> > +	response->resp_size = IAP_PROGRAM_RESP_SIZE;
> > +	message->cmd_opcode = IAP_PROGRAM_CMD_OPCODE;
> > +	message->cmd_data_size = IAP_PROGRAM_CMD_DATA_SIZE;
> > +	message->response = response;
> > +	message->cmd_data = IAP_PROGRAM_CMD_DATA;
> > +	message->mbox_offset = IAP_IMAGE_INDEX;
> > +	message->resp_offset = IAP_DEFAULT_RESP_OFFSET;
> > +
> > +	pr_info("ran IAP_PROGRAM_CMD_OPCODE\n");
> > +	ret = mpfs_blocking_transaction(priv->sys_controller, message);
> > +	if (ret)
> > +		goto out;
> > +
> > +	/*
> > +	 * This return 0 is dead code. Either the IAP will fail, or it will pass
> > +	 * & the FPGA will be rebooted in which case mpfs_blocking_transaction()
> > +	 * will never return and Linux will die.
> > +	 */
> > +	return 0;
> > +
> > +out:
> > +	devm_kfree(priv->dev, response_msg);
> > +	devm_kfree(priv->dev, response);
> > +	devm_kfree(priv->dev, message);
> > +	return ret;
> > +}
> > +
> > +static const struct fpga_manager_ops mpf_iap_ops = {
> > +	.state = mpf_iap_state,
> > +	.write_init = mpf_iap_write_init,
> > +	.write = mpf_iap_write,
> > +	.write_complete = mpf_iap_write_complete,
> > +};
> > +
> > +static int mpf_iap_run(struct device *dev)
> > +{
> > +	struct fpga_manager *mgr;
> > +	struct fpga_image_info *info;
> > +	int ret;
> > +
> > +	printk("starting to test the fpga manager\n");
> > +
> > +	mgr = fpga_mgr_get(dev);
> > +	info = fpga_image_info_alloc(dev);
> > +
> > +	info->firmware_name = devm_kstrdup(dev, "pf_bitstream.fw", GFP_KERNEL);
> > +	ret = fpga_mgr_lock(mgr);
> > +	if (ret) {
> > +		dev_err(dev, "couldnt lock the manager\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = fpga_mgr_load(mgr, info);
> > +	if (ret) {
> > +		dev_err(dev, "couldnt load the firmware\n");
> > +		return ret;
> > +	}
> > +
> > +	fpga_mgr_unlock(mgr);
> > +	fpga_mgr_put(mgr);
> > +	fpga_image_info_free(info);
> > +
> > +	dev_info(dev, "test complete\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int mpf_iap_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct mpf_iap_priv *priv;
> > +	struct fpga_manager *mgr;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->sys_controller = mpfs_sys_controller_get(dev);
> > +	if (IS_ERR(priv->sys_controller))
> > +		return dev_err_probe(dev, PTR_ERR(priv->sys_controller),
> > +				     "Could not register as a sub device of the system controller\n");
> > +
> > +	priv->dev = dev;
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	mgr = devm_fpga_mgr_register(dev, "Microchip MPF(S) IAP FPGA Manager",
> > +				     &mpf_iap_ops, priv);
> > +	if (IS_ERR(mgr))
> > +		return dev_err_probe(dev, PTR_ERR(mgr),
> > +				     "Could not register FPGA manager.\n");
> > +
> > +	enum fpga_mgr_states state = mpf_iap_state(mgr);
> > +	//ret = mpf_iap_run(dev);
> > +	ret = 1;
> > +	if (ret)
> > +		dev_err_probe(dev, ret, "IAP failed");
> > +
> > +	dev_info(dev, "Registered Microchip MPF(S) IAP FPGA Manager %u\n", state);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver mpf_iap_driver = {
> > +	.driver = {
> > +		.name = "mpfs-iap",
> > +	},
> > +	.probe = mpf_iap_probe,
> > +};
> > +module_platform_driver(mpf_iap_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Conor Dooley <conor.dooley at microchip.com>");
> > +MODULE_DESCRIPTION("PolarFire SoC IAP FPGA reprogramming");
> > -- 
> > 2.37.2
> > 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



More information about the linux-riscv mailing list