Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [v3 PATCH 8/8] RISC-V: Assign hwcap only according to boot cpu.
To:     Christoph Hellwig <hch@infradead.org>
Cc:     "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        =?UTF-8?Q?Patrick_St=c3=a4hlin?= <me@packi.ch>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        Jason Cooper <jason@lakedaemon.net>,
        Alan Kao <alankao@andestech.com>,
        Dmitriy Cherkasov <dmitriy@oss-tech.org>,
        Anup Patel <anup@brainfault.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Zong Li <zongbox@gmail.com>,
        Palmer Dabbelt <palmer@sifive.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Andreas Schwab <schwab@suse.de>,
        Marc Zyngier <marc.zyngier@arm.com>,
        Thomas Gleixner <tglx@linutronix.de>
References: <1549590681-24125-1-git-send-email-atish.patra@wdc.com>
 <1549590681-24125-9-git-send-email-atish.patra@wdc.com>
 <20190208091133.GD16932@infradead.org>
From:   Atish Patra <atish.patra@wdc.com>
Message-ID: <8f5fd0c8-0320-00b1-4ddf-b1225be352c8@wdc.com>
Date:   Fri, 8 Feb 2019 15:02:52 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0)
 Gecko/20100101 Thunderbird/60.5.0
MIME-Version: 1.0
In-Reply-To: <20190208091133.GD16932@infradead.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2/8/19 1:11 AM, Christoph Hellwig wrote:
>> +	 * We don't support running Linux on hertergenous ISA systems.
>> +	 * But first "okay" processor might not be the boot cpu.
>> +	 * Check the ISA of boot cpu.
> 
> Please use up your available 80 characters per line in comments.
> 
I will fix it.

>> +		/*
>> +		 * All "okay" hart should have same isa. We don't know how to
>> +		 * handle if they don't. Throw a warning for now.
>> +		 */
>> +		if (elf_hwcap && temp_hwcap != elf_hwcap)
>> +			pr_warn("isa mismatch: 0x%lx != 0x%lx\n",
>> +				elf_hwcap, temp_hwcap);
>> +
>> +		if (hartid == boot_cpu_hartid)
>> +			boot_hwcap = temp_hwcap;
>> +		elf_hwcap = temp_hwcap;
> 
> So we always set elf_hwcap to the capabilities of the previous cpu.
> 
>> +		temp_hwcap = 0;
> 
> I think tmp_hwcap should be declared and initialized inside the outer loop
> instead having to manually reset it like this.
> 
>> +	}
>>   
>> +	elf_hwcap = boot_hwcap;
> 
> And then reset it here to the boot cpu.
> 
> Shoudn't we only report the features supported by all cores?  Otherwise
> we'll still have problems if the boot cpu supports a feature, but not
> others.
> 

Hmm. The other side of the argument is boot cpu does have a feature that 
is not supported by other hart that didn't even boot.
The user space may execute something based on boot cpu capability but 
that won't be enabled.

At least, in this way we know that we are compatible completely with 
boot cpu capabilities. Thoughts ?

Regards,
Atish
> Something like:
> 
> 	for () {
> 		unsigned long this_hwcap = 0;
> 
> 		for (i = 0; i < strlen(isa); i++)
> 			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
> 
> 		if (elf_hwcap)
> 			elf_hwcap &= this_hwcap;
> 		else
> 			elf_hwcap = this_hwcap;
> 	}
> 
>