Messages in this thread | | | Date | Fri, 29 Jul 2022 13:43:47 +0530 | From | "Rao, Appana Durga Kedareswara" <> | 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 <appana.durga.rao@xilinx.com> >> >> 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 <appana.durga.kedareswara.rao@amd.com> >> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com> > > 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: appana.durga.rao@xilinx.com >> +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: appana.durga.rao@xilinx.com >> +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 <appana.durga.kedareswara.rao@amd.com> >> +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 <ludovic.desroches@microchip.com> >> M: Tudor Ambarus <tudor.ambarus@microchip.com> >> 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
| |