linux-kernel - Re: [RFC PATCH v8 16/35] x86/apic: Simplify bitwise operations on APIC bitmap

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: <20250714105248.GFaHThgLB9QnrW2xLW@fat_crate.local>
Date: Mon, 14 Jul 2025 12:52:48 +0200
From: Borislav Petkov <bp@...en8.de>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Neeraj Upadhyay <Neeraj.Upadhyay@....com>, linux-kernel@...r.kernel.org,
	tglx@...utronix.de, mingo@...hat.com, dave.hansen@...ux.intel.com,
	Thomas.Lendacky@....com, nikunj@....com, Santosh.Shukla@....com,
	Vasant.Hegde@....com, Suravee.Suthikulpanit@....com,
	David.Kaplan@....com, x86@...nel.org, hpa@...or.com,
	peterz@...radead.org, pbonzini@...hat.com, kvm@...r.kernel.org,
	kirill.shutemov@...ux.intel.com, huibo.wang@....com,
	naveen.rao@....com, kai.huang@...el.com
Subject: Re: [RFC PATCH v8 16/35] x86/apic: Simplify bitwise operations on
 APIC bitmap

On Wed, Jul 09, 2025 at 07:35:41AM -0700, Sean Christopherson wrote:
> On Wed, Jul 09, 2025, Neeraj Upadhyay wrote:
> > Use 'regs' as a contiguous linear bitmap for bitwise operations in
> > apic_{set|clear|test}_vector(). This makes the code simpler by eliminating
> 
> That's very debatable.  I don't find this code to be any simpler.  Quite the
> opposite; it adds yet another open coded math exercise, which is so "simple"
> that it warrants its own comment to explain what it's doing.
> 
> I'm not dead set against this, but I'd strongly prefer to drop this patch.

> > +static inline unsigned int get_vec_bit(unsigned int vec)
> > +{
> > +	/*
> > +	 * The registers are 32-bit wide and 16-byte aligned.
> > +	 * Compensate for the resulting bit number spacing.
> > +	 */
> > +	return vec + 96 * (vec / 32);

I kinda agree. The naked 96 doesn't tell me anything. If we do this, the
explaination of what this thing does should be crystal clear, perhaps even
with an example. And the naked numbers need to be defines with proper names.

Also:

>     This change results in slight increase in generated code size for
>     gcc-14.2.
>     
>     - Without change

What is the asm supposed to tell me?

The new change gets a LEA which is noticeable or so?

The generated code size increase is, what, a couple of bytes? Who cares?

We add asm to commit messages when it is really important. Doesn't seem so to
me here but maybe I'm missing an angle...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette