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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 14 Mar 2022 19:11:50 -0400
From:   Brendan Higgins <brendanhiggins@...gle.com>
To:     Vincent Whitchurch <vincent.whitchurch@...s.com>
Cc:     linux-kernel@...r.kernel.org, kernel@...s.com,
        devicetree@...r.kernel.org, linux-um@...ts.infradead.org,
        shuah@...nel.org, linux-kselftest@...r.kernel.org,
        jic23@...nel.org, linux-iio@...r.kernel.org, lgirdwood@...il.com,
        broonie@...nel.org, a.zummo@...ertech.it,
        alexandre.belloni@...tlin.com, linux-rtc@...r.kernel.org,
        corbet@....net, linux-doc@...r.kernel.org
Subject: Re: [RFC v1 07/10] iio: light: opt3001: add roadtest

On Fri, Mar 11, 2022 at 11:24 AM Vincent Whitchurch
<vincent.whitchurch@...s.com> wrote:
>
> Add a regression test for the problem fixed by the following patch,
> which would require specific environmental conditions to be able to be
> reproduced and regression-tested on real hardware:
>
>  iio: light: opt3001: Fixed timeout error when 0 lux
>  https://lore.kernel.org/lkml/20210920125351.6569-1-valek@2n.cz/
>
> No other aspects of the driver are tested.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@...s.com>
> ---
>  .../roadtest/roadtest/tests/iio/__init__.py   |  0
>  .../roadtest/roadtest/tests/iio/config        |  1 +
>  .../roadtest/tests/iio/light/__init__.py      |  0
>  .../roadtest/roadtest/tests/iio/light/config  |  1 +
>  .../roadtest/tests/iio/light/test_opt3001.py  | 95 +++++++++++++++++++
>  5 files changed, 97 insertions(+)
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/__init__.py
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/config
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/light/__init__.py
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/light/config
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py
>
> diff --git a/tools/testing/roadtest/roadtest/tests/iio/__init__.py b/tools/testing/roadtest/roadtest/tests/iio/__init__.py
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tools/testing/roadtest/roadtest/tests/iio/config b/tools/testing/roadtest/roadtest/tests/iio/config
> new file mode 100644
> index 000000000000..a08d9e23ce38
> --- /dev/null
> +++ b/tools/testing/roadtest/roadtest/tests/iio/config
> @@ -0,0 +1 @@
> +CONFIG_IIO=y
> diff --git a/tools/testing/roadtest/roadtest/tests/iio/light/__init__.py b/tools/testing/roadtest/roadtest/tests/iio/light/__init__.py
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/tools/testing/roadtest/roadtest/tests/iio/light/config b/tools/testing/roadtest/roadtest/tests/iio/light/config
> new file mode 100644
> index 000000000000..b9753f2d0728
> --- /dev/null
> +++ b/tools/testing/roadtest/roadtest/tests/iio/light/config
> @@ -0,0 +1 @@
> +CONFIG_OPT3001=m
> diff --git a/tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py b/tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py
> new file mode 100644
> index 000000000000..abf20b8f3516
> --- /dev/null
> +++ b/tools/testing/roadtest/roadtest/tests/iio/light/test_opt3001.py
> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +# Copyright Axis Communications AB
> +
> +from typing import Any, Final
> +
> +from roadtest.backend.i2c import SMBusModel
> +from roadtest.core.devicetree import DtFragment, DtVar
> +from roadtest.core.hardware import Hardware
> +from roadtest.core.modules import insmod, rmmod
> +from roadtest.core.suite import UMLTestCase
> +from roadtest.core.sysfs import I2CDriver, read_float
> +
> +REG_RESULT: Final = 0x00
> +REG_CONFIGURATION: Final = 0x01
> +REG_LOW_LIMIT: Final = 0x02
> +REG_HIGH_LIMIT: Final = 0x03
> +REG_MANUFACTURER_ID: Final = 0x7E
> +REG_DEVICE_ID: Final = 0x7F
> +
> +REG_CONFIGURATION_CRF: Final = 1 << 7
> +
> +
> +class OPT3001(SMBusModel):
> +    def __init__(self, **kwargs: Any) -> None:
> +        super().__init__(regbytes=2, byteorder="big", **kwargs)
> +        # Reset values from datasheet
> +        self.regs = {
> +            REG_RESULT: 0x0000,
> +            REG_CONFIGURATION: 0xC810,
> +            REG_LOW_LIMIT: 0xC000,
> +            REG_HIGH_LIMIT: 0xBFFF,
> +            REG_MANUFACTURER_ID: 0x5449,
> +            REG_DEVICE_ID: 0x3001,
> +        }
> +
> +    def reg_read(self, addr: int) -> int:
> +        val = self.regs[addr]
> +
> +        if addr == REG_CONFIGURATION:
> +            # Always indicate that the conversion is ready.  This is good
> +            # enough for our current purposes.
> +            val |= REG_CONFIGURATION_CRF
> +
> +        return val
> +
> +    def reg_write(self, addr: int, val: int) -> None:
> +        assert addr in self.regs
> +        self.regs[addr] = val
> +
> +
> +class TestOPT3001(UMLTestCase):

I am partial to starting with UML since there are a lot of nice easy
things about starting there; however, I imagine people will eventually
want to use this on other architectures (speaking from experience).
How difficult do you think it would be to extend this to support
manipulating fake devices in say QEMU?

I also have some colleagues inside of Google that worked on some
projects to simulate simple devices on an FPGA to test software and
adjacent devices in a conceptually similar way; one of these teams
built a Domain Specific Language kind of like roadtest to implement
the tests and the environment for the tests. The main reason I mention
this here is I am thinking about maybe one day having an API you can
implement so you can run your roadtests on UML, QEMU, or on any
emulator or hardware testbed that implements the appropriate API.

I'll try to dig up some people who might be interested and add them here.

> +    dts = DtFragment(
> +        src="""
> +&i2c {
> +    light-sensor@...dr$ {
> +        compatible = "ti,opt3001";
> +        reg = <0x$addr$>;
> +    };
> +};
> +        """,
> +        variables={
> +            "addr": DtVar.I2C_ADDR,
> +        },
> +    )
> +
> +    @classmethod
> +    def setUpClass(cls) -> None:
> +        insmod("opt3001")
> +
> +    @classmethod
> +    def tearDownClass(cls) -> None:
> +        rmmod("opt3001")
> +
> +    def setUp(self) -> None:
> +        self.driver = I2CDriver("opt3001")
> +        self.hw = Hardware("i2c")
> +        self.hw.load_model(OPT3001)
> +
> +    def tearDown(self) -> None:
> +        self.hw.close()
> +
> +    def test_illuminance(self) -> None:
> +        data = [
> +            # Some values from datasheet, and 0
> +            (0b_0000_0000_0000_0000, 0),
> +            (0b_0000_0000_0000_0001, 0.01),
> +            (0b_0011_0100_0101_0110, 88.80),
> +            (0b_0111_1000_1001_1010, 2818.56),
> +        ]
> +        with self.driver.bind(self.dts["addr"]) as dev:
> +            luxfile = dev.path / "iio:device0/in_illuminance_input"
> +
> +            for regval, lux in data:
> +                self.hw.reg_write(REG_RESULT, regval)
> +                self.assertEqual(read_float(luxfile), lux)

I love the framework; this looks very easy to use.

One nit about this test; it seems like you cover just one test case
here - the happy path. Can you cover some other one? Particularly some
error paths?

Sorry, I am not trying to be cheeky here; it looks like this driver
actually should probably be fully (or very close to fully) testable
via roadtest as I understand it. It only looks like there are a
handful of cases to cover for the driver: the device is busy, the
device returned something invalid, the user requested something
invalid, and several SMBus read/write failures - it really only looks
like there are a handful of paths and I think they are all accessible
via the I2C interface (except for maybe the user requesting something
invalid).

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ