Re: [Xen-devel] [PATCH v2 2/2] x86/HVM: fix ID handling of x2APIC emulation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.09.14 at 12:53, <tim@xxxxxxx> wrote:
> At 08:57 +0100 on 12 Sep (1410508628), Jan Beulich wrote:
>> >> @@ -1254,6 +1255,29 @@ HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_s
>> >>  HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs, lapic_load_regs,
>> >>                            1, HVMSR_PER_VCPU);
>> >>  
>> >> +void vlapic_domain_unpause(const struct domain *d)
>> >> +{
>> >> +    /*
>> >> +     * Following lapic_load_hidden()/lapic_load_regs() we may need to
>> >> +     * correct ID and LDR when they come from an old, broken hypervisor.
>> >> +     */
>> > 
>> > This seems like aweful overhead for the domain_{,un}pause() path.
>> 
>> It's the best I could think of (and domain un-pausing shouldn't be that
>> frequent an operation I would hope), since ...
>> 
>> >  Why can't it be fixed up once in lapic_load_{regs,hidden}(),
>> 
>> ... both of these would have to have got carried out and ...
> 
> You're already tracking whether either of them happens so you could
> track whether both have happened and do the fixup then.  Loading a VM
> with only the hidden state and not the register state is not something
> we care about (and in that case, by construction, LDR won't be 1).

I had considered that, but dropped the idea because the two pieces
are independent, and hence I don't think we should (implicitly) disallow
it. Nor do I think we should (implicitly) disallow restoring the same
state more than once before resuming the guest (or vCPU). I.e. if I
was to go that route, we should then first formally (even if just
verbally) tighten the requirements on these context load operations.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel