Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date:   Tue, 12 Jan 2021 11:51:50 +0000
From:   Marc Zyngier <maz@kernel.org>
To:     Suzuki K Poulose <suzuki.poulose@arm.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu,
        linux-kernel@vger.kernel.org, Will Deacon <will@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        David Brazdil <dbrazdil@google.com>,
        Alexandru Elisei <alexandru.elisei@arm.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Jing Zhang <jingzhangos@google.com>,
        Ajay Patil <pajay@qti.qualcomm.com>,
        Prasad Sodagudi <psodagud@codeaurora.org>,
        Srinivas Ramana <sramana@codeaurora.org>,
        James Morse <james.morse@arm.com>,
        Julien Thierry <julien.thierry.kdev@gmail.com>,
        kernel-team@android.com
Subject: Re: [PATCH v3 09/21] arm64: cpufeature: Add global feature override
 facility
In-Reply-To: <d98aed718a26d0455d5549d53f97db06@kernel.org>
References: <20210111132811.2455113-1-maz@kernel.org>
 <20210111132811.2455113-10-maz@kernel.org> <20210111184154.GC17941@gaia>
 <129db8bd3913a90c96d4cfe4f55e27a0@kernel.org>
 <a122aa5c-4af9-e236-db82-db0ed885e0a5@arm.com>
 <d98aed718a26d0455d5549d53f97db06@kernel.org>
User-Agent: Roundcube Webmail/1.4.9
Message-ID: <a477277b3941dc6650571f8fe29fe4f6@kernel.org>
Precedence: bulk

On 2021-01-12 11:50, Marc Zyngier wrote:
> 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.

Actually, we can only *disable* the PMU altogether. Same question 
though...

         M.

> 
> Is it something we care about?
> 
> Thanks,
> 
>         M.

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