[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v7 02/10] xen/x86: Add initial x2APIC ID to the per-vLAPIC save area
Hi, On Wed Oct 30, 2024 at 6:37 AM GMT, Jan Beulich wrote: > On 29.10.2024 21:30, Andrew Cooper wrote: > > On 21/10/2024 4:45 pm, Alejandro Vallejo wrote: > >> @@ -310,19 +309,16 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf, > >> break; > >> > >> case 0xb: > >> - /* > >> - * In principle, this leaf is Intel-only. In practice, it is > >> tightly > >> - * coupled with x2apic, and we offer an x2apic-capable APIC > >> emulation > >> - * to guests on AMD hardware as well. > >> - * > >> - * TODO: Rework topology logic. > >> - */ > >> if ( p->basic.x2apic ) > >> { > >> *(uint8_t *)&res->c = subleaf; > >> > >> - /* Fix the x2APIC identifier. */ > >> - res->d = v->vcpu_id * 2; > >> + /* > >> + * Fix the x2APIC identifier. The PV side is nonsensical, but > >> + * we've always shown it like this so it's kept for compat. > >> + */ > > > > In hindsight I should changed "Fix the x2APIC identifier." when I > > reworked this logic, but oh well - better late than never. > > > > /* The x2APIC_ID is per-vCPU, and fixed irrespective of the requested > > subleaf. */ > > Can we perhaps avoid "fix" in this comment? "Adjusted", "overwritten", or > some such ought to do, without carrying a hint towards some bug somewhere. I understood "fix" there as "pin" rather than "unbreak". Regardless I can also rewrite it as "The x2APIC ID is per-vCPU and shown on all subleafs" > > >> --- a/xen/include/public/arch-x86/hvm/save.h > >> +++ b/xen/include/public/arch-x86/hvm/save.h > >> @@ -394,6 +394,8 @@ struct hvm_hw_lapic { > >> uint32_t disabled; /* VLAPIC_xx_DISABLED */ > >> uint32_t timer_divisor; > >> uint64_t tdt_msr; > >> + uint32_t x2apic_id; > >> + uint32_t rsvd_zero; > > > > ... we do normally spell it _rsvd; to make it extra extra clear that > > people shouldn't be doing anything with it. > > Alternatively, to carry the "zero" in the name, how about _mbz? > > Jan I'd prefer that to _rsvd, if anything to make it patently clear that leaving rubble is not ok. Cheers, Alejandro
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |