linux-kernel - Re: [PATCH v3 09/21] arm64: cpufeature: Add global feature override facility

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: <d98aed718a26d0455d5549d53f97db06@kernel.org>
Date:   Tue, 12 Jan 2021 11:50:14 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Suzuki K Poulose <suzuki.poulose@....com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        David Brazdil <dbrazdil@...gle.com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Jing Zhang <jingzhangos@...gle.com>,
        Ajay Patil <pajay@....qualcomm.com>,
        Prasad Sodagudi <psodagud@...eaurora.org>,
        Srinivas Ramana <sramana@...eaurora.org>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        kernel-team@...roid.com
Subject: Re: [PATCH v3 09/21] arm64: cpufeature: Add global feature override
 facility

Hi Suzuki,

On 2021-01-12 09:17, Suzuki K Poulose wrote:
> Hi Marc,
> 
> On 1/11/21 7:48 PM, Marc Zyngier wrote:

[...]

>> diff --git a/arch/arm64/kernel/cpufeature.c 
>> b/arch/arm64/kernel/cpufeature.c
>> index 894af60b9669..00d99e593b65 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -774,6 +774,7 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, 
>> u64 new)
>>       u64 strict_mask = ~0x0ULL;
>>       u64 user_mask = 0;
>>       u64 valid_mask = 0;
>> +    u64 override_val = 0, override_mask = 0;
>> 
>>       const struct arm64_ftr_bits *ftrp;
>>       struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>> @@ -781,9 +782,35 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, 
>> u64 new)
>>       if (!reg)
>>           return;
>> 
>> +    if (reg->override_mask && reg->override_val) {
>> +        override_mask = *reg->override_mask;
>> +        override_val = *reg->override_val;
>> +    }
>> +
>>       for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
>>           u64 ftr_mask = arm64_ftr_mask(ftrp);
>>           s64 ftr_new = arm64_ftr_value(ftrp, new);
>> +        s64 ftr_ovr = arm64_ftr_value(ftrp, override_val);
>> +
>> +        if ((ftr_mask & override_mask) == ftr_mask) {
>> +            if (ftr_ovr < ftr_new) {
> 
> Here we assume that all the features are FTR_LOWER_SAFE. We could
> probably use arm64_ftr_safe_value(ftrp, ftr_new, ftr_ovr) here ?
> That would cover us for both HIGHER_SAFE and LOWER_SAFE features.
> However that may be restrictive for FTR_EXACT, as we the safe
> value would be set to "ftr->safe_val". I guess that may be better
> than forcing to use an unsafe value for the boot CPU, which could
> anyway conflict with the other CPUs and eventually trigger the
> ftr alue to be safe_val.

I like the idea of using the helper, as it cleanups up the code a bit.
However, not being to set a feature to a certain value could be 
restrictive,
as in general, it means that we can only disable a feature and not 
adjust
its level of support.

Take PMUVER for example: with the helper, I can't override it from v8.4 
to
v8.1. I can only go to v8.0.

Is it something we care about?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...