Re: [PATCH v2 1/5] VMX: fix interaction of APIC-V and Viridian emulation

[George Dunlap <george.dunlap@eu.citrix.com>]

On 24/06/13 14:26, Jan Beulich wrote:
>>>> On 24.06.13 at 15:09, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> On 24/06/13 13:52, Jan Beulich wrote:
>>>>>> On 24.06.13 at 12:10, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>> On 24/06/13 08:03, Jan Beulich wrote:
>>>>> Viridian using a synthetic MSR for issuing EOI notifications bypasses
>>>>> the normal in-processor handling, which would clear
>>>>> GUEST_INTR_STATUS.SVI. Hence we need to do this in software in order
>>>>> for future interrupts to get delivered.
>>>>>
>>>>> Based on analysis by Yang Z Zhang <yang.z.zhang@intel.com>.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Hmm... so there are three paths which may end up calling this vmx EOI
>>>> code -- from viridian.c:wrmsr_vidiridan_regs(), from
>>>> vlapic.c:vlapic_reg_write(), and vmx_handle_eoi_write().
>>> This is the very reason why I favored patch 2 over this one for
>>> 4.3 ...
>> Yes, I think I didn't realize that when I looked at the patch on
>> Friday.  (It was the end of a very tiring week.)
>>
>> What other operating systems have you tested patch #2 with?  IIRC Vista
>> and Win7 both also have extensions, IIRC.
> Win7 surely has been tested, but I doubt Vista has (we don't
> routinely do that).
>
>> Also, has either #1 or #2 been tested on AMD boxen?
> No, and I don't see the point. The actor #1 adds is simply NULL for
> SVM (and hence behavior doesn't change), and the flag tested in
> #2 is VMX specific too (so behavior doesn't change either).
>
>> Choosing #1 involves the risk that we've missed something an will make
>> one of those three cases *not* like real hardware, which seems fairly
>> small.  Choosing #2 involves the risk that MS may not have implemented
>> the feature flag checking properly -- they almost surely test it *with*
>> the feature flag much more than *without* it.  Even if they do test
>> without it, they may not test with the particular combination of flags
>> that we are proposing.
>>
>> So overall, I still tend to think #1 is probably less risky.  But as I
>> said, I'm willing to go with either one.
> Which might as well mean to go with both, provided we get acks
> soon.

Um, I think exactly the opposite.  The question we want to ask now is, 
"What option will fix the problem with the lowest risk of having another 
problem crop up?"  If we choose *both*, then we get a failure if there 
is a secondary problem in *either* patch.

That's like saying, "I'm not sure which of these revolvers is loaded/has 
more bullets, so if I'm going to play Russian roulette, I might as well 
do it with both."  :-)

   -George