Re: [PATCH 00/33] KVM: PPC: Fix IRQ race in magic page code

From: Scott Wood <scottwood@freescale.com>
To: Alexander Graf <agraf@suse.de>
Cc: <kvm-ppc@vger.kernel.org>, <kvm@vger.kernel.org>
Subject: Re: [PATCH 00/33] KVM: PPC: Fix IRQ race in magic page code
Date: Tue, 24 Jun 2014 19:21:29 -0500	[thread overview]
Message-ID: <1403655689.2435.55.camel@snotra.buserror.net> (raw)
In-Reply-To: <53AA0C78.60703@suse.de>

On Wed, 2014-06-25 at 01:40 +0200, Alexander Graf wrote:
> On 25.06.14 01:15, Scott Wood wrote:
> > On Wed, 2014-06-25 at 00:41 +0200, Alexander Graf wrote:
> >> On 24.06.14 20:53, Scott Wood wrote:
> >>> The timer interrupt works, but I'm not fully convinced that it's a good
> >> idea for things like MC events which we also block during critical
> >> sections on e500v2.
> > Are you concerned about the guest seeing machine checks that are (more)
> > asynchronous with the error condition?  e500v2 machine checks are always
> > asynchronous.  From the core manual:
> >
> >          Machine check interrupts are typically caused by a hardware or
> >          memory subsystem failure or by an attempt to access an invalid
> >          address. They may be caused indirectly by execution of an
> >          instruction, but may not be recognized or reported until long
> >          after the processor has executed past the instruction that
> >          caused the machine check. As such, machine check interrupts are
> >          not thought of as synchronous or asynchronous nor as precise or
> >          imprecise.
> >
> > I don't think the lag would be a problem, and certainly it's better than
> > the current situation.
> 
> So what value would you set the timer to? If the value is too small, we 
> never finish the critical section. If it's too big, we add lots of jitter.

Maybe something like 100us?

Single stepping would be better, though.

> >> Single stepping is hard enough to get right on interaction between QEMU,
> >> KVM and the guest. I didn't really want to make that stuff any more
> >> complicated.
> > I'm not sure that it would add much complexity.  We'd just need to check
> > whether any source other than the magic page turned wants DCBR0_IC on,
> > to determine whether to exit to userspace or not.
> 
> What if the guest is single stepping itself? How do we determine when to 
> unset the bit again? When we get out of the critical section? How do we 
> know what the value was before we set it?

Keep track of each requester of single stepping separately, and only
ever set the real bit by ORing them.

-Scott