> > 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
