From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Mario Limonciello <mario.limonciello@dell.com>
Cc: Andreas Noever <andreas.noever@gmail.com>,
Michael Jamet <michael.jamet@intel.com>,
Yehezkel Bernat <YehezkelShB@gmail.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] thunderbolt: Add support for separating the flush to SPI and authenticate
Date: Tue, 23 Jun 2020 17:32:52 +0300 [thread overview]
Message-ID: <20200623143252.GD2795@lahna.fi.intel.com> (raw)
In-Reply-To: <20200622185758.28145-2-mario.limonciello@dell.com>
On Mon, Jun 22, 2020 at 01:57:57PM -0500, Mario Limonciello wrote:
> This allows userspace to have a shorter period of time that the device
> is unusable and to call it at a more convenient time.
>
> For example flushing the image may happen while the user is using the
> machine and authenticating/rebooting may happen while logging out.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
> .../ABI/testing/sysfs-bus-thunderbolt | 11 ++++-
> drivers/thunderbolt/nvm.c | 1 +
> drivers/thunderbolt/switch.c | 42 ++++++++++++-------
> drivers/thunderbolt/tb.h | 2 +
> 4 files changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
> index bd504ed323e8..7d0500b4d58a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
> +++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
> @@ -178,11 +178,18 @@ KernelVersion: 4.13
> Contact: thunderbolt-software@lists.01.org
> Description: When new NVM image is written to the non-active NVM
> area (through non_activeX NVMem device), the
> - authentication procedure is started by writing 1 to
> - this file. If everything goes well, the device is
> + authentication procedure is started by writing to
> + this file.
> + If everything goes well, the device is
> restarted with the new NVM firmware. If the image
> verification fails an error code is returned instead.
>
> + This file will accept writing values "1" or "2"
> + - Writing "1" will flush the image to the storage
> + area and authenticate the image in one action.
> + - Writing "2" will run some basic validation on the image
> + and flush it to the storage area.
> +
> When read holds status of the last authentication
> operation if an error occurred during the process. This
> is directly the status value from the DMA configuration
> diff --git a/drivers/thunderbolt/nvm.c b/drivers/thunderbolt/nvm.c
> index 4c6aa06ab3d5..29de6d95c6e7 100644
> --- a/drivers/thunderbolt/nvm.c
> +++ b/drivers/thunderbolt/nvm.c
> @@ -100,6 +100,7 @@ int tb_nvm_write_buf(struct tb_nvm *nvm, unsigned int offset, void *val,
> return -ENOMEM;
> }
>
> + nvm->flushed = false;
This means every write invalidates the "flushed" state, right?
> nvm->buf_data_size = offset + bytes;
> memcpy(nvm->buf + offset, val, bytes);
> return 0;
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 817c66c7adcf..bbfbfebeee7f 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -26,6 +26,11 @@ struct nvm_auth_status {
> u32 status;
> };
>
> +enum nvm_write_ops {
> + WRITE_AND_AUTHENTICATE = 1,
> + WRITE_ONLY = 2,
> +};
> +
> /*
> * Hold NVM authentication failure status per switch This information
> * needs to stay around even when the switch gets power cycled so we
> @@ -155,8 +160,12 @@ static int nvm_validate_and_write(struct tb_switch *sw)
> }
>
> if (tb_switch_is_usb4(sw))
> - return usb4_switch_nvm_write(sw, 0, buf, image_size);
> - return dma_port_flash_write(sw->dma_port, 0, buf, image_size);
> + ret = usb4_switch_nvm_write(sw, 0, buf, image_size);
> + else
> + ret = dma_port_flash_write(sw->dma_port, 0, buf, image_size);
> + if (!ret)
> + sw->nvm->flushed = true;
> + return ret;
> }
>
> static int nvm_authenticate_host_dma_port(struct tb_switch *sw)
> @@ -1488,7 +1497,7 @@ static ssize_t nvm_authenticate_store(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t count)
> {
> struct tb_switch *sw = tb_to_switch(dev);
> - bool val;
> + int val;
> int ret;
>
> pm_runtime_get_sync(&sw->dev);
> @@ -1504,25 +1513,28 @@ static ssize_t nvm_authenticate_store(struct device *dev,
> goto exit_unlock;
> }
>
> - ret = kstrtobool(buf, &val);
> + ret = kstrtoint(buf, 10, &val);
> if (ret)
> goto exit_unlock;
>
> /* Always clear the authentication status */
> nvm_clear_auth_status(sw);
>
> - if (val) {
> - if (!sw->nvm->buf) {
> - ret = -EINVAL;
> - goto exit_unlock;
> - }
> -
> - ret = nvm_validate_and_write(sw);
> - if (ret)
> - goto exit_unlock;
> + if (val > 0) {
> + if (!sw->nvm->flushed) {
> + if (!sw->nvm->buf) {
> + ret = -EINVAL;
> + goto exit_unlock;
> + }
>
> - sw->nvm->authenticating = true;
> - ret = nvm_authenticate(sw);
> + ret = nvm_validate_and_write(sw);
> + if (ret || val == WRITE_ONLY)
> + goto exit_unlock;
> + }
> + if (val == WRITE_AND_AUTHENTICATE) {
> + sw->nvm->authenticating = true;
> + ret = nvm_authenticate(sw);
> + }
> }
>
> exit_unlock:
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 736d1589c31e..43a8ca2eb3d8 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -42,6 +42,7 @@
> *
> * The user of this structure needs to handle serialization of possible
> * concurrent access.
> + * @flushed: The image has been flushed to the storage area
This should go below the @authenticating description.
> */
> struct tb_nvm {
> struct device *dev;
> @@ -53,6 +54,7 @@ struct tb_nvm {
> void *buf;
> size_t buf_data_size;
> bool authenticating;
> + bool flushed;
> };
>
> #define TB_SWITCH_KEY_SIZE 32
> --
> 2.25.1
next prev parent reply other threads:[~2020-06-23 14:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-22 18:57 [PATCH v2 0/2] Allow breaking up Thunderbolt/USB4 updates Mario Limonciello
2020-06-22 18:57 ` [PATCH v2 1/2] thunderbolt: Add support for separating the flush to SPI and authenticate Mario Limonciello
2020-06-23 14:32 ` Mika Westerberg [this message]
2020-06-23 14:41 ` Mario.Limonciello
2020-06-22 18:57 ` [PATCH v2 2/2] thunderbolt: Add support for authenticate on disconnect Mario Limonciello
2020-06-23 14:39 ` Mika Westerberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200623143252.GD2795@lahna.fi.intel.com \
--to=mika.westerberg@linux.intel.com \
--cc=YehezkelShB@gmail.com \
--cc=andreas.noever@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mario.limonciello@dell.com \
--cc=michael.jamet@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).