On 2023-02-10 at 15:37:01 -0800, Russ Weight wrote:
> Hi Yilun,
>
> On 2/10/23 01:47, Xu Yilun wrote:
> > On 2023-02-09 at 13:13:28 +0200, Ilpo Järvinen wrote:
> >> Add MAX 10 BMC log driver for accessing BMC event log, FPGA image
> >> directory, and bill-of-materials (BOM) info partitions on FPGA card's
> >> flash. Use the nvmem API to expose the event MAX 10 BMC event logs to
> >> userspace.
> > I didn't look into the details yet, but I suppose this driver is not
> > for FPGA domain, which focuses on FPGA region re-configuration and
> > re-enumeration of devices in FPGA region.
>
> This driver was originally placed (without much thought) under drivers/mfd. It is
> a sub-device of the BMC, not a multi-function device. I have looked for other
> instances of log drivers and they seem to be grouped with the technology they are
> logging for. For example:
>
> ./net/bridge/netfilter/ebt_log.c
> ./net/bridge/netfilter/ebt_nflog.c
> ./net/netfilter/nft_log.c
> ./net/netfilter/nfnetlink_log.c
I see these log drivers interacts with net core. But what this log
driver is doing, is just to provide the nvmem interface for BMC flash.
>
> There is no drivers/log subdirectory. Since this driver is used exclusively with
> FPGA devices, drivers/fpga seemed like the best option. Do you have any suggestions
Try not to include all kinds of sub device drivers in FPGA domain. Most
sub devices in FPGA act the same as in ASIC.
> for a better place to put this driver?
I'm not quite sure. Maybe drivers/nvmem?
Thanks,
Yilun
>
> Thanks,
> - Russ
>
> >
> > Thanks,
> > Yilun
> >
> >> The PMCI MAX 10 BMC contains high and low timestamp registers that are
> >> periodically written by the host driver to facilitate BMC event logs.
> >> Add a kernel worker thread to update the BMC timestamp registers. The
> >> frequency of timestamp updates is controlled through sysfs file (the
> >> default frequency is once per minute).
> >>
> >> Co-developed-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> >> Signed-off-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> >> Co-developed-by: Tianfei Zhang <tianfei.zhang@xxxxxxxxx>
> >> Signed-off-by: Tianfei Zhang <tianfei.zhang@xxxxxxxxx>
> >> Co-developed-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> >> ---
> >>
> >> This requires commit 4f43a6e80ba9 ("fpga: m10bmc-sec: Add support for
> >> N6000") from ib-mfd-fpga-hwmon-6.3-1 (the 11th patch of the M10 BMC split
> >> series that wasn't picked up at first and still isn't in for-fpga-v6.3-rc1).
> >>
> >> .../testing/sysfs-driver-intel-m10-bmc-log | 37 +++
> >> MAINTAINERS | 4 +-
> >> drivers/fpga/Kconfig | 9 +
> >> drivers/fpga/Makefile | 3 +-
> >> drivers/fpga/intel-m10-bmc-log.c | 264 ++++++++++++++++++
> >> drivers/mfd/intel-m10-bmc-pmci.c | 1 +
> >> include/linux/mfd/intel-m10-bmc.h | 13 +
> >> 7 files changed, 329 insertions(+), 2 deletions(-)
> >> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-log
> >> create mode 100644 drivers/fpga/intel-m10-bmc-log.c
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-log b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-log
> >> new file mode 100644
> >> index 000000000000..a170e675d11e
> >> --- /dev/null
> >> +++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-log
> >> @@ -0,0 +1,37 @@
> >> +What: /sys/bus/platform/devices/intel-m10-bmc-log.*.auto/time_sync_frequency
> >> +Date: Apr 2023
> >> +KernelVersion: 6.3
> >> +Contact: Russ Weight <russell.h.weight@xxxxxxxxx>
> >> +Description: Read/write. This sysfs node controls the frequency (in
> >> + seconds) that the host writes to the MAX10 BMC registers
> >> + to synchronize the timestamp registers used for the BMC
> >> + event log. Write zero to stop the timestamp synchronization.
> >> + Write a non-zero integer value to restart or modify the
> >> + update frequency. Reading from this file will return the
> >> + same integer value.
> >> + Format: %u
> >> +
> >> +What: /sys/bus/platform/devices/intel-m10-bmc-log.*.auto/bmc_event_log*/nvmem
> >> +Date: Apr 2023
> >> +KernelVersion: 6.3
> >> +Contact: Tianfei zhang <tianfei.zhang@xxxxxxxxx>
> >> +Description: Read-only. This file returns the contents of the "evemt log"
> >> + partition in flash. This partition includes the event and
> >> + error info for the BMC.
> >> +
> >> +What: /sys/bus/platform/devices/intel-m10-bmc-log.*.auto/fpga_image_directory*/nvmem
> >> +Date: Apr 2023
> >> +KernelVersion: 6.3
> >> +Contact: Tianfei zhang <tianfei.zhang@xxxxxxxxx>
> >> +Description: Read-only. This file returns the contents of the "FPGA image
> >> + directory" partition in flash. This partition includes
> >> + information like the FPGA Image versions and state.
> >> +
> >> +What: /sys/bus/platform/devices/intel-m10-bmc-log.*.auto/bom_info*/nvmem
> >> +Date: Apr 2023
> >> +KernelVersion: 6.3
> >> +Contact: Tianfei zhang <tianfei.zhang@xxxxxxxxx>
> >> +Description: Read-only. This file returns the contents of the "BOM info"
> >> + partition in flash. This partition includes information such
> >> + as the bill of materials (BOM) critical components like
> >> + PBA#, MMID.
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index ddfa4f8b3c80..1b2427ba6729 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -8075,11 +8075,13 @@ F: Documentation/fpga/
> >> F: drivers/fpga/
> >> F: include/linux/fpga/
> >>
> >> -INTEL MAX10 BMC SECURE UPDATES
> >> +INTEL MAX10 BMC SUBDRIVERS
> >> M: Russ Weight <russell.h.weight@xxxxxxxxx>
> >> L: linux-fpga@xxxxxxxxxxxxxxx
> >> S: Maintained
> >> +F: Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-log
> >> F: Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update
> >> +F: drivers/fpga/intel-m10-bmc-log.c
> >> F: drivers/fpga/intel-m10-bmc-sec-update.c
> >>
> >> MICROCHIP POLARFIRE FPGA DRIVERS
> >> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> >> index 0a00763b9f28..773667e3b027 100644
> >> --- a/drivers/fpga/Kconfig
> >> +++ b/drivers/fpga/Kconfig
> >> @@ -244,6 +244,15 @@ config FPGA_MGR_VERSAL_FPGA
> >>
> >> To compile this as a module, choose M here.
> >>
> >> +config FPGA_M10_BMC_LOG
> >> + tristate "Intel MAX 10 Board Management Controller Log Driver"
> >> + depends on MFD_INTEL_M10_BMC_CORE
> >> + help
> >> + Support for the Intel MAX 10 Board Management Controller (BMC)
> >> + event log, event log timestamp synchronization, and other
> >> + information on flash partitions (available images and
> >> + bill-of-materials critical information).
> >> +
> >> config FPGA_M10_BMC_SEC_UPDATE
> >> tristate "Intel MAX10 BMC Secure Update driver"
> >> depends on MFD_INTEL_M10_BMC_CORE
> >> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> >> index 72e554b4d2f7..1db25ad3d76a 100644
> >> --- a/drivers/fpga/Makefile
> >> +++ b/drivers/fpga/Makefile
> >> @@ -25,7 +25,8 @@ obj-$(CONFIG_FPGA_MGR_LATTICE_SYSCONFIG_SPI) += lattice-sysconfig-spi.o
> >> obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o
> >> obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o
> >>
> >> -# FPGA Secure Update Drivers
> >> +# MAX10 subdrivers
> >> +obj-$(CONFIG_FPGA_M10_BMC_LOG) += intel-m10-bmc-log.o
> >> obj-$(CONFIG_FPGA_M10_BMC_SEC_UPDATE) += intel-m10-bmc-sec-update.o
> >>
> >> # FPGA Bridge Drivers
> >> diff --git a/drivers/fpga/intel-m10-bmc-log.c b/drivers/fpga/intel-m10-bmc-log.c
> >> new file mode 100644
> >> index 000000000000..4e7a54afe786
> >> --- /dev/null
> >> +++ b/drivers/fpga/intel-m10-bmc-log.c
> >> @@ -0,0 +1,264 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Intel Max10 Board Management Controller Log Driver
> >> + *
> >> + * Copyright (C) 2021-2023 Intel Corporation.
> >> + */
> >> +
> >> +#include <linux/bitfield.h>
> >> +#include <linux/dev_printk.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/nvmem-provider.h>
> >> +#include <linux/mod_devicetable.h>
> >> +#include <linux/types.h>
> >> +
> >> +#include <linux/mfd/intel-m10-bmc.h>
> >> +
> >> +#define M10BMC_TIMESTAMP_FREQ 60 /* 60 secs between updates */
> >> +
> >> +struct m10bmc_log_cfg {
> >> + int el_size;
> >> + unsigned long el_off;
> >> +
> >> + int id_size;
> >> + unsigned long id_off;
> >> +
> >> + int bi_size;
> >> + unsigned long bi_off;
> >> +};
> >> +
> >> +struct m10bmc_log {
> >> + struct device *dev;
> >> + struct intel_m10bmc *m10bmc;
> >> + unsigned int freq_s; /* update frequency in seconds */
> >> + struct delayed_work dwork;
> >> + const struct m10bmc_log_cfg *log_cfg;
> >> + struct nvmem_device *bmc_event_log_nvmem;
> >> + struct nvmem_device *fpga_image_dir_nvmem;
> >> + struct nvmem_device *bom_info_nvmem;
> >> +};
> >> +
> >> +static void m10bmc_log_time_sync(struct work_struct *work)
> >> +{
> >> + struct delayed_work *dwork = to_delayed_work(work);
> >> + const struct m10bmc_csr_map *csr_map;
> >> + struct m10bmc_log *log;
> >> + s64 time_ms;
> >> + int ret;
> >> +
> >> + log = container_of(dwork, struct m10bmc_log, dwork);
> >> + csr_map = log->m10bmc->info->csr_map;
> >> +
> >> + time_ms = ktime_to_ms(ktime_get_real());
> >> + ret = regmap_write(log->m10bmc->regmap, csr_map->base + M10BMC_N6000_TIME_HIGH,
> >> + upper_32_bits(time_ms));
> >> + if (!ret) {
> >> + ret = regmap_write(log->m10bmc->regmap, csr_map->base + M10BMC_N6000_TIME_LOW,
> >> + lower_32_bits(time_ms));
> >> + }
> >> + if (ret)
> >> + dev_err_once(log->dev, "Failed to update BMC timestamp: %d\n", ret);
> >> +
> >> + schedule_delayed_work(&log->dwork, log->freq_s * HZ);
> >> +}
> >> +
> >> +static ssize_t time_sync_frequency_store(struct device *dev, struct device_attribute *attr,
> >> + const char *buf, size_t count)
> >> +{
> >> + struct m10bmc_log *ddata = dev_get_drvdata(dev);
> >> + unsigned int old_freq = ddata->freq_s;
> >> + int ret;
> >> +
> >> + ret = kstrtouint(buf, 0, &ddata->freq_s);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (old_freq)
> >> + cancel_delayed_work_sync(&ddata->dwork);
> >> +
> >> + if (ddata->freq_s)
> >> + m10bmc_log_time_sync(&ddata->dwork.work);
> >> +
> >> + return count;
> >> +}
> >> +
> >> +static ssize_t time_sync_frequency_show(struct device *dev, struct device_attribute *attr,
> >> + char *buf)
> >> +{
> >> + struct m10bmc_log *ddata = dev_get_drvdata(dev);
> >> +
> >> + return sysfs_emit(buf, "%u\n", ddata->freq_s);
> >> +}
> >> +static DEVICE_ATTR_RW(time_sync_frequency);
> >> +
> >> +static struct attribute *m10bmc_log_attrs[] = {
> >> + &dev_attr_time_sync_frequency.attr,
> >> + NULL,
> >> +};
> >> +ATTRIBUTE_GROUPS(m10bmc_log);
> >> +
> >> +static int bmc_nvmem_read(struct m10bmc_log *ddata, unsigned int addr,
> >> + unsigned int off, void *val, size_t count)
> >> +{
> >> + struct intel_m10bmc *m10bmc = ddata->m10bmc;
> >> + int ret;
> >> +
> >> + ret = m10bmc->flash_bulk_ops->read(m10bmc, val, addr + off, count);
> >> + if (ret) {
> >> + if (ret != -EBUSY)
> >> + dev_err(ddata->dev, "failed to read flash %x (%d)\n", addr, ret);
> >> + return ret;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int bmc_event_log_nvmem_read(void *priv, unsigned int off, void *val, size_t count)
> >> +{
> >> + struct m10bmc_log *ddata = priv;
> >> +
> >> + return bmc_nvmem_read(ddata, ddata->log_cfg->el_off, off, val, count);
> >> +}
> >> +
> >> +static int fpga_image_dir_nvmem_read(void *priv, unsigned int off, void *val, size_t count)
> >> +{
> >> + struct m10bmc_log *ddata = priv;
> >> +
> >> + return bmc_nvmem_read(ddata, ddata->log_cfg->id_off, off, val, count);
> >> +}
> >> +
> >> +static int bom_info_nvmem_read(void *priv, unsigned int off, void *val, size_t count)
> >> +{
> >> + struct m10bmc_log *ddata = priv;
> >> +
> >> + return bmc_nvmem_read(ddata, ddata->log_cfg->bi_off, off, val, count);
> >> +}
> >> +
> >> +static struct nvmem_config bmc_event_log_nvmem_config = {
> >> + .name = "bmc_event_log",
> >> + .stride = 4,
> >> + .word_size = 1,
> >> + .reg_read = bmc_event_log_nvmem_read,
> >> + .id = NVMEM_DEVID_AUTO,
> >> +};
> >> +
> >> +static struct nvmem_config fpga_image_dir_nvmem_config = {
> >> + .name = "fpga_image_directory",
> >> + .stride = 4,
> >> + .word_size = 1,
> >> + .reg_read = fpga_image_dir_nvmem_read,
> >> + .id = NVMEM_DEVID_AUTO,
> >> +};
> >> +
> >> +static struct nvmem_config bom_info_nvmem_config = {
> >> + .name = "bom_info",
> >> + .stride = 4,
> >> + .word_size = 1,
> >> + .reg_read = bom_info_nvmem_read,
> >> + .id = NVMEM_DEVID_AUTO,
> >> +};
> >> +
> >> +static int m10bmc_log_probe(struct platform_device *pdev)
> >> +{
> >> + const struct platform_device_id *id = platform_get_device_id(pdev);
> >> + struct intel_m10bmc *m10bmc = dev_get_drvdata(pdev->dev.parent);
> >> + struct nvmem_config nvconfig;
> >> + struct m10bmc_log *ddata;
> >> +
> >> + if (WARN_ON_ONCE(!m10bmc->flash_bulk_ops))
> >> + return -ENODEV;
> >> +
> >> + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> >> + if (!ddata)
> >> + return -ENOMEM;
> >> +
> >> + ddata->dev = &pdev->dev;
> >> + ddata->m10bmc = m10bmc;
> >> + ddata->freq_s = M10BMC_TIMESTAMP_FREQ;
> >> + INIT_DELAYED_WORK(&ddata->dwork, m10bmc_log_time_sync);
> >> + ddata->log_cfg = (struct m10bmc_log_cfg *)id->driver_data;
> >> + dev_set_drvdata(&pdev->dev, ddata);
> >> +
> >> + if (ddata->log_cfg->el_size > 0) {
> >> + m10bmc_log_time_sync(&ddata->dwork.work);
> >> +
> >> + memcpy(&nvconfig, &bmc_event_log_nvmem_config, sizeof(bmc_event_log_nvmem_config));
> >> + nvconfig.dev = ddata->dev;
> >> + nvconfig.priv = ddata;
> >> + nvconfig.size = ddata->log_cfg->el_size;
> >> +
> >> + ddata->bmc_event_log_nvmem = devm_nvmem_register(ddata->dev, &nvconfig);
> >> + if (IS_ERR(ddata->bmc_event_log_nvmem))
> >> + return PTR_ERR(ddata->bmc_event_log_nvmem);
> >> + }
> >> +
> >> + if (ddata->log_cfg->id_size > 0) {
> >> + memcpy(&nvconfig, &fpga_image_dir_nvmem_config, sizeof(fpga_image_dir_nvmem_config));
> >> + nvconfig.dev = ddata->dev;
> >> + nvconfig.priv = ddata;
> >> + nvconfig.size = ddata->log_cfg->id_size;
> >> +
> >> + ddata->fpga_image_dir_nvmem = devm_nvmem_register(ddata->dev, &nvconfig);
> >> + if (IS_ERR(ddata->fpga_image_dir_nvmem))
> >> + return PTR_ERR(ddata->fpga_image_dir_nvmem);
> >> + }
> >> +
> >> + if (ddata->log_cfg->bi_size > 0) {
> >> + memcpy(&nvconfig, &bom_info_nvmem_config, sizeof(bom_info_nvmem_config));
> >> + nvconfig.dev = ddata->dev;
> >> + nvconfig.priv = ddata;
> >> + nvconfig.size = ddata->log_cfg->bi_size;
> >> +
> >> + ddata->bom_info_nvmem = devm_nvmem_register(ddata->dev, &nvconfig);
> >> + if (IS_ERR(ddata->bom_info_nvmem))
> >> + return PTR_ERR(ddata->bom_info_nvmem);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int m10bmc_log_remove(struct platform_device *pdev)
> >> +{
> >> + struct m10bmc_log *ddata = dev_get_drvdata(&pdev->dev);
> >> +
> >> + cancel_delayed_work_sync(&ddata->dwork);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct m10bmc_log_cfg m10bmc_log_n6000_cfg = {
> >> + .el_size = M10BMC_N6000_ERROR_LOG_SIZE,
> >> + .el_off = M10BMC_N6000_ERROR_LOG_ADDR,
> >> +
> >> + .id_size = M10BMC_N6000_FPGA_IMAGE_DIR_SIZE,
> >> + .id_off = M10BMC_N6000_FPGA_IMAGE_DIR_ADDR,
> >> +
> >> + .bi_size = M10BMC_N6000_BOM_INFO_SIZE,
> >> + .bi_off = M10BMC_N6000_BOM_INFO_ADDR,
> >> +};
> >> +
> >> +static const struct platform_device_id intel_m10bmc_log_ids[] = {
> >> + {
> >> + .name = "n6000bmc-log",
> >> + .driver_data = (unsigned long)&m10bmc_log_n6000_cfg,
> >> + },
> >> + { }
> >> +};
> >> +
> >> +static struct platform_driver intel_m10bmc_log_driver = {
> >> + .probe = m10bmc_log_probe,
> >> + .remove = m10bmc_log_remove,
> >> + .driver = {
> >> + .name = "intel-m10-bmc-log",
> >> + .dev_groups = m10bmc_log_groups,
> >> + },
> >> + .id_table = intel_m10bmc_log_ids,
> >> +};
> >> +module_platform_driver(intel_m10bmc_log_driver);
> >> +
> >> +MODULE_DEVICE_TABLE(platform, intel_m10bmc_log_ids);
> >> +MODULE_AUTHOR("Intel Corporation");
> >> +MODULE_DESCRIPTION("Intel MAX 10 BMC log driver");
> >> +MODULE_LICENSE("GPL");
> >> diff --git a/drivers/mfd/intel-m10-bmc-pmci.c b/drivers/mfd/intel-m10-bmc-pmci.c
> >> index 8821f1876dd6..f8803b7bb98e 100644
> >> --- a/drivers/mfd/intel-m10-bmc-pmci.c
> >> +++ b/drivers/mfd/intel-m10-bmc-pmci.c
> >> @@ -350,6 +350,7 @@ static struct regmap_config m10bmc_pmci_regmap_config = {
> >> static struct mfd_cell m10bmc_pmci_n6000_bmc_subdevs[] = {
> >> { .name = "n6000bmc-hwmon" },
> >> { .name = "n6000bmc-sec-update" },
> >> + { .name = "n6000bmc-log" },
> >> };
> >>
> >> static const struct m10bmc_csr_map m10bmc_n6000_csr_map = {
> >> diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
> >> index 1812ebfa11a8..1079e580e9e6 100644
> >> --- a/include/linux/mfd/intel-m10-bmc.h
> >> +++ b/include/linux/mfd/intel-m10-bmc.h
> >> @@ -125,6 +125,9 @@
> >> #define M10BMC_N6000_SYS_BASE 0x0
> >> #define M10BMC_N6000_SYS_END 0xfff
> >>
> >> +#define M10BMC_N6000_TIME_LOW 0x178
> >> +#define M10BMC_N6000_TIME_HIGH 0x17c
> >> +
> >> #define M10BMC_N6000_DOORBELL 0x1c0
> >> #define M10BMC_N6000_AUTH_RESULT 0x1c4
> >> #define AUTH_RESULT_RSU_STATUS GENMASK(23, 16)
> >> @@ -134,6 +137,16 @@
> >> #define M10BMC_N6000_MAC_LOW 0x20
> >> #define M10BMC_N6000_MAC_HIGH (M10BMC_N6000_MAC_LOW + 4)
> >>
> >> +/* Addresses for BMC log data in FLASH */
> >> +#define M10BMC_N6000_ERROR_LOG_ADDR 0x7fb0000
> >> +#define M10BMC_N6000_ERROR_LOG_SIZE 0x40000
> >> +
> >> +#define M10BMC_N6000_FPGA_IMAGE_DIR_ADDR 0x7ff6000
> >> +#define M10BMC_N6000_FPGA_IMAGE_DIR_SIZE 0x3000
> >> +
> >> +#define M10BMC_N6000_BOM_INFO_ADDR 0x7ff0000
> >> +#define M10BMC_N6000_BOM_INFO_SIZE 0x2000
> >> +
> >> /* Addresses for security related data in FLASH */
> >> #define M10BMC_N6000_BMC_REH_ADDR 0x7ffc004
> >> #define M10BMC_N6000_BMC_PROG_ADDR 0x7ffc000
> >> --
> >> 2.30.2
> >>
>
