On 01.10.2024 14:38, Alejandro Vallejo wrote: > If toolstack were to upload LAPIC contexts as part of domain creation it
If it were to - yes. But it doesn't, an peeking ahead in the series I also couldn't spot this changing. Hence I don#t think I see why this change would be needed, and why ... > would encounter a problem were the architectural state does not reflect > the APIC ID in the hidden state. This patch ensures updates to the > hidden state trigger an update in the architectural registers so the > APIC ID in both is consistent. > > Signed-off-by: Alejandro Vallejo <[email protected]> > --- > xen/arch/x86/hvm/vlapic.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c > index 02570f9dd63a..a8183c3023da 100644 > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -1640,7 +1640,27 @@ static int cf_check lapic_load_hidden(struct domain > *d, hvm_domain_context_t *h) > > s->loaded.hw = 1; > if ( s->loaded.regs ) > + { > + /* > + * We already processed architectural regs in lapic_load_regs(), so > + * this must be a migration. Fix up inconsistencies from any older > Xen. > + */ > lapic_load_fixup(s); > + } > + else > + { > + /* > + * We haven't seen architectural regs so this could be a migration > or a > + * plain domain create. In the domain create case it's fine to modify > + * the architectural state to align it to the APIC ID that was just > + * uploaded and in the migrate case it doesn't matter because the > + * architectural state will be replaced by the LAPIC_REGS ctx later > on. > + */ ... a comment would need to mention a case that never really happens, thus only risking to cause confusion. Jan > + if ( vlapic_x2apic_mode(s) ) > + set_x2apic_id(s); > + else > + vlapic_set_reg(s, APIC_ID, SET_xAPIC_ID(s->hw.x2apic_id)); > + } > > hvm_update_vlapic_mode(v); >
