[PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM

Tong Ho posted 9 patches 2 years, 6 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210917052400.1249094-1-tong.ho@xilinx.com
Maintainers: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>
docs/system/arm/xlnx-versal-virt.rst |  49 ++
hw/Kconfig                           |   2 +
hw/arm/Kconfig                       |   2 +
hw/arm/xlnx-versal-virt.c            |  88 +++
hw/arm/xlnx-versal.c                 |  57 ++
hw/arm/xlnx-zcu102.c                 |  30 +
hw/arm/xlnx-zynqmp.c                 |  49 ++
hw/nvram/Kconfig                     |  19 +
hw/nvram/meson.build                 |   8 +
hw/nvram/xlnx-bbram.c                | 545 +++++++++++++++++
hw/nvram/xlnx-efuse-crc.c            | 119 ++++
hw/nvram/xlnx-efuse.c                | 280 +++++++++
hw/nvram/xlnx-versal-efuse-cache.c   | 114 ++++
hw/nvram/xlnx-versal-efuse-ctrl.c    | 783 ++++++++++++++++++++++++
hw/nvram/xlnx-zynqmp-efuse.c         | 855 +++++++++++++++++++++++++++
include/hw/arm/xlnx-versal.h         |  15 +
include/hw/arm/xlnx-zynqmp.h         |   5 +
include/hw/nvram/xlnx-bbram.h        |  54 ++
include/hw/nvram/xlnx-efuse.h        | 132 +++++
include/hw/nvram/xlnx-versal-efuse.h |  68 +++
include/hw/nvram/xlnx-zynqmp-efuse.h |  44 ++
21 files changed, 3318 insertions(+)
create mode 100644 hw/nvram/xlnx-bbram.c
create mode 100644 hw/nvram/xlnx-efuse-crc.c
create mode 100644 hw/nvram/xlnx-efuse.c
create mode 100644 hw/nvram/xlnx-versal-efuse-cache.c
create mode 100644 hw/nvram/xlnx-versal-efuse-ctrl.c
create mode 100644 hw/nvram/xlnx-zynqmp-efuse.c
create mode 100644 include/hw/nvram/xlnx-bbram.h
create mode 100644 include/hw/nvram/xlnx-efuse.h
create mode 100644 include/hw/nvram/xlnx-versal-efuse.h
create mode 100644 include/hw/nvram/xlnx-zynqmp-efuse.h
[PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM
Posted by Tong Ho 2 years, 6 months ago
This series implements the Xilinx eFUSE and BBRAM devices for
the Versal and ZynqMP product families.

Furthermore, both new devices are connected to the xlnx-versal-virt
board and the xlnx-zcu102 board.

See changes in docs/system/arm/xlnx-versal-virt.rst for detail.

---

Changelogs:

v2->v3:
* Move drive-backend attaching to board models
* Use OBJECT_DECLARE_SIMPLE_TYPE instead of OBJECT_CHECK
* Use reg_array->mem directly instead of an extra mr-container
* Add doc comments for .h file API
* Remove "qemu/osdep.h" from .h files
* Remove ad-hoc endianess detection
* Remove error_abort from device models
* Remove empty vmstate and mininum_version_id_old
* Remove unused #define macros
* Remove unavailable references from comments
* Kconfig & meson.build:
  - Add CONFIG_XLNX_EFUSE_CRC in patch 1
  - Select bbram and efuse devices from boards' Kconfig
  - Remove 'if' from meson.build
* Fix spelling and wording in comments and docs

v1->v2:
* Move doc change from 1st to last of this series
* Remove outdated comment of 'autogenerated by xregqemu.py'
  from all affected files.

---

Tong Ho (9):
  hw/nvram: Introduce Xilinx eFuse QOM
  hw/nvram: Introduce Xilinx Versal eFuse device
  hw/nvram: Introduce Xilinx ZynqMP eFuse device
  hw/nvram: Introduce Xilinx battery-backed ram
  hw/arm: xlnx-versal-virt: Add Xilinx BBRAM device
  hw/arm: xlnx-versal-virt: Add Xilinx eFUSE device
  hw/arm: xlnx-zcu102: Add Xilinx BBRAM device
  hw/arm: xlnx-zcu102: Add Xilinx eFUSE device
  docs/system/arm: xlnx-versal-virt: BBRAM and eFUSE Usage

 docs/system/arm/xlnx-versal-virt.rst |  49 ++
 hw/Kconfig                           |   2 +
 hw/arm/Kconfig                       |   2 +
 hw/arm/xlnx-versal-virt.c            |  88 +++
 hw/arm/xlnx-versal.c                 |  57 ++
 hw/arm/xlnx-zcu102.c                 |  30 +
 hw/arm/xlnx-zynqmp.c                 |  49 ++
 hw/nvram/Kconfig                     |  19 +
 hw/nvram/meson.build                 |   8 +
 hw/nvram/xlnx-bbram.c                | 545 +++++++++++++++++
 hw/nvram/xlnx-efuse-crc.c            | 119 ++++
 hw/nvram/xlnx-efuse.c                | 280 +++++++++
 hw/nvram/xlnx-versal-efuse-cache.c   | 114 ++++
 hw/nvram/xlnx-versal-efuse-ctrl.c    | 783 ++++++++++++++++++++++++
 hw/nvram/xlnx-zynqmp-efuse.c         | 855 +++++++++++++++++++++++++++
 include/hw/arm/xlnx-versal.h         |  15 +
 include/hw/arm/xlnx-zynqmp.h         |   5 +
 include/hw/nvram/xlnx-bbram.h        |  54 ++
 include/hw/nvram/xlnx-efuse.h        | 132 +++++
 include/hw/nvram/xlnx-versal-efuse.h |  68 +++
 include/hw/nvram/xlnx-zynqmp-efuse.h |  44 ++
 21 files changed, 3318 insertions(+)
 create mode 100644 hw/nvram/xlnx-bbram.c
 create mode 100644 hw/nvram/xlnx-efuse-crc.c
 create mode 100644 hw/nvram/xlnx-efuse.c
 create mode 100644 hw/nvram/xlnx-versal-efuse-cache.c
 create mode 100644 hw/nvram/xlnx-versal-efuse-ctrl.c
 create mode 100644 hw/nvram/xlnx-zynqmp-efuse.c
 create mode 100644 include/hw/nvram/xlnx-bbram.h
 create mode 100644 include/hw/nvram/xlnx-efuse.h
 create mode 100644 include/hw/nvram/xlnx-versal-efuse.h
 create mode 100644 include/hw/nvram/xlnx-zynqmp-efuse.h

-- 
2.25.1


Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM
Posted by Peter Maydell 2 years, 6 months ago
On Fri, 17 Sept 2021 at 06:24, Tong Ho <tong.ho@xilinx.com> wrote:
>
> This series implements the Xilinx eFUSE and BBRAM devices for
> the Versal and ZynqMP product families.
>
> Furthermore, both new devices are connected to the xlnx-versal-virt
> board and the xlnx-zcu102 board.
>
> See changes in docs/system/arm/xlnx-versal-virt.rst for detail.
>
> ---




Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM
Posted by Peter Maydell 2 years, 5 months ago
On Fri, 17 Sept 2021 at 06:24, Tong Ho <tong.ho@xilinx.com> wrote:
>
> This series implements the Xilinx eFUSE and BBRAM devices for
> the Versal and ZynqMP product families.
>
> Furthermore, both new devices are connected to the xlnx-versal-virt
> board and the xlnx-zcu102 board.
>
> See changes in docs/system/arm/xlnx-versal-virt.rst for detail.

Hi -- now this has landed upstream, Coverity points out a
lot of memory leaks on error or logging paths, where
the code does things like:

*** CID 1464071:  Resource leaks  (RESOURCE_LEAK)
/qemu/hw/nvram/xlnx-versal-efuse-ctrl.c: 628 in efuse_ctrl_reg_write()
622         dev = reg_array->mem.owner;
623         assert(dev);
624
625         s = XLNX_VERSAL_EFUSE_CTRL(dev);
626
627         if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
>>>     CID 1464071:  Resource leaks  (RESOURCE_LEAK)
>>>     Failing to save or free storage allocated by "object_get_canonical_path((Object *)s)" leaks it.
628             qemu_log_mask(LOG_GUEST_ERROR,
629                           "%s[reg_0x%02lx]: Attempt to write
locked register.\n",
630                           object_get_canonical_path(OBJECT(s)), (long)addr);
631         } else {
632             register_write_memory(opaque, addr, data, size);
633         }

You need to free the memory here. A good pattern is how it's
done in xlnx-zynqmp-can.c with g_autofree:

    if (ARRAY_FIELD_EX32(s->regs, SOFTWARE_RESET_REGISTER, SRST)) {
        g_autofree char *path = object_get_canonical_path(OBJECT(s));

        qemu_log_mask(LOG_GUEST_ERROR, "%s: Attempting to transfer data while"
                      " data while controller is in reset mode.\n",
                      path);
        return false;
    }

Could somebody send some followup patches that fix all of these,
please? (There are 10 coverity issues, covering probably all of
the places where object_get_canonical_path() is used in this series.)

thanks
-- PMM

RE: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM
Posted by Tong Ho 2 years, 5 months ago
Hi Peter,

I will follow up with patches to fix the memory leaks.

Where can I get a copy of the Coverity reports that have the 10 issues you indicated?

Thanks,
Tong

-----Original Message-----
From: Peter Maydell <peter.maydell@linaro.org> 
Sent: Saturday, October 2, 2021 3:28 AM
To: Tong Ho <tongh@xilinx.com>
Cc: qemu-arm <qemu-arm@nongnu.org>; QEMU Developers <qemu-devel@nongnu.org>; Alistair Francis <alistair@alistair23.me>; Edgar E. Iglesias <edgar.iglesias@gmail.com>
Subject: Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM

On Fri, 17 Sept 2021 at 06:24, Tong Ho <tong.ho@xilinx.com> wrote:
>
> This series implements the Xilinx eFUSE and BBRAM devices for the 
> Versal and ZynqMP product families.
>
> Furthermore, both new devices are connected to the xlnx-versal-virt 
> board and the xlnx-zcu102 board.
>
> See changes in docs/system/arm/xlnx-versal-virt.rst for detail.

Hi -- now this has landed upstream, Coverity points out a lot of memory leaks on error or logging paths, where the code does things like:

*** CID 1464071:  Resource leaks  (RESOURCE_LEAK)
/qemu/hw/nvram/xlnx-versal-efuse-ctrl.c: 628 in efuse_ctrl_reg_write()
622         dev = reg_array->mem.owner;
623         assert(dev);
624
625         s = XLNX_VERSAL_EFUSE_CTRL(dev);
626
627         if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
>>>     CID 1464071:  Resource leaks  (RESOURCE_LEAK)
>>>     Failing to save or free storage allocated by "object_get_canonical_path((Object *)s)" leaks it.
628             qemu_log_mask(LOG_GUEST_ERROR,
629                           "%s[reg_0x%02lx]: Attempt to write
locked register.\n",
630                           object_get_canonical_path(OBJECT(s)), (long)addr);
631         } else {
632             register_write_memory(opaque, addr, data, size);
633         }

You need to free the memory here. A good pattern is how it's done in xlnx-zynqmp-can.c with g_autofree:

    if (ARRAY_FIELD_EX32(s->regs, SOFTWARE_RESET_REGISTER, SRST)) {
        g_autofree char *path = object_get_canonical_path(OBJECT(s));

        qemu_log_mask(LOG_GUEST_ERROR, "%s: Attempting to transfer data while"
                      " data while controller is in reset mode.\n",
                      path);
        return false;
    }

Could somebody send some followup patches that fix all of these, please? (There are 10 coverity issues, covering probably all of the places where object_get_canonical_path() is used in this series.)

thanks
-- PMM
Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM
Posted by Peter Maydell 2 years, 4 months ago
On Mon, 4 Oct 2021 at 16:54, Tong Ho <tongh@xilinx.com> wrote:
>
> Hi Peter,
>
> I will follow up with patches to fix the memory leaks.
>
> Where can I get a copy of the Coverity reports that have the 10 issues you indicated?

They're on https://scan.coverity.com/projects/qemu?tab=overview
You'll need to create an account and request access there.

-- PMM