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]
Message-ID: <a5443c2ddf8246d987c4b9a70ba6239a@huawei.com>
Date:   Mon, 18 Sep 2023 15:03:52 +0000
From:   Shiju Jose <shiju.jose@...wei.com>
To:     David Hildenbrand <david@...hat.com>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        Linuxarm <linuxarm@...wei.com>
CC:     "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "naoya.horiguchi@....com" <naoya.horiguchi@....com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "james.morse@....com" <james.morse@....com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "jiaqiyan@...gle.com" <jiaqiyan@...gle.com>,
        "jthoughton@...gle.com" <jthoughton@...gle.com>,
        "somasundaram.a@....com" <somasundaram.a@....com>,
        "erdemaktas@...gle.com" <erdemaktas@...gle.com>,
        "pgonda@...gle.com" <pgonda@...gle.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        "duenwen@...gle.com" <duenwen@...gle.com>,
        "Vilas.Sridharan@....com" <Vilas.Sridharan@....com>,
        "mike.malvestuto@...el.com" <mike.malvestuto@...el.com>,
        "gthelen@...gle.com" <gthelen@...gle.com>,
        tanxiaofei <tanxiaofei@...wei.com>,
        "Zengtao (B)" <prime.zeng@...ilicon.com>
Subject: RE: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
 documentation for scrub driver



>-----Original Message-----
>From: David Hildenbrand <david@...hat.com>
>Sent: 18 September 2023 13:35
>To: Jonathan Cameron <jonathan.cameron@...wei.com>; Linuxarm
><linuxarm@...wei.com>
>Cc: Shiju Jose <shiju.jose@...wei.com>; linux-acpi@...r.kernel.org; linux-
>mm@...ck.org; linux-kernel@...r.kernel.org; rafael@...nel.org;
>lenb@...nel.org; naoya.horiguchi@....com; tony.luck@...el.com;
>james.morse@....com; dave.hansen@...ux.intel.com; jiaqiyan@...gle.com;
>jthoughton@...gle.com; somasundaram.a@....com;
>erdemaktas@...gle.com; pgonda@...gle.com; rientjes@...gle.com;
>duenwen@...gle.com; Vilas.Sridharan@....com; mike.malvestuto@...el.com;
>gthelen@...gle.com; tanxiaofei <tanxiaofei@...wei.com>; Zengtao (B)
><prime.zeng@...ilicon.com>
>Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>documentation for scrub driver
>
>On 18.09.23 14:28, Jonathan Cameron wrote:
>> On Mon, 18 Sep 2023 14:15:33 +0200
>> David Hildenbrand <david@...hat.com> wrote:
>>
>>> On 18.09.23 12:25, Shiju Jose wrote:
>>>> Hi David,
>>>>
>>>> Thanks for looking into this.
>>>>
>>>>> -----Original Message-----
>>>>> From: David Hildenbrand <david@...hat.com>
>>>>> Sent: 18 September 2023 08:24
>>>>> To: Shiju Jose <shiju.jose@...wei.com>; linux-acpi@...r.kernel.org;
>>>>> linux- mm@...ck.org; linux-kernel@...r.kernel.org
>>>>> Cc: rafael@...nel.org; lenb@...nel.org; naoya.horiguchi@....com;
>>>>> tony.luck@...el.com; james.morse@....com;
>>>>> dave.hansen@...ux.intel.com; jiaqiyan@...gle.com;
>>>>> jthoughton@...gle.com; somasundaram.a@....com;
>>>>> erdemaktas@...gle.com; pgonda@...gle.com; rientjes@...gle.com;
>>>>> duenwen@...gle.com; Vilas.Sridharan@....com;
>>>>> mike.malvestuto@...el.com; gthelen@...gle.com; Linuxarm
>>>>> <linuxarm@...wei.com>; Jonathan Cameron
>>>>> <jonathan.cameron@...wei.com>; tanxiaofei <tanxiaofei@...wei.com>;
>>>>> Zengtao (B) <prime.zeng@...ilicon.com>
>>>>> Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>>>>> documentation for scrub driver
>>>>>
>>>>> On 15.09.23 19:28, shiju.jose@...wei.com wrote:
>>>>>> From: Shiju Jose <shiju.jose@...wei.com>
>>>>>>
>>>>>> Add documentation for scrub driver, supports configure scrub
>>>>>> parameters, in Documentation/scrub-configure.rst
>>>>>>
>>>>>> Signed-off-by: Shiju Jose <shiju.jose@...wei.com>
>>>>>> ---
>>>>>>     Documentation/scrub-configure.rst | 55
>>>>> +++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 55 insertions(+)
>>>>>>     create mode 100644 Documentation/scrub-configure.rst
>>>>>>
>>>>>> diff --git a/Documentation/scrub-configure.rst
>>>>>> b/Documentation/scrub-configure.rst
>>>>>> new file mode 100644
>>>>>> index 000000000000..9f8581b88788
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/scrub-configure.rst
>>>>>> @@ -0,0 +1,55 @@
>>>>>> +==========================
>>>>>> +Scrub subsystem driver
>>>>>> +==========================
>>>>>> +
>>>>>> +Copyright (c) 2023 HiSilicon Limited.
>>>>>> +
>>>>>> +:Author:   Shiju Jose <shiju.jose@...wei.com>
>>>>>> +:License:  The GNU Free Documentation License, Version 1.2
>>>>>> +          (dual licensed under the GPL v2) :Original Reviewers:
>>>>>> +
>>>>>> +- Written for: 6.7
>>>>>> +- Updated for:
>>>>>> +
>>>>>> +Introduction
>>>>>> +------------
>>>>>> +The scrub subsystem driver provides the interface for configure
>>>>>> +the
>>>>>
>>>>> "... interface for configuring memory scrubbers in the system."
>>>>>
>>>>> are we only configuring firmware/hw-based memory scrubbing? I assume
>so.
>>>> The scrub control could be used for the SW  based memory scrubbing too.
>>>
>>> Okay, looks like there is not too much hw/firmware specific in there
>>> (besides these weird range changes).
>>> [...]
>>>
>>>>>> +-------
>>>>>> +
>>>>>> +  The usage takes the form shown in this example::
>>>>>> +
>>>>>> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
>>>>>> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
>>>>>> +    # cat /sys/class/scrub/scrub0/region0/speed_available
>>>>>> +    # 1-60
>>>>>> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
>>>>>> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
>>>>>> +
>>>>>> +    # cat /sys/class/scrub/scrub0/region0/speed
>>>>>> +    # 0x19
>>>>>
>>>>> Is it reasonable to return the speed as hex? You set it as dec.
>>>> Presently return speed  as hex to reduce the number of callback
>>>> function needed for reading the hex/dec data because the values for
>>>> the address range need to be in hex.
>>>
>>> If speed_available returns dec, speed better also return dec IMHO.
>>>
>>>>
>>>>>
>>>>>> +    # cat /sys/class/scrub/scrub0/region0/addr_base
>>>>>> +    # 0x100000
>>>>>
>>>>> But didn't we set it to 0x300000 ...
>>>> This is an emulated example for testing the RASF/RAS2 definition.
>>>> According to the RASF & RAS2 definition, the actual address range in
>>>> the platform could vary from the requested address range for the patrol
>scrubbing.
>>>> "The platform calculates the nearest patrol scrub boundary address
>>>> from where it can start". The platform returns the actual address
>>>> range in response to GET_PATROL_PARAMETERS command to the
>firmware.
>>>> Please see section 5.2.21.2.1 Hardware-based Memory Scrubbing ,
>>>> Table 5.87: Parameter Block Structure for PATROL_SCRUB in the ACPI
>>>> 6.5 specification.
>>>>
>>>
>>> So you configure [0x300000 - 0x400000] and you get [0x100000 -
>>> 0x300000]
>>>
>>> How does that make any sense? :)
>>>
>>> Shouldn't we rather return an error when setting a range that is
>>> impossible, instead of the hardware deciding to scrub something
>>> completely different (as can be seen in the example)?
>>>
>>
>> A broader scrub is probably reasonable, but agreed that scrubbing
>> narrower is 'interesting' as not scrubbing the memory requeseted.
>
>It's not even narrower. Both ranges don't even intersect! (sorry to say, but this
>configuration interface doesn't make any sense if hardware just does
>*something* else).
>
>If you can't configure it properly, fail with an error.
>
>> It's really annoying that neither ACPI table provides any proper
>> discoverability.  Whilst we can fix that long term, we are stuck with
>> a clunky poke it and see interface in the meantime.
>
>Can't you set it, briefly enable it, and read the values back? Then, you can
>complain to the user that the configured range is impossible.

Will try to add report to the user that the configured address range is not possible.

>
>--
>Cheers,
>
>David / dhildenb

Thanks,
Shiju

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ