Subject: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP

From: Appana Durga Kedareswara rao <[email protected]>

The Triple Modular Redundancy(TMR) provides functional fault injection by
changing selected MicroBlaze instructions, which provides the possibility
to verify that the TMR subsystem error detection and fault recovery logic
is working properly, provided sysfs entries which allow the user to inject
a fault.

Usage:
echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err

Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
---
Changes for v2:
--> Added Examples for sysfs entries
--> Removed uneeded struct dev from the driver private structure
--> Updated driver to use sysfs_emit() API instead of sprintf() API
--> Added error checks wherever applicable.
--> Fixed sysfs registration.
.../testing/sysfs-driver-xilinx-tmr-inject | 16 ++
MAINTAINERS | 7 +
drivers/misc/Kconfig | 10 +
drivers/misc/Makefile | 1 +
drivers/misc/xilinx_tmr_inject.c | 186 ++++++++++++++++++
5 files changed, 220 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
create mode 100644 drivers/misc/xilinx_tmr_inject.c

diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
new file mode 100644
index 000000000000..d274b30ee24c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
@@ -0,0 +1,16 @@
+What: /sys/devices/platform/amba_pl/<dev>/inject_err
+Date: June 2022
+Contact: [email protected]
+Description: This control file allows to inject fault using tmr inject.
+ This file is write only.
+ Example:
+ # echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
+
+What: /sys/devices/platform/amba_pl/<dev>/inject_cpuid
+Date: June 2022
+Contact: [email protected]
+Description: This control file allows to configure the CPU identifier
+ to enable fault injection.
+ This file is write only.
+ Example:
+ # echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_cpuid
diff --git a/MAINTAINERS b/MAINTAINERS
index 732fd9ae7d9f..c903b45c204a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13087,6 +13087,13 @@ F: Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
F: Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
F: drivers/misc/xilinx_tmr_manager.c

+MICROBLAZE TMR INJECT
+M: Appana Durga Kedareswara rao <[email protected]>
+S: Supported
+F: Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
+F: Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
+F: drivers/misc/xilinx_tmr_inject.c
+
MICROCHIP AT91 DMA DRIVERS
M: Ludovic Desroches <[email protected]>
M: Tudor Ambarus <[email protected]>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 555ae2e33b91..0989c36f3051 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -493,6 +493,16 @@ config TMR_MANAGER

Say N here unless you know what you are doing.

+config TMR_INJECT
+ bool "Select TMR Inject"
+ depends on TMR_MANAGER
+ help
+ This option enables the driver developed for TMR Inject.
+ The Triple Modular Redundancy(TMR) Inject provides
+ fault injection.
+
+ Say N here unless you know what you are doing.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 28b9803f909b..e9d0a709e207 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -61,3 +61,4 @@ obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
obj-$(CONFIG_OPEN_DICE) += open-dice.o
obj-$(CONFIG_TMR_MANAGER) += xilinx_tmr_manager.o
+obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
diff --git a/drivers/misc/xilinx_tmr_inject.c b/drivers/misc/xilinx_tmr_inject.c
new file mode 100644
index 000000000000..930d89e90b61
--- /dev/null
+++ b/drivers/misc/xilinx_tmr_inject.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Xilinx TMR Inject IP.
+ *
+ * Copyright (C) 2022 Xilinx, Inc.
+ *
+ * Description:
+ * This driver is developed for TMR Inject IP,The Triple Modular Redundancy(TMR)
+ * Inject provides fault injection.
+ * Fault injection and detection features are provided through sysfs entries
+ * which allow the user to generate a fault.
+ */
+
+#include <asm/xilinx_mb_manager.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+/* TMR Inject Register offsets */
+#define XTMR_INJECT_CR_OFFSET 0x0
+#define XTMR_INJECT_AIR_OFFSET 0x4
+#define XTMR_INJECT_IIR_OFFSET 0xC
+#define XTMR_INJECT_EAIR_OFFSET 0x10
+#define XTMR_INJECT_ERR_OFFSET 0x204
+
+/* Register Bitmasks/shifts */
+#define XTMR_INJECT_CR_CPUID_SHIFT 8
+#define XTMR_INJECT_CR_IE_SHIFT 10
+#define XTMR_INJECT_IIR_ADDR_MASK GENMASK(31, 16)
+
+#define XTMR_INJECT_MAGIC_MAX_VAL 255
+
+/**
+ * struct xtmr_inject_dev - Driver data for TMR Inject
+ * @regs: device physical base address
+ * @cr_val: control register value
+ * @magic: Magic hardware configuration value
+ * @err_cnt: error statistics count
+ */
+struct xtmr_inject_dev {
+ void __iomem *regs;
+ u32 cr_val;
+ u32 magic;
+ u32 err_cnt;
+};
+
+/* IO accessors */
+static inline void xtmr_inject_write(struct xtmr_inject_dev *xtmr_inject,
+ u32 addr, u32 value)
+{
+ iowrite32(value, xtmr_inject->regs + addr);
+}
+
+static inline u32 xtmr_inject_read(struct xtmr_inject_dev *xtmr_inject,
+ u32 addr)
+{
+ return ioread32(xtmr_inject->regs + addr);
+}
+
+static ssize_t inject_err_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t size)
+{
+ int ret;
+ long value;
+
+ ret = kstrtoul(buf, 16, &value);
+ if (ret)
+ return ret;
+
+ if (value > 1)
+ return -EINVAL;
+
+ xmb_inject_err();
+
+ return size;
+}
+static DEVICE_ATTR_WO(inject_err);
+
+static ssize_t inject_cpuid_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct xtmr_inject_dev *xtmr_inject = dev_get_drvdata(dev);
+ int ret;
+ long value;
+
+ ret = kstrtoul(buf, 0, &value);
+ if (ret)
+ return ret;
+
+ if (value > 3)
+ return -EINVAL;
+
+ xtmr_inject->cr_val |= (value << XTMR_INJECT_CR_CPUID_SHIFT);
+ xtmr_inject_write(xtmr_inject, XTMR_INJECT_CR_OFFSET,
+ xtmr_inject->cr_val);
+
+ return size;
+}
+static DEVICE_ATTR_WO(inject_cpuid);
+
+static struct attribute *xtmr_inject_dev_attrs[] = {
+ &dev_attr_inject_err.attr,
+ &dev_attr_inject_cpuid.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(xtmr_inject_dev);
+
+static void xtmr_inject_init(struct xtmr_inject_dev *xtmr_inject)
+{
+ /* Allow fault injection */
+ xtmr_inject->cr_val = xtmr_inject->magic |
+ (1 << XTMR_INJECT_CR_IE_SHIFT) |
+ (1 << XTMR_INJECT_CR_CPUID_SHIFT);
+ xtmr_inject_write(xtmr_inject, XTMR_INJECT_CR_OFFSET,
+ xtmr_inject->cr_val);
+ /* Initialize the address inject and instruction inject registers */
+ xtmr_inject_write(xtmr_inject, XTMR_INJECT_AIR_OFFSET,
+ XMB_INJECT_ERR_OFFSET);
+ xtmr_inject_write(xtmr_inject, XTMR_INJECT_IIR_OFFSET,
+ XMB_INJECT_ERR_OFFSET & XTMR_INJECT_IIR_ADDR_MASK);
+}
+
+/**
+ * xtmr_inject_probe - Driver probe function
+ * @pdev: Pointer to the platform_device structure
+ *
+ * This is the driver probe routine. It does all the memory
+ * allocation and creates sysfs entries for the device.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int xtmr_inject_probe(struct platform_device *pdev)
+{
+ struct xtmr_inject_dev *xtmr_inject;
+ int err;
+
+ xtmr_inject = devm_kzalloc(&pdev->dev, sizeof(*xtmr_inject),
+ GFP_KERNEL);
+ if (!xtmr_inject)
+ return -ENOMEM;
+
+ xtmr_inject->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(xtmr_inject->regs))
+ return PTR_ERR(xtmr_inject->regs);
+
+ err = of_property_read_u32(pdev->dev.of_node, "xlnx,magic",
+ &xtmr_inject->magic);
+ if (err < 0) {
+ dev_err(&pdev->dev, "unable to read xlnx,magic property");
+ return err;
+ }
+
+ if (xtmr_inject->magic > XTMR_INJECT_MAGIC_MAX_VAL) {
+ dev_err(&pdev->dev, "invalid xlnx,magic property value");
+ return -EINVAL;
+ }
+
+ /* Initialize TMR Inject */
+ xtmr_inject_init(xtmr_inject);
+
+ platform_set_drvdata(pdev, xtmr_inject);
+
+ return 0;
+}
+
+static const struct of_device_id xtmr_inject_of_match[] = {
+ {
+ .compatible = "xlnx,tmr-inject-1.0",
+ },
+ { /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, xtmr_inject_of_match);
+
+static struct platform_driver xtmr_inject_driver = {
+ .driver = {
+ .name = "xilinx-tmr_inject",
+ .of_match_table = xtmr_inject_of_match,
+ .dev_groups = xtmr_inject_dev_groups,
+ },
+ .probe = xtmr_inject_probe,
+};
+module_platform_driver(xtmr_inject_driver);
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Xilinx TMR Inject Driver");
+MODULE_LICENSE("GPL");
--
2.25.1


2022-07-28 15:09:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP

On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
> From: Appana Durga Kedareswara rao <[email protected]>
>
> The Triple Modular Redundancy(TMR) provides functional fault injection by
> changing selected MicroBlaze instructions, which provides the possibility
> to verify that the TMR subsystem error detection and fault recovery logic
> is working properly, provided sysfs entries which allow the user to inject
> a fault.

We already have a fault-injection api, why are you not using that?

>
> Usage:
> echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
>
> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>

Odd, just one is needed.

> ---
> Changes for v2:
> --> Added Examples for sysfs entries
> --> Removed uneeded struct dev from the driver private structure
> --> Updated driver to use sysfs_emit() API instead of sprintf() API
> --> Added error checks wherever applicable.
> --> Fixed sysfs registration.
> .../testing/sysfs-driver-xilinx-tmr-inject | 16 ++
> MAINTAINERS | 7 +
> drivers/misc/Kconfig | 10 +
> drivers/misc/Makefile | 1 +
> drivers/misc/xilinx_tmr_inject.c | 186 ++++++++++++++++++
> 5 files changed, 220 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> create mode 100644 drivers/misc/xilinx_tmr_inject.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> new file mode 100644
> index 000000000000..d274b30ee24c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> @@ -0,0 +1,16 @@
> +What: /sys/devices/platform/amba_pl/<dev>/inject_err
> +Date: June 2022

It's not June anymore, even when you sent this patch :(

> +Contact: [email protected]
> +Description: This control file allows to inject fault using tmr inject.
> + This file is write only.
> + Example:
> + # echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
> +
> +What: /sys/devices/platform/amba_pl/<dev>/inject_cpuid
> +Date: June 2022
> +Contact: [email protected]
> +Description: This control file allows to configure the CPU identifier
> + to enable fault injection.
> + This file is write only.
> + Example:
> + # echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_cpuid

What errors and faults happen? Where is that documented? What happens
when you write to these sysfs files? Does the system crash? Why would
you want to use them ever?



> diff --git a/MAINTAINERS b/MAINTAINERS
> index 732fd9ae7d9f..c903b45c204a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13087,6 +13087,13 @@ F: Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
> F: Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
> F: drivers/misc/xilinx_tmr_manager.c
>
> +MICROBLAZE TMR INJECT
> +M: Appana Durga Kedareswara rao <[email protected]>
> +S: Supported
> +F: Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
> +F: Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
> +F: drivers/misc/xilinx_tmr_inject.c
> +
> MICROCHIP AT91 DMA DRIVERS
> M: Ludovic Desroches <[email protected]>
> M: Tudor Ambarus <[email protected]>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 555ae2e33b91..0989c36f3051 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -493,6 +493,16 @@ config TMR_MANAGER
>
> Say N here unless you know what you are doing.
>
> +config TMR_INJECT
> + bool "Select TMR Inject"
> + depends on TMR_MANAGER
> + help
> + This option enables the driver developed for TMR Inject.
> + The Triple Modular Redundancy(TMR) Inject provides
> + fault injection.
> +
> + Say N here unless you know what you are doing.

Why can't this be a module?



> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 28b9803f909b..e9d0a709e207 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -61,3 +61,4 @@ obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
> obj-$(CONFIG_OPEN_DICE) += open-dice.o
> obj-$(CONFIG_TMR_MANAGER) += xilinx_tmr_manager.o
> +obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
> diff --git a/drivers/misc/xilinx_tmr_inject.c b/drivers/misc/xilinx_tmr_inject.c
> new file mode 100644
> index 000000000000..930d89e90b61
> --- /dev/null
> +++ b/drivers/misc/xilinx_tmr_inject.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Xilinx TMR Inject IP.
> + *
> + * Copyright (C) 2022 Xilinx, Inc.
> + *
> + * Description:
> + * This driver is developed for TMR Inject IP,The Triple Modular Redundancy(TMR)
> + * Inject provides fault injection.
> + * Fault injection and detection features are provided through sysfs entries
> + * which allow the user to generate a fault.
> + */
> +
> +#include <asm/xilinx_mb_manager.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +
> +/* TMR Inject Register offsets */
> +#define XTMR_INJECT_CR_OFFSET 0x0
> +#define XTMR_INJECT_AIR_OFFSET 0x4
> +#define XTMR_INJECT_IIR_OFFSET 0xC
> +#define XTMR_INJECT_EAIR_OFFSET 0x10
> +#define XTMR_INJECT_ERR_OFFSET 0x204
> +
> +/* Register Bitmasks/shifts */
> +#define XTMR_INJECT_CR_CPUID_SHIFT 8
> +#define XTMR_INJECT_CR_IE_SHIFT 10
> +#define XTMR_INJECT_IIR_ADDR_MASK GENMASK(31, 16)
> +
> +#define XTMR_INJECT_MAGIC_MAX_VAL 255
> +
> +/**
> + * struct xtmr_inject_dev - Driver data for TMR Inject
> + * @regs: device physical base address
> + * @cr_val: control register value
> + * @magic: Magic hardware configuration value
> + * @err_cnt: error statistics count
> + */
> +struct xtmr_inject_dev {
> + void __iomem *regs;
> + u32 cr_val;
> + u32 magic;
> + u32 err_cnt;
> +};
> +
> +/* IO accessors */
> +static inline void xtmr_inject_write(struct xtmr_inject_dev *xtmr_inject,
> + u32 addr, u32 value)
> +{
> + iowrite32(value, xtmr_inject->regs + addr);
> +}
> +
> +static inline u32 xtmr_inject_read(struct xtmr_inject_dev *xtmr_inject,
> + u32 addr)
> +{
> + return ioread32(xtmr_inject->regs + addr);
> +}
> +
> +static ssize_t inject_err_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t size)
> +{
> + int ret;
> + long value;
> +
> + ret = kstrtoul(buf, 16, &value);
> + if (ret)
> + return ret;
> +
> + if (value > 1)
> + return -EINVAL;

That does not match your documentation :(


> +
> + xmb_inject_err();
> +
> + return size;
> +}
> +static DEVICE_ATTR_WO(inject_err);
> +
> +static ssize_t inject_cpuid_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct xtmr_inject_dev *xtmr_inject = dev_get_drvdata(dev);
> + int ret;
> + long value;
> +
> + ret = kstrtoul(buf, 0, &value);
> + if (ret)
> + return ret;
> +
> + if (value > 3)
> + return -EINVAL;

Again, does not match the documentation at all.

confused,

greg k-h

Subject: Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP

Hi Greg,

Thanks for the review.

On 28/07/22 7:54 pm, Greg KH wrote:
> On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
>> From: Appana Durga Kedareswara rao <[email protected]>
>>
>> The Triple Modular Redundancy(TMR) provides functional fault injection by
>> changing selected MicroBlaze instructions, which provides the possibility
>> to verify that the TMR subsystem error detection and fault recovery logic
>> is working properly, provided sysfs entries which allow the user to inject
>> a fault.
>
> We already have a fault-injection api, why are you not using that?
>

Inorder to inject the error using TMR inject IP, The API
which injects the error should be executed from Processor LMB,
below sysfs entry calls microblaze core API xmb_inject_err()
which switches the processor to real mode and injects the error,
Please find the code corresponds to xmb_inject_err() API here:
https://www.spinics.net/lists/arm-kernel/msg991888.html


>>
>> Usage:
>> echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
>>
>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>> Signed-off-by: Appana Durga Kedareswara rao <[email protected]>
>
> Odd, just one is needed.

Sure will fix in next version.
>
>> ---
>> Changes for v2:
>> --> Added Examples for sysfs entries
>> --> Removed uneeded struct dev from the driver private structure
>> --> Updated driver to use sysfs_emit() API instead of sprintf() API
>> --> Added error checks wherever applicable.
>> --> Fixed sysfs registration.
>> .../testing/sysfs-driver-xilinx-tmr-inject | 16 ++
>> MAINTAINERS | 7 +
>> drivers/misc/Kconfig | 10 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/xilinx_tmr_inject.c | 186 ++++++++++++++++++
>> 5 files changed, 220 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> create mode 100644 drivers/misc/xilinx_tmr_inject.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> new file mode 100644
>> index 000000000000..d274b30ee24c
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> @@ -0,0 +1,16 @@
>> +What: /sys/devices/platform/amba_pl/<dev>/inject_err
>> +Date: June 2022
>
> It's not June anymore, even when you sent this patch :(

Will fix in next version
>
>> +Contact: [email protected]
>> +Description: This control file allows to inject fault using tmr inject.
>> + This file is write only.
>> + Example:
>> + # echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_err
>> +
>> +What: /sys/devices/platform/amba_pl/<dev>/inject_cpuid
>> +Date: June 2022
>> +Contact: [email protected]
>> +Description: This control file allows to configure the CPU identifier
>> + to enable fault injection.
>> + This file is write only.
>> + Example:
>> + # echo 1 > /sys/devices/platform/amba_pl/44a30000.tmr_inject/inject_cpuid
>
> What errors and faults happen? Where is that documented? What happens
> when you write to these sysfs files? Does the system crash? Why would
> you want to use them ever?
>
>

TMR subsystem has 3 Microblaze processor cores, by default driver is
configured to inject the error at processor core-0, This sysfs entry
provides a mechanism to inject the fault at the user-specified core.

>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 732fd9ae7d9f..c903b45c204a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13087,6 +13087,13 @@ F: Documentation/ABI/testing/sysfs-driver-xilinx-tmr-manager
>> F: Documentation/devicetree/bindings/misc/xlnx,tmr-manager.yaml
>> F: drivers/misc/xilinx_tmr_manager.c
>>
>> +MICROBLAZE TMR INJECT
>> +M: Appana Durga Kedareswara rao <[email protected]>
>> +S: Supported
>> +F: Documentation/ABI/testing/sysfs-driver-xilinx-tmr-inject
>> +F: Documentation/devicetree/bindings/misc/xlnx,tmr-inject.yaml
>> +F: drivers/misc/xilinx_tmr_inject.c
>> +
>> MICROCHIP AT91 DMA DRIVERS
>> M: Ludovic Desroches <[email protected]>
>> M: Tudor Ambarus <[email protected]>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 555ae2e33b91..0989c36f3051 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -493,6 +493,16 @@ config TMR_MANAGER
>>
>> Say N here unless you know what you are doing.
>>
>> +config TMR_INJECT
>> + bool "Select TMR Inject"
>> + depends on TMR_MANAGER
>> + help
>> + This option enables the driver developed for TMR Inject.
>> + The Triple Modular Redundancy(TMR) Inject provides
>> + fault injection.
>> +
>> + Say N here unless you know what you are doing.
>
> Why can't this be a module?
>
>

We can use this driver as a module will fix in next version.

>
>> +
>> source "drivers/misc/c2port/Kconfig"
>> source "drivers/misc/eeprom/Kconfig"
>> source "drivers/misc/cb710/Kconfig"
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 28b9803f909b..e9d0a709e207 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -61,3 +61,4 @@ obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
>> obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
>> obj-$(CONFIG_OPEN_DICE) += open-dice.o
>> obj-$(CONFIG_TMR_MANAGER) += xilinx_tmr_manager.o
>> +obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
>> diff --git a/drivers/misc/xilinx_tmr_inject.c b/drivers/misc/xilinx_tmr_inject.c
>> new file mode 100644
>> index 000000000000..930d89e90b61
>> --- /dev/null
>> +++ b/drivers/misc/xilinx_tmr_inject.c
>> @@ -0,0 +1,186 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for Xilinx TMR Inject IP.
>> + *
>> + * Copyright (C) 2022 Xilinx, Inc.
>> + *
>> + * Description:
>> + * This driver is developed for TMR Inject IP,The Triple Modular Redundancy(TMR)
>> + * Inject provides fault injection.
>> + * Fault injection and detection features are provided through sysfs entries
>> + * which allow the user to generate a fault.
>> + */
>> +
>> +#include <asm/xilinx_mb_manager.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +
>> +/* TMR Inject Register offsets */
>> +#define XTMR_INJECT_CR_OFFSET 0x0
>> +#define XTMR_INJECT_AIR_OFFSET 0x4
>> +#define XTMR_INJECT_IIR_OFFSET 0xC
>> +#define XTMR_INJECT_EAIR_OFFSET 0x10
>> +#define XTMR_INJECT_ERR_OFFSET 0x204
>> +
>> +/* Register Bitmasks/shifts */
>> +#define XTMR_INJECT_CR_CPUID_SHIFT 8
>> +#define XTMR_INJECT_CR_IE_SHIFT 10
>> +#define XTMR_INJECT_IIR_ADDR_MASK GENMASK(31, 16)
>> +
>> +#define XTMR_INJECT_MAGIC_MAX_VAL 255
>> +
>> +/**
>> + * struct xtmr_inject_dev - Driver data for TMR Inject
>> + * @regs: device physical base address
>> + * @cr_val: control register value
>> + * @magic: Magic hardware configuration value
>> + * @err_cnt: error statistics count
>> + */
>> +struct xtmr_inject_dev {
>> + void __iomem *regs;
>> + u32 cr_val;
>> + u32 magic;
>> + u32 err_cnt;
>> +};
>> +
>> +/* IO accessors */
>> +static inline void xtmr_inject_write(struct xtmr_inject_dev *xtmr_inject,
>> + u32 addr, u32 value)
>> +{
>> + iowrite32(value, xtmr_inject->regs + addr);
>> +}
>> +
>> +static inline u32 xtmr_inject_read(struct xtmr_inject_dev *xtmr_inject,
>> + u32 addr)
>> +{
>> + return ioread32(xtmr_inject->regs + addr);
>> +}
>> +
>> +static ssize_t inject_err_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf,
>> + size_t size)
>> +{
>> + int ret;
>> + long value;
>> +
>> + ret = kstrtoul(buf, 16, &value);
>> + if (ret)
>> + return ret;
>> +
>> + if (value > 1)
>> + return -EINVAL;
>
> That does not match your documentation :(
>

Will fix the documentation in next version

>
>> +
>> + xmb_inject_err();
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_WO(inject_err);
>> +
>> +static ssize_t inject_cpuid_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t size)
>> +{
>> + struct xtmr_inject_dev *xtmr_inject = dev_get_drvdata(dev);
>> + int ret;
>> + long value;
>> +
>> + ret = kstrtoul(buf, 0, &value);
>> + if (ret)
>> + return ret;
>> +
>> + if (value > 3)
>> + return -EINVAL;
>
> Again, does not match the documentation at all.
>

Will fix the documentation in next version


> confused,

sorry for the improper sysfs documentation will fix in next version.

Regards,
Kedar.
>
> greg k-h

2022-07-29 09:51:47

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP

On Fri, Jul 29, 2022 at 01:43:47PM +0530, Rao, Appana Durga Kedareswara wrote:
> Hi Greg,
>
> Thanks for the review.
>
> On 28/07/22 7:54 pm, Greg KH wrote:
> > On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
> > > From: Appana Durga Kedareswara rao <[email protected]>
> > >
> > > The Triple Modular Redundancy(TMR) provides functional fault injection by
> > > changing selected MicroBlaze instructions, which provides the possibility
> > > to verify that the TMR subsystem error detection and fault recovery logic
> > > is working properly, provided sysfs entries which allow the user to inject
> > > a fault.
> >
> > We already have a fault-injection api, why are you not using that?
> >
>
> Inorder to inject the error using TMR inject IP, The API
> which injects the error should be executed from Processor LMB,
> below sysfs entry calls microblaze core API xmb_inject_err()
> which switches the processor to real mode and injects the error,
> Please find the code corresponds to xmb_inject_err() API here:
> https://www.spinics.net/lists/arm-kernel/msg991888.html

You did not answer the question. Why are you not using the in-kernel
fault injection userspace api and why have you created your own?

Also, please use lore.kernel.org links when possible.

thanks,

greg k-h

Subject: Re: [PATCH v2 4/4] drivers: misc: Add Support for TMR Inject IP

Hi Greg,

On 29/07/22 2:32 pm, Greg KH wrote:
> On Fri, Jul 29, 2022 at 01:43:47PM +0530, Rao, Appana Durga Kedareswara wrote:
>> Hi Greg,
>>
>> Thanks for the review.
>>
>> On 28/07/22 7:54 pm, Greg KH wrote:
>>> On Wed, Jul 20, 2022 at 11:30:16AM +0530, Appana Durga Kedareswara rao wrote:
>>>> From: Appana Durga Kedareswara rao <[email protected]>
>>>>
>>>> The Triple Modular Redundancy(TMR) provides functional fault injection by
>>>> changing selected MicroBlaze instructions, which provides the possibility
>>>> to verify that the TMR subsystem error detection and fault recovery logic
>>>> is working properly, provided sysfs entries which allow the user to inject
>>>> a fault.
>>>
>>> We already have a fault-injection api, why are you not using that?
>>>
>>
>> Inorder to inject the error using TMR inject IP, The API
>> which injects the error should be executed from Processor LMB,
>> below sysfs entry calls microblaze core API xmb_inject_err()
>> which switches the processor to real mode and injects the error,
>> Please find the code corresponds to xmb_inject_err() API here:
>> https://www.spinics.net/lists/arm-kernel/msg991888.html
>
> You did not answer the question. Why are you not using the in-kernel
> fault injection userspace api and why have you created your own?

will update the driver to use fault injection user space API in next
version.

>
> Also, please use lore.kernel.org links when possible.

Sure

Regards,
Kedar.
>
> thanks,
>
> greg k-h