On Sun, Mar 05, 2023 at 12:38:00AM +0800, Xu Yilun wrote:
> On 2023-02-17 at 16:40:22 +0000, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> >
> > Add support for Auto Update reprogramming of the FPGA fabric on
> > PolarFire SoC.
> >
> > Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > ---
> > drivers/fpga/Kconfig | 9 +
> > drivers/fpga/Makefile | 1 +
> > drivers/fpga/microchip-auto-update.c | 495 +++++++++++++++++++++++++++
> > 3 files changed, 505 insertions(+)
> > create mode 100644 drivers/fpga/microchip-auto-update.c
> > + /*
> > + * To verify that Auto Update is possible, the "Query Security Service
> Why verify the possibility here, if Auto Update is not possible, the
> Auto Update device should not be populated, is it?
Good point, I'll check this in probe instead.
> > + /*
> > + * Populate the image address and then zero out the next directory so
> > + * that the system controller doesn't complain if in "Single Image"
> > + * mode.
> > + */
> > + memcpy(buffer + AUTO_UPDATE_UPGRADE_DIRECTORY, &image_address, AUTO_UPDATE_DIRECTORY_WIDTH);
> > + memset(buffer + AUTO_UPDATE_BLANK_DIRECTORY, 0x0, AUTO_UPDATE_DIRECTORY_WIDTH);
>
> I'm wondering why the image address should be written for every
> updating? Seems it is only related to the flash size, not related to
> the to-be-programmed bitstream.
Yah, it doesn't need to be. I'll check it against the expected value &
only write it if needed.
> > + dev_info(priv->dev, "Running verification of Upgrade Image\n");
> > + ret = mpfs_blocking_transaction(priv->sys_controller, message);
> > + if (ret | response->resp_status) {
> > + dev_warn(priv->dev, "Verification of Upgrade Image failed!\n");
> > + ret = ret ? ret : -EBADMSG;
>
> If verification failed, what happens to the written flash? Auto roll
> back?
Nope, that should be left up to userspace to decide what to do. I've got
some improvement to do to the mailbox driver that sits behind
mpfs_blocking_transaction() that I thought was not allowed by the
mailbox framework, so should be able to report better errors for this in
the future.
> > + }
> > +
> > + dev_info(priv->dev, "Verification of Upgrade Image passed!\n");
> > +// /*
> > +// * If the validation has passed, initiate Auto Update.
> > +// * 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 Auto Update either:
> > +// * - it passes and the board reboots
> > +// * - it fails and the board reboots to recover
> > +// * - the system controller aborts and we exit "gracefully".
> > +// * "gracefully" since there is no interrupt produced & it just times
> > +// * out.
> > +// */
> > +// response->resp_msg = response_msg;
> > +// response->resp_size = AUTO_UPDATE_PROGRAM_RESP_SIZE;
> > +// message->cmd_opcode = AUTO_UPDATE_PROGRAM_CMD_OPCODE;
> > +// message->cmd_data_size = AUTO_UPDATE_PROGRAM_CMD_DATA_SIZE;
> > +// message->response = response;
> > +// message->cmd_data = AUTO_UPDATE_PROGRAM_CMD_DATA;
> > +// message->mbox_offset = 0; //field is ignored
> > +// message->resp_offset = AUTO_UPDATE_DEFAULT_RESP_OFFSET;
> > +//
> > +// dev_info(priv->dev, "Running Auto Update command\n");
> > +// ret = mpfs_blocking_transaction(priv->sys_controller, message);
> > +// if (ret && ret != -ETIMEDOUT)
> > +// goto out;
> > +//
> > +// /* *remove this for auto update*
> > +// * This return 0 is dead code. Either the Auto Update 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;
>
> Why comment out this code block?
It was meant to be removed & must have snuck back in a rebase. This is my
test code that initiates the update from Linux, rather than at reboot.
I'm going to take a look at Russ' driver before I submit another version
of this (and the underlying mailbox stuff also needs changes).
Thanks for taking a look,
Conor.
Attachment:
signature.asc
Description: PGP signature
