| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f0ea25fe-782b-ed35-d977-3fe16724193c@amd.com>
Date: Fri, 29 Jul 2022 13:43:47 +0530
From: "Rao, Appana Durga Kedareswara" <appanad@....com>
To: Greg KH <gregkh@...uxfoundation.org>,
Appana Durga Kedareswara rao
<appana.durga.kedareswara.rao@....com>
CC: <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<michal.simek@...inx.com>, <derek.kiernan@...inx.com>,
<dragan.cvetic@...inx.com>, <arnd@...db.de>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <git@....com>,
<git@...inx.com>,
Appana Durga Kedareswara rao <appana.durga.rao@...inx.com>
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@...inx.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@....com>
>> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@...inx.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@...inx.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@...inx.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@....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@...rochip.com>
>> M: Tudor Ambarus <tudor.ambarus@...rochip.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
Powered by blists - more mailing lists