On Tue, May 12, 2009 at 01:35:31PM +0800, Dong, Eddie wrote: > Gleb Natapov wrote: > > On Mon, May 11, 2009 at 09:04:52AM +0800, Dong, Eddie wrote: > >> > >>> There is not point referring to current code. Current code does not > >>> handle serial exceptions properly. So fix it in your patch > >>> otherwise I propose to use my patch that fixes current code > >>> (http://patchwork.kernel.org/patch/21829/). > >>> > >> > >> I would like Avi to decide. As comments to the difference of 2 > >> patches, my undrestanding is that I am addressing the problem base > >> on SDM 5-4 with the answer to serial injection as first in first > >> service. Your patch doesn;t solve generic double fault case for > >> example exception 11 on 11, or GP on GP which needs to be converted > >> to #DF per SDM, rather you only handle the case the secondary > >> exception is PF, and servicing PF. > >> > > There is nothing to decide really. I prefer your patch with serial > > exception handling fixed. If you'll not do it I'll do it. > > OK, an additional patch will be constructive but my position is neutral. The reason (mentioned) is: > > 1: Current KVM just WARN_ON for those case (and never be hit), so the this patch won't introduce > additional issues. Either printk or WARN_ON to notify us in case we met the problem in future is safer way for me. > But current KVM also replace pending exception with a newer one after WARN_ON. I agree that real OSes (at least common ones) never hit this case. But it is possible to hit it from a guest and I have a test case. > 2: In case of real "serial ecception" happens, from architectural point of view, I think we'd better consult Table 5-2 to prioritize them, which is neither reserving former exception nor overwritting. But as you mentioned, the list is not completed. My point is that this is another complicated scenario that we should spend time in future, but not related to current patch. > If you can get more complete info about what real CPU does in case of simultaneous exceptions it would be nice. I think CPU is smart enough to understand when second exception happened while trying to handle the first one and handle the second one first in this case. Otherwise I don't see how it could work. > 3: This function will soon needs to be extended to cover IRQ case too, which needs to push back the overwritten IRQ. We need a total solution for this, so I prefer to do that some time later. > I don't think that IRQ should be handled by this function. At leas it should still be stored in its own queue. > 4: I prefer to split issue if possible. > > That is OK, You can send two patches. The first one will WARN_ON and overwrite exception like the current code does. And the second one will remove WARN_ON explaining that this case is actually possible to trigger from a guest. > > > >> I can check with internal architecture to see what does "handle > >> exceptions serially" mean in really. For me serial means first in > >> first out, and thus we should remain 1st exception. > >> > > There is a table 5.2 that defines an order between some events. The > > table is not complete, I don't see #DE there for instance. But > > consider > > this case: #DE (or #NP) happens while exception stack is paged out so > > #PF happens next. #PF is handled by TSS gate so it uses its own stack > > and it fixes exception stack in its handler. If we drop #PF because > > #DE is already waiting we will keep trying to inject #DE > > indefinitely. The result is hanging QEMU process eating 100% cpu > > time. If we replace #DE with #PF on the other hand then #PF handler > > will fix exception stack instruction that caused #DE will be > > re-executed, #DE regenerated and handled properly. So which scenario > > do you prefer? > > See above. > > Thx, eddie -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
