linux-kernel - Re: [RFC PATCH v3 34/37] kvx: Add power controller driver

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: <4ac2f7d0-11f4-4cb9-b0f8-2961caca9c2f@kalrayinc.com>
Date: Fri, 23 Aug 2024 15:07:24 +0200
From: Yann Sionneau <ysionneau@...rayinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, linux-kernel@...r.kernel.org
Cc: Jonathan Borne <jborne@...rayinc.com>, Julian Vetter
 <jvetter@...rayinc.com>, Clement Leger <clement@...ment-leger.fr>, Louis
 Morhet <lmorhet@...rayinc.com>, Marius Gligor <mgligor@...rayinc.com>, Jules
 Maselbas <jmaselbas@...v.net>, bpf@...r.kernel.org
Subject: Re: [RFC PATCH v3 34/37] kvx: Add power controller driver

Hello Krzysztof,

On 22/07/2024 14:37, Krzysztof Kozlowski wrote:
> On 22/07/2024 11:41, ysionneau@...rayinc.com wrote:
>> From: Yann Sionneau <ysionneau@...rayinc.com>
>>
>> The Power Controller (pwr-ctrl) controls cores reset and wake-up
>> procedure.
>> [...]
>> +
>> +static bool pwr_ctrl_not_initialized = true;
> Do not use inverted meanings.
Ack, I will fix this.
>
>> +
>> +/**
>> + * kvx_pwr_ctrl_cpu_poweron() - Wakeup a cpu
>> + * @cpu: cpu to wakeup
>> + */
>> +int __init kvx_pwr_ctrl_cpu_poweron(unsigned int cpu)
>> +{
>> +	int ret = 0;
>> +
>> +	if (pwr_ctrl_not_initialized) {
>> +		pr_err("KVX power controller not initialized!\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	/* Set PE boot address */
>> +	writeq((unsigned long long)kvx_start,
> Addresses use kernel_ulong_t
Ack, I will fix this.
>
>> +			kvx_pwr_controller.regs + KVX_PWR_CTRL_RESET_PC_OFFSET);
>> +	/* Wake up processor ! */
>> +	writeq(1ULL << cpu,
> That's BIT
Ack, I will fix this and replace with BITULL(cpu).
>
>> +	       kvx_pwr_controller.regs + PWR_CTRL_WUP_SET_OFFSET);
>> +	/* Then clear wakeup to allow processor to sleep */
>> +	writeq(1ULL << cpu,
> BIT
Ack.
>
>> +	       kvx_pwr_controller.regs + PWR_CTRL_WUP_CLEAR_OFFSET);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct smp_operations coolidge_smp_ops __initconst = {
>> +	.smp_boot_secondary = kvx_pwr_ctrl_cpu_poweron,
>> +};
>> +
>> +static int __init kvx_pwr_ctrl_probe(void)
> That's not a probe, please rename to avoid confusion. Or make it a
> proper device driver.

Ok, I will probably rename it kvx_pwr_ctrl_init()

Thanks!

-- 

Yann