linux-kernel - Re: [PATCH] memory: dfl-emif: add the DFL EMIF private feature driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Fri, 11 Sep 2020 17:16:29 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Xu Yilun <yilun.xu@...el.com>
Cc:     mdf@...nel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        trix@...hat.com, matthew.gerlach@...ux.intel.com,
        russell.h.weight@...el.com, lgoncalv@...hat.com, hao.wu@...el.com
Subject: Re: [PATCH] memory: dfl-emif: add the DFL EMIF private feature driver

On Fri, 11 Sep 2020 at 16:48, Xu Yilun <yilun.xu@...el.com> wrote:
>
> On Fri, Sep 11, 2020 at 11:03:50AM +0200, Krzysztof Kozlowski wrote:
> > On Fri, 11 Sep 2020 at 10:46, Xu Yilun <yilun.xu@...el.com> wrote:
> > >
> > > Sorry I missed one comments, see inline.
> > >
> > > On Tue, Sep 08, 2020 at 12:01:02PM +0200, Krzysztof Kozlowski wrote:
> > > > On Tue, Sep 08, 2020 at 04:27:25PM +0800, Xu Yilun wrote:
> > > > > This driver is for the EMIF private feature implemented under FPGA
> > > > > Device Feature List (DFL) framework. It is used to expose memory
> > > > > interface status information as well as memory clearing control.
> > > > >
> > > > > The purpose of memory clearing block is to zero out all private memory
> > > > > when FPGA is to be reprogrammed. This gives users a reliable method to
> > > > > prevent potential data leakage.
> > > > >
> > > > > Signed-off-by: Xu Yilun <yilun.xu@...el.com>
> > > > > Signed-off-by: Russ Weight <russell.h.weight@...el.com>
> > > > > ---
> > > > >  .../ABI/testing/sysfs-bus-dfl-devices-emif         |  25 +++
> > > > >  drivers/memory/Kconfig                             |   9 +
> > > > >  drivers/memory/Makefile                            |   2 +
> > > > >  drivers/memory/dfl-emif.c                          | 211 +++++++++++++++++++++
> > > > >  4 files changed, 247 insertions(+)
> > > > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl-devices-emif
> > > > >  create mode 100644 drivers/memory/dfl-emif.c
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-dfl-devices-emif b/Documentation/ABI/testing/sysfs-bus-dfl-devices-emif
> > > > > new file mode 100644
> > > > > index 0000000..33d557e
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-dfl-devices-emif
> > > > > @@ -0,0 +1,25 @@
> > > > > +What:              /sys/bus/dfl/devices/dfl_dev.X/infX_cal_fail
> > > > > +Date:              Sep 2020
> > > > > +KernelVersion:     5.10
> > > > > +Contact:   Xu Yilun <yilun.xu@...el.com>
> > > > > +Description:       Read-only. It indicates if the calibration is failed on this
> > > > > +           memory interface. "1" for calibration failure, "0" for OK.
> > > >
> > > > "if the calibration failed"
> > > >
> > > > > +           Format: %u
> > > > > +
> > > > > +What:              /sys/bus/dfl/devices/dfl_dev.X/infX_init_done
> > > > > +Date:              Sep 2020
> > > > > +KernelVersion:     5.10
> > > > > +Contact:   Xu Yilun <yilun.xu@...el.com>
> > > > > +Description:       Read-only. It indicates if the initialization is complete on
> > > > > +           this memory interface. "1" for initialization complete, "0"
> > > > > +           for not yet.
> > > > > +           Format: %u
> > > >
> > > > "if the initialization completed"
> > > >
> > > > > +
> > > > > +What:              /sys/bus/dfl/devices/dfl_dev.X/infX_clear
> > > > > +Date:              Sep 2020
> > > > > +KernelVersion:     5.10
> > > > > +Contact:   Xu Yilun <yilun.xu@...el.com>
> > > > > +Description:       Write-only. Writing "1" to this file will zero out all memory
> > > > > +           data in this memory interface. Writing other values are
> > > > > +           invalid.
> > > >
> > > > "Writing of other values is invalid."
> > > >
> > > > > +           Format: %u
> > > > > diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> > > > > index 8072204..fb0858f 100644
> > > > > --- a/drivers/memory/Kconfig
> > > > > +++ b/drivers/memory/Kconfig
> > > > > @@ -215,6 +215,15 @@ config STM32_FMC2_EBI
> > > > >       devices (like SRAM, ethernet adapters, FPGAs, LCD displays, ...) on
> > > > >       SOCs containing the FMC2 External Bus Interface.
> > > > >
> > > > > +config FPGA_DFL_EMIF
> > > > > +   tristate "DFL EMIF private feature support"
> > > > > +   depends on FPGA_DFL && HAS_IOMEM
> > > >
> > > > Cannot be compile tested without FPGA_DFL?
> > >
> > > We need this FPGA_DFL dependency. The driver will use the
> > > __dfl_driver_register(), which is defined in drivers/fpga/dfl.c, and
> > > FPGA_DFL enables the compiling of dfl.c
> >
> > Yeah but the DFL headers provide empty stubs for such case, don't
> > they? If they don't, probably they should.
>
> The DFL headers don't provide the empty stubs, why they should? A DFL
> driver should not be selected when the DFL framework & DFL bus is not
> enabled in system.

Good point, this is not a consumer API. Then the config entry looks fine.

Best regards,
Krzysztof