On 9/18/2020 3:18 PM, Andrew Jones wrote:
> On Thu, Sep 17, 2020 at 08:00:55PM +0800, Peng Liang wrote:
>> If we want to emulate ID registers, we need to initialize ID registers
>> firstly. This commit is to add a helper function to traverse
>> arm64_ftr_regs so that we can initialize ID registers from
>> arm64_ftr_regs.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@xxxxxxxxxx>
>> Signed-off-by: Peng Liang <liangpeng10@xxxxxxxxxx>
>> ---
>> arch/arm64/include/asm/cpufeature.h | 2 ++
>> arch/arm64/kernel/cpufeature.c | 13 +++++++++++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>> index 89b4f0142c28..2ba7c4f11d8a 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -79,6 +79,8 @@ struct arm64_ftr_reg {
>>
>> extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
>>
>> +int arm64_cpu_ftr_regs_traverse(int (*op)(u32, u64, void *), void *argp);
>> +
>> /*
>> * CPU capabilities:
>> *
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 6424584be01e..698b32705544 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -1112,6 +1112,19 @@ u64 read_sanitised_ftr_reg(u32 id)
>> return regp->sys_val;
>> }
>>
>> +int arm64_cpu_ftr_regs_traverse(int (*op)(u32, u64, void *), void *argp)
>> +{
>> + int i, ret;
>> +
>> + for (i = 0; i < ARRAY_SIZE(arm64_ftr_regs); i++) {
>> + ret = (*op)(arm64_ftr_regs[i].sys_id,
>> + arm64_ftr_regs[i].reg->sys_val, argp);
>> + if (ret < 0)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> #define read_sysreg_case(r) \
>> case r: return read_sysreg_s(r)
>>
>> --
>> 2.26.2
>>
>
> Skimming the rest of the patches to see how this is used I only saw a
> single callsite. Why wouldn't we just put this simple for-loop right
> there at that callsite? Or, IOW, I think this traverse function should
> be dropped.
>
> Thanks,
> drew
>
> .
>
arm64_ftr_regs is defined as a static array in arch/arm64/kernel/cpufeature.c,
which is not a virtualization-related file. Putting this simple for-loop
right there will make cpufeature.c depend on kvm_host.h. Is this a good idea?
Thanks,
Peng
