qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paul Brook <paul@codesourcery.com>
To: qemu-devel@nongnu.org
Cc: "Alberto Treviño" <alberto@byu.edu>,
	"Marcelo Tosatti" <mtosatti@redhat.com>
Subject: Re: [Qemu-devel] Re: LSI: avoid infinite loops
Date: Thu, 19 Jun 2008 23:33:04 +0100	[thread overview]
Message-ID: <200806192333.05194.paul@codesourcery.com> (raw)
In-Reply-To: <20080619215340.GA20454@dmt.cnet>

> > > > > > > The Windows driver has SCRIPTS code which busy loops on main
> > > > > > > memory. So give the CPU's a chance to run if that happens.
> > > > > >
> > > > > > I'm kinda surprised this works.  What causes the scripts engine
> > > > > > to be restarted?
> > > > >
> > > > > LSI_ISTAT0_SIGP.
> > > >
> > > > In that case my surprise continues, and this is looking like an
> > > > unbelievably horrid hack.
> > > >
> > > > By my reading you're making LSI_ISTAT0_SIGP effect whatever
> > > > instruction happens to be executing when we stall. You get doubly
> > > > lucky because (a) the guest OS decides to bang on SIGP, even though
> > > > it doesn't need to. And (b) the last instruction executed happens to
> > > > have set dnad to a value that "works". I'm guessing you always happen
> > > > to stop execution on the conditional jump instruction and taking that
> > > > jump doesn't cause any bad effects, right?
> > >
> > > Oh, I'd also be worried what happens if an async IO operation completes
> > > at this point. lsi_command_complete is liable to trample all over your
> > > state.
> >
> > So what do you suggest as a proper fix?
>
> What do you suggest as a proper fix to this problem?

At minimum you need to address the issues I've raised with your current patch.  
Stalling execution temporarily every few hundred instructions and waiting for 
SIGP (or some other trigger) before resuming may be acceptable.  Aborting 
execution and relying on very specific guest OS behavior to give correct 
results is not.  The current code is written with the assumption that 
execution will only stop at very specific points. Your patch breaks this 
assumption.

Ideally you'd also do proper loop detection rather than setting an arbitrary 
limit. I wouldn't be surprised if a good OS can create very long queues.

Paul

      reply	other threads:[~2008-06-19 22:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-07 23:02 [Qemu-devel] LSI: avoid infinite loops Marcelo Tosatti
2008-05-07 23:21 ` [Qemu-devel] " Paul Brook
2008-05-07 23:42   ` Marcelo Tosatti
2008-05-08  0:39     ` Paul Brook
2008-05-08  0:52       ` Paul Brook
2008-05-08  3:13         ` Marcelo Tosatti
2008-06-19 21:53           ` Marcelo Tosatti
2008-06-19 22:33             ` Paul Brook [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200806192333.05194.paul@codesourcery.com \
    --to=paul@codesourcery.com \
    --cc=alberto@byu.edu \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).