Re: [PATCH] x86: add support DM&P devices

From: Marcos Del Sol Vives <marcos@orca.pet>
To: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86: add support DM&P devices
Date: Thu, 14 Oct 2021 18:29:12 +0000	[thread overview]
Message-ID: <a7e54bfa-a015-2be7-e2c0-7bab47cc2b4a@orca.pet> (raw)
In-Reply-To: <YWcQDYY9CuWKsayl@zn.tnic>

El 13/10/2021 a las 16:57, Borislav Petkov escribió:
> On Fri, Oct 08, 2021 at 06:22:46PM +0200, Marcos Del Sol Vives wrote:
>> +config CPU_SUP_DMP_32
>> +	default y
>> +	bool "Support DM&P processors" if PROCESSOR_SELECT
>> +	depends on !64BIT
> 
> You mean
> 
> 	depends on X86_32
> 
> ?
> 
> Wikipedia says those things are 32-bit.
> 

I used here !64BIT because it is what CPU_SUP_TRANSMETA_32 and
CPU_SUP_UMC_32 (the only other two 32-bit-only processors on
Kconfig.cpu) are also using.

Using X86_32 makes total sense, in fact I originally used that, but for
consistency I changed it to !64BIT to match existing flags.

Should I change it then? Should I also change the other two, possibly in
a different patch?

>> +	help
>> +	  This enables detection, tunings and quirks for DM&P processors
>> +
>> +	  You need this enabled if you want your kernel to run on a
>> +	  DM&P CPU. Disabling this option on other types of CPUs
> 
> So I'm not sure about the nomenclature: those CPUs are called Vortex86
> and DM&P is simply the next owner of the IP:
> 
> "Vortex86 previously belonged to SiS, which got the basic design from
> Rise Technology.[1] SiS sold it to DM&P Electronics[2] in Taiwan."
> 
> So I'm thinking we should call everything Vortex, the file vortex.c, the
> vendor define X86_VENDOR_VORTEX and so on.

Makes total sense. Will change it for v2.

>> +	  makes the kernel a tiny bit smaller. Disabling it on a DM&P
>> +	  CPU might render the kernel unbootable.
> 
> Why unbootable? It looks like those are perfect clones: "No special init
> required for DM&P processors." it says in the patch. :)
> 

I used that text because it's what every other x86 processor flag is
also using, even those that also do not do any special initialization.

For example, the CPU_SUP_UMC_32 flag also has the same warning, yet
arch/x86/kernel/cpu/umc.c reads "UMC chips appear to be only either 386
or 486, so no special init takes place". I thus assumed this was
standard text, in case at some point an special init is required.

Do you think it should be then reworded, or should I keep it to mantain
consistency with other existing flag descriptions?

Greetings and thanks for your time,
Marcos