From: Cyril Bur <cyrilbur@gmail•com>
To: Daniel Axtens <dja@axtens•net>
Cc: linuxppc-dev@ozlabs•org, mikey@neuling•org,
"Matthew R. Ochs" <mrochs@linux•vnet.ibm.com>,
imunsie@au•ibm.com, Manoj Kumar <kumarmn@us•ibm.com>
Subject: Re: [PATCH v2 08/10] cxl: Allow the kernel to trust that an image won't change on PERST.
Date: Tue, 11 Aug 2015 17:15:38 +1000 [thread overview]
Message-ID: <20150811171538.477c3842@camb691> (raw)
In-Reply-To: <1438061323-20710-9-git-send-email-dja@axtens.net>
On Tue, 28 Jul 2015 15:28:41 +1000
Daniel Axtens <dja@axtens•net> wrote:
> Provide a kernel API and a sysfs entry which allow a user to specify
> that when a card is PERSTed, it's image will stay the same, allowing
> it to participate in EEH.
>
> cxl_reset is used to reflash the card. In that case, we cannot safely
> assert that the image will not change. Therefore, disallow cxl_reset
> if the flag is set.
>
So I'm not super all over the putting all sorts of code inside CONFIG_CXL_EEH,
I understand that there is another driver being merged and they'll use
CONFIG_CXL_EEH so that both this driver and the other driver can go in the same
merge window but does this mean you need to put it around everything here?
I may have misunderstood what you've told me but if the other driver depends on
work done in this one (and not the other way around), if they depend on
CONFIG_CXL_EEH which you create in the last patch, then they cannot be built
until this series exists, so they can't have issues.
The one catch is that this series as is waits untill the last patch to actually
create the symbol, and therefore compile everything so lets be sure you don't
break bisecting. You might need to rethink the order of things in 8/10 and 9/10,
I can't see anything obvious if it helps...
> Signed-off-by: Daniel Axtens <dja@axtens•net>
> ---
> Documentation/ABI/testing/sysfs-class-cxl | 10 ++++++++++
> drivers/misc/cxl/api.c | 9 +++++++++
> drivers/misc/cxl/cxl.h | 3 +++
> drivers/misc/cxl/pci.c | 11 +++++++++++
> drivers/misc/cxl/sysfs.c | 30 ++++++++++++++++++++++++++++++
> include/misc/cxl.h | 12 ++++++++++++
> 6 files changed, 75 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-cxl b/Documentation/ABI/testing/sysfs-class-cxl
> index acfe9df83139..b07e86d4597f 100644
> --- a/Documentation/ABI/testing/sysfs-class-cxl
> +++ b/Documentation/ABI/testing/sysfs-class-cxl
> @@ -223,3 +223,13 @@ Description: write only
> Writing 1 will issue a PERST to card which may cause the card
> to reload the FPGA depending on load_image_on_perst.
> Users: https://github.com/ibm-capi/libcxl
> +
> +What: /sys/class/cxl/<card>/perst_reloads_same_image
> +Date: July 2015
> +Contact: linuxppc-dev@lists•ozlabs.org
> +Description: read/write
> + Trust that when an image is reloaded via PERST, it will not
> + have changed.
> + 0 = don't trust, the image may be different (default)
> + 1 = trust that the image will not change.
> +Users: https://github.com/ibm-capi/libcxl
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 729e0851167d..c1012ced0323 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -327,3 +327,12 @@ int cxl_afu_reset(struct cxl_context *ctx)
> return cxl_afu_check_and_enable(afu);
> }
> EXPORT_SYMBOL_GPL(cxl_afu_reset);
> +
> +#ifdef CONFIG_CXL_EEH
> +void cxl_perst_reloads_same_image(struct cxl_afu *afu,
> + bool perst_reloads_same_image)
> +{
> + afu->adapter->perst_same_image = perst_reloads_same_image;
> +}
> +EXPORT_SYMBOL_GPL(cxl_perst_reloads_same_image);
> +#endif
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index 88a88c445e2a..6dd4158f76ac 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -493,6 +493,9 @@ struct cxl {
> bool user_image_loaded;
> bool perst_loads_image;
> bool perst_select_user;
> +#ifdef CONFIG_CXL_EEH
> + bool perst_same_image;
> +#endif
> };
>
> int cxl_alloc_one_irq(struct cxl *adapter);
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 0acf9e62733e..b6a189b35323 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -875,6 +875,14 @@ int cxl_reset(struct cxl *adapter)
> int i;
> u32 val;
>
> +#ifdef CONFIG_CXL_EEH
> + if (adapter->perst_same_image) {
> + dev_warn(&dev->dev,
> + "cxl: refusing to reset/reflash when perst_reloads_same_image is set.\n");
> + return -EINVAL;
> + }
> +#endif
> +
> dev_info(&dev->dev, "CXL reset\n");
>
> /* pcie_warm_reset requests a fundamental pci reset which includes a
> @@ -1148,6 +1156,9 @@ static struct cxl *cxl_init_adapter(struct pci_dev *dev)
> * configure/reconfigure
> */
> adapter->perst_loads_image = true;
> +#ifdef CONFIG_CXL_EEH
> + adapter->perst_same_image = false;
> +#endif
>
> if ((rc = cxl_configure_adapter(adapter, dev))) {
> pci_disable_device(dev);
> diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
> index 31f38bc71a3d..4bcb63258e3e 100644
> --- a/drivers/misc/cxl/sysfs.c
> +++ b/drivers/misc/cxl/sysfs.c
> @@ -112,12 +112,42 @@ static ssize_t load_image_on_perst_store(struct device *device,
> return count;
> }
>
> +#ifdef CONFIG_CXL_EEH
> +static ssize_t perst_reloads_same_image_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl *adapter = to_cxl_adapter(device);
> +
> + return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->perst_same_image);
> +}
> +
> +static ssize_t perst_reloads_same_image_store(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cxl *adapter = to_cxl_adapter(device);
> + int rc;
> + int val;
> +
> + rc = sscanf(buf, "%i", &val);
> + if ((rc != 1) || !(val == 1 || val == 0))
> + return -EINVAL;
> +
> + adapter->perst_same_image = (val == 1 ? true : false);
> + return count;
> +}
> +#endif /* CONFIG_CXL_EEH */
> +
> static struct device_attribute adapter_attrs[] = {
> __ATTR_RO(caia_version),
> __ATTR_RO(psl_revision),
> __ATTR_RO(base_image),
> __ATTR_RO(image_loaded),
> __ATTR_RW(load_image_on_perst),
> +#ifdef CONFIG_CXL_EEH
> + __ATTR_RW(perst_reloads_same_image),
> +#endif
> __ATTR(reset, S_IWUSR, NULL, reset_adapter_store),
> };
>
> diff --git a/include/misc/cxl.h b/include/misc/cxl.h
> index 7a6c1d6cc173..7f2cdc6caab3 100644
> --- a/include/misc/cxl.h
> +++ b/include/misc/cxl.h
> @@ -200,4 +200,16 @@ unsigned int cxl_fd_poll(struct file *file, struct poll_table_struct *poll);
> ssize_t cxl_fd_read(struct file *file, char __user *buf, size_t count,
> loff_t *off);
>
> +#ifdef CONFIG_CXL_EEH
> +/*
> + * For EEH, a driver may want to assert a PERST will reload the same image
> + * from flash into the FPGA.
> + *
> + * This is a property of the entire adapter, not a single AFU, so drivers
> + * should set this property with care!
> + */
> +void cxl_perst_reloads_same_image(struct cxl_afu *afu,
> + bool perst_reloads_same_image);
> +#endif /* CONFIG_CXL_EEH */
> +
> #endif /* _MISC_CXL_H */
next prev parent reply other threads:[~2015-08-11 7:14 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-28 5:28 [PATCH v2 00/10] CXL EEH Handling Daniel Axtens
2015-07-28 5:28 ` [PATCH v2 01/10] cxl: Drop commands if the PCI channel is not in normal state Daniel Axtens
2015-08-11 3:31 ` Cyril Bur
2015-08-11 4:11 ` Daniel Axtens
2015-07-28 5:28 ` [PATCH v2 02/10] cxl: Allocate and release the SPA with the AFU Daniel Axtens
2015-08-11 3:42 ` Cyril Bur
2015-08-11 4:16 ` Daniel Axtens
2015-07-28 5:28 ` [PATCH v2 03/10] cxl: Make IRQ release idempotent Daniel Axtens
2015-08-11 3:44 ` Cyril Bur
2015-07-28 5:28 ` [PATCH v2 04/10] cxl: Clean up adapter MMIO unmap path Daniel Axtens
2015-08-11 3:52 ` Cyril Bur
2015-08-11 6:38 ` Daniel Axtens
2015-07-28 5:28 ` [PATCH v2 05/10] cxl: Refactor adaptor init/teardown Daniel Axtens
2015-08-11 6:01 ` Cyril Bur
2015-08-11 22:38 ` Daniel Axtens
2015-08-12 10:14 ` David Laight
2015-08-12 21:58 ` Daniel Axtens
2015-07-28 5:28 ` [PATCH v2 06/10] cxl: Refactor AFU init/teardown Daniel Axtens
2015-08-11 3:59 ` Cyril Bur
2015-07-28 5:28 ` [PATCH v2 07/10] cxl: Don't remove AFUs/vPHBs in cxl_reset Daniel Axtens
2015-08-11 5:57 ` Cyril Bur
2015-07-28 5:28 ` [PATCH v2 08/10] cxl: Allow the kernel to trust that an image won't change on PERST Daniel Axtens
2015-08-11 7:15 ` Cyril Bur [this message]
2015-08-11 11:22 ` Daniel Axtens
2015-08-11 23:47 ` Daniel Axtens
2015-07-28 5:28 ` [PATCH v2 09/10] cxl: EEH support Daniel Axtens
2015-08-11 7:23 ` Cyril Bur
2015-07-28 5:28 ` [PATCH v2 10/10] cxl: Add CONFIG_CXL_EEH symbol Daniel Axtens
2015-08-11 3:59 ` Cyril Bur
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=20150811171538.477c3842@camb691 \
--to=cyrilbur@gmail$(echo .)com \
--cc=dja@axtens$(echo .)net \
--cc=imunsie@au$(echo .)ibm.com \
--cc=kumarmn@us$(echo .)ibm.com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=mikey@neuling$(echo .)org \
--cc=mrochs@linux$(echo .)vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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