Re: Bug: Windows 2003 fails to install on xen-unstable tip

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

On 23/04/13 17:04, Jan Beulich wrote:
>>>> On 23.04.13 at 17:46, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> On 23/04/13 16:02, Jan Beulich wrote:
>>>>>> On 23.04.13 at 16:57, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>> On 23/04/13 15:55, Jan Beulich wrote:
>>>>>>>> On 23.04.13 at 16:21, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>>>> On 23/04/13 14:00, Jan Beulich wrote:
>>>>>>>>>> On 23.04.13 at 13:52, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>>>>>> On 23/04/13 12:33, Jan Beulich wrote:
>>>>>>> Just went through that change again: The only thing that changes
>>>>>>> is that while rtc_periodic_cb() (simply setting REG_C flags) got
>>>>>>> called at the end of pt_intr_post(), rtc_periodic_interrupt() now
>>>>>>> gets called from pt_update_irq(), and therefore takes care of
>>>>>>> asserting the IRQ itself (which originally happened inside
>>>>>>> pt_update_irq()) along with setting REG_C flags. Bottom line -
>>>>>>> all the patch changes is when exactly REG_PF (and possibly
>>>>>>> REG_IRQF) get set, and whether the IRQ actually gets asserted.
>>>>>> So that last bit seems to do these things:
>>>>>> a. Changes when the handling happens in the Xen interrupt handler
>>>>>> b. Causes assert/deassert to only happen if !(C.PF) && (B.PIE)
>>>>>>       (Before it happened unconditionally)
>>>>>> c. Causes C.IRQF=1 only if (same condition above)
>>>>>>      (Before it happened unconditionally)
>>>>>> d. Runs destroy_periodic_timer() if C.PF already
>>>>>>
>>>>>> So just to figure out what it was that w2k3 wanted, I tried a bunch of
>>>>>> variations:
>>>>>>
>>>>>> * All of above
>>>>>>      BAD
>>>>>> * a only; always assert/deassert + set C.IRQF
>>>>>>      GOOD
>>>>>> * always assert/deassert, but leave destroy_periodic_timer and IRQF
>>>>>> setting alone
>>>>>>      FAIL
>>>>>> * destroy if C.PF, assert/deassert, set IRQF if setting C.PF (don't
>>>>>> check B.PIE)
>>>>>>      FAIL
>>>>>> * destroy if C.PF, always assert/deassert + set C.IRQF
>>>>>>      FAIL
>>>>>> * never destroy, assert/deassert + set C.IRQF if setting C.PF
>>>>>>      FAIL
>>>>>> * never destroy, assert/deassert + set C.IRQF if !C.IRQF
>>>>>>      FAIL
>>>>>>
>>>>>> In short, it seems that w2k3 basically expects an unlimited number of
>>>>>> attempts to actually deliver the interrupt.
>>>>> Not always doing the deassert/assert pair was actually part of Tim's
>>>>> subsequent changes - before that it got called conditionally upon
>>>>> B.PIE, but always set C.IRQF and always deasserted and asserted.
>>>>> Since we know from the debug log that B.PIE is set, I fail to see how
>>>>> the code prior to "x86/hvm: Centralize and simplify the RTC IRQ logic"
>>>>> would have not worked, but the above case turned out GOOD.
>>>>>
>>>>> So did his earlier 3 changes perhaps fix the issue, and the fourth re-
>>>>> introduced it?
>>>> If you've got a git hash I can try to revert it.
>>> 527824f41f5fac9cba3d4441b2e73d3118d98837
>> Reverting that c/s has no effect.
> To me that contradicts your earlier findings, but the whole story
> shows that I must be missing something.

With that c/s reverted, it still only does the deassert/assert/set 
C.IRQF if !C.PF && B.PIE; so from my previous findings I would have 
expected the test to fail.

>
>> And if I then make the change that made it work before -- namely,
>> unconditionally calling rtc_toggle_irq in rtc_periodic_interrupt() --
>> then the w2k3 installer just spins instead of hangs.
>>
>> At this point I think it's worth asking: is raising needless IRQs and
>> running non-used pmtimers really causing that big of an issue? Given
>> that I've just spent a whole day trying to debug this, wouldn't it be
>> better just to revert all the rtc-related changesets from 620d5da?
> Reverting everything, as said a number of times before, is certainly
> not the right thing.

Well we can't release with w2k3 not booting -- that's not the right 
thing either.  :-)

Anyway, if you have patches you want me to try I can give them a spin; 
but other than that I don't think I'm going to spend any more time 
trying to track down this particular problem.

  -George