From: Paul Brook <paul@codesourcery.com>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] Compile dma only once
Date: Sun, 30 May 2010 01:13:08 +0100 [thread overview]
Message-ID: <201005300113.08998.paul@codesourcery.com> (raw)
In-Reply-To: <AANLkTiloZgAX-QpE908YAJ_orLL1n_W2WR-2TIdEAkTO@mail.gmail.com>
> On Fri, May 28, 2010 at 7:34 PM, Paul Brook <paul@codesourcery.com> wrote:
> >> Use a qemu_irq to request CPU exit.
> >
> > Needing to request a CPU exit at all is just wrong. See previous
> > discussions about how any use of qemu_bh_schedule_idle is fundamentally
> > broken.
>
> I agree for the device case. Is the attached patch then OK?
No. You can't remove code without understanding why it was there in the first
place. I'm pretty sure this will break FDC emulation, or at least make it
unfeasibly slow.
The underlying problem is that devices (in particular the FDC) don't
communicate properly with the DMA engine. Instead they rely on the DMA device
polling state at poorly defined intervals. Removing DMA_schedule without
removing qemu_bh_schedule_idle is almost certainly wrong.
My main objection to you original patch is that it introduces a new API for
something that is just plain wrong. At minimum it needs a comment documenting
that this function should never be used for anything - It only exists because
we're too lazy to fix legacy code.
> But what about other uses (with the patch applied):
I was just referring to device emulation.
qemu_notify_event is also pretty bogus. It is a horrible hack to workaround
deficiencies in the network API.
Paul
next prev parent reply other threads:[~2010-05-30 0:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-18 19:51 [Qemu-devel] [PATCH] Compile dma only once Blue Swirl
2010-05-28 19:34 ` Paul Brook
2010-05-29 8:13 ` Blue Swirl
2010-05-30 0:13 ` Paul Brook [this message]
-- strict thread matches above, loose matches on Subject: below --
2010-05-18 19:51 Blue Swirl
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=201005300113.08998.paul@codesourcery.com \
--to=paul@codesourcery.com \
--cc=blauwirbel@gmail.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).