linux-kernel - Re: [RFC PATCH v8 15/35] x86/apic: Unionize apic regs for 32bit/64bit access w/o type casting

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: <be596f16-3a03-4ad0-b3d0-c6737174534a@amd.com>
Date: Thu, 10 Jul 2025 09:13:11 +0530
From: Neeraj Upadhyay <Neeraj.Upadhyay@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: linux-kernel@...r.kernel.org, bp@...en8.de, 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 15/35] x86/apic: Unionize apic regs for 32bit/64bit
 access w/o type casting


> 
> NAK.
> 
> I really, *really* don't like this patch.  IMO, the casting code is more "obvious"
> and thus easier to follow.  And there is still casting going on, i.e. to a
> "struct apic_page".
> 
> _If_ we want to go this route, then all of the open coded literals need to be
> replaced with sizeof().  But I'd still very strongly prefer we not do this in
> the first place.
> 
> Jumping ahead a bit, I also recommend the secure AVIC stuff name its global
> varaible "secure_apic_page", because just "apic_page" could result in avoidable
> collisions.
> 
> There are also a number of extraneous local variables in x2apic_savic.c, some of
> which are actively dangerous.  E.g. using a local "bitmap" in savic_eoi() makes
> it possible to reuse a pointer and access the wrong bitmap.
> 

Thanks for the reviews, inputs and suggested cleanups! I have addressed them for v9 at

https://github.com/AMDESE/linux-kvm/commits/savic-guest-latest

I have changed

    struct secure_apic_page {
	u8 *regs[PAGE_SIZE];
    } __aligned(PAGE_SIZE);


to

    struct secure_apic_page {
	u8 regs[PAGE_SIZE];
    } __aligned(PAGE_SIZE);


... and changed 

    struct secure_apic_page *ap = this_cpu_ptr(secure_apic_page);

to 

    void *ap = this_cpu_ptr(secure_apic_page);

in savic_write(), savic_setup()


- Neeraj