Re: [Qemu-devel] [PULL 24/37] qtest: Add set_irq_in command to set IRQ/GPIO level

[Matthew Ogilvie via Qemu-devel]

On Mon, Jan 07, 2019 at 04:31:04PM +, Peter Maydell wrote:
> From: Steffen Görtz 
> 
> Adds a new qtest command "set_irq_in" which allows
> to set qemu gpio lines to a given level.
> 
> Based on https://lists.gnu.org/archive/html/qemu-devel/2012-12/msg02363.html
> which never got merged.
> 
> Signed-off-by: Steffen Görtz 
> Reviewed-by: Stefan Hajnoczi 
> Reviewed-by: Thomas Huth 
> Reviewed-by: Laurent Vivier 
> Signed-off-by: Stefan Hajnoczi 
> Message-id: 20190103091119.9367-2-stefa...@redhat.com
> Originally-by: Matthew Ogilvie 

It is kind of interesting to see a part of my old thread(s)
resurrected, even if it isn't the part I was actually interested
in.

Note that my miniinfo email account no longer exists, but I
can still be reached at mmogilvi+q...@zoho.com if desired.

Regarding the patch, I haven't kept up with changes to qemu
well enough to really have a useful opinion, although superficially
it looks good.  Perhaps it would make sense to include both
my old and new email addresses in the sign-off section, to both
tie it to the old threads and make it easier to reach me
if someone wants to contact me?  (But I'm not sure of a "clean"
way to do so, and it is probably low priority.  Perhaps this
current email is good enough?)

- Matthew Ogilvie

P.S.: Explanation and keywords to find this message (and me) with
search tools:

With regards to qemu, my primary interest (from that thread and
related 2012 threads) is/was being able to run
"Microport UNIX System V/386 v2.1" (ca. 1987) under qemu, which
especially required:
   - Fixing various inaccuracies in qemu's model of the i8259
 interrupt controller.
   - Implementing very old graphics card model(s) that Microport UNIX
 actually knows how to control properly.  Perhaps original
 IBM monochrome display adaptor (MDA), CGA, and/or hercules
 graphics card.  Or else my nasty qemu hackish patches I wrote
 that allowed VGA to work in text mode despite Microport driving
 it like CGA in a way that didn't work with real VGA hardware.

Re: [Qemu-devel] fix clearing i8259 IRQ lines (Was: Should the i8259 devices remain no-user?)

[Matthew Ogilvie]

On Wed, Oct 16, 2013 at 06:23:11PM +0200, Paolo Bonzini wrote:
 Il 16/10/2013 18:21, BALATON Zoltan ha scritto:
  A bit off topic but this reminded me of these patches:
 
  http://patchwork.ozlabs.org/patch/206753/
  http://patchwork.ozlabs.org/patch/208252/
 
  which never got merged. Is there a chance that these fixes get merged
  sometimes or is there an explanation why it won't be fixed? As far as I
  remember the patches were reviewed and multiple versions were proposed
  but at the end no decision was reached on which one to merge and it was
  just left uncorrected.

 Right, thank you very much.  ISTR the unanswered question was what to do
 about migration, but I need to reread all the threads.

 Paolo

Essentially correct.

Although the 8259 (interrupts) model is clearly wrong with respect
to clearing an IRQ request line, only one ancient unimportant guest
(Microport UNIX ca. 1987) seems to care, and there are potentially
significant risks to more important guests if we try to fix it:

Risks: The 8254 (timers) model is wrong in various ways, some of
which are hidden by the incorrect 8259 model, and fixing it could
potentially break migration, depending on exact circumstances.
Also, it isn't clear if there are other device models depending
on the incorrect 8259 that would also need to be fixed.

Similar changes are needed in KVM for consistency, although some of
the 8254 modes are implemented in a more simplistic way (pulses
handled as fast as possible directly, instead of
1-millisecond-long pulses on real hardware).  Note that I was
never able to get my guest running successfully under KVM; I'm
not sure what the remaining problems were.

Also, the patch series included a few other things:
  - A couple of low priority fixes that can still be worked
around without code changes, but could probably qualify
as trivial patches.
  - Some test cases to test for the 8259 problem.
  - Plus an optional VGA hack to make it work when
my ancient guest tries to directly (no BIOS) configure it
for CGA text mode.
I didn't get much feedback about these.

-

If someone actually showed real interest in actually merging
these, including the selection of a migration compatibility
strategy they would actually be willing to merge (and above:
other devices, KVM, etc), I could look into updating
the patches to match.  But the if parts aren't looking
particularly likely.  This seems like a rather core-level
wide-implication change for a newbie to be messing
with.  (I've already spent noticably more time on qemu
patches than I had intended to spend total on playing with
this guest, although I may continue if I have a clearly
defined strategy.)

- Matthew Ogilvie

Re: [Qemu-devel] [PATCH KVM v2 1/4] KVM: fix i8254 IRQ0 to be normally high

[Matthew Ogilvie]

On Fri, Jan 11, 2013 at 05:45:28PM +0200, Gleb Natapov wrote:
 On Thu, Jan 10, 2013 at 11:40:07PM -0700, Matthew Ogilvie wrote:
  On Tue, Jan 08, 2013 at 09:31:19AM +0200, Gleb Natapov wrote:
   On Mon, Jan 07, 2013 at 06:17:22PM -0600, mmogi...@miniinfo.net wrote:
On Mon, 7 Jan 2013 11:39:18 +0200, Gleb Natapov g...@redhat.com wrote:
 On Wed, Dec 26, 2012 at 10:39:53PM -0700, Matthew Ogilvie wrote:
 Reading the spec, it is clear that most modes normally leave the IRQ
 output line high, and only pulse it low to generate a leading edge.
 Especially the most commonly used mode 2.
 
 The KVM i8254 model does not try to emulate the duration of the 
 pulse at
 all, so just swap the high/low settings it to leave it high most of
 the time.
 
 This fix is a prerequisite to improving the i8259 model to handle
 the trailing edge of an interupt request as indicated in its spec:
 If it gets a trailing edge of an IRQ line before it starts to service
 the interrupt, the request should be canceled.
 
 See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
 or search the net for 23124406.pdf.
 
 Risks:
 
 There is a risk that migrating a running guest between versions
 with and without this patch will lose or gain a single timer
 interrupt during the migration process.  The only case where
 Can you elaborate on how exactly this can happen? Do not see it.
 

KVM 8254: In the corrected model, when the count expires, the model
briefly pulses output low and then high again, with the low to high
transition being what triggers the interrupt.  In the old model,
when the count expires, the model expects the output line
to already be low, and briefly pulses it high (triggering the
interrupt) and then low again.  But if the line was already
high (because it migrated from the corrected model),
this won't generate a new leading edge (low to high) and won't
trigger a new interrupt (the first post-back-migration pulse turns
into a simple trailing edge instead of a pulse).

Unless there is something I'm missing?

   No, I missed that pic-last_irr/ioapic-irr  will be migrated as 1. But
   this means that next interrupt after migration from new to old will
   always be lost.  What about clearing pit bit from last_irr/irr before
   migration? Should not affect new-new migration and should fix new-old
   one. The only problem is that we may need to consult irq routing table
   to know how pit is connected to ioapic.
  
  We should not clear both last_irr and irr.  That
  cancels the interrupt early if the CPU hasn't started servicing
  it yet:  If the guest CPU has interrupts disabled
  and hasn't begun to service the interrupt, a new-new migration could
  lose the interrupt much earlier in the countdown cycle than it otherwise
  might be lost.
  
 I am talking about last_irr in pic and irr in ioapic. Of course we
 shouldn't clear pic-irr on migration. ioapic uses irr to detect edge.

I probably need to look into the details of how the ioapic is supposed
to work.  Sigh.

 
  Potentially we could clear the last_irr bit only.  This effectively
  makes migration behave like the old unfixed code.  But if this is
  considered acceptable, I would be more inclined to not fix IRQ0 at all,
 If we do not fix IRQ0 next timer interrupt after migration will always be
 lost.

Obviously true if the trailing edge consumption logic for the
IRQ0 line is corrected in the 8259.  But if it isn't:

I probably could have been clearer that we could leave the 8254 unchanged,
and adjust the 8259 fix to leave IRQ0 as-is (with the incorrect handling
of a trailing edge - only other IRQ lines would be fixed), and then
timer interrupts would just work exactly as they do now.
This minimal fix would would probably be the lowest risk of
breaking something that currently works, but I don't know if
people would go for a patch that intentionally leaves in known
breakage in IRQ0...  This is option 1 below.

--

If we cleared last_irr in the qemu model during migration,
we risk getting an EXTRA interrupt when
migrating mode 4 (misremembered as mode 2 in an earlier
email below) from old to new models.  (If the sequence goes: line
is asserted, guest migrates, interrupt is handled [all in less than
a 8254 clock tick (approx 1 MHz)], then the off-by-one edge
in the new model triggers another leading edge...)  In contrast,
this might not be a risk in the KVM model as currently implemented.

Maybe this objection is not important, and you like option 4
listed below.

---

So I'm still not sure what overall fix strategy the main qemu and kvm
maintainers would like the best.  There are several possibilities,
but they all seem to have notable downsides.

Some possible fix strategies:

1. Only fix the IRQ2 (cascade) line in 8259.  Leave trailing edge
   logic for other lines as-is, and don't touch the 8254

Re: [Qemu-devel] [PATCH KVM v2 2/4] KVM: additional i8254 output fixes

[Matthew Ogilvie]

On Tue, Jan 08, 2013 at 09:41:36AM +0200, Gleb Natapov wrote:
 On Mon, Jan 07, 2013 at 06:35:47PM -0600, mmogi...@miniinfo.net wrote:
  On Mon, 7 Jan 2013 14:04:03 +0200, Gleb Natapov g...@redhat.com wrote:
   On Wed, Dec 26, 2012 at 10:39:54PM -0700, Matthew Ogilvie wrote:
   Make git_get_out() consistent with spec.  Currently pit_get_out()
   doesn't affect IRQ0, but it can be read by the guest in other ways.
   This makes it consistent with proposed changes in qemu's i8254 model
   as well.
   
   See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
   or search the net for 23124406.pdf.
   
   Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
   ---
arch/x86/kvm/i8254.c | 44 ++--
1 file changed, 34 insertions(+), 10 deletions(-)
   
   diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
   index cd4ec60..fd38938 100644
   --- a/arch/x86/kvm/i8254.c
   +++ b/arch/x86/kvm/i8254.c
   @@ -144,6 +144,10 @@ static int pit_get_count(struct kvm *kvm, int
   channel)

WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));

   +/* FIXME: Add some way to represent a paused timer and return
   + *   the paused-at counter value, to better model gate pausing,
   + *   wait until next CLK pulse to load counter logic, etc.
   + */
t = kpit_elapsed(kvm, c, channel);
d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);

   @@ -155,8 +159,7 @@ static int pit_get_count(struct kvm *kvm, int
   channel)
counter = (c-count - d)  0x;
break;
case 3:
   -/* XXX: may be incorrect for odd counts */
   -counter = c-count - (mod_64((2 * d), c-count));
   +counter = (c-count - (mod_64((2 * d), c-count)))  
   0xfffe;
break;
default:
counter = c-count - mod_64(d, c-count);
   @@ -180,20 +183,18 @@ static int pit_get_out(struct kvm *kvm, int
   channel)
switch (c-mode) {
default:
case 0:
   -out = (d = c-count);
   -break;
case 1:
   -out = (d  c-count);
   +out = (d = c-count);
break;
case 2:
   -out = ((mod_64(d, c-count) == 0)  (d != 0));
   +out = (mod_64(d, c-count) != (c-count - 1) || c-gate 
   == 0);
break;
case 3:
   -out = (mod_64(d, c-count)  ((c-count + 1)  1));
   +out = (mod_64(d, c-count)  ((c-count + 1)  1) || 
   c-gate == 0);
break;
case 4:
case 5:
   -out = (d == c-count);
   +out = (d != c-count);
break;
}

   @@ -367,7 +368,7 @@ static void pit_load_count(struct kvm *kvm, int
   channel, u32 val)

/*
 * The largest possible initial count is 0; this is equivalent
   - * to 216 for binary counting and 104 for BCD counting.
   + * to pow(2,16) for binary counting and pow(10,4) for BCD 
   counting.
 */
if (val == 0)
val = 0x1;
   @@ -376,6 +377,26 @@ static void pit_load_count(struct kvm *kvm, int
   channel, u32 val)

if (channel != 0) {
ps-channels[channel].count_load_time = ktime_get();
   +
   +/* In gate-triggered one-shot modes,
   + * indirectly model some pit_get_out()
   + * cases by setting the load time way
   + * back until gate-triggered.
   + * (Generally only affects reading status
   + * from channel 2 speaker,
   + * due to hard-wired gates on other
   + * channels.)
   + *
   + * FIXME: This might be redesigned if a paused
   + * timer state is added for pit_get_count().
   + */
   +if (ps-channels[channel].mode == 1 ||
   +ps-channels[channel].mode == 5) {
   +u64 delta = muldiv64(val+2, NSEC_PER_SEC, 
   KVM_PIT_FREQ);
   +ps-channels[channel].count_load_time =
   +   
   ktime_sub(ps-channels[channel].count_load_time,
   +  ns_to_ktime(delta));
   I do not understand what are you trying to do here. You assume that
   trigger will happen 2 clocks after counter is loaded?
   
  
  Modes 1 and 5 are single-shot, and they do not start counting until GATE
  is triggered, potentially well after count is loaded.  So this is
  attempting to model the start of countdown has not been triggered
  state as being mostly identical to the already triggered and also
  expired some number of clocks (2) ago state.
  
 So

Re: [Qemu-devel] [PATCH KVM v2 1/4] KVM: fix i8254 IRQ0 to be normally high

[Matthew Ogilvie]

On Tue, Jan 08, 2013 at 09:31:19AM +0200, Gleb Natapov wrote:
 On Mon, Jan 07, 2013 at 06:17:22PM -0600, mmogi...@miniinfo.net wrote:
  On Mon, 7 Jan 2013 11:39:18 +0200, Gleb Natapov g...@redhat.com wrote:
   On Wed, Dec 26, 2012 at 10:39:53PM -0700, Matthew Ogilvie wrote:
   Reading the spec, it is clear that most modes normally leave the IRQ
   output line high, and only pulse it low to generate a leading edge.
   Especially the most commonly used mode 2.
   
   The KVM i8254 model does not try to emulate the duration of the pulse at
   all, so just swap the high/low settings it to leave it high most of
   the time.
   
   This fix is a prerequisite to improving the i8259 model to handle
   the trailing edge of an interupt request as indicated in its spec:
   If it gets a trailing edge of an IRQ line before it starts to service
   the interrupt, the request should be canceled.
   
   See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
   or search the net for 23124406.pdf.
   
   Risks:
   
   There is a risk that migrating a running guest between versions
   with and without this patch will lose or gain a single timer
   interrupt during the migration process.  The only case where
   Can you elaborate on how exactly this can happen? Do not see it.
   
  
  KVM 8254: In the corrected model, when the count expires, the model
  briefly pulses output low and then high again, with the low to high
  transition being what triggers the interrupt.  In the old model,
  when the count expires, the model expects the output line
  to already be low, and briefly pulses it high (triggering the
  interrupt) and then low again.  But if the line was already
  high (because it migrated from the corrected model),
  this won't generate a new leading edge (low to high) and won't
  trigger a new interrupt (the first post-back-migration pulse turns
  into a simple trailing edge instead of a pulse).
  
  Unless there is something I'm missing?
  
 No, I missed that pic-last_irr/ioapic-irr  will be migrated as 1. But
 this means that next interrupt after migration from new to old will
 always be lost.  What about clearing pit bit from last_irr/irr before
 migration? Should not affect new-new migration and should fix new-old
 one. The only problem is that we may need to consult irq routing table
 to know how pit is connected to ioapic.

We should not clear both last_irr and irr.  That
cancels the interrupt early if the CPU hasn't started servicing
it yet:  If the guest CPU has interrupts disabled
and hasn't begun to service the interrupt, a new-new migration could
lose the interrupt much earlier in the countdown cycle than it otherwise
might be lost.

Potentially we could clear the last_irr bit only.  This effectively
makes migration behave like the old unfixed code.  But if this is
considered acceptable, I would be more inclined to not fix IRQ0 at all,
rather than make IRQ0 work subtly differently normally vs during
migration.  One of my earlier patch series (qemu v7 dated Nov 25)
attempts to use basically this strategy for the qemu model, at
least in the short term (and then potentially fix it properly
in the longer term), although the details of series might
be tweaked.

Or the minimal-risk strategy is to ONLY fix the cascade IRQ2's
trailing edge [the original i825* problem that spawned this whole
thing], leaving all other IRQs as-is.

 
 Still do not see how can we gain one interrupt.

In most cases I don't think we get an extra interrupt from
a direct fix.  But some kinds of attempts to work around missing
interrupts during migration can cause cause extra interrupts in other
cases.  In the old qemu model (but perhaps not kvm), the mode 2 leading
edge occurs one clock tick earlier than it ought to.  So if you
try to be tricky with adjusting levels during migration, you
may introduce possible cases where it gets an interrupt in
the old model, then migrates and gets another interrupt one tick
later in the new model...

Also, it occurs to me that maybe there might be an off-by-one issue in
the kvm model of mode 2 that ought to be fixed as well?  Although the  
zero length pulse in kvm suggests that one-off issues in counters do  
not matter.  In the qemu model, the leading edge of OUT in mode 2
moves by one tick...

 
  The qemu 8254 model actually models each edge at independent
  clock ticks instead of combining both into a very brief pulse at one time.
  I've found it handy to draw out old and new timing diagrams on paper
  (for each mode), and then carefully think about what happens with respect
  to levels and edges when you transition back and forth between old and
  new models at various points in the timing cycle.  (Note I've spent more
  time examining the qemu models rather than the kvm models.)
  
 Yes, drawing it definitely helps :)
 
   this is likely to be serious is probably losing a single-shot (mode 4)
   interrupt, but if my understanding of how things work is good, then
   that should only

[Qemu-devel] [PATCH KVM v2 4/4] KVM: i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 arch/x86/kvm/i8259.c | 28 +++-
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 76d8dc1..63f731b 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -95,26 +95,20 @@ static inline int pic_set_irq1(struct kvm_kpic_state *s, 
int irq, int level)
 {
int mask, ret = 1;
mask = 1  irq;
-   if (s-elcr  mask) /* level triggered */
-   if (level) {
+   if (level) {
+   if ((s-last_irr  mask) == 0 ||  /* edge for edge-triggered */
+   (s-elcr  mask)) {   /* or level triggered */
ret = !(s-irr  mask);
s-irr |= mask;
-   s-last_irr |= mask;
-   } else {
-   s-irr = ~mask;
-   s-last_irr = ~mask;
-   }
-   else/* edge triggered */
-   if (level) {
-   if ((s-last_irr  mask) == 0) {
-   ret = !(s-irr  mask);
-   s-irr |= mask;
-   }
-   s-last_irr |= mask;
-   } else {
-   s-irr = ~mask;
-   s-last_irr = ~mask;
}
+   s-last_irr |= mask;
+   } else {
+   /* Dropping level clears the interrupt regardless of edge
+* trigger vs level trigger.
+*/
+   s-irr = ~mask;
+   s-last_irr = ~mask;
+   }
 
return (s-imr  mask) ? -1 : ret;
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH KVM v2 1/4] KVM: fix i8254 IRQ0 to be normally high

[Matthew Ogilvie]

Reading the spec, it is clear that most modes normally leave the IRQ
output line high, and only pulse it low to generate a leading edge.
Especially the most commonly used mode 2.

The KVM i8254 model does not try to emulate the duration of the pulse at
all, so just swap the high/low settings it to leave it high most of
the time.

This fix is a prerequisite to improving the i8259 model to handle
the trailing edge of an interupt request as indicated in its spec:
If it gets a trailing edge of an IRQ line before it starts to service
the interrupt, the request should be canceled.

See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
or search the net for 23124406.pdf.

Risks:

There is a risk that migrating a running guest between versions
with and without this patch will lose or gain a single timer
interrupt during the migration process.  The only case where
this is likely to be serious is probably losing a single-shot (mode 4)
interrupt, but if my understanding of how things work is good, then
that should only be possible if a whole slew of conditions are
all met:

 1. The guest is configured to run in a tickless mode (like
modern Linux).
 2. The guest is for some reason still using the i8254 rather
than something more modern like an HPET.  (The combination
of 1 and 2 should be rare.)
 3. The migration is going from a fixed version back to the
old version.  (Not sure how common this is, but it should
be rarer than migrating from old to new.)
 4. There are not going to be any timely events/interrupts
(keyboard, network, process sleeps, etc) that cause the guest
to reset the PIT mode 4 one-shot counter soon enough.

This combination should be rare enough that more complicated
solutions are not worth the effort.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 arch/x86/kvm/i8254.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index c1d30b2..cd4ec60 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -290,8 +290,12 @@ static void pit_do_work(struct kthread_work *work)
}
spin_unlock(ps-inject_lock);
if (inject) {
-   kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 1);
+   /* Clear previous interrupt, then create a rising
+* edge to request another interupt, and leave it at
+* level=1 until time to inject another one.
+*/
kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 0);
+   kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 1);
 
/*
 * Provides NMI watchdog support via Virtual Wire mode.
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH KVM v2 0/4] fix KVM i8259 IRQ trailing-edge logic

[Matthew Ogilvie]

Changes since version 1 (from Sep 9):
   * Split off patch 1; this is the critical prerequisite to
 make the i8254 work with the fixed i8259.
   * Add patch 2, to make additional changes to the i8254
 to make it consistent with the spec and with proposed changes
 to qemu's i8254 model.

Background:

According to the spec, the i8259 will cancel an interrupt if
the line goes low before it starts servicing it, even when
it is considered edge-triggered.  This is a problem
with an old Microport UNIX guest (ca 1987), where the
guest masks off IRQ14 in the slave i8259, but the host's
master i8259 model will still try to deliver an interrupt even after
IRQ2 has been brought low, resulting in a spurious interrupt (IRQ15).

Before the i8259 can be fixed, the i8254 model needs to be fixed
so it doesn't depend on the broken i8259.

Alternative: This could be narrowly fixed for IRQ2 only with no
risk at all (and no need to touch the i8254), but if possible it
seems reasonable to fix it for all lines at the same time.

But there may be some risk:

First, I think I've convinced myself that between the i8254 and i8259,
the only substantial migration breakage should be when a
whole series of conditions are met, and the combination
should be rare enough not to worry about it.  See writeup
in my qemu patch series (version 8; Dec 16).

Second, there is also the possibility that other devices are relying
on the broken model.  I'm especially concerned with various users
of a function called

qemu_irq_pulse()

in the qemu source tree, which immediately lowers IRQ line after
raising it.  If any of those calls are routed straight into
the i8259 (as opposed to an APIC or some other chip), then it
probably won't behave as desired.

I'll probably dig into qemu_irq_pulse() callers more carefully
eventually, but there are lot of them, and any high-level incite
anyone can provide into them would be helpful.

See the Dec 16 patch series I sent to the qemu mailing list for
more information.
http://search.gmane.org/?query=ogilviegroup=gmane.comp.emulators.qemu

Matthew Ogilvie (4):
  KVM: fix i8254 IRQ0 to be normally high
  KVM: additional i8254 output fixes
  KVM: fix i8259 interrupt high to low transition logic
  KVM: i8259: refactor pic_set_irq level logic

 arch/x86/kvm/i8254.c | 50 +++---
 arch/x86/kvm/i8259.c | 36 ++--
 2 files changed, 53 insertions(+), 33 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH KVM v2 3/4] KVM: fix i8259 interrupt high to low transition logic

[Matthew Ogilvie]

Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered
and then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt to the CPU even
though IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked UNIX kernel.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 arch/x86/kvm/i8259.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index cc31f7c..76d8dc1 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -111,8 +111,10 @@ static inline int pic_set_irq1(struct kvm_kpic_state *s, 
int irq, int level)
s-irr |= mask;
}
s-last_irr |= mask;
-   } else
+   } else {
+   s-irr = ~mask;
s-last_irr = ~mask;
+   }
 
return (s-imr  mask) ? -1 : ret;
 }
@@ -169,14 +171,10 @@ static void pic_update_irq(struct kvm_pic *s)
 {
int irq2, irq;
 
+   /* slave PIC notifies master PIC via IRQ2 */
irq2 = pic_get_irq(s-pics[1]);
-   if (irq2 = 0) {
-   /*
-* if irq request by slave pic, signal master PIC
-*/
-   pic_set_irq1(s-pics[0], 2, 1);
-   pic_set_irq1(s-pics[0], 2, 0);
-   }
+   pic_set_irq1(s-pics[0], 2, irq2 = 0);
+
irq = pic_get_irq(s-pics[0]);
pic_irq_request(s-kvm, irq = 0);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH KVM v2 2/4] KVM: additional i8254 output fixes

[Matthew Ogilvie]

Make git_get_out() consistent with spec.  Currently pit_get_out()
doesn't affect IRQ0, but it can be read by the guest in other ways.
This makes it consistent with proposed changes in qemu's i8254 model
as well.

See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
or search the net for 23124406.pdf.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 arch/x86/kvm/i8254.c | 44 ++--
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index cd4ec60..fd38938 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -144,6 +144,10 @@ static int pit_get_count(struct kvm *kvm, int channel)
 
WARN_ON(!mutex_is_locked(kvm-arch.vpit-pit_state.lock));
 
+   /* FIXME: Add some way to represent a paused timer and return
+*   the paused-at counter value, to better model gate pausing,
+*   wait until next CLK pulse to load counter logic, etc.
+*/
t = kpit_elapsed(kvm, c, channel);
d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);
 
@@ -155,8 +159,7 @@ static int pit_get_count(struct kvm *kvm, int channel)
counter = (c-count - d)  0x;
break;
case 3:
-   /* XXX: may be incorrect for odd counts */
-   counter = c-count - (mod_64((2 * d), c-count));
+   counter = (c-count - (mod_64((2 * d), c-count)))  0xfffe;
break;
default:
counter = c-count - mod_64(d, c-count);
@@ -180,20 +183,18 @@ static int pit_get_out(struct kvm *kvm, int channel)
switch (c-mode) {
default:
case 0:
-   out = (d = c-count);
-   break;
case 1:
-   out = (d  c-count);
+   out = (d = c-count);
break;
case 2:
-   out = ((mod_64(d, c-count) == 0)  (d != 0));
+   out = (mod_64(d, c-count) != (c-count - 1) || c-gate == 0);
break;
case 3:
-   out = (mod_64(d, c-count)  ((c-count + 1)  1));
+   out = (mod_64(d, c-count)  ((c-count + 1)  1) || c-gate 
== 0);
break;
case 4:
case 5:
-   out = (d == c-count);
+   out = (d != c-count);
break;
}
 
@@ -367,7 +368,7 @@ static void pit_load_count(struct kvm *kvm, int channel, 
u32 val)
 
/*
 * The largest possible initial count is 0; this is equivalent
-* to 216 for binary counting and 104 for BCD counting.
+* to pow(2,16) for binary counting and pow(10,4) for BCD counting.
 */
if (val == 0)
val = 0x1;
@@ -376,6 +377,26 @@ static void pit_load_count(struct kvm *kvm, int channel, 
u32 val)
 
if (channel != 0) {
ps-channels[channel].count_load_time = ktime_get();
+
+   /* In gate-triggered one-shot modes,
+* indirectly model some pit_get_out()
+* cases by setting the load time way
+* back until gate-triggered.
+* (Generally only affects reading status
+* from channel 2 speaker,
+* due to hard-wired gates on other
+* channels.)
+*
+* FIXME: This might be redesigned if a paused
+* timer state is added for pit_get_count().
+*/
+   if (ps-channels[channel].mode == 1 ||
+   ps-channels[channel].mode == 5) {
+   u64 delta = muldiv64(val+2, NSEC_PER_SEC, KVM_PIT_FREQ);
+   ps-channels[channel].count_load_time =
+ktime_sub(ps-channels[channel].count_load_time,
+  ns_to_ktime(delta));
+   }
return;
}
 
@@ -383,7 +404,6 @@ static void pit_load_count(struct kvm *kvm, int channel, 
u32 val)
 * mode 1 is one shot, mode 2 is period, otherwise del timer */
switch (ps-channels[0].mode) {
case 0:
-   case 1:
 /* FIXME: enhance mode 4 precision */
case 4:
create_pit_timer(kvm, val, 0);
@@ -393,6 +413,10 @@ static void pit_load_count(struct kvm *kvm, int channel, 
u32 val)
create_pit_timer(kvm, val, 1);
break;
default:
+   /* Modes 1 and 5 are triggered by gate leading edge,
+* but channel 0's gate is hard-wired high and has
+* no edges (on normal real hardware).
+*/
destroy_pit_timer(kvm-arch.vpit);
}
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 01/10] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 40efa8a..84f2f16 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2053,8 +2053,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index af0ba4d..60c25ba 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 804db60..72ceaaf 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index efda173..10ba9b5 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 07/10] i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index 95587cd..9b2ec40 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,26 +140,18 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
-if (s-elcr  mask) {
-/* level triggered */
-if (level) {
+if (level) {
+if ((s-last_irr  mask) == 0 ||  /* edge for edge triggered */
+(s-elcr  mask)) {   /* or level triggered */
 s-irr |= mask;
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
 }
+s-last_irr |= mask;
 } else {
-/* edge triggered */
-if (level) {
-if ((s-last_irr  mask) == 0) {
-s-irr |= mask;
-}
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
-}
+/* Dropping level clears the interrupt regardless of edge trigger
+ * vs level trigger.
+ */
+s-irr = ~mask;
+s-last_irr = ~mask;
 }
 pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 05/10] fix i8254 output logic to match the spec

[Matthew Ogilvie]

This patch fixes i8254 output line (IRQ0) logic to match the spec.
Basically, IRQ0 line should normally be high, and only occasionally
go low to cause an edge.  This is an important prerequisite to
fixing the i8259 interrupt controller to cancel an unhandled interrupt
when the IRQ line goes low.

More details:
  * Fix high-vs-low counting logic to match the timing diagrams
and descriptions in Intel's documentation.
  * Improve reading back the count in mode 3.
  * Handle GATE input properly with respect to the OUT line, and add
a FIXME comment for GATE-paused counting and reading
back the counter.  GATE is hard wired high for channel 0 (IRQ0), but
it can be software controlled on channel 2 (PC speaker).

See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
or search the net for 23124406.pdf.

Risks:

There is a risk that migrating between versions of qemu
with and without this patch will lose or gain a single timer
interrupt during the migration process.  The only case where
this is likely to be serious is probably losing a single-shot (mode 4)
interrupt, but if my understanding of how things work is good, then
that should only be possible if a whole slew of conditions are
all met:

 1. The guest is configured to run in a tickless mode (like
modern Linux).
 2. The guest is for some reason still using the i8254 rather
than something more modern like an HPET.  (The combination
of 1 and 2 should be rare.)
 3. The migration is going from a fixed version back to the
old version.  (Not sure how common this is, but it should
be rarer than migrating from old to new.)
 4. There are not going to be any timely events/interrupts
(keyboard, network, process sleeps, etc) that cause the guest
to reset the PIT mode 4 one-shot counter soon enough.

This combination should be rare enough that more complicated
solutions are not worth the effort.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8254.c| 24 ++--
 hw/i8254_common.c | 18 ++
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index bea5f92..edb5b7a 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -39,6 +39,15 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t 
current_time);
 
 static int pit_get_count(PITChannelState *s)
 {
+/* FIXME: Add some way to represent a paused timer and return
+ *   the paused-at counter value, to better model:
+ * - gate-triggered modes (1 and 5),
+ * - gate-pausable modes,
+ * - [maybe] the wait until next CLK pulse to load counter logic,
+ * - [maybe/not clear] half-loaded counter logic for mode 0, and
+ *   the null count status bit,
+ * - etc.
+ */
 uint64_t d;
 int counter;
 
@@ -52,8 +61,7 @@ static int pit_get_count(PITChannelState *s)
 counter = (s-count - d)  0x;
 break;
 case 3:
-/* XXX: may be incorrect for odd counts */
-counter = s-count - ((2 * d) % s-count);
+counter = (s-count - ((2 * d) % s-count))  0xfffe;
 break;
 default:
 counter = s-count - (d % s-count);
@@ -98,6 +106,18 @@ static inline void pit_load_count(PITChannelState *s, int 
val)
 if (val == 0)
 val = 0x1;
 s-count_load_time = qemu_get_clock_ns(vm_clock);
+
+/* In gate-triggered one-shot modes, indirectly model some pit_get_out()
+ * cases by setting the load time way back until gate-triggered.
+ * (Generally only affects reading status from channel 2 speaker,
+ * due to hard-wired gates on other channels.)
+ *
+ * FIXME: This might be redesigned if a paused timer state is added
+ * for pic_get_count().
+ */
+if (s-mode == 1 || s-mode == 5)
+s-count_load_time -= muldiv64(val+2, get_ticks_per_sec(), PIT_FREQ);
+
 s-count = val;
 pit_irq_timer_update(s, s-count_load_time);
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index a03d7cd..dc13c99 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -50,24 +50,18 @@ int pit_get_out(PITChannelState *s, int64_t current_time)
 switch (s-mode) {
 default:
 case 0:
-out = (d = s-count);
-break;
 case 1:
-out = (d  s-count);
+out = (d = s-count);
 break;
 case 2:
-if ((d % s-count) == 0  d != 0) {
-out = 1;
-} else {
-out = 0;
-}
+out = (d % s-count) != (s-count - 1) || s-gate == 0;
 break;
 case 3:
-out = (d % s-count)  ((s-count + 1)  1);
+out = (d % s-count)  ((s-count + 1)  1) || s-gate == 0;
 break;
 case 4:
 case 5:
-out = (d == s-count);
+out = (d != s-count);
 break;
 }
 return out;
@@ -93,10 +87,10 @@ int64_t pit_get_next_transition_time(PITChannelState *s, 
int64_t current_time)
 break;
 case 2:
 base = (d / s-count) * s

[Qemu-devel] [PATCH v8 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

This series makes a series of mostly-unrelated fixes to allow
running an old Microport UNIX (ca 1987) guest under qemu.

The most intrusive of the fixes is modifying how the i8259 handles
the trailing edge of an interrupt request (the trailing edge ought
to cancel interrupt, even when the line is edge-triggered).

Changes since version 7:
   * Abandon attempts to be fancy about handling cross version migration
 in the i8254 model.  Just fix it directly.  Analysis suggests
 migration issues should essentially always be minor - see comments
 for patch 5.
   * Added qtest-based test/pic-test.c (and associated infrastructure)
 to test the i8259's handling of the trailing edge of an interrupt
 request, to prevent future regressions.

I'm still wondering if maybe the non-i825[49]-related patches should be
split off from the rest of this series?  Any chance some of them (at
least the trivial ones) could just be included immediately?

I still need to refine the KVM patches I posted back in September
to match these latest changes.  Maybe I'll get to that next weekend.


Potential issue with this:


I'm not sure if it is a problem or not, but while figuring out
how to hookup pic-test.c, I noticed the existence of the

qemu_irq_pulse()

function in hw/irq.h.  It is raising an IRQ line only
to immediately lower it again, which for a real i8259
(at least) effectively cancels the interrupt request before
it could ever be serviced.  It is used in the following files:

hw/bonito.c
hw/dma.c
hw/grlib_apbuart.c
hw/grlib_gptimer.c
hw/hpet.c
hw/milkymist-ac97.c
hw/milkymist-minimac2.c
hw/milkymist-pfpu.c
hw/milkymist-softusb.c
hw/milkymist-sysctl.c
hw/milkymist-tmu2.c
hw/omap1.c
hw/omap_gptimer.c
hw/onenand.c
hw/spapr_events.c
hw/spapr_llan.c
hw/spapr_pci.c
hw/spapr_vio.c
hw/spapr_vty.c
hw/stellaris.c
hw/xilinx_ethlite.c

If any of these calls are ever routed into the i8259, then I doubt
they will behave as desired once the i8259 is fixed to handle the
trailing edge of an interrupt request as indicated in the
spec (patch 6).  (That is, the devices use over-simplified interrupt logic
that relies on the currently broken behavior of the i8259 model.)

On the other hand, maybe some or all of these devices are only used
in situations where the the IRQ line is fed into something else (like
an ioapic configured to invert the polarity of the line, or perhaps
some other non-intel interrupt chip that doesn't have this
canceling behavior), in which case they should be fine.

Can anyone shed some light on this?  I don't really know much
of anything about any of the devices in the above list (or any
other devices that might manually do something similar without using
qemu_irq_pulse()), so I'm not sure if there are real problems here
or not.

If more than a couple of these devices are wrong, then I'm tempted to
only fix the cascade interrupt (IRQ2) in the i8259 (leaving others
as-is), until such time as all these device models are improved.

-

Matthew Ogilvie (10):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  fix i8254 output logic to match the spec
  i8259: fix so that dropping IRQ level always clears the interrupt
request
  i8259: refactor pic_set_irq level logic
  qtest: fix qemu_irq_intercept_out()
  qtest: add set_irq_{in,out} infrastructure for testing interrupt
controllers
  add test/pic-test.c

 hw/cirrus_vga.c   |   4 +-
 hw/i8254.c|  24 ++-
 hw/i8254_common.c |  18 +++-
 hw/i8259.c|  34 +++
 hw/ide/cmd646.c   |   5 ++-
 hw/ide/via.c  |   5 ++-
 hw/irq.c  |  11 +++--
 hw/irq.h  |   2 +-
 hw/pc.h   |   4 ++
 hw/vga.c  |  37 
 qemu-options.hx   |  27 +++-
 qtest.c   |  53 ++-
 tests/Makefile|   2 +
 tests/libqtest.c  |  12 ++
 tests/libqtest.h  |  48 +
 tests/pic-test.c  | 127 ++
 vl.c  |  62 +-
 17 files changed, 403 insertions(+), 72 deletions(-)
 create mode 100644 tests/pic-test.c

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 03/10] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 qemu-options.hx | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 231cc4f..c50f737 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1007,7 +1007,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -1032,6 +1032,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 08/10] qtest: fix qemu_irq_intercept_out()

[Matthew Ogilvie]

For the 8259 (at least), we need to modify the entries in gpio_out
(which is pointing at PICCommonState::int_out) in-place rather than
change it to point to a totally different table.  The 8259 sends its
output to int_out even if gpio_out is a different table.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/irq.c | 11 ---
 hw/irq.h |  2 +-
 qtest.c  |  2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/irq.c b/hw/irq.c
index f4e2a78..60fa152 100644
--- a/hw/irq.c
+++ b/hw/irq.c
@@ -129,8 +129,13 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, 
qemu_irq_handler handler, int n)
 }
 }
 
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int 
n)
+void qemu_irq_intercept_out(qemu_irq *gpio_out, qemu_irq_handler handler, int 
n)
 {
-qemu_irq *old_irqs = *gpio_out;
-*gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
+int i;
+qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n);
+for (i = 0; i  n; i++) {
+*old_irqs[i] = *gpio_out[i];
+gpio_out[i]-handler = handler;
+gpio_out[i]-opaque = old_irqs;
+}
 }
diff --git a/hw/irq.h b/hw/irq.h
index 610e6b7..8dc26cf 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -52,6 +52,6 @@ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n);
 /* For internal use in qtest.  Similar to qemu_irq_split, but operating
on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int 
n);
+void qemu_irq_intercept_out(qemu_irq *gpio_out, qemu_irq_handler handler, int 
n);
 
 #endif
diff --git a/qtest.c b/qtest.c
index fbfab4e..6965910 100644
--- a/qtest.c
+++ b/qtest.c
@@ -232,7 +232,7 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 }
 
 if (words[0][14] == 'o') {
-qemu_irq_intercept_out(dev-gpio_out, qtest_irq_handler, 
dev-num_gpio_out);
+qemu_irq_intercept_out(dev-gpio_out, qtest_irq_handler, 
dev-num_gpio_out);
 } else {
 qemu_irq_intercept_in(dev-gpio_in, qtest_irq_handler, 
dev-num_gpio_in);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 10/10] add test/pic-test.c

[Matthew Ogilvie]

Microport UNIX System V/386 v2.1 (ca. 1987) does something similar
to the slave_imr() test, and doesn't run if the canceling behavior
of IRQ2 doesn't work.

I don't know anything that depends on the canceling behavior of
other interrupts (master_basic() or slave_basic() tests), but
it seems best to fix the issue generically if possible.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 tests/Makefile   |   2 +
 tests/pic-test.c | 127 +++
 2 files changed, 129 insertions(+)
 create mode 100644 tests/pic-test.c

diff --git a/tests/Makefile b/tests/Makefile
index b60f0fb..d21b0d5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -25,6 +25,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 check-qtest-i386-y = tests/fdc-test$(EXESUF)
 check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
+check-qtest-i386-y += tests/pic-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
@@ -75,6 +76,7 @@ tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o 
tests/test-qmp-marsh
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o 
$(test-qapi-obj-y)
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
+tests/pic-test$(EXESUF): tests/pic-test.o $(trace-obj-y)
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
diff --git a/tests/pic-test.c b/tests/pic-test.c
new file mode 100644
index 000..57b8bee
--- /dev/null
+++ b/tests/pic-test.c
@@ -0,0 +1,127 @@
+/*
+ * QTest testcase for the 8259 interrupt controller
+ *
+ * Copyright (c) 2012 Matthew Ogilvie
+ *
+ * Authors:
+ *  Matthew Ogilvie mmogilvi_q...@miniinfo.net
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include libqtest.h
+
+#include glib.h
+
+static void init_i8259(void)
+{
+/* master: */
+outb(0x20,0x13); /* ICW1: edge-triggered, multiple 8259s, ICW4 needed */
+outb(0x21,0x08); /* ICW2: map to int08-int0f */
+outb(0x21,0x04); /* ICW3: IR2 connected to slave */
+outb(0x21,0x01); /* ICW4: not-SFNM, not-buffered, manual EOI, 8086 mode */
+
+outb(0x21,0xfa); /* OCW1/IMR: mask off all except IRQ2 and IRQ0 */
+
+/* slave: */
+outb(0xa0,0x13); /* ICW1: edge-triggered, multiple 8259s, ICW4 needed */
+outb(0xa1,0x70); /* ICW2: map to int70-int77 */
+outb(0xa1,0x02); /* ICW3: connected to master IR2 */
+outb(0xa1,0x01); /* ICW4: not-SFNM, not-buffered, manual EOI, 8086 mode */
+
+outb(0xa1,0x3f); /* OCW1/IMR: mask off all except IRQ14 and IRQ15 */
+}
+
+static void set_irq(int num, int level)
+{
+if(num  8) {
+set_irq_in(i8259-slave, num - 8, level);
+} else {
+set_irq_in(i8259-master, num, level);
+}
+}
+
+static void slave_imr(void)
+{
+init_i8259();
+
+g_assert_cmpint(get_irq(0), ==, 0);
+set_irq(14, 1);
+g_assert_cmpint(get_irq(0), ==, 1);
+
+outb(0xa1,0xff); /* OCW1/IMR: mask off all slave IRQs */
+g_assert_cmpint(get_irq(0), ==, 0);
+
+outb(0xa0,0x0a); /* slave OCW3: read IRR */
+g_assert_cmpint(inb(0xa0)0x40, ==, 0x40); /* IRR still has IRQ14 */
+outb(0x20,0x0a); /* master OCW3: read IRR */
+g_assert_cmpint(inb(0x20)0x04, ==, 0x00); /* IRR does not have IRQ2 */
+
+outb(0xa1,0x3f); /* OCW1/IMR: unmask IRQ14 and IRQ15 */
+g_assert_cmpint(get_irq(0), ==, 1);
+
+outb(0xa0,0x0a); /* slave OCW3: read IRR */
+g_assert_cmpint(inb(0xa0)0x40, ==, 0x40); /* IRR has IRQ14 */
+outb(0x20,0x0a); /* master OCW3: read IRR */
+g_assert_cmpint(inb(0x20)0x04, ==, 0x04); /* IRR has IRQ2 */
+}
+
+static void master_cancel(void)
+{
+init_i8259();
+
+g_assert_cmpint(get_irq(0), ==, 0);
+set_irq(0, 1);
+g_assert_cmpint(get_irq(0), ==, 1);
+set_irq(0, 0);
+g_assert_cmpint(get_irq(0), ==, 0);
+}
+
+static void slave_cancel(void)
+{
+init_i8259();
+
+g_assert_cmpint(get_irq(0), ==, 0);
+set_irq(14, 1);
+g_assert_cmpint(get_irq(0), ==, 1);
+set_irq(14, 0);
+g_assert_cmpint(get_irq(0), ==, 0);
+}
+
+int main(int argc, char **argv)
+{
+QTestState *s = NULL;
+int ret;
+
+g_test_init(argc, argv, NULL);
+
+s = qtest_start(/*-qtest-log /dev/tty */ -display none);
+qtest_irq_intercept_out(s, i8259-master);
+
+/* FUTURE: Before testing obscure cases, first test basic
+ *  interrupt delivery to the CPU and EOI functionality.
+ *  But that requires some kind of abstracted hook into
+ *  cpu_get_pic_interrupt() vs other architecture equivalents,
+ *  and the abstraction doesn't currently exist.
+ */
+
+/* I know of at least one (admittedly

[Qemu-devel] [PATCH v8 02/10] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 3ebf01f..12786f0 100644
--- a/vl.c
+++ b/vl.c
@@ -2528,8 +2528,9 @@ int main(int argc, char **argv, char **envp)
 const char *kernel_filename, *kernel_cmdline;
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2584,8 +2585,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2633,7 +2633,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2656,21 +2656,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2704,7 +2691,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2736,7 +2726,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 04/10] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/pc.h |  4 
 hw/vga.c| 37 +
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index 2237e86..9b9538b 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -152,6 +152,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 int isa_vga_mm_init(hwaddr vram_base,
 hwaddr ctrl_base, int it_shift,
 MemoryRegion *address_space);
diff --git a/hw/vga.c b/hw/vga.c
index c266161..2ddd09d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -552,14 +552,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xd;
+} else if (s-cr_index == VGA_CRTC_CURSOR_END 
+   val == 7 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xe;
+}
 }
-return;
 }
 s-cr[s-cr_index] = val;
 
@@ -1896,7 +1913,11 @@ static void vga_update_display(void *opaque)
 /* nothing to do */
 } else {
 full_update = 0;
-if (!(s-ar_index  0x20)) {
+if (!(s-ar_index  0x20) 
+/* extra CGA compatibility hacks (not in standard VGA) */
+(!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index c50f737..3f1e122 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1037,6 +1037,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default for CGA fonts
+instead of VGA

[Qemu-devel] [PATCH v8 06/10] i8259: fix so that dropping IRQ level always clears the interrupt request

[Matthew Ogilvie]

Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered and
then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked kernel.

Output from a test program:
---
Real hardware [Pentium 4]:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
[I also see a final IRR= occasionally.  Probably just happened to
ask it while the timer (IRQ0) line is low.  It masks off most IRQ's, including
timer.]
---
Unpatched qemu:
  cmdRead unmask IRR=4015 mask IRR=4015 sti irq15 unmask IRR=4015 DONE
[Presumably IRQ4 (0x10 - qemu's serial device model?) had a transient
edge during initialization, but had been masked off even before I
masked it off?]
---
Patched qemu:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
---

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

There is a risk that some other device models depend on the broken
behavior.  See my question about qemu_irq_pulse() in the cover
letter.

 hw/i8259.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 60c25ba..95587cd 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -157,6 +157,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 s-last_irr |= mask;
 } else {
+s-irr = ~mask;
 s-last_irr = ~mask;
 }
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 09/10] qtest: add set_irq_{in, out} infrastructure for testing interrupt controllers

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c   |  6 ++
 qtest.c  | 51 +++
 tests/libqtest.c | 12 
 tests/libqtest.h | 48 
 4 files changed, 117 insertions(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 9b2ec40..5f09f2f 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -453,6 +453,9 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
 }
 
 isa_pic = dev-qdev;
+object_property_add_link(OBJECT(bus),
+ i8259-master, TYPE_DEVICE,
+ (Object **)isa_pic, NULL);
 
 dev = i8259_init_chip(isa-i8259, bus, false);
 
@@ -462,6 +465,9 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
 }
 
 slave_pic = DO_UPCAST(PICCommonState, dev, dev);
+object_property_add_link(OBJECT(bus),
+ i8259-slave, TYPE_DEVICE,
+ (Object **)slave_pic, NULL);
 
 return irq_set;
 }
diff --git a/qtest.c b/qtest.c
index 6965910..d4c2dd7 100644
--- a/qtest.c
+++ b/qtest.c
@@ -117,6 +117,21 @@ static bool qtest_opened;
  * where NUM is an IRQ number.  For the PC, interrupts can be intercepted
  * simply with irq_intercept_in ioapic (note that IRQ0 comes out with
  * NUM=0 even though it is remapped to GSI 2).
+ *
+ * Testing interrupt handler chips like the i8259:
+ *
+ *   set_irq_in QOM-PATH IRQ LEVEL
+ *   OK
+ *
+ *   set_irq_out QOM-PATH IRQ LEVEL
+ *   OK
+ *
+ * Forcibly set the given interrupt pin to the given level (from the
+ * consumer's point of view).
+ *
+ * FUTURE: Some abstracted mechanism to initiate delivery of an
+ *   interrupt to the CPU, returning the interrupt vector number
+ *   that would be delivered to that CPU.
  */
 
 static int hex2nib(char ch)
@@ -239,7 +254,43 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 irq_intercept_dev = dev;
 qtest_send_prefix(chr);
 qtest_send(chr, OK\n);
+} else if (strcmp(words[0], set_irq_in) == 0 ||
+   strcmp(words[0], set_irq_out) == 0) {
+DeviceState *dev;
+qemu_irq *irqs;
+int num_irqs;
+int num;
+int level;
+
+g_assert(words[1]  words[2]  words[3]);
+
+dev = DEVICE(object_resolve_path(words[1], NULL));
+if (!dev) {
+qtest_send_prefix(chr);
+qtest_send(chr, FAIL Unknown device\n);
+return;
+}
 
+if (words[0][8] == 'o') {
+irqs = dev-gpio_out;
+num_irqs = dev-num_gpio_out;
+} else {
+irqs = dev-gpio_in;
+num_irqs = dev-num_gpio_in;
+}
+
+num = strtol(words[2], NULL, 0);
+if (num  0 || num = num_irqs) {
+qtest_send_prefix(chr);
+qtest_send(chr, FAIL Bad IRQ number\n);
+return;
+}
+
+level = strtol(words[3], NULL, 0);
+
+qemu_set_irq(irqs[num], level != 0);
+qtest_send_prefix(chr);
+qtest_send(chr, OK\n);
 } else if (strcmp(words[0], outb) == 0 ||
strcmp(words[0], outw) == 0 ||
strcmp(words[0], outl) == 0) {
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71b84c1..f6160ad 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -379,6 +379,18 @@ void qtest_irq_intercept_in(QTestState *s, const char 
*qom_path)
 qtest_rsp(s, 0);
 }
 
+void qtest_set_irq_in(QTestState *s, const char *qom_path, int num, int level)
+{
+qtest_sendf(s, set_irq_in %s %d %d\n, qom_path, num, level);
+qtest_rsp(s, 0);
+}
+
+void qtest_set_irq_out(QTestState *s, const char *qom_path, int num, int level)
+{
+qtest_sendf(s, set_irq_out %s %d %d\n, qom_path, num, level);
+qtest_rsp(s, 0);
+}
+
 static void qtest_out(QTestState *s, const char *cmd, uint16_t addr, uint32_t 
value)
 {
 qtest_sendf(s, %s 0x%x 0x%x\n, cmd, addr, value);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index c8ade85..a045fc7 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -76,6 +76,30 @@ void qtest_irq_intercept_in(QTestState *s, const char 
*string);
 void qtest_irq_intercept_out(QTestState *s, const char *string);
 
 /**
+ * qtest_set_irq_in:
+ * @s: QTestState instance to operate on.
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-in pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+void qtest_set_irq_in(QTestState *s, const char *string, int irq, int level);
+
+/**
+ * qtest_set_irq_out:
+ * @s: QTestState instance to operate on.
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-out pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+void

Re: [Qemu-devel] [PATCH v7 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

On Tue, Dec 11, 2012 at 11:20:05AM +, Jamie Lokier wrote:
 Matthew Ogilvie wrote:
  2. Just fix it immediately, and don't worry about migration.  Squash
 the last few patches together.  A single missed periodic
 timer tick that only happens when migrating
 between versions of qemu is probably not a significant
 concern.  (Unless someone knows of an OS that actually runs
 the i8254 in single shot mode 4, where a missed interrupt
 could cause a hang or something?)
 
 Hi Matthew,
 
 Such as Linux?  0x38 looks like mode 4 to me.  I suspect it's used in
 tickless mode when there isn't a better clock event source.
 
 linux/drivers/clocksource/i8253.c:
 
 #ifdef CONFIG_CLKEVT_I8253
/* ... */
 
 case CLOCK_EVT_MODE_ONESHOT:
 /* One shot setup */
 outb_p(0x38, PIT_MODE);
 
/* ... */

I'm not very familiar with how Linux selects and uses timers, but
I think the full qemu fix will only lose a mode 4 interrupt during
migration if several conditions are all met:

  1. Guest's kernel is configured to be tickless.
  2. Guest's kernel is using the 8254, and not something better
 like the HPET.  (Perhaps the better device is configured out
 of the kernel, and/or explicitly left out of qemu's list of
 devices.  Both of which I would expect to be rare.)
  3. The user is migrating from a newer/fixed version of qemu, back
 to an older version of qemu.  (Not sure how common this is.  It
 may be common in some kind of cluster environment where you
 take down one real machine at a time for upgrades and one
 of the upgraded machines needs to be taken back down, or if you
 migrate between completely different data centers that are on
 different upgrade cyles, or something.)

If any of these three conditions is NOT met, then I don't expect it
to be possible to lose an interrupt in mode 4.  Perhaps the combination
can be considered rare enough not to worry about?

It may generate a immediate interrupt (instead of delayed
properly) if the guest resets the one-shot mode due to some
other device's interrupt (fixed normal high vs old normal low),
but hopefully the guest won't be bothered too much by a single
such interrupt.

Also, for guests that use periodic modes (2 or 3), it is still possible
it might gain or lose 1 periodic timer interrupt during migration
between versions of qemu, but that probably isn't a big deal.

NOTE: I'm assumming that the HPET qemu model doesn't have similar trailing
edge or off-by-one-tick problems as the 8254 model.  Nor any other
device, for that matter.

If such breakage is considered minor enough and/or rare enough, then
I certainly think the simplicity of this option is attractive.

 
  4. Support both old and fixed i8254 models, selectable at runtime
 with a command line option.  (Question: What should such an
 option look like?)  This may be the best way to actually
 change the 8254, but I'm not sure changes are even needed.
 It's certainly getting rather far afield from running Microport
 UNIX...
 
 I can't see a reason to have the old behaviour, if every guest works
 with the new one, except for this awkward cross-version migration
 thing.

Yes, the only reason to even consider having both old and new behavior
around is if the migration thing is considered significant enough to
warrant it.

One argument for making it configurable is if the user has an environment
where you expect to occasionally migrate back to old versions, then you
might want to start with the old model even if you initially boot
the guest on a new version qemu.

 
 I guess ideally, device emulations would be versioned when their
 behaviour changes, rather like shared libraries are, and the
 appropriate old version kept around to be loaded for a particular
 machine that's still running with it.  Sounds a bit complicated though.
 
 Best,
 -- Jamie

Re: [Qemu-devel] [PATCH v7 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

On Tue, Dec 11, 2012 at 06:19:56PM +0200, Gleb Natapov wrote:
 On Sun, Nov 25, 2012 at 02:51:36PM -0700, Matthew Ogilvie wrote:
  
  Still needed:
  
* Corresponding KVM patches.  The best approach may depend
  on what option is selected for qemu above.
   * Note that KVM uses a simplified model that doesn't try
 to emulate the trailing edge of the interrupt very well
 at all.  I'm not proposing to change this aspect of it.
   * A patch analogous to 7 should be easy.
   * Patches 8 through 10 are also fairly easy by themselves.
 But now we start having an explosion of combinations
 of versions of KVM and qemu and migration to/from, and it
 might be better to:
 Why explosion of combinations? I do not see any changes in
 migration code in your series, so as long as we care about migration
 from in-kernel irqchip to in-kernel irqchip we should be independent
 from qemu version, no?

You may be correct.  I'm a little hazy on the details of how things are
split between KVM and QEMU.  Are there situations that do ioport read/write
handling within qemu rather than KVM?  How about things like pit_get_out(),
pit_get_next_transition_time(), etc in qemu/hw/i8254_common.c?  (If
not used when KVM is enabled, then why are they common?)  What
are the implications if qemu and KVM implementations of such
functions disagree?

 
   * Or more involved fixes would involve new ioctl()'s and
 command line arguments to select old or fixed 8254 models
 dynamically.  See below.
  
  
  Alternative options for improving the i8254 model and migration:
  
  1. Don't fix 8254 at all.  Just apply through patch 7 or 8, and don't try
 to make any additional fixes.  I don't know of any guests that need
 improvements, so this could be a viable option.
  
  2. Just fix it immediately, and don't worry about migration.  Squash
 the last few patches together.  A single missed periodic
 timer tick that only happens when migrating
 between versions of qemu is probably not a significant
 concern.  (Unless someone knows of an OS that actually runs
 the i8254 in single shot mode 4, where a missed interrupt
 could cause a hang or something?)
  
 If migration can fail only with the single, rarely (if ever) used mode,
 I honestly like this option the most.

As long as it is truly rare, I agree.  I'm just not sure if the rare
qualification is actually true or not.  See also my response to Jamie
Lokier about Linux's tickless configuration.

 
  3. Use patches 8 and 9 now, and patch 10 sometime in the future.
 If it was just qemu, this would be attractive.  But when you
 also need to worry about a bunch of combinations of versions of
 qemu and KVM and migration, this is looking less attractive.
  
  4. Support both old and fixed i8254 models, selectable at runtime
 with a command line option.  (Question: What should such an
 option look like?)  This may be the best way to actually
 change the 8254, but I'm not sure changes are even needed.
 It's certainly getting rather far afield from running Microport
 UNIX...
 If we will start doing it for each bug...
 

Re: [Qemu-devel] [PATCH v7 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

On Mon, Dec 10, 2012 at 10:47:59AM -0600, Anthony Liguori wrote:
 Jan Kiszka jan.kis...@web.de writes:
 
  On 2012-12-10 06:14, Matthew Ogilvie wrote:
  On Sun, Nov 25, 2012 at 02:51:36PM -0700, Matthew Ogilvie wrote:
  This series makes a series of mostly-unrelated fixes to allow
  running an old Microport UNIX (ca 1987) guest under qemu.
 
  Changes since version 6:
 * Patches 1 through 6 haven't changed, other than resolving
   a couple of simple conflicts.
 * Patch 7 fixes IRQ0 by just making it work like before,
   rather than fixing it properly.  This avoids possible risk
   to cross-version migration, etc.
 * Patches 8, 9, and 10 provide one possible gradual transition path
   to properly fix the 8254 model with relatively little risk to
   migration/etc.  The idea is that 8 and 9 could be applied
   immediately in preparation for a future fix, and then the
   actual fix (10) could be applied sometime in the future when
   migrating to or from pre-patch-9 versions is no longer a concern.
  I am not actually aware of ANY guest that actually needs
   an improved 8254 model, but this provides one way to improve
   it if desired.
 
  
  Ping?
  
  What would it take to get some variation of this series
  into 1.4?  The last feedback I've seen was against version 5, back
  in September.
  http://search.gmane.org/?query=ogilviegroup=gmane.comp.emulators.qemu
 
  I suppose it's primarily a question of time for some reviewer(s). Sorry,
  I wasn't able to look at it yet, maybe I will have a chance next week.
 
 If you added a test case for the i8254 using the mc146818rtc qtest test
 case as an example, you would very likely attract more reviewers.
 
 It would also make it easier to ensure that the issues you're fixing
 here don't regress in the future too.
 
 Regards,
 
 Anthony Liguori

I'll look into adding some qtest test case(s), starting along the same
lines as the i8259 kvm-unit-tests case I posted Sep 9, 2012, (and the
standalone test it was based on).  See also
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2

Note that for the i8259/i8254 stuff, strictly speaking
I only really need a fix to the trailing-edge handling of the
cascade (IRQ2) line of the i8259.  A more general fix (all IRQ lines)
might be nice, but such a general fix (especially IRQ0) exposes
bugs in the i8254 model.  And fixing that poses at least a
small risk to cross-version guest migration.

All of which raises the strategic question of how much scope creap of
fixing/changing low level device models should I worry about, to
address a problem only seen in a very rare guest?  Vs some kind
of narrower, non-general fix (IRQ2 only, or all-except-IRQ0, or
something else).

Also, there is a CGA compatibility hack I need.  As well as
three trivial patches that I don't actually need, but seem
like easy fixes.

 
 
  
  
  Split up this series?
 
  I'm not sure what the next steps are to get these into qemu, other
  than waiting for 1.4 for at least the non-trivial parts?
 
  Patches 1 through 3 could be considered independent trivial patches.
  Would splitting them apart improve the changes of getting them into qemu?
 
  Patch 4 isn't quite trivial, but it is well isolated (other than
  small documentation conflicts against patch 3).  Should it be split
  off?  It hasn't changed since version 3, but nobody has really
  commented on it.
 
  Patches 5 through 10 are interrelated, and should remain related in
  a series.
 
  
  Still needed:
 
* Corresponding KVM patches.  The best approach may depend
  on what option is selected for qemu above.
   * Note that KVM uses a simplified model that doesn't try
 to emulate the trailing edge of the interrupt very well
 at all.  I'm not proposing to change this aspect of it.
   * A patch analogous to 7 should be easy.
   * Patches 8 through 10 are also fairly easy by themselves.
 But now we start having an explosion of combinations
 of versions of KVM and qemu and migration to/from, and it
 might be better to:
   * Or more involved fixes would involve new ioctl()'s and
 command line arguments to select old or fixed 8254 models
 dynamically.  See below.
  
  Any preferences?
 
  As Avi left, I'm putting Gleb and Marcelo on CC.
 
  
 
  
  Alternative options for improving the i8254 model and migration:
 
  1. Don't fix 8254 at all.  Just apply through patch 7 or 8, and don't try
 to make any additional fixes.  I don't know of any guests that need
 improvements, so this could be a viable option.
  
  Or:
  1.1. Don't fix any 8259 lines either, except for the one line (IRQ2) that
   is giving me trouble.  (Recall that the original problem is the guest
   masking off IRQ14 in the 8259, and the resulting IRQ2 trailing edge
   isn't handled correctly in the master 8259

Re: [Qemu-devel] [PATCH v7 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

On Sun, Nov 25, 2012 at 02:51:36PM -0700, Matthew Ogilvie wrote:
 This series makes a series of mostly-unrelated fixes to allow
 running an old Microport UNIX (ca 1987) guest under qemu.
 
 Changes since version 6:
* Patches 1 through 6 haven't changed, other than resolving
  a couple of simple conflicts.
* Patch 7 fixes IRQ0 by just making it work like before,
  rather than fixing it properly.  This avoids possible risk
  to cross-version migration, etc.
* Patches 8, 9, and 10 provide one possible gradual transition path
  to properly fix the 8254 model with relatively little risk to
  migration/etc.  The idea is that 8 and 9 could be applied
  immediately in preparation for a future fix, and then the
  actual fix (10) could be applied sometime in the future when
  migrating to or from pre-patch-9 versions is no longer a concern.
 I am not actually aware of ANY guest that actually needs
  an improved 8254 model, but this provides one way to improve
  it if desired.
 

Ping?

What would it take to get some variation of this series
into 1.4?  The last feedback I've seen was against version 5, back
in September.
http://search.gmane.org/?query=ogilviegroup=gmane.comp.emulators.qemu

 
 Split up this series?
 
 I'm not sure what the next steps are to get these into qemu, other
 than waiting for 1.4 for at least the non-trivial parts?
 
 Patches 1 through 3 could be considered independent trivial patches.
 Would splitting them apart improve the changes of getting them into qemu?
 
 Patch 4 isn't quite trivial, but it is well isolated (other than
 small documentation conflicts against patch 3).  Should it be split
 off?  It hasn't changed since version 3, but nobody has really
 commented on it.
 
 Patches 5 through 10 are interrelated, and should remain related in
 a series.
 
 
 Still needed:
 
   * Corresponding KVM patches.  The best approach may depend
 on what option is selected for qemu above.
  * Note that KVM uses a simplified model that doesn't try
to emulate the trailing edge of the interrupt very well
at all.  I'm not proposing to change this aspect of it.
  * A patch analogous to 7 should be easy.
  * Patches 8 through 10 are also fairly easy by themselves.
But now we start having an explosion of combinations
of versions of KVM and qemu and migration to/from, and it
might be better to:
  * Or more involved fixes would involve new ioctl()'s and
command line arguments to select old or fixed 8254 models
dynamically.  See below.

Any preferences?

 
 
 Alternative options for improving the i8254 model and migration:
 
 1. Don't fix 8254 at all.  Just apply through patch 7 or 8, and don't try
to make any additional fixes.  I don't know of any guests that need
improvements, so this could be a viable option.

Or:
1.1. Don't fix any 8259 lines either, except for the one line (IRQ2) that
 is giving me trouble.  (Recall that the original problem is the guest
 masking off IRQ14 in the 8259, and the resulting IRQ2 trailing edge
 isn't handled correctly in the master 8259, resulting in a
 spurious interrupt.)

 
 2. Just fix it immediately, and don't worry about migration.  Squash
the last few patches together.  A single missed periodic
timer tick that only happens when migrating
between versions of qemu is probably not a significant
concern.  (Unless someone knows of an OS that actually runs
the i8254 in single shot mode 4, where a missed interrupt
could cause a hang or something?)
 
 3. Use patches 8 and 9 now, and patch 10 sometime in the future.
If it was just qemu, this would be attractive.  But when you
also need to worry about a bunch of combinations of versions of
qemu and KVM and migration, this is looking less attractive.
 
 4. Support both old and fixed i8254 models, selectable at runtime
with a command line option.  (Question: What should such an
option look like?)  This may be the best way to actually
change the 8254, but I'm not sure changes are even needed.
It's certainly getting rather far afield from running Microport
UNIX...
 
 
 
 Matthew Ogilvie (10):
   fix some debug printf format strings
   vl: fix -hdachs/-hda argument order parsing issues
   qemu-options.hx: mention retrace= VGA option
   vga: add some optional CGA compatibility hacks
   i8259: fix so that dropping IRQ level always clears the interrupt
 request
   i8259: refactor pic_set_irq level logic
   i8254/i8259: workaround to make IRQ0 work like before
   i8254: add comments about fixing timings
   i8254: prepare for migration compatibility with future fixes
   FOR FUTURE: fix i8254/i8259 IRQ0 line logic

[Qemu-devel] [PATCH v7 02/10] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index c8e9c78..c6c7d95 100644
--- a/vl.c
+++ b/vl.c
@@ -2533,8 +2533,9 @@ int main(int argc, char **argv, char **envp)
 const char *kernel_filename, *kernel_cmdline;
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2589,8 +2590,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2638,7 +2638,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2661,21 +2661,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2709,7 +2696,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2741,7 +2731,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v7 03/10] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 qemu-options.hx | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 9bb29d3..ebc138d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1007,7 +1007,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -1032,6 +1032,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v7 04/10] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/pc.h |  4 
 hw/vga.c| 37 +
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index e7993ca..d033ee3 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -149,6 +149,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 int isa_vga_mm_init(hwaddr vram_base,
 hwaddr ctrl_base, int it_shift,
 MemoryRegion *address_space);
diff --git a/hw/vga.c b/hw/vga.c
index 2b0200a..660961b 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -552,14 +552,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xd;
+} else if (s-cr_index == VGA_CRTC_CURSOR_END 
+   val == 7 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xe;
+}
 }
-return;
 }
 s-cr[s-cr_index] = val;
 
@@ -1896,7 +1913,11 @@ static void vga_update_display(void *opaque)
 /* nothing to do */
 } else {
 full_update = 0;
-if (!(s-ar_index  0x20)) {
+if (!(s-ar_index  0x20) 
+/* extra CGA compatibility hacks (not in standard VGA) */
+(!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index ebc138d..7d6fc72 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1037,6 +1037,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default for CGA fonts
+instead of VGA

[Qemu-devel] [PATCH v7 09/10] i8254: prepare for migration compatibility with future fixes

[Matthew Ogilvie]

Under normal operation this patch should have no effect.  But
it adds some redundant (no-change) calls to qemu_set_irq(),
so that if a running host image is migrated from a future version
of qemu that has various fixes to the i8254 output line logic,
this version can still deliver exactly the correct number of
IRQ0 leading edges to the i8259 at as close as possible to the
correct time.

The plan is to incorporate this now, and then much later (years?)
we can implement the actual fixes to the i8254, after migration to/from
qemu versions without this transitional fix is no longer a concern.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Alternatives:

An alternative for mode 2 might involve tweaking
pit_get_next_transition_time() to create an extra pseudo-transition
at the same clock tick that will eventually have the real transition
(so it has 3 transitions per period).  But it may be best to keep
the migration hacks together in one place.

Or support both the old model and a new/fixed model, selectable at
runtime by command line switch.

 hw/i8254.c | 85 --
 1 file changed, 78 insertions(+), 7 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index 982e8e7..ffe6e27 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -35,7 +35,8 @@
 #define RW_STATE_WORD0 3
 #define RW_STATE_WORD1 4
 
-static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
+static void pit_irq_timer_update(PITChannelState *s, int64_t current_time,
+ bool edge);
 
 static int pit_get_count(PITChannelState *s)
 {
@@ -90,7 +91,7 @@ static void pit_set_channel_gate(PITCommonState *s, 
PITChannelState *sc,
 if (sc-gate  val) {
 /* restart counting on rising edge */
 sc-count_load_time = qemu_get_clock_ns(vm_clock);
-pit_irq_timer_update(sc, sc-count_load_time);
+pit_irq_timer_update(sc, sc-count_load_time, false);
 }
 break;
 case 2:
@@ -98,7 +99,7 @@ static void pit_set_channel_gate(PITCommonState *s, 
PITChannelState *sc,
 if (sc-gate  val) {
 /* restart counting on rising edge */
 sc-count_load_time = qemu_get_clock_ns(vm_clock);
-pit_irq_timer_update(sc, sc-count_load_time);
+pit_irq_timer_update(sc, sc-count_load_time, false);
 }
 /* XXX: disable/enable counting */
 break;
@@ -128,7 +129,7 @@ static inline void pit_load_count(PITChannelState *s, int 
val)
  */
 
 s-count = val;
-pit_irq_timer_update(s, s-count_load_time);
+pit_irq_timer_update(s, s-count_load_time, false);
 }
 
 /* if already latched, do not latch again */
@@ -262,7 +263,8 @@ static uint64_t pit_ioport_read(void *opaque, hwaddr addr,
 return ret;
 }
 
-static void pit_irq_timer_update(PITChannelState *s, int64_t current_time)
+static void pit_irq_timer_update(PITChannelState *s, int64_t current_time,
+ bool edge)
 {
 int64_t expire_time;
 int irq_level;
@@ -272,6 +274,75 @@ static void pit_irq_timer_update(PITChannelState *s, 
int64_t current_time)
 }
 expire_time = pit_get_next_transition_time(s, current_time);
 irq_level = pit_get_out(s, current_time);
+
+/* FIXME i8254_timing_fixes: Fixing pic_get_out() timings needs
+ *  to happen in stages.  For now, ensure that exactly one leading
+ *  edge is detected in the 8259 somewhere near counter expiration,
+ *  even if migrating to/from a future version that has corrected
+ *  (different) IRQ0 transitions from pic_get_out().
+ *
+ *  Quick description of how this works for various modes when
+ *  migrating between old and new versions.  To understand this,
+ *  it can help to draw out a timing diagram showing both this/old and
+ *  future version IRQ0 levels over time.
+ * mode 0: no change.
+ * mode 2: The leading edge stays at the same counter value,
+ * but the timing of the trailing edge moves
+ * from one CLK tick after the leading edge (this/old
+ * version) to one CLK tick before the next trailing
+ * edge (future version).  Migration cases:
+ *   - This high IRQ0 migrated to future version: Nothing special;
+ * always correct.
+ *   - This low IRQ0 migrated to future version: May be set low
+ * again a second time (noop) when counter gets to 1, but
+ * otherwise nothing special.
+ *   - Future low IRQ0 migrated to this/old version: Nothing special;
+ * always correct.
+ *   - Future high migrated to this/old version: Do a
+ * high-low-high sequence when the next leading edge is needed.
+ * The sequence may simplify to noop-low-high in some cases
+ * depending on if migrates less than a tick since it went
+ * high (so

[Qemu-devel] [PATCH v7 10/10] FOR FUTURE: fix i8254/i8259 IRQ0 line logic

[Matthew Ogilvie]

This patch fixes i8254 output line logic to match the spec, and
eliminates compatibility logic that was added as a transition
strategy for fixing the i8254 without breaking migration.

Tentatively, the idea is to apply this patch sometime in the
distant future (years from now?) after versions of qemu that this
can't migrate to/from reliably are no longer in use.  See the
previous patch for the preparatory support for this patch.

This patch probably shouldn't go in now, unless it is determined
that the migration concerns are overblown - in which case it should
probably be squashed in with earlier patches instead of kept separate.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8254.c| 106 +++---
 hw/i8254_common.c |  58 --
 hw/i8259.c|  19 +-
 3 files changed, 20 insertions(+), 163 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index ffe6e27..edb5b7a 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -35,8 +35,7 @@
 #define RW_STATE_WORD0 3
 #define RW_STATE_WORD1 4
 
-static void pit_irq_timer_update(PITChannelState *s, int64_t current_time,
- bool edge);
+static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
 
 static int pit_get_count(PITChannelState *s)
 {
@@ -62,12 +61,7 @@ static int pit_get_count(PITChannelState *s)
 counter = (s-count - d)  0x;
 break;
 case 3:
-/* XXX: may be incorrect for odd counts
- * FIXME i8254_timing_fixes: fix this by changing it to something
- * like:
- *   counter = (s-count - ((2 * d) % s-count))  0xfffe;
- */
-counter = s-count - ((2 * d) % s-count);
+counter = (s-count - ((2 * d) % s-count))  0xfffe;
 break;
 default:
 counter = s-count - (d % s-count);
@@ -91,7 +85,7 @@ static void pit_set_channel_gate(PITCommonState *s, 
PITChannelState *sc,
 if (sc-gate  val) {
 /* restart counting on rising edge */
 sc-count_load_time = qemu_get_clock_ns(vm_clock);
-pit_irq_timer_update(sc, sc-count_load_time, false);
+pit_irq_timer_update(sc, sc-count_load_time);
 }
 break;
 case 2:
@@ -99,7 +93,7 @@ static void pit_set_channel_gate(PITCommonState *s, 
PITChannelState *sc,
 if (sc-gate  val) {
 /* restart counting on rising edge */
 sc-count_load_time = qemu_get_clock_ns(vm_clock);
-pit_irq_timer_update(sc, sc-count_load_time, false);
+pit_irq_timer_update(sc, sc-count_load_time);
 }
 /* XXX: disable/enable counting */
 break;
@@ -113,23 +107,19 @@ static inline void pit_load_count(PITChannelState *s, int 
val)
 val = 0x1;
 s-count_load_time = qemu_get_clock_ns(vm_clock);
 
-/* FIXME i8254_timing_fixes:
- * In gate-triggered one-shot modes, indirectly model some pit_get_out()
- * cases by setting the load time way back until gate-triggered,
- * using code such as:
- *
- * if (s-mode == 1 || s-mode == 5)
- * s-count_load_time -= muldiv64(val+2, get_ticks_per_sec(), 
PIT_FREQ);
- *
+/* In gate-triggered one-shot modes, indirectly model some pit_get_out()
+ * cases by setting the load time way back until gate-triggered.
  * (Generally only affects reading status from channel 2 speaker,
  * due to hard-wired gates on other channels.)
  *
- * FIXME Or this might be redesigned if a paused timer state is added
+ * FIXME: This might be redesigned if a paused timer state is added
  * for pic_get_count().
  */
+if (s-mode == 1 || s-mode == 5)
+s-count_load_time -= muldiv64(val+2, get_ticks_per_sec(), PIT_FREQ);
 
 s-count = val;
-pit_irq_timer_update(s, s-count_load_time, false);
+pit_irq_timer_update(s, s-count_load_time);
 }
 
 /* if already latched, do not latch again */
@@ -263,8 +253,7 @@ static uint64_t pit_ioport_read(void *opaque, hwaddr addr,
 return ret;
 }
 
-static void pit_irq_timer_update(PITChannelState *s, int64_t current_time,
- bool edge)
+static void pit_irq_timer_update(PITChannelState *s, int64_t current_time)
 {
 int64_t expire_time;
 int irq_level;
@@ -274,75 +263,6 @@ static void pit_irq_timer_update(PITChannelState *s, 
int64_t current_time,
 }
 expire_time = pit_get_next_transition_time(s, current_time);
 irq_level = pit_get_out(s, current_time);
-
-/* FIXME i8254_timing_fixes: Fixing pic_get_out() timings needs
- *  to happen in stages.  For now, ensure that exactly one leading
- *  edge is detected in the 8259 somewhere near counter expiration,
- *  even if migrating to/from a future version that has corrected
- *  (different) IRQ0 transitions from pic_get_out().
- *
- *  Quick description of how this works for various modes when
- *  migrating

[Qemu-devel] [PATCH v7 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

This series makes a series of mostly-unrelated fixes to allow
running an old Microport UNIX (ca 1987) guest under qemu.

Changes since version 6:
   * Patches 1 through 6 haven't changed, other than resolving
 a couple of simple conflicts.
   * Patch 7 fixes IRQ0 by just making it work like before,
 rather than fixing it properly.  This avoids possible risk
 to cross-version migration, etc.
   * Patches 8, 9, and 10 provide one possible gradual transition path
 to properly fix the 8254 model with relatively little risk to
 migration/etc.  The idea is that 8 and 9 could be applied
 immediately in preparation for a future fix, and then the
 actual fix (10) could be applied sometime in the future when
 migrating to or from pre-patch-9 versions is no longer a concern.
I am not actually aware of ANY guest that actually needs
 an improved 8254 model, but this provides one way to improve
 it if desired.


Split up this series?

I'm not sure what the next steps are to get these into qemu, other
than waiting for 1.4 for at least the non-trivial parts?

Patches 1 through 3 could be considered independent trivial patches.
Would splitting them apart improve the changes of getting them into qemu?

Patch 4 isn't quite trivial, but it is well isolated (other than
small documentation conflicts against patch 3).  Should it be split
off?  It hasn't changed since version 3, but nobody has really
commented on it.

Patches 5 through 10 are interrelated, and should remain related in
a series.


Still needed:

  * Corresponding KVM patches.  The best approach may depend
on what option is selected for qemu above.
 * Note that KVM uses a simplified model that doesn't try
   to emulate the trailing edge of the interrupt very well
   at all.  I'm not proposing to change this aspect of it.
 * A patch analogous to 7 should be easy.
 * Patches 8 through 10 are also fairly easy by themselves.
   But now we start having an explosion of combinations
   of versions of KVM and qemu and migration to/from, and it
   might be better to:
 * Or more involved fixes would involve new ioctl()'s and
   command line arguments to select old or fixed 8254 models
   dynamically.  See below.


Alternative options for improving the i8254 model and migration:

1. Don't fix 8254 at all.  Just apply through patch 7 or 8, and don't try
   to make any additional fixes.  I don't know of any guests that need
   improvements, so this could be a viable option.

2. Just fix it immediately, and don't worry about migration.  Squash
   the last few patches together.  A single missed periodic
   timer tick that only happens when migrating
   between versions of qemu is probably not a significant
   concern.  (Unless someone knows of an OS that actually runs
   the i8254 in single shot mode 4, where a missed interrupt
   could cause a hang or something?)

3. Use patches 8 and 9 now, and patch 10 sometime in the future.
   If it was just qemu, this would be attractive.  But when you
   also need to worry about a bunch of combinations of versions of
   qemu and KVM and migration, this is looking less attractive.

4. Support both old and fixed i8254 models, selectable at runtime
   with a command line option.  (Question: What should such an
   option look like?)  This may be the best way to actually
   change the 8254, but I'm not sure changes are even needed.
   It's certainly getting rather far afield from running Microport
   UNIX...



Matthew Ogilvie (10):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  i8259: fix so that dropping IRQ level always clears the interrupt
request
  i8259: refactor pic_set_irq level logic
  i8254/i8259: workaround to make IRQ0 work like before
  i8254: add comments about fixing timings
  i8254: prepare for migration compatibility with future fixes
  FOR FUTURE: fix i8254/i8259 IRQ0 line logic

 hw/cirrus_vga.c   |  4 ++--
 hw/i8254.c| 24 +++--
 hw/i8254_common.c | 18 ++--
 hw/i8259.c| 28 ++---
 hw/ide/cmd646.c   |  5 +++--
 hw/ide/via.c  |  5 +++--
 hw/pc.h   |  4 
 hw/vga.c  | 37 ++---
 qemu-options.hx   | 27 +++-
 vl.c  | 62 ---
 10 files changed, 147 insertions(+), 67 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v7 05/10] i8259: fix so that dropping IRQ level always clears the interrupt request

[Matthew Ogilvie]

Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered and
then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked kernel.

Output from a test program:
---
Real hardware [Pentium 4]:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
[I also see a final IRR= occasionally.  Probably just happened to
ask it while the timer (IRQ0) line is low.  It masks off most IRQ's, including
timer.]
---
Unpatched qemu:
  cmdRead unmask IRR=4015 mask IRR=4015 sti irq15 unmask IRR=4015 DONE
[Presumably IRQ4 (0x10 - qemu's serial device model?) had a transient
edge during initialization, but had been masked off even before I
masked it off?]
---
Patched qemu:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
---

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 60c25ba..95587cd 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -157,6 +157,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 s-last_irr |= mask;
 } else {
+s-irr = ~mask;
 s-last_irr = ~mask;
 }
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v7 07/10] i8254/i8259: workaround to make IRQ0 work like before

[Matthew Ogilvie]

Someday it should be fixed properly, but doing so may
break migration.  So go with an incremental approach instead.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index 9b2ec40..71cc09a 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -150,8 +150,25 @@ static void pic_set_irq(void *opaque, int irq, int level)
 /* Dropping level clears the interrupt regardless of edge trigger
  * vs level trigger.
  */
-s-irr = ~mask;
 s-last_irr = ~mask;
+
+/* Migration compatibility hack:
+ *
+ * The i8254 timer model is wrong in a number of ways,
+ * including lowering IRQ0 much earlier than it should.
+ *
+ * FIXME i8254_timing_fixes: Eventually the i8254
+ *  should be fixed, but it isn't
+ *  trivial to do so in a way that avoids possible problems with
+ *  migration (lost or gained timer ticks).  So for now, make the
+ *  i8254 work the same way that it worked in qemu 1.2, and
+ *  leave irr for IRQ0 alone in the i8259 here:
+ */
+if (irq == 0  s-master) {
+mask = 0;
+}
+
+s-irr = ~mask;
 }
 pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v7 08/10] i8254: add comments about fixing timings

[Matthew Ogilvie]

There may be risk of problems with migration if these are just
fixed blindly, but at least comment what it ought to be changed to.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8254.c| 31 ++-
 hw/i8254_common.c | 40 
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index bea5f92..982e8e7 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -39,6 +39,15 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t 
current_time);
 
 static int pit_get_count(PITChannelState *s)
 {
+/* FIXME: Add some way to represent a paused timer and return
+ *   the paused-at counter value, to better model:
+ * - gate-triggered modes (1 and 5),
+ * - gate-pausable modes,
+ * - [maybe] the wait until next CLK pulse to load counter logic,
+ * - [maybe/not clear] half-loaded counter logic for mode 0, and
+ *   the null count status bit,
+ * - etc.
+ */
 uint64_t d;
 int counter;
 
@@ -52,7 +61,11 @@ static int pit_get_count(PITChannelState *s)
 counter = (s-count - d)  0x;
 break;
 case 3:
-/* XXX: may be incorrect for odd counts */
+/* XXX: may be incorrect for odd counts
+ * FIXME i8254_timing_fixes: fix this by changing it to something
+ * like:
+ *   counter = (s-count - ((2 * d) % s-count))  0xfffe;
+ */
 counter = s-count - ((2 * d) % s-count);
 break;
 default:
@@ -98,6 +111,22 @@ static inline void pit_load_count(PITChannelState *s, int 
val)
 if (val == 0)
 val = 0x1;
 s-count_load_time = qemu_get_clock_ns(vm_clock);
+
+/* FIXME i8254_timing_fixes:
+ * In gate-triggered one-shot modes, indirectly model some pit_get_out()
+ * cases by setting the load time way back until gate-triggered,
+ * using code such as:
+ *
+ * if (s-mode == 1 || s-mode == 5)
+ * s-count_load_time -= muldiv64(val+2, get_ticks_per_sec(), 
PIT_FREQ);
+ *
+ * (Generally only affects reading status from channel 2 speaker,
+ * due to hard-wired gates on other channels.)
+ *
+ * FIXME Or this might be redesigned if a paused timer state is added
+ * for pic_get_count().
+ */
+
 s-count = val;
 pit_irq_timer_update(s, s-count_load_time);
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index a03d7cd..33f352f 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -53,9 +53,30 @@ int pit_get_out(PITChannelState *s, int64_t current_time)
 out = (d = s-count);
 break;
 case 1:
+/* FIXME i8254_timing_fixes: There are various problems
+ *  with qemu's 8254 model (especially when IRQ0's trailing
+ *  edge occurs), but fixing them directly might cause
+ *  problems with migration.  So just comment the fixes for
+ *  now.
+ *(Based on reading timing diagrams in Intel's 8254 spec
+ *  sheet, downloadable from
+ *  http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
+ *  or other places.)
+ * FIXME i8254_timing_fixes: Both mode 0 and mode 1 should use
+ *out = (d = s-count);
+ *  So mode 0 can fall-through into a fixed mode 1.
+ *  The difference between modes 0 and 1 is in the gate triggering.
+ *  See also an adjustment to make to count_load_time
+ *  when count is loaded for mode 1.
+ */
 out = (d  s-count);
 break;
 case 2:
+/* FIXME i8254_timing_fixes: This should simply be:
+ *out = (d % s-count) != (s-count - 1) || s-gate == 0;
+ *  Gate is hard-wired 1 for channel 0 (IRQ0), but it can be
+ *  adjusted in software for channel 2 (PC speaker).
+ */
 if ((d % s-count) == 0  d != 0) {
 out = 1;
 } else {
@@ -63,10 +84,21 @@ int pit_get_out(PITChannelState *s, int64_t current_time)
 }
 break;
 case 3:
+/* FIXME i8254_timing_fixes: This should be:
+ *out = (d % s-count)  ((s-count + 1)  1) || s-gate == 0;
+ *  Gate is hard-wired 1 for channel 0 (IRQ0), but it can be
+ *  adjusted in software for channel 2 (PC speaker).
+ */
 out = (d % s-count)  ((s-count + 1)  1);
 break;
 case 4:
 case 5:
+/* FIXME i8254_timing_fixes: The logic is backwards, and should be:
+ *out = (d != s-count);
+ *  The difference between modes 4 and 5 is in the gate triggering.
+ *  See also an adjustment to make to count_load_time
+ *  when count is loaded for mode 5.
+ */
 out = (d == s-count);
 break;
 }
@@ -93,6 +125,14 @@ int64_t pit_get_next_transition_time(PITChannelState *s, 
int64_t current_time)
 break;
 case 2:
 base = (d / s-count) * s-count;
+/* FIXME

[Qemu-devel] [PATCH v7 01/10] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 9bef96e..da60fc3 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2052,8 +2052,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index af0ba4d..60c25ba 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 804db60..72ceaaf 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index efda173..10ba9b5 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v7 06/10] i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index 95587cd..9b2ec40 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,26 +140,18 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
-if (s-elcr  mask) {
-/* level triggered */
-if (level) {
+if (level) {
+if ((s-last_irr  mask) == 0 ||  /* edge for edge triggered */
+(s-elcr  mask)) {   /* or level triggered */
 s-irr |= mask;
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
 }
+s-last_irr |= mask;
 } else {
-/* edge triggered */
-if (level) {
-if ((s-last_irr  mask) == 0) {
-s-irr |= mask;
-}
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
-}
+/* Dropping level clears the interrupt regardless of edge trigger
+ * vs level trigger.
+ */
+s-irr = ~mask;
+s-last_irr = ~mask;
 }
 pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5

Re: [Qemu-devel] [PATCH v5 5/6] i8259: fix so that dropping IRQ level always clears the interrupt request

[Matthew Ogilvie]

On Mon, Nov 19, 2012 at 04:28:31PM +0100, BALATON Zoltan wrote:
 On Sun, 9 Sep 2012, Matthew Ogilvie wrote:
  Intel's definition of edge triggered means: asserted with a
  low-to-high transition at the time an interrupt is registered and
  then kept high until the interrupt is served via one of the
  EOI mechanisms or goes away unhandled.
 
  So the only difference between edge triggered and level triggered
  is in the leading edge, with no difference in the trailing edge.
 
 What happened to this patch? Any chance it will be in 1.3?
 
 Thanks,
 BALATON Zoltan

I kind of doubt it will make it into 1.3, although I would like to
get it in eventually.  Maybe the first few essentially unrelated
trivial patches can make it in?

Yours is the first comment I've seen since I've posted version 6 of
the series (this particular patch is the same as version 5).  The lack
of feedback combined with other demands on my time have prevented me
from progressing on this for over a month and a half.

Status:

   * We can't completely fix the i8259 model without addressing some
 issues in the i8254 model as well.  The i8254 issues (very short
 duty cycle on IRQ0 causing lost timer ticks) become rather
 severe if you try to fix the i8259 without fixing the i8254.
   * But the obvious direct fixes to the i8254 may risk causing issues with
 breaking migration between pre-fix and post-fix versions of qemu,
 due to lost and/or extra timer interrupts.  I'm not sure how big
 of an issue this is in practice, but it is a concern.
   * I've outlined some possible ways to address the migration issue
 in the cover letter of version 6 of the patch series.
   * Similar issue cascades exist in the in-kernel KVM model as well.
 The i8254 issues are even more severe in KVM: timer interrupts
 are always lost [0-length duty cycle in KVM model], instead of
 sometimes lost [non-0 but tiny duty cycle in qemu model].
   * I'm not aware of any similar problems in other device interrupt
 models, but I haven't really checked them, either.

Next step?:

In the absense of any other suggestions, I'm thinking about rolling
a version of the series that leaves IRQ0 (timer) working the way
it does in qemu 1.2.  Except in the i8254 I'll immediately preceed
each intended to be leading edge transition with an
explicit lowering of IRQ0, so that if migrating from some
future really-fixed version that leaves IRQ0 high most
of the time, it will still recognize the new edge.  A real fix
would then be delayed (years?) until versions without this first
stage fix are no longer in production.  I can probably
get this done this weekend.  [Although I'm not sure how to
deal with the issue that some of the i8254 modes' leading edge
transitions are off by one count.  Perhaps we'll need multiple
intermediate stages, or one of the other strategies outlined in
the version 6 cover letter.]

Or does anyone have a better suggestion?

 - Matthew Ogilvie

[Qemu-devel] [PATCH v6 2/8] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 8d305ca..e1d2f7e 100644
--- a/vl.c
+++ b/vl.c
@@ -2362,8 +2362,9 @@ int main(int argc, char **argv, char **envp)
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
 DisplayChangeListener *dcl;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2418,8 +2419,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2467,7 +2467,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2485,21 +2485,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2533,7 +2520,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2565,7 +2555,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 4/8] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/pc.h |  4 
 hw/vga.c| 37 +
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index e4db071..37e2f87 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -176,6 +176,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 static inline DeviceState *isa_vga_init(ISABus *bus)
 {
 ISADevice *dev;
diff --git a/hw/vga.c b/hw/vga.c
index afaef0d..15b5f0d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -552,14 +552,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xd;
+} else if (s-cr_index == VGA_CRTC_CURSOR_END 
+   val == 7 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xe;
+}
 }
-return;
 }
 s-cr[s-cr_index] = val;
 
@@ -1891,7 +1908,11 @@ static void vga_update_display(void *opaque)
 /* nothing to do */
 } else {
 full_update = 0;
-if (!(s-ar_index  0x20)) {
+if (!(s-ar_index  0x20) 
+/* extra CGA compatibility hacks (not in standard VGA) */
+(!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index d473e1c..5ba9c19 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1005,6 +1005,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default for CGA fonts
+instead of VGA fonts.
+@item all
+Enable all CGA hacks.  More CGA hacks may

[Qemu-devel] [PATCH v6 1/8] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 9a0a565..107254d 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2055,8 +2055,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index 53daf78..6587666 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, 
target_phys_addr_t addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index e0b9443..dd2855e 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t 
addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b20e4f0..948a469 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 6/8] i8259: fix so that dropping IRQ level always clears the interrupt request

[Matthew Ogilvie]

Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered and
then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked kernel.

Output from a test program:
---
Real hardware [Pentium 4]:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
[I also see a final IRR= occasionally.  Probably just happened to
ask it while the timer (IRQ0) line is low.  It masks off most IRQ's, including
timer.]
---
Unpatched qemu:
  cmdRead unmask IRR=4015 mask IRR=4015 sti irq15 unmask IRR=4015 DONE
[Presumably IRQ4 (0x10 - qemu's serial device model?) had a transient
edge during initialization, but had been masked off even before I
masked it off?]
---
Patched qemu:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
---

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 6587666..c011787 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -157,6 +157,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 s-last_irr |= mask;
 } else {
+s-irr = ~mask;
 s-last_irr = ~mask;
 }
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 5/8] i8254: fix inaccuracies in pit_get_out()

[Matthew Ogilvie]

   * Fix high-vs-low counting logic to match the timing diagrams
 and descriptions in Intel's documentation (23124406.pdf).
   * Improve reading back the count in mode 3.
   * Handle GATE input properly with respect to the OUT line, and add
 a FIXME comment for reading back the counter.  GATE is hard wired
 high for channel 0 (IRQ0), but it can be software controlled on
 channel 2 (PC speaker).

The output line is now high much more often than before, especially
in modes 2 and 4, which might cause updates of the timer chip
to generate extra interrupts.  But this should only be an issue for
some migration cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

There are still some things not quite modeled correctly.
See the cover letter of this patch series for details, and
the FIXME comments added to the code.

 hw/i8254.c| 24 ++--
 hw/i8254_common.c | 18 ++
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index 77bd5e8..9168016 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -39,6 +39,15 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t 
current_time);
 
 static int pit_get_count(PITChannelState *s)
 {
+/* FIXME: Add some way to represent a paused timer and return
+ *   the paused-at counter value, to better model:
+ * - gate-triggered modes (1 and 5),
+ * - gate-pausable modes,
+ * - [maybe] the wait until next CLK pulse to load counter logic,
+ * - [maybe/not clear] half-loaded counter logic for mode 0, and
+ *   the null count status bit,
+ * - etc.
+ */
 uint64_t d;
 int counter;
 
@@ -52,8 +61,7 @@ static int pit_get_count(PITChannelState *s)
 counter = (s-count - d)  0x;
 break;
 case 3:
-/* XXX: may be incorrect for odd counts */
-counter = s-count - ((2 * d) % s-count);
+counter = (s-count - ((2 * d) % s-count))  0xfffe;
 break;
 default:
 counter = s-count - (d % s-count);
@@ -98,6 +106,18 @@ static inline void pit_load_count(PITChannelState *s, int 
val)
 if (val == 0)
 val = 0x1;
 s-count_load_time = qemu_get_clock_ns(vm_clock);
+
+/* In gate-triggered one-shot modes, indirectly model some pit_get_out()
+ * cases by setting the load time way back until gate-triggered.
+ * (Generally only affects reading status from channel 2 speaker,
+ * due to hard-wired gates on other channels.)
+ *
+ * FIXME: This might be redesigned if a paused timer state is added
+ * for pic_get_count().
+ */
+if (s-mode == 1 || s-mode == 5)
+s-count_load_time -= muldiv64(val+2, get_ticks_per_sec(), PIT_FREQ);
+
 s-count = val;
 pit_irq_timer_update(s, s-count_load_time);
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index a03d7cd..dc13c99 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -50,24 +50,18 @@ int pit_get_out(PITChannelState *s, int64_t current_time)
 switch (s-mode) {
 default:
 case 0:
-out = (d = s-count);
-break;
 case 1:
-out = (d  s-count);
+out = (d = s-count);
 break;
 case 2:
-if ((d % s-count) == 0  d != 0) {
-out = 1;
-} else {
-out = 0;
-}
+out = (d % s-count) != (s-count - 1) || s-gate == 0;
 break;
 case 3:
-out = (d % s-count)  ((s-count + 1)  1);
+out = (d % s-count)  ((s-count + 1)  1) || s-gate == 0;
 break;
 case 4:
 case 5:
-out = (d == s-count);
+out = (d != s-count);
 break;
 }
 return out;
@@ -93,10 +87,10 @@ int64_t pit_get_next_transition_time(PITChannelState *s, 
int64_t current_time)
 break;
 case 2:
 base = (d / s-count) * s-count;
-if ((d - base) == 0  d != 0) {
+if ((d - base) == s-count-1) {
 next_time = base + s-count;
 } else {
-next_time = base + s-count + 1;
+next_time = base + s-count - 1;
 }
 break;
 case 3:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 7/8] i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index c011787..1ba9b3a 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,26 +140,18 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
-if (s-elcr  mask) {
-/* level triggered */
-if (level) {
+if (level) {
+if ((s-last_irr  mask) == 0 ||  /* edge for edge triggered */
+(s-elcr  mask)) {   /* or level triggered */
 s-irr |= mask;
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
 }
+s-last_irr |= mask;
 } else {
-/* edge triggered */
-if (level) {
-if ((s-last_irr  mask) == 0) {
-s-irr |= mask;
-}
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
-}
+/* Dropping level clears the interrupt regardless of edge trigger
+ * vs level trigger.
+ */
+s-irr = ~mask;
+s-last_irr = ~mask;
 }
 pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 0/8] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

, and a command line switch
   to select one.  At some point, change the default to the more accurate
   model.  And maybe drop the old model sometime in the distant future.

Anyone have any better ideas?  Or preferences for or against any
of the above?

-
Some notes about the KVM (kernel mode) 8254 PIT (timer) model:

As far as it goes, the kernel model appears to need the
similar changes and have similar remaining inaccuracies
was the qemu model.  But it also has other issues:

But it will also have problems related to the different way the
kernel 8254 ties the OUT logic to the interrupt controller.
The kernel makes no attempt to model two edges separately; instead
it delivers both at the same time, in essentially a zero-width
pulse.

This model never looses a tick; it delays delivering a new leading
edge until the previous leading edge has been acknowledged with
an EOI in the 8259.  This functionality may be necessary due to how
the kernel schedules tasks, and the nature of the kernel's timers
(otherwise on a heavily loaded host, the guest could unavoidably
lose a large number of ticks).

It also has the edges backwards.  The almost-zero-width pulse is
currently briefly pulsing positive instead of briefly pulsing
negative, which is clearly won't work at all with a proper model
of the trailing edge.

Clearly the order of the edges in the brief pulse needs to be fixed
if we want to fix the interrupt trailing edge model.  (See migration
considerations above...)

What is less clear is if it is worth the effort to try to model the
trailing edge more accurately than that.  Note that if we want to
keep the tick catchup logic, then anytime it is catching up it
is probably not desirable to leave it low for long.  That would
delay or prevent catching up, so in the catching up case, we
would essentially fall back on the current simplified model.

Potentially we could model the low level in a few cases (like
when starting mode 0, or getting halfway through counting
mode 3, or the one-CLK-cylce low in various other modes, or we
get the EOI after the IRQ0 should be low, etc), as long as
it isn't trying to catch up.  This could lead to somewhat random
durations of IRQ0=low, which might cause more
intermitment/confusing bugs than just always modeling it as
short-duration.

Perhaps if some rare guest cares about IRQ0=low duration,
then user could just disable the in-kernel IRQ chip for that guest?

-

Matthew Ogilvie (8):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  i8254: fix inaccuracies in pit_get_out()
  i8259: fix so that dropping IRQ level always clears the interrupt
request
  i8259: refactor pic_set_irq level logic
  i8259/i8254: migration workaround for timer

 hw/cirrus_vga.c   |  4 ++--
 hw/i8254.c| 24 ++--
 hw/i8254_common.c | 18 +--
 hw/i8259.c| 67 +--
 hw/ide/cmd646.c   |  5 +++--
 hw/ide/via.c  |  5 +++--
 hw/pc.h   |  4 
 hw/vga.c  | 37 +++---
 qemu-options.hx   | 27 +-
 vl.c  | 62 +-
 10 files changed, 186 insertions(+), 67 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 3/8] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 qemu-options.hx | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 7d97f96..d473e1c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -975,7 +975,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -1000,6 +1000,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v6 8/8] i8259/i8254: migration workaround for timer

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

It is not at all clear that this is the best way to handle this.
See the detailed notes in the cover letter of this patch series.

UPDATE: Also, some fixes moved the leading edge by 1 CLK
tick (CLK ticks at about 1.1 MHz), and some strategies like this
might risk extra interrupts just from that.

--

 hw/i8259.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 1ba9b3a..4b3c6c9 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -153,6 +153,45 @@ static void pic_set_irq(void *opaque, int irq, int level)
 s-irr = ~mask;
 s-last_irr = ~mask;
 }
+
+/* Back-migration compatibility hack:
+ * As of late 2012, the PIT timer model was incorrectly
+ * pulsing the the IRQ0 line high for only a short time to
+ * indicate an interrupt.  It counted on a conceptual bug in
+ * the PIC irq model to latch onto and and deliver the
+ * interrupt even after it became low again.  (Normally lowering
+ * an IRQ line before it is serviced should cancel the
+ * interrupt.)
+ *
+ * In late 2012 the model has been improved to match hardware
+ * much better by only pulsing low for a short time (in most
+ * PIT modes), but unfortunately that means if you back-migrate
+ * a guest to a version without this fix, the next interrupt
+ * won't have its own leading edge at all, and will
+ * be lost.
+ *
+ * The following hack will allow both the current
+ * interrupt to be serviced properly, and the next one
+ * as well, regardless of which version the migration is
+ * restored on.
+ *
+ * Unfortunately, this has a small possibility of causing
+ * an extra IRQ0 in cases that would not have in the old 2012
+ * model, nor on real hardware.  Specifically, if the current
+ * interrupt is processed, and then something causes an
+ * pit_irq_timer_update() to the same high level it was previously
+ * updated with.  Re-setting various PIT modes (like 4) could
+ * do this, for example.
+ *
+ * At some point in the future (years from now?),
+ * when back-migration to the old 2012 version is
+ * no longer important, it should be safe to just delete
+ * this hack.
+ */
+if (irq==0  s-master) {
+s-last_irr = ~1;
+}
+
 pic_update_irq(s);
 }
 
-- 
1.7.10.2.484.gcd07cc5

Re: [Qemu-devel] [PATCH 1/2] KVM: fix i8259 interrupt high to low transition logic

[Matthew Ogilvie]

On Wed, Sep 12, 2012 at 10:57:57AM +0200, Jan Kiszka wrote:
 On 2012-09-12 10:51, Avi Kivity wrote:
  On 09/12/2012 11:48 AM, Jan Kiszka wrote:
  On 2012-09-12 10:01, Avi Kivity wrote:
  On 09/10/2012 04:29 AM, Matthew Ogilvie wrote:
  Intel's definition of edge triggered means: asserted with a
  low-to-high transition at the time an interrupt is registered
  and then kept high until the interrupt is served via one of the
  EOI mechanisms or goes away unhandled.
 
  So the only difference between edge triggered and level triggered
  is in the leading edge, with no difference in the trailing edge.
 
  This bug manifested itself when the guest was Microport UNIX
  System V/386 v2.1 (ca. 1987), because it would sometimes mask
  off IRQ14 in the slave IMR after it had already been asserted.
  The master would still try to deliver an interrupt even though
  IRQ2 had dropped again, resulting in a spurious interupt
  (IRQ15) and a panicked UNIX kernel.
  diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
  index adba28f..5cbba99 100644
  --- a/arch/x86/kvm/i8254.c
  +++ b/arch/x86/kvm/i8254.c
  @@ -302,8 +302,12 @@ static void pit_do_work(struct kthread_work *work)
   }
   spin_unlock(ps-inject_lock);
   if (inject) {
  -kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 1);
  +/* Clear previous interrupt, then create a rising
  + * edge to request another interupt, and leave it at
  + * level=1 until time to inject another one.
  + */
   kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 0);
  +kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 1);
   
   /*
 
  I thought I understood this, now I'm not sure.  How can this be correct?
   Real hardware doesn't act like this.
 
  What if the PIT is disabled after this?  You're injecting a spurious
  interrupt then.
 
  Yes, the PIT has to raise the output as long as specified, i.e.
  according to the datasheet. That's important now due to the corrections
  to the PIC. We can then carefully check if there is room for
  simplifications / optimizations. I also cannot imagine that the above
  already fulfills these requirements.
 
  And if the PIT is disabled by the HPET, we need to clear the output
  explicitly as we inject the IRQ#0 under a different source ID than
  userspace HPET does (which will logically take over IRQ#0 control). The
  kernel would otherwise OR both sources to an incorrect result.
 
  
  I guess we need to double the hrtimer rate then in order to generate a
  square wave.  It's getting ridiculous how accurate our model needs to be.
 
 I would suggest to solve this for the userspace model first, ensure that
 it works properly in all modes, maybe optimize it, and then decide how
 to map all this on kernel space. As long as we have two models, we can
 also make use of them.

Thoughts about the 8254 PIT:

First, this summary of (real) 8254 PIT behavior seems fairly
good, as far it goes:

On Tue, Sep 04, 2012 at 07:27:38PM +0100, Maciej W. Rozycki wrote:
  * The 8254 PIT is normally configured in mode 2 or 3 in the PC/AT
architecture.  In the former its output is high (active) all the time
except from one (last) clock cycle.  In the latter a wave that has a
duty cycle close or equal to 0.5 (depending on whether the divider is
odd or even) is produced, so no short pulses either.  I don't remember
the other four modes -- have a look at the datasheet if interested, but
I reckon they're not really compatible with the wiring anyway, e.g. the
gate is hardwired enabled.

I've also just skimmed parts of the 8254 section of The Indispensable PC
Hardware Book, by Hans-Peter Messmer, Copyright 1994 Addison-Wesley,
although I probably ought to read it more carefully.

Under normal conditions, the 8254 part of the patch above should be
indistinguishable from previous behavior.  The 8259's IRR will
still show up as 1 until the interrupt is actually serviced,
and no new interrupt will be serviced after one is serviced until
another edge is injected via the high-low-high transition of the new
code.  (Unless the guest resets the 8259 or maybe messes with IMR,
but real hardware would generate extra interrupts in such cases as
well.)

The new code sounds much closer to mode 2 described by
Maciej, compared to the old code - except the duty cycle is
effectively 100 percent instead of 99.[some number of 9's] percent.

-
But there might be some concerns in abnormal conditions:

   * If some guest is actually depending on a 50 percent duty cycle
 (maybe some kind of polling rather than interrupts), I would
 expect it to be just as broken before this patch as after,
 unless it is really weird (handles continuous 1's more
 gracefully than continuous 0's).

According to the my book, mode 3 isn't normally
 used to create interrupts: The generated square-wave

Re: [Qemu-devel] [PATCH v5 5/6] i8259: fix so that dropping IRQ level always clears the interrupt request

[Matthew Ogilvie]

On Mon, Sep 10, 2012 at 11:09:27AM +0200, Jan Kiszka wrote:
 On 2012-09-10 10:56, Avi Kivity wrote:
  On 09/10/2012 04:27 AM, Matthew Ogilvie wrote:
  Intel's definition of edge triggered means: asserted with a
  low-to-high transition at the time an interrupt is registered and
  then kept high until the interrupt is served via one of the
  EOI mechanisms or goes away unhandled.
 
  So the only difference between edge triggered and level triggered
  is in the leading edge, with no difference in the trailing edge.
  
  Hard to believe.  So an edge while cpu interrupts are disabled is ignored?
 
 No, this is about the PIC, not the CPU interrupt inputs.
 
 Matthew, did you verify this on real hardware by reading back the IRR as
 I suggested?
 
 Jan

I hadn't before, but now that I've checked, it's as expected:

---
Real hardware [Pentium 4]:

  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE

[I also see a final IRR= occasionally.  Probably just happened to
ask it while the timer (IRQ0) line is low (without the new understanding
of the trailing edge of an edge triggered interrupt, this would
have been confusing).  I have most IRQ's (including
timer) masked off.]

---
Unpatched qemu:

  cmdRead unmask IRR=4015 mask IRR=4015 sti irq15 unmask IRR=4015 DONE

[Presumably IRQ4 (0x10 - qemu's serial device model?) had a transient
edge during initialization, but had been masked off even before I
masked it off?]

---
Patched qemu:

  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE


  - Matthew

Re: [Qemu-devel] [PATCH 1/2] KVM: fix i8259 interrupt high to low transition logic

[Matthew Ogilvie]

On Tue, Sep 11, 2012 at 01:49:51AM +0100, Maciej W. Rozycki wrote:
 On Sun, 9 Sep 2012, Matthew Ogilvie wrote:
 
  This bug manifested itself when the guest was Microport UNIX
  System V/386 v2.1 (ca. 1987), because it would sometimes mask
  off IRQ14 in the slave IMR after it had already been asserted.
  The master would still try to deliver an interrupt even though
  IRQ2 had dropped again, resulting in a spurious interupt
  (IRQ15) and a panicked UNIX kernel.
 
  That is quite weird actually -- from my experience the spurious vector is 
 never sent from a slave (quite understandably -- since the interrupt is 
 gone and no other is pending, the master has no reason to select a slave 
 to supply a vector and therefore supplies the spurious vector itself) and 
 therefore a spurious IRQ7 is always issued regardless of whether the 
 discarded request came from a slave or from the master.

Keep in mind that this paragraph is describing QEMU's 8259 device
model behavior (and also KVM's), not real hardware.  Reading the
unpatched code, the master clearly latches on to the momentary IRQ2,
does not cancel it when it is cleared again, and ultimately delivers
a spurious IRQ15.

As for what the OS is doing with the IRQ15 (or IRQ7), I only have a large
dissamebly listing (with only a vague idea of it's overall interrupt
handling strategy), and some printf logs of stuff happening in the
8259 model when the OS is running (more useful).

 
  Is there a bug elsewhere then too?  I would have expected a reasonable 
 (and especially an old-school) x86 OS to be able to cope with spurious 
 8259A interrupts, but then obviously one would expect them on IRQ7 only.
 
   Maciej

[Qemu-devel] [PATCH v5 0/6] Running Microport UNIX (ca 1987)

[Matthew Ogilvie]

Changes since previous version:

   * The first 4 patches haven't changed since version 3.
   * New patch 5: The 8259 patch has been totally redesigned again, this
 time based on a new understanding that on real hardware, if
 the trailing edge of an interrupt arrives before the interrupt
 is serviced, then it cancels the interrupt, just like a level
 triggered interrupt.  See earlier email discussion.
   * New patch 6 just refactors the code (no functionality change)
 after the one line fix in patch 5.

I'm also sending a couple of patches for related projects, separately:

   * Two KVM (Linux kernel) patches that do roughly the same thing as
 patches 5 and 6, only for the in-kernel PIC.
   * A patch for the kvm-unit-tests project that adds a test case
 to demonstrate the trailing edge behavior.

Matthew Ogilvie (6):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  i8259: fix so that dropping IRQ level always clears the interrupt
request
  i8259: refactor pic_set_irq level logic

 hw/cirrus_vga.c |  4 ++--
 hw/i8259.c  | 28 ++
 hw/ide/cmd646.c |  5 +++--
 hw/ide/via.c|  5 +++--
 hw/pc.h |  4 
 hw/vga.c| 37 ++
 qemu-options.hx | 27 -
 vl.c| 62 ++---
 8 files changed, 119 insertions(+), 53 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v5 1/6] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index e8dcc6b..68c36f3 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2055,8 +2055,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index 53daf78..6587666 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, 
target_phys_addr_t addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index e0b9443..dd2855e 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t 
addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b20e4f0..948a469 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH] [kvm-unit-tests] add x86/test8259.S

[Matthew Ogilvie]

This tests how the master i8259 handles a falling IRQ2 cascade
interrupt when the original interrupt is masked off in the slave's
IMR register.  It should cancel the interrupt, and not deliver
anything to the CPU until it is unmasked again.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

On Mon, Sep 03, 2012 at 09:08:58AM +0200, Paolo Bonzini wrote:
 You can write a test for kvm-unit-tests.  These tests can be written in
 C, including interrupt handlers (see lib/x86/isr.h).  The git tree is at
 git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git.

I attempted to rewrite this in C, but ran into snags with
figuring out how to bypass APIC stuff that the unit test framework
is setting up (I don't know anything about APIC), the VM would
reboot rather than service IRQ14/IRQ15 correctly, etc.  So I
just adapted my assembly language boot sector test instead.
I could send the non-functional C code, if anyone wants it.

I also considered adding it to x86/realmode.c, but there is
relatively little sharable real mode infrastructure between them.
(Setting up real mode interrupt shims to tiny C functions where the
shim is likely bigger than the converted C, etc...)

Unlike most kvm-unit-test programs, this one duplicates the output on the
VGA, so in theory it can actually be usefully run on real hardware
without the qemu test devices.  Unfortunately, grub gives an
error #7 Loading below 1MB is not supported.  The original
bootsector version from the following URL works better (use grub's
chainloader /filename/), although it seems to hang as soon as
it accesses the hard disk on my newest machine (issues with
SATA drive, maybe?).
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2


 config-x86-common.mak |  10 +-
 x86/test8259.S| 587 ++
 2 files changed, 596 insertions(+), 1 deletion(-)
 create mode 100644 x86/test8259.S

diff --git a/config-x86-common.mak b/config-x86-common.mak
index c76cd11..1a57079 100644
--- a/config-x86-common.mak
+++ b/config-x86-common.mak
@@ -34,7 +34,8 @@ tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
$(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \
$(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \
$(TEST_DIR)/kvmclock_test.flat  $(TEST_DIR)/eventinj.flat \
-   $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat 
$(TEST_DIR)/asyncpf.flat
+   $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat \
+   $(TEST_DIR)/asyncpf.flat $(TEST_DIR)/test8259.flat
 
 ifdef API
 tests-common += api/api-sample
@@ -92,6 +93,13 @@ $(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
 
 $(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
 
+$(TEST_DIR)/test8259.elf: $(TEST_DIR)/test8259.o
+   $(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
+
+$(TEST_DIR)/test8259.o: bits = 32
+
+$(TEST_DIR)/test8259.o: CFLAGS += -m32
+
 $(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
 
 arch_clean:
diff --git a/x86/test8259.S b/x86/test8259.S
new file mode 100644
index 000..93e09c6
--- /dev/null
+++ b/x86/test8259.S
@@ -0,0 +1,593 @@
+# Copyright 2012 Matthew Ogilvie
+# Released under LGPLv2.
+#
+# This program demonstrates that edge triggered interrupts are
+# canceled on the trailing edge.  Especially with respect to
+# the cascade IRQ2 and the slave's IMR register.
+#
+# Strategy: Tested by masking out IRQ14 in the slave's IMR register, and
+# noticing that you don't get an interrupt until it is unmasked.
+# Search for INTERESTING_PART below for the heart of the test.
+#
+# This outputs both to the VGA (useful for running on a real
+# machine) and the same test devices as the rest of kvm-unit-tests.
+#
+# Assumes that BIOS and the bootloader left interrupts, the VGA, etc
+# all in the standard real mode/DOS configurations, as specified in
+# the multiboot specification:
+# 
http://www.gnu.org/software/grub/manual/multiboot/multiboot.html#Machine-state
+#
+# Adapted from plain boot-sector version:
+# http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2
+
+.section .init
+.code32
+mb_magic = 0x1BADB002
+mb_flags = 0x0
+
+# multiboot header
+.long mb_magic, mb_flags, 0 - (mb_magic + mb_flags)
+
+.globl start
+.data
+. = . + 4096
+
+.section .text
+start:
+# Multiboot tries to be helpful with protected mode, but it's
+# just in the way of a simple test like this.
+lgdt r_gdt_descr
+ljmp $8, $backToRealMode
+backToRealMode:
+.code16
+mov $16, %eax
+mov %ax, %ds
+mov %ax, %es
+mov %ax, %fs
+mov %ax, %gs
+mov %ax, %ss
+mov %cr0, %eax
+btc $0, %eax
+mov %eax, %cr0
+ljmp $0, $realmode_entry
+
+#define VGA_SEGMENT 0xb800
+/*#define VGA_SEGMENT 0xb000*/ /* MDA */
+
+#define VGA_CRTC_BASE 0x3d4
+/*#define

[Qemu-devel] [PATCH v5 6/6] i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index c011787..1ba9b3a 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,26 +140,18 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
-if (s-elcr  mask) {
-/* level triggered */
-if (level) {
+if (level) {
+if ((s-last_irr  mask) == 0 ||  /* edge for edge triggered */
+(s-elcr  mask)) {   /* or level triggered */
 s-irr |= mask;
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
 }
+s-last_irr |= mask;
 } else {
-/* edge triggered */
-if (level) {
-if ((s-last_irr  mask) == 0) {
-s-irr |= mask;
-}
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
-}
+/* Dropping level clears the interrupt regardless of edge trigger
+ * vs level trigger.
+ */
+s-irr = ~mask;
+s-last_irr = ~mask;
 }
 pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 2/2] KVM: i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 arch/x86/kvm/i8259.c | 28 +++-
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index b0c2346..d6c005c 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -95,26 +95,20 @@ static inline int pic_set_irq1(struct kvm_kpic_state *s, 
int irq, int level)
 {
int mask, ret = 1;
mask = 1  irq;
-   if (s-elcr  mask) /* level triggered */
-   if (level) {
+   if (level) {
+   if ((s-last_irr  mask) == 0 ||  /* edge for edge-triggered */
+   (s-elcr  mask)) {   /* or level triggered */
ret = !(s-irr  mask);
s-irr |= mask;
-   s-last_irr |= mask;
-   } else {
-   s-irr = ~mask;
-   s-last_irr = ~mask;
-   }
-   else/* edge triggered */
-   if (level) {
-   if ((s-last_irr  mask) == 0) {
-   ret = !(s-irr  mask);
-   s-irr |= mask;
-   }
-   s-last_irr |= mask;
-   } else {
-   s-irr = ~mask;
-   s-last_irr = ~mask;
}
+   s-last_irr |= mask;
+   } else {
+   /* Dropping level clears the interrupt regardless of edge
+* trigger vs level trigger.
+*/
+   s-irr = ~mask;
+   s-last_irr = ~mask;
+   }
 
return (s-imr  mask) ? -1 : ret;
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v5 3/6] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 qemu-options.hx | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 3c411c4..3e8085d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -945,7 +945,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -970,6 +970,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 1/2] KVM: fix i8259 interrupt high to low transition logic

[Matthew Ogilvie]

Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered
and then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked UNIX kernel.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

There has been some discussions about this on the qemu mailing
list; for example see
http://www.mail-archive.com/qemu-devel@nongnu.org/msg129071.html

This fixes the test program I wrote to demonstrate the problem.  See
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2
(for source code to a floppy disk boot sector) or a kvm-unit-test patch
I'm posting separately.

Unfortunately, Microport UNIX still has some some unknown interrupt
problem when run under qemu with KVM enabled, although it
runs fine under plain QEMU [no KVM] with the corresponding
QEMU patch.  The unknown interrupt bug actually triggers
much earlier than this one: this one doesn't show up until
the boot floppy accesses the hard drive after some prompts,
but the unknown bug shows up early in bootup.  Also, the specific
function in which UNIX panics is different: this
one in splint(); the unknown bug in splx().  Also, the KVM
behavior is not affected by the presense or absense of this patch.

I'll probably continue looking into the unknown problem, but I'm not
sure when I'll have any results.

Despite this unknown problem, I'm confident that this known problem
should be fixed, and if not fixed will cause problems when I figure
out the unknown problem.

 arch/x86/kvm/i8254.c |  6 +-
 arch/x86/kvm/i8259.c | 14 ++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index adba28f..5cbba99 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -302,8 +302,12 @@ static void pit_do_work(struct kthread_work *work)
}
spin_unlock(ps-inject_lock);
if (inject) {
-   kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 1);
+   /* Clear previous interrupt, then create a rising
+* edge to request another interupt, and leave it at
+* level=1 until time to inject another one.
+*/
kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 0);
+   kvm_set_irq(kvm, kvm-arch.vpit-irq_source_id, 0, 1);
 
/*
 * Provides NMI watchdog support via Virtual Wire mode.
diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index e498b18..b0c2346 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -111,8 +111,10 @@ static inline int pic_set_irq1(struct kvm_kpic_state *s, 
int irq, int level)
s-irr |= mask;
}
s-last_irr |= mask;
-   } else
+   } else {
+   s-irr = ~mask;
s-last_irr = ~mask;
+   }
 
return (s-imr  mask) ? -1 : ret;
 }
@@ -169,14 +171,10 @@ static void pic_update_irq(struct kvm_pic *s)
 {
int irq2, irq;
 
+   /* slave PIC notifies master PIC via IRQ2 */
irq2 = pic_get_irq(s-pics[1]);
-   if (irq2 = 0) {
-   /*
-* if irq request by slave pic, signal master PIC
-*/
-   pic_set_irq1(s-pics[0], 2, 1);
-   pic_set_irq1(s-pics[0], 2, 0);
-   }
+   pic_set_irq1(s-pics[0], 2, irq2 = 0);
+
irq = pic_get_irq(s-pics[0]);
pic_irq_request(s-kvm, irq = 0);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v5 5/6] i8259: fix so that dropping IRQ level always clears the interrupt request

[Matthew Ogilvie]

Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered and
then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked kernel.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

If you missed the previous thread about this, see
http://www.mail-archive.com/qemu-devel@nongnu.org/msg129071.html

 hw/i8259.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 6587666..c011787 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -157,6 +157,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 s-last_irr |= mask;
 } else {
+s-irr = ~mask;
 s-last_irr = ~mask;
 }
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v5 4/6] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/pc.h |  4 
 hw/vga.c| 37 +
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index e4db071..37e2f87 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -176,6 +176,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 static inline DeviceState *isa_vga_init(ISABus *bus)
 {
 ISADevice *dev;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..fb08dc0 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -547,14 +547,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xd;
+} else if (s-cr_index == VGA_CRTC_CURSOR_END 
+   val == 7 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xe;
+}
 }
-return;
 }
 s-cr[s-cr_index] = val;
 
@@ -1886,7 +1903,11 @@ static void vga_update_display(void *opaque)
 /* nothing to do */
 } else {
 full_update = 0;
-if (!(s-ar_index  0x20)) {
+if (!(s-ar_index  0x20) 
+/* extra CGA compatibility hacks (not in standard VGA) */
+(!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index 3e8085d..68925f3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -975,6 +975,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default for CGA fonts
+instead of VGA fonts.
+@item all
+Enable all CGA hacks.  More CGA hacks may

[Qemu-devel] [PATCH v5 2/6] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 7c577fa..febfd62 100644
--- a/vl.c
+++ b/vl.c
@@ -2352,8 +2352,9 @@ int main(int argc, char **argv, char **envp)
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
 DisplayChangeListener *dcl;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2408,8 +2409,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2457,7 +2457,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2475,21 +2475,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2523,7 +2510,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2555,7 +2545,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5

Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register

[Matthew Ogilvie]

On Tue, Sep 04, 2012 at 04:42:35PM +0200, Paolo Bonzini wrote:
 Il 04/09/2012 16:29, Maciej W. Rozycki ha scritto:
   So first of all, the *output* of the 8259A is always edge triggered, 
  regardless of whether it's the master or one of the slaves (only one slave 
  is used in the PC/AT architecture, but up to eight are supported; the 
  PC/XT had none).  
 
 I swear I read all your message :) but this seems to be the crux.  It means
 that something like this ought to fix the bug too.  Matthew, can you post
 your code or test it?

The test program source can be downloaded from
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2

I intend to rewrite it for kvm-unit-tests as you suggested earlier, but
likely not before this weekend.

 
 diff --git a/hw/i8259.c b/hw/i8259.c
 index 53daf78..3dc1dff 100644
 --- a/hw/i8259.c
 +++ b/hw/i8259.c
 @@ -104,12 +104,11 @@ static void pic_update_irq(PICCommonState *s)
  int irq;
  
  irq = pic_get_irq(s);
 +qemu_irq_lower(s-int_out[0]);
  if (irq = 0) {
  DPRINTF(pic%d: imr=%x irr=%x padd=%d\n,
  s-master ? 0 : 1, s-imr, s-irr, s-priority_add);
  qemu_irq_raise(s-int_out[0]);
 -} else {
 -qemu_irq_lower(s-int_out[0]);
  }
  }

This doesn't work; the master still locks onto the original low to high
edge, and doesn't cancel it on the high to low edge.  I haven't had a
chance to look into or test your (or any other) KVM patches yet,
although I'll probably get to it this weekend.

According to later discussion, the main issue is actually the input
IRQ behavior on a high to low transition; hence the following fixes
both the test program and the Microport UNIX problem:

diff --git a/hw/i8259.c b/hw/i8259.c
index 6587666..c011787 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -143,22 +143,23 @@ static void pic_set_irq(void *opaque, int irq, int level)
 if (s-elcr  mask) {
 /* level triggered */
 if (level) {
 s-irr |= mask;
 s-last_irr |= mask;
 } else {
 s-irr = ~mask;
 s-last_irr = ~mask;
 }
 } else {
 /* edge triggered */
 if (level) {
 if ((s-last_irr  mask) == 0) {
 s-irr |= mask;
 }
 s-last_irr |= mask;
 } else {
+s-irr = ~mask;
 s-last_irr = ~mask;
 }
 }
 pic_update_irq(s);
 }
 

Perhaps it would be worth it to swap around the ifs a little bit
to avoid the (!level) duplication, and clarify that the only difference
is in the low to high transition?

 - Matthew Ogilvie

[Qemu-devel] [PATCH v4 0/5] Running Microport UNIX (ca 1987)

[Matthew Ogilvie]

This series is fixing issues I found when getting qemu to run
Micoport UNIX System V/386, v 2.1 (ca 1987), although most of
the patches are completely independent of each other.

Changes since v2 and v3:

   - Drop the -no-spurious-interrupts patch.  (It might still be useful
 as an end-user workaround for other potential interrupt bugs, but I'm
 not going to push for it.)

   - Add a completely new patch to fix for how the master i8259
 handles IRQ2 when the original interrupt (say IRQ14) is
 dynamically masked off in the slave via the IMR register.
 This is supported by a test program I wrote.  There are probably
 some tweaks still desired (KVM at least), but I'm fairly
 confident this basic approach is correct.

   - Squash in the remaining two small v3 incremental patches into v2.

   - [applied] The mov to/from crN/drN patch has been applied (and not
 reverted), and is no longer included with this series.


Matthew Ogilvie (5):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  i8259: fix dynamically masking slave IRQs with IMR register

 hw/cirrus_vga.c |  4 ++--
 hw/i8259.c  | 15 -
 hw/i8259_common.c   |  2 ++
 hw/i8259_internal.h |  1 +
 hw/ide/cmd646.c |  5 +++--
 hw/ide/via.c|  5 +++--
 hw/pc.h |  4 
 hw/vga.c| 37 +---
 qemu-options.hx | 27 ++-
 vl.c| 62 +++--
 10 files changed, 121 insertions(+), 41 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v4 2/5] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 7c577fa..febfd62 100644
--- a/vl.c
+++ b/vl.c
@@ -2352,8 +2352,9 @@ int main(int argc, char **argv, char **envp)
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
 DisplayChangeListener *dcl;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2408,8 +2409,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2457,7 +2457,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2475,21 +2475,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2523,7 +2510,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2555,7 +2545,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v4 1/5] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Changes since v2: The v3 tweak (adding back a dropped 02) has been
squashed in.

 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index e8dcc6b..68c36f3 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2055,8 +2055,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index 53daf78..6587666 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, 
target_phys_addr_t addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index e0b9443..dd2855e 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t 
addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b20e4f0..948a469 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v4 4/5] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Change since v2: The v3 tweak (fix conditions for palette blanking hack)
has been squashed in.

 hw/pc.h |  4 
 hw/vga.c| 37 +
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index e4db071..37e2f87 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -176,6 +176,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 static inline DeviceState *isa_vga_init(ISABus *bus)
 {
 ISADevice *dev;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..fb08dc0 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -547,14 +547,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xd;
+} else if (s-cr_index == VGA_CRTC_CURSOR_END 
+   val == 7 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xe;
+}
 }
-return;
 }
 s-cr[s-cr_index] = val;
 
@@ -1886,7 +1903,11 @@ static void vga_update_display(void *opaque)
 /* nothing to do */
 } else {
 full_update = 0;
-if (!(s-ar_index  0x20)) {
+if (!(s-ar_index  0x20) 
+/* extra CGA compatibility hacks (not in standard VGA) */
+(!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index 3e8085d..68925f3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -975,6 +975,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default

[Qemu-devel] [PATCH v4 3/5] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 qemu-options.hx | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 3c411c4..3e8085d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -945,7 +945,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -970,6 +970,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register

[Matthew Ogilvie]

Although I haven't found any specs that say so, on real hardware
I have a test program that shows if you mask off the slave
interrupt (say IRQ14) in the IMR after it has already been raised,
the master (IRQ2) gets canceled (that is, IRQ2 acts like it is level
triggered).  Without this patch, qemu delivers a
spurious interrupt (IRQ15) instead when running the test program.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

I've written a test program (in the form of a floppy disk boot sector)
that demonstrates that qemu's emulation of IRQ2 propagation from the
slave i8259 to the master does not work correctly when the CPU has
interrupts disabled and it masks off the original interrupt (IRQ14)
in the slave's IMR register.  This was based on simplifying breakage
observed when trying to run an old Microport UNIX system (ca 1987).

Earlier I speculated that maybe the ELCR bit for IRQ2 was incorrect,
but now I don't think changing the bit (from the target's
perspective) would be a good idea.  See below.

You can download the source code for the test program from
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2
It can be compiled using a standard GNU i386 toolchain on Linux.

The heart of the test program is:

cli

  # i8259:imm: mask off everything except IRQ2
movb $0xfb,%al # master (only IRQ2 clear)
outb %al,$0x21
movb $0xff,%al # slave
outb %al,$0xa1

mov $.msgCmdRead,%ax
call print
call initIrqHandlers
call scheduleIrq14

call .largeDelay   # note: IRQ14 raised while this is waiting

mov $.msgUnmask,%ax
call print
movb $0x3f,%al # unmask IRQ14 and IRQ15
outb %al,$0xa1

call .largeDelay   # (probably not important)

mov $.msgMask,%ax
call print
movb $0xff,%al # mask IRQ14 and IRQ15 again
outb %al,$0xa1

call .largeDelay   # (probably not important)

mov $.msgSti,%ax
call print
sti

call .largeDelay   # note: spurious interrupt (IRQ15) from qemu

cli
mov $.msgUnmask,%ax
call print
movb $0x3f,%al # unmask IRQ14 and IRQ15
outb %al,$0xa1
sti

call .largeDelay   # we should finally see IRQ14 here?

  # DONE:
cli
movw $.msgDone, %ax
call print
  .Lhalt:
hlt
jmp .Lhalt

In qemu, the master treats IRQ2 as edge triggered, and delivers a
spurious interrupt if the CPU re-enables interrupts after
the slave's IMR is masked off (it also doesn't seem to deliver
the real interrupt when the IMR is unmasked, but maybe that is
because I'm not correctly acknowledging the spurious interrupt).

  - Qemu output (without this patch):
elcr=0c00 cmdRead ummask mask sti irq15 unmask DONE

But on real hardware, the master seems to treat IRQ2 as level triggered,
and doesn't deliver an interrupt to the CPU until the slave unmasks IRQ14.
I've tried this on several machines, including a non-PCI 486 that
does not seem to have ELCR registers.

  - Typical output (pentium/pentium pro/dual AMD Athlon/etc; slight
variations in some elcr bits depending on machine, but bit 0x04
is always clear):
elcr=0c20 cmdRead unmask mask sti unmask irq14 DONE

  - 486 without elcr (just an ISA bus):
elcr=e0e0 cmdRead unmask mask sti unmask irq14 DONE

  - One failure: It doesn't boot properly (no output) with a USB floppy
drive on my Intel Core I7.  Guess: The test program just barely
fits in a sector, with no room for any tables (partition/etc)
that BIOS might check for if it isn't an original, native floppy
drive.

---

I've found a few descriptions of programming the i8259.
The closest thing I've found to a detailed spec is in
iAPX 86, 88 User's Manual, dated August 1981:
http://ebookbrowse.com/1981-iapx-86-88-users-manual-pdf-d3089962
It has a significant section titled Using the 8259A Programmable
Interrupt Controller starting on page 438 or A-135.

But none of my sources seem to specify how master/slave cascading
interacts with the IMR (mask register) and edge trigger mode,
although it talks about those things individually.
Also, given the date it isn't surprising that it doesn't mention
the elcr registers at all.

I have not found any real specs for the ELCR registers, but I've found a
few hints:

  - Two 8 bit registers: one for master (0x4d0) and one for slave (0x4d1).
  - One bit per IRQ line: 0 for edge trigger, 1 for level trigger.
  - Not present unless the machine has EISA or PCI buses.  Plain
ISA doesn't have it.
  - Might be implemented completely external to the actual i8259s.
  - As seen in test program output above, ELCR bit 0x04 is clear,
indicating that IRQ2 is supposedly edge triggered, even though
actual tested behavior is level triggered.
  - A google search (8259 elcr -linux -qemu) [exclude

Re: [Qemu-devel] [PATCH v2 6/6] i8259: add -no-spurious-interrupt-hack option

[Matthew Ogilvie]

On Fri, Aug 24, 2012 at 07:40:36AM +0200, Jan Kiszka wrote:
 On 2012-08-23 08:24, Matthew Ogilvie wrote:
  This patch provides a way to optionally suppress spurious interrupts,

[snip]

  I'm not sure why it only sporadically hits this sequence of events.
  There doesn't seem to be other IRQs asserted or serviced anywhere
  in the near past; the last several were all IRQ14's.  But I can't
  help feeling I'm not reading the log output correctly or something,
  because that doesn't make sense.  Maybe there is there some kind
  of a-few-instructions delay before a CPU interrupt is actually
  deliviered after interrupts are enabled, or some delay in raising
  IRQ14 after a hard drive operation is requested, and such delays
  need to fall into a narrow window of opportunity left by UNIX?
  
  I can get a disassembly of the UNIX kernel using a coff-enabled
  build of GNU objdump, giving function names but not much else.
  But I haven't studied it in enough detail to actually find the
  relevant code path that is manipulating imr as described above.
  However, this old post outlines some of the high level theory
  of UNIX spl*() functions:
  http://www.linuxmisc.com/29-unix-internals/4e6c1f6fa2e41670.htm
  
  If anyone wants to look into this further, I can provide access to the
  initial boot install floppy, at least.  Email me.  (Without the rest
  of the install disks, it isn't much use for anything except testing
  virtual machines like qemu against rare corner cases...)
  
  
  Alternative Approaches:
  
  An alternative to this patch that might work (I haven't tried) would
  be to have BIOS set the master's elcr register 0x04 bit, making IRQ2
  level triggered instead of edge triggered.  I'm not sure what other
  effects this might have.  Maybe it would actually be a more accurate
  model (I haven't checked documentation; maybe slave mode of a
  IRQ line into the master is supposed to be level triggered?)
  
  Or perhaps find a way to model the minimum timescale that a interrupt
  request needs to be active to be recognized?
  
  Or maybe my analysis isn't correct; I wasn't able to find the
  relevant code path in the UNIX kernel.

[snip]

 
 Has to mention or even actively warn that it doesn't work with KVM and
 its in-kernel irqchip (as that PIC model lacks your hack).

I'll make an incremental patch to the documentation soon.

 
 However, I strongly suspect you are nastily papering over an issue in
 some device model. So I would prefer to dig deeper before installing
 this in upstream (also due to its dependency on the userspace PIC model).

This is certainly possible.  I'm not an expert on the whole interrupt
subsystem design in a PC.  But other than the wild speculation above
(making IRQ2 level triggered via elcr, or some kind of timing preventing the
edge triggering from catching a very short blip), I'm not sure what
to look for.

- Matthew Ogilvie

[Qemu-devel] [PATCH v3 0/3] Microport UNIX series (was: [PATCH v2 0/6] ...)

[Matthew Ogilvie]

This version is an incremental on top of version 2 to address
various concerns.  At least as of this moment, version 2 has
been applied upstream, so this version is incremental.

On Fri, Aug 24, 2012 at 07:44:16AM +0200, Jan Kiszka wrote:
 On 2012-08-24 05:58, malc wrote:
  Applied, thanks.

 Err, does this comply with the -rcX process? Patch 6 alone has been on
 the list for less than a day. Only now I was able to comment on it, and
 I would prefer to not have it merged that easily.

Although I would prefer to apply these patches, I'm also a little
surprised how quickly they were accepted.  I had thought I had
already missed the merge window.

If the ultimate decision is to leave in some or all of the version 2
patches, then we should probably also include the corresponding
version 3 incremental patches.  Otherwise, if some or all of them
are reverted for the release, I can post a version 4 with version
3 squashed in later.

Overall, the first four patches of version 2 seem more like simple
bug fixes than the last two.  Maybe keep at least the first 4 (possilby
with just the first patch of version 3 as well)?


Matthew Ogilvie (3):
  debug printf (cirrus_vga): fixup unintended format change
  vga cga_hack=palette_blanking: narrower conditions for hack
  doc: mention that -no-spurious-interrupt-hack doesn't work with KVM

 hw/cirrus_vga.c | 2 +-
 hw/vga.c| 5 +++--
 qemu-options.hx | 5 +++--
 3 files changed, 7 insertions(+), 5 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 1/3] debug printf (cirrus_vga): fixup unintended format change

[Matthew Ogilvie]

I unintentionally dropped an 02 from one of the format strings
in commit 145c7c880ff520a9, as noted by Andreas Färber afaer...@suse.de.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

The 02 in debug code seems extremely low priority, but on the other
hand, there is no good reason to change it.

 hw/cirrus_vga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 909899d..68c36f3 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2055,7 +2055,7 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value % PRIx64 \n,
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
addr, mem_value);
 #endif
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 3/3] doc: mention that -no-spurious-interrupt-hack doesn't work with KVM

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

On Fri, Aug 24, 2012 at 07:40:36AM +0200, Jan Kiszka wrote:
 Has to mention or even actively warn that it doesn't work with KVM and
 its in-kernel irqchip (as that PIC model lacks your hack).

 qemu-options.hx | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index b28e853..d6bfc34 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1198,7 +1198,8 @@ STEXI
 Use it as a workaround for operating systems that drive PICs in a way that
 can generate spurious interrupts, but the OS doesn't handle spurious
 interrupts gracefully.  (e.g. late 80s/early 90s versions of ATT UNIX
-and derivatives)
+and derivatives)  Warning: This does not work with KVM's in-kernel
+PIC model.
 ETEXI
 
 HXCOMM Deprecated by -rtc
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 2/3] vga cga_hack=palette_blanking: narrower conditions for hack

[Matthew Ogilvie]

In commit 482f7bf86b43af9f, I mistakenly inverted the logic
I intended for ar_flip_flop.  I intended to allow the GMODE_BLANK
case as soon as any palette register was modified.

Also include minor tweak to documentation about how to list
multiple hacks on the command line.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/vga.c| 5 +++--
 qemu-options.hx | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index a65fc26..fb08dc0 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1904,9 +1904,10 @@ static void vga_update_display(void *opaque)
 } else {
 full_update = 0;
 if (!(s-ar_index  0x20) 
-/* extra CGA compatibility hacks (not in standard VGA */
+/* extra CGA compatibility hacks (not in standard VGA) */
 (!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
- (s-ar_index != 0  s-ar_flip_flop))) {
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index 2a6d829..b28e853 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -975,7 +975,7 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
-@item cga_hacks=@var{hack1}[+@var{hack2},[...]]
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
 Enable various extra CGA compatibility hacks for programs that are
 trying to directly set CGA modes without BIOS assistance nor
 real knowledge of EGA/VGA.  These might only work with -vga std.
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v2 1/6] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

This version of the patch adds i8259.c.

An alternative approach might be to eliminate these printf's,
and/or replace them with trace*() calls, but until someone gets
around to doing so...

 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index e8dcc6b..909899d 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2055,8 +2055,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value % PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index 53daf78..6587666 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, 
target_phys_addr_t addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index e0b9443..dd2855e 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t 
addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b20e4f0..948a469 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v2 0/6] Running Microport UNIX (ca 1987)

[Matthew Ogilvie]

After applying this version 2 of this patch series, I can
successfully run Micoport UNIX System V/386, v 2.1 (ca 1987)
under qemu.  (although not if I try to enable KVM)

Version 1 of this series was posted about 4 weeks ago.  See
http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=15654

The patches are all independent, except that the documentation part
of patch 5 (vga) adds onto patch 4 (retrace=) changes.

Patches 2 (mov crN), 5 (vga/cga), and 6 (spurious interrupts) are
required to run this UNIX.  The other three patches are trivial
improvements I noticed while tracking down the main issues.

The first four patches are probably trivially obvious.

The last two patches might be a little controversial, since they
add hacks to work around what could be argued are operating system
bugs.  But I've tried to make them minimal impact (leave
them disabled by default, isolate relevant code in minimal number
of places, etc), and tried to implement and describe them so that
they might be useful for other old OS's and programs besides my old
version of UNIX.

==

Just for reference, in case someone else wants to debug similar issues
with other operating systems, here are some notes about running and
debugging this UNIX system under qemu:

 - This version of UNIX seems to hard code the number of tracks
   per cylinder for the hard drive to 17.  So build your drive image
   with that in mind, and tell qemu to use 17 tracks per cylinder.
 - This version of UNIX seems to think it doesn't have any RAM unless
   I configure the virtual machine with 17MB or less RAM.  I've mostly
   been reducing that to 15 while debugging other issues, just to
   eliminate any possible problems at 16.  Someday I'll try 17 again.
 - I use a command line similar to the following (from a shell script):
 qemu-system-i386 -monitor stdio -m 15 -hdachs 977,5,17 -hda $diskC \
-drive file=$installDisk,if=floppy,snapshot=on -no-fd-bootchk \
-vga std,cga_hacks=palette_blanking+font_height \
-no-spurious-interrupt-hack
 - -no-shutdown and -no-reboot were also handy for tracking some of the
   early bootup issues (mov crN patch).
 - Without my cga hacks patch, you can get a snapshot of the screen
   by running pmemsave 0xb8000 0x8000 screenDump.out in the monitor,
   and then examining every other byte of screenDump.out externally.
 - Other tools:
- I can mount the first install floppy in Linux if I skip the
  first track:
mount -t sysv -r -o loop,offset=15K $installDisk /mnt/misc
- I can also mount the UNIX hard drive in Linux, but I don't
  know a good way to find the correct offset.  UNIX seems to use
  it's own partition scheme within a DOS-style partition, so it
  doesn't work to just use the offset of the (DOS) partition.
  kpartx and pvscan sounded promising, but only seem to find DOS
  partitions.  Perhaps reboot with the max_part option on a
  kernel configured with the correct partitioning scheme
  enabled?  I found the offset by brute force trying every sector
  on the the above hard disk.  The actual number likely depends
  on a lot of things.
mount -t sysv -r -o loop,offset=5178880 $diskC /mnt/misc
- GNU objdump can dissassemble the kernel with something
  like objdump -s -d $MOUNTPOINT/unix from Linux, including
  function names but not much else.  But objdump needs to be
  configured with something like:
./configure -enable-target=i386-foobar-coff
- gdb can recognize function names from UNIX kernel if configured
  with something like ./configure -target=i386-foobar-coff.  Use
  qemu's -s option, run gdb $MOUNTPOINT/unix, and issue the
  gdb command target remote:1234.  After the floppy boots (kernel
  loaded in RAM), but before it accesses the hard disk, I
  could set breakpoints early in panic like break splintpanic2.
  I could examine registers (info registers or
  info all-registers) and memory, but the call stack
  tended to be truncated early.

==

Matthew Ogilvie (6):
  fix some debug printf format strings
  target-i386/translate.c: mov to/from crN/drN: ignore mod bits
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  i8259: add -no-spurious-interrupt-hack option

 cpu-exec.c  | 12 +
 hw/cirrus_vga.c |  4 +--
 hw/i8259.c  | 21 +++-
 hw/ide/cmd646.c |  5 ++--
 hw/ide/via.c|  5 ++--
 hw/pc.h |  4 +++
 hw/vga.c| 39 +++--
 qemu-options.hx | 38 +++-
 sysemu.h|  1 +
 target-i386/translate.c | 14 ---
 vl.c| 66 +
 11 files changed, 163 insertions(+), 46 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v2 6/6] i8259: add -no-spurious-interrupt-hack option

[Matthew Ogilvie]

This patch provides a way to optionally suppress spurious interrupts,
as a workaround for systems described below:

Some old operating systems do not handle spurious interrupts well,
and qemu tends to generate them significantly more often than
real hardware.

Examples:
  - Microport UNIX System V/386 v 2.1 (ca 1987)
(The main problem I'm fixing: Without this patch, it panics
sporadically when accessing the hard disk.)
  - ATT UNIX System V/386 Release 4.0 Version 2.1a (ca 1991)
See screenshot in QEMU Official OS Support List:
http://www.claunia.com/qemu/objectManager.php?sClass=applicationiId=9
(I don't have this system to test.)
  - A report about OS/2 boot lockup from 2004 by Hampa Hug:
http://lists.nongnu.org/archive/html/qemu-devel/2004-09/msg00367.html
(My patch was partially inspired by his.)
Also: http://lists.nongnu.org/archive/html/qemu-devel/2005-06/msg00243.html
(I don't have this system to test.)

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Note: checkpatches.pl gives an error about initializing the global 
int no_spurious_interrupt_hack = 0;, even though existing lines
near it are doing the same thing.  Should I give precedence to
checkpatches.pl, or nearby code?

There was no version 1 of this patch; this was the last thing I had to
work around to get UNIX running.

High level symptoms:
   1. Despite using this UNIX system for nearly 10 years (ca 1987-1996)
  on an early 80386, I don't remember ever seeing any crash like
  this.  I vaguely remember I may have had one or two crashes for
  which I don't have other explanations that perhaps could have
  been this, but I don't remember the error messages to confirm it.
   2. It is somewhat random when UNIX crashes when running in qemu.
   - Sometimes it crashes the first time the floppy-based installer
 tries to access the hard disk (partition table?).
   - Other times (though fairly rarely), it actually finishes
 formatting and copying the first disk's files to the
 hard disk without crashing.
   - On the other hand, I've never seen it successfully boot from
 the hard disk without this patch.  An attempt to boot from
 the hard drive always panics quite early.
   3. I tried -win2k-hack instead, thinking maybe the hard disk is just
  responding faster than UNIX expected.  But it doesn't seem
  to have any effect.  UNIX still panics sporadically the same way.
   - TANGENT: I was going to see if my patch provides an
 alternative fix for installing Windows 2000, but
 I was unable to reproduce the original -win2k-hack problem at
 all (with neither -win2k-hack NOR this patch).  Maybe
 some other change has fixed it some other way?  Or maybe
 it is only an issue in configurations I didn't test?
 (KVM instead of TCG?  Less RAM?  Something else?)
It might be worth doing a little more investigation,
 and eliminating the -win2k-hack option if appropriate.
   4. If I enable KVM, I get a different error very early in
  bootup (in splx function instead of splint), and this patch
  doesn't help.


My low level analysis of what is going on:

It is hard to track down all the details, but based on logging a
lot of qemu IRQ stuff, and setting a breakpoint in the earliest
panic-related UNIX function using gdb, it looks like:

   1. It is near the end of servicing a previous IRQ14 from the
  hard disk.
   2. The processor has interrupts disabled (I think), while UNIX
  clears the slave 8259's IMR (mask) register (sets it to 0), allowing
  all interrupts to be passed on to the master.
   3. While in that state, IRQ14 is raised (on the slave), which
  gets propagated to the master (IRQ2), but the CPU
  is not interrupted yet.
   4. UNIX then masks the slave 8259's IMR register
  completely (sets to 0xff).
   5. Because the master elcr register is set (by BIOS; UNIX never
  touches it) to edge trigger for IRQ2, the master latched on
  to IRQ2 earlier, and continues to assert the processors INT line
  (the env-interrupt_requestCPU_INTERRUPT_HARD bit) even
  after all slave IRQs have been masked off (clearing the input
  IRQ2).
   6. Finally, UNIX enables CPU interrupts and the interrupt is delivered
  to the CPU, which ends up as a spurious IRQ15 due to the
  slave's imr register.  UNIX doesn't know what to do with
  that, and panics/halts.

I'm not sure why it only sporadically hits this sequence of events.
There doesn't seem to be other IRQs asserted or serviced anywhere
in the near past; the last several were all IRQ14's.  But I can't
help feeling I'm not reading the log output correctly or something,
because that doesn't make sense.  Maybe there is there some kind
of a-few-instructions delay before a CPU interrupt is actually
deliviered after interrupts are enabled, or some delay in raising

[Qemu-devel] [PATCH v2 3/6] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Version 1 of this patch had the confusing subject
Re: [Qemu-devel] [PATCH 0/3] Attempting to run Microport UNIX (ca 1987).

This version reworks things a little to avoid duplicated code between
the -hda and just a file cases, based on Markus Armbruster's
comments.

An alternative approach would be to just get rid of the -hdachs
option, and require the use of -drive for anything that -hda can't
handle by itself.



 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 7c577fa..febfd62 100644
--- a/vl.c
+++ b/vl.c
@@ -2352,8 +2352,9 @@ int main(int argc, char **argv, char **envp)
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
 DisplayChangeListener *dcl;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2408,8 +2409,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2457,7 +2457,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2475,21 +2475,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2523,7 +2510,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2555,7 +2545,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v2 2/6] target-i386/translate.c: mov to/from crN/drN: ignore mod bits

[Matthew Ogilvie]

From AMD's documentation (multiple versions of 24594.pdf):
 This instruction is always treated as a register-to-register (MOD = 11)
 instruction, regardless of the encoding of the MOD field in the MODR/M
 byte.

Also, Microport UNIX System V/386 v 2.1 (ca 1987) runs fine on
real Intel 386 and 486 CPU's (at least), but does not run in qemu without
this patch.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

This version of the patch tweaks some of the comments to refer to
AMD's documentation, based on malc av1...@comtv.ru's response to
version 1.  It is functionally identical.



 target-i386/translate.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7ab2ccb..eb0cabc 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7551,8 +7551,11 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 gen_exception(s, EXCP0D_GPF, pc_start - s-cs_base);
 } else {
 modrm = cpu_ldub_code(cpu_single_env, s-pc++);
-if ((modrm  0xc0) != 0xc0)
-goto illegal_op;
+/* Ignore the mod bits (assume (modrm0xc0)==0xc0).
+ * AMD documentation (24594.pdf) and testing of
+ * intel 386 and 486 processors all show that the mod bits
+ * are assumed to be 1's, regardless of actual values.
+ */
 rm = (modrm  7) | REX_B(s);
 reg = ((modrm  3)  7) | rex_r;
 if (CODE64(s))
@@ -7594,8 +7597,11 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 gen_exception(s, EXCP0D_GPF, pc_start - s-cs_base);
 } else {
 modrm = cpu_ldub_code(cpu_single_env, s-pc++);
-if ((modrm  0xc0) != 0xc0)
-goto illegal_op;
+/* Ignore the mod bits (assume (modrm0xc0)==0xc0).
+ * AMD documentation (24594.pdf) and testing of
+ * intel 386 and 486 processors all show that the mod bits
+ * are assumed to be 1's, regardless of actual values.
+ */
 rm = (modrm  7) | REX_B(s);
 reg = ((modrm  3)  7) | rex_r;
 if (CODE64(s))
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v2 5/6] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Note: checkpatches.pl gives an error about initializing the global 
int vga_cga_hacks = 0;, even though existing lines near it are
doing the same thing.  Should I give precedence to checkpatches.pl, or
nearby code?

Another approach would be to add real MDA and/or CGA support.
Perhaps -vga mda and/or -vga cga command line options, and
then make a split off hw/mda.c that is much simplified compared to
vga.c.

Note that even if qemu had a real -vga cga option, something like
this patch might still be useful to allow running both old/fragile
CGA programs and newer VGA programs in a single virtual machine.
One downside of real CGA is the relatively ugly low-res font
CGA uses (I think) for text mode, not just graphics mode.

Although irrelevant to UNIX, this idea of implementing old graphics
cards could be further extended:
  - -vga hgc (Hercules) support would probably be easy to add
to MDA, after MDA was cleanly separated out.
  - -vga hgc+cga, (or perhaps -vga hgc,secondary=cga,
or something else), to support a dual graphics card
configurations.  I believe the real hardware supported various
combinations: (hgc|mda)\+(cga|std|...),
(cga|std|...)\+(hgc|mda), cga\+(std|...), and (std|...)\+cga.
Also, with a PCI bus, two of the same kind of card may be supported
in some cases.  In such a configuration, I would expect each
graphics card would have it's own display window in the host OS.
  - Also, in my research about CGA stuff, I noticed that there were
a few other old rare cards that appear to be minor variations of CGA
and/or MDA (a combination CGA/HGC, a color HGC, a higher-resolution
CGA, etc).  Once you had a clean plain-CGA implementation,
many of these could probably be implemented as optional enable flags
fairly easily.

Does anyone have any thoughts about the best way of adding MDA, HGC,
CGA, or similar old video cards to qemu?



 hw/pc.h |  4 
 hw/vga.c| 39 +++
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index e4db071..37e2f87 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -176,6 +176,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 static inline DeviceState *isa_vga_init(ISABus *bus)
 {
 ISADevice *dev;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..08ec4bd 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -547,14 +547,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val

[Qemu-devel] [PATCH v2 4/6] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

This is the first version of this patch.  I noticed this was missing
when I wanted to add documentation for my own VGA option in the next
patch...

===

 qemu-options.hx | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 3c411c4..104d228 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -944,7 +944,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -971,6 +971,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

Re: [Qemu-devel] [PATCH 0/3] Attempting to run Microport UNIX (ca 1987)

[Matthew Ogilvie]

On Sat, Jul 28, 2012 at 08:33:54AM +0200, Markus Armbruster wrote:
 Matthew Ogilvie mmogilvi_q...@miniinfo.net writes:
[...]
1. It doesn't seem to recognize the hard drive geometry, even
   if I use -hdachs and keep it carefully inside ancient
   limitations.  Note that at the time, hard drives did not support
   self-identification commands for the geometry; you had to
   configure it in BIOS.   I also have some old notes from when my
   dad was asking Microport about compatibility; apparently
   they wanted to know the specific BIOS version in order to
   decide about compatibility.  Maybe UNIX is bypassing later
   standards for looking up geometry, and trying to get it in
   some non-standard way (straight from CMOS or something?)
 
 Please run QEMU with -trace events=trace-events, where trace-events is a
 file containing the line hd_geometry_*.  Post results, along with your
 full command line.
 
 [...]

Thanks for the suggestion.  I've narrowed down a couple of problems
based on it:

First, the -hdachs command line option is silently ignored depending
on its relative order compared to other command line options.  I've
attached a patch below.



Second, this UNIX kernel and/or bootloader always seems to think
it has 17 sectors per track, no matter what I tell qemu (using a blank
image; no partition table).  But UNIX does recognize the number
of cylinders and heads correctly, and I've verified that
the number of sectors per track is also correct in the fixed disk
parameter table, by using the monitor's x (examine) command. 

Potentially guess_chs_for_size() could be enhanced to return 17
for small drive images when appropriate, to be consistent with most
legacy drives having 17.  Or maybe there is some more
obscure way to get UNIX to recognize non-17 sectors.
Or either document a third party way to put a basic partition table
on a new image, or add that ability to qemu-img, so that code
like that in guess_disk_lchs() or similar code that might exist
in some operating systems would have something to work with.
I also found a 1988 posting that suggests some leads for
further investigation:
http://www.megalextoria.com/usenet-archive/news076f1/b96/comp/unix/microport/0798.html

But I'm not really inclined to worry much more about this
right now, because I have a usable workaround: Always define
my disk image size to be consistent with 17 sectors per
track, and tell qemu to use 17.



(Now if I can just figure out what's up with the sporadic panic
in UNIX's interrupt handlers...)



From: Matthew Ogilvie mmogilvi_q...@miniinfo.net
Date: Sat, 28 Jul 2012 17:01:14 -0600
Subject: [PATCH] vl.c: fix -hdachs/-hda argument order parsing issues

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems
reasonable.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index c18bb80..e0611eb 100644
--- a/vl.c
+++ b/vl.c
@@ -2374,7 +2374,18 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+char buf[256];
+if (cyls == 0)
+snprintf(buf, sizeof(buf), %s, HD_OPTS);
+else
+snprintf(buf, sizeof(buf),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], buf);
 } else {
 const QEMUOption *popt;
 
@@ -2404,7 +2415,7 @@ int main(int argc, char **argv, char **envp)
  ,trans=lba :
  translation == BIOS_ATA_TRANSLATION_NONE ?
  ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, buf);
 break;
 }
 case QEMU_OPTION_hdb:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 0/3] Attempting to run Microport UNIX (ca 1987)

[Matthew Ogilvie]

I've recently been trying to get an ancient version of UNIX
I used to use working in qemu:

Micoport UNIX System V/386, v 2.1 (ca 1987).

I used this from about 1987 until about 1996, when I first
got Linux.

With a couple of small patches, I can get qemu to boot and run UNIX
from the first install floppy, although I still have some sporadic
issues accessing the hard drive.

I don't know of any reason the first two patches of this series
shouldn't be applied now.

But the last patch (VGA) is a temporary hack just to make UNIX
usuable, and is currently not appropriate for general use.  The
commit comment discusses some possible strategies for improving
it; does anyone want to weigh in?

I've also noticed some other problems that I have not yet
tracked down:

---
Hard Drive Issues:
---

I also encounter a couple of hard drive issues, which I haven't
investigated as carefully, nor attempted to work around:

  1. It doesn't seem to recognize the hard drive geometry, even
 if I use -hdachs and keep it carefully inside ancient
 limitations.  Note that at the time, hard drives did not support
 self-identification commands for the geometry; you had to
 configure it in BIOS.   I also have some old notes from when my
 dad was asking Microport about compatibility; apparently
 they wanted to know the specific BIOS version in order to
 decide about compatibility.  Maybe UNIX is bypassing later
 standards for looking up geometry, and trying to get it in
 some non-standard way (straight from CMOS or something?)

  2. Although it can access the hard drive for a short time, when
 accessing the hard disk, the kernel will sporadially panic
 with a message about a logic error in a splint() method,
 which dissasembly shows is called from interrupt
 handlers.  (See also
 http://www.linuxmisc.com/29-unix-internals/4e6c1f6fa2e41670.htm
 for some general background information I found about UNIX
 kernel spl*() functions.)

I haven't spent much time investigating either of these yet, but
in my limited investigation so far, it hasn't yet run long enough
to complete the first phase of installation to the hard drive (when
it is running from floppy).

---
KVM Issue:
---

Finally, if KVM is enabled, I get different results.  It (somehow)
seems to make it past the control register instructions even with or
without my patch 2, but the UNIX kernel very quickly panics with
a logic error in an splx() method [apparently for restoring
interrupt controller bitmasks; see link above], before it runs
any userspace code.

I haven't investigated this, either.

---

If anyone is interested in examining this system for themselves,
contact me and I can send you at least the first (bootable)
installation floppy image.

--

Matthew Ogilvie (3):
  fix debug printf 64bit format strings
  target-i386/translate.c: mov to/from crN/drN: ignore mod bits
  HACKS to make vga text mode work with old Microport UNIX (ca 1987)

 hw/cirrus_vga.c |  4 ++--
 hw/ide/cmd646.c |  5 +++--
 hw/ide/via.c|  5 +++--
 hw/vga.c|  5 +++--
 target-i386/translate.c | 16 
 5 files changed, 23 insertions(+), 12 deletions(-)

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 1/3] fix some debug printf 64bit format strings

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/cirrus_vga.c | 4 ++--
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 623dd68..24ddea6 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2055,8 +2055,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value % PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index bf8ece4..5ae9e25 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t 
addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index eec5136..96c6273 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, target_phys_addr_t 
addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH 3/3] HACKS for vga text mode with Microport UNIX (ca 1987)

[Matthew Ogilvie]

Background:


I don't really think it is appropriate to include this patch in it's
current form, but I'm posting it to illustrate some of the obscure
things that Microport UNIX System V/386 v 2.1 (ca 1987) is trying
to do to the video card.

On real hardware, this version of UNIX could drive an MDA graphics
card (actually an HGC [hurcules graphics card] in text mode only)
just fine.  BUT, the VGA card and monitor would always
display snow (timings were out of sync, and/or other problems).
In 1987, I initially had an HGC-only system, and added a VGA
to it later as a second graphics card and monitor.  DOS could switch
between them (the mode command), but UNIX was only usable on the HGC.

The fact that it couldn't run a real VGA properly suggests that
hacking the emulated VGA isn't really the best solution.  A better
approach might be to add specific support for graphics cards
(MDA, HGC, and/or maybe plain-CGA) that UNIX works properly on.

If anyone is interested in examining this UNIX system
directly (for these VGA issues or other issues), let me know and
I can email you at least the first (bootable) installation
floppy image.


Visible Problems in Qemu:


Running and debugging UNIX in qemu, I see two main VGA-related
problems, and a few associated nits (not counting non-VGA issues
written up separately):

1. (Blank screen): UNIX weirdly writes exactly
   one byte to 0x3c0 (VGA_ATT_W).  After reading
   from 0x3da (which resets 0x3c0 to index mode),
   it writes a 0 to 0x3c0, and then doesn't touch the register again.
   It doesn't actually modify the contents of the data behind any index.
   Writing zero also clears the 0x20 palette access bit, which forces
   qemu vga_update_display() to treat the screen as GMODE_BLANK.
- My hackish workaround will not blank the screen if the index
  register is still 0.  Perhaps this hack could be (slightly)
  improved to look at the index/data flip-flop as well, under
  the theory that until the OS actually starts modifying things,
  it is OK for the card to keep using the old palette?

2. (Characters cut in half): UNIX appears to be trying
   to program the CRTC under the assumption that the text
   mode is using a font that is only 8 pixels high, instead
   of 16.  I would guess it is trying to treat it like a CGA card.
- (main problem): It changes CRTC register 9 (0x3d4[9]/0x3d5)
  from 0x4f to 0x07.  The 0xf vs 7 halves the height of
  the character, cutting off the bottom halves.  The 0x40
  bit doesn't seem to hurt, but documentation explains how
  it has something to do with split screen scrolling.
- My hackish workaround is to ignore attempts to modify
  register 9 if the lock bit in register 0x11 is locked.
- UNIX is also changing the cursor-height-within-a-character
  values from (0xe through 0xf) to (6 through 7), effectively
  moving the cursor to the middle of the character.
  (I haven't tried to work around this, but it would be
  trivial to disable these registers under some conditions TBD.
  Or perhaps translate the supplied values with scaling logic.)
- UNIX changes register 8 (0x3d4[8]/0x3d5) from 0 to 2, but
  this doesn't seem to hurt.  (Documentation says it has to
  do with smooth text mode scrolling, but UNIX only sets it
  once...)
- UNIX also attempts to change registers 0-7 (0x3d4[0-7]/0x3d5), but
  these attempts are ignored by qemu because of the lock bit
  in register 0x11 (0x3d4[0x11]/0x3d5).  (UNIX doesn't seem to
  know anything about registers greater than 0xf.)

3. I haven't looked to see if it programming other VGA subsystems
   strangely, but it wouldn't surprise me.


Possible Real Solutions:


1. The ideal solution would probably be to implement an emulated plain MDA
   adapter, and a way to select it on the command line.  (It could also be
   extended to optionally be HGC-capable and/or to support a
   dual monitor with a CGA/VGA [with two windows for the two
   monitors], as well.)
  Superficially, emulating an MDA or HGC card itself probably
   wouldn't be too hard (mostly just copy, strip down [simplify], and
   tweak the VGA support), but I'm still a little vague on the best
   way to get it all hooked into the overall system cleanly,
   and appropriate configurations options (perhaps options
   like -vga mda, -vga mda+std, -vgs cirrus+hgc, etc?).
  This seems like a moderate amount of work, but it has the highest
   likelyhood of being useful for running other ancient software,
   rather than a hackish workaround for one rare OS.

2. Perhaps add some special command line option to enable hacks similar
   to what I do in this patch (normally disabled).  This is probably
   the easiest.  Any suggestions for command-line options, etc?

3. I wonder if maybe an alternative approach to these problems would be
   to somehow identify that the target is trying to use the VGA 

[Qemu-devel] [PATCH 2/3] target-i386/translate.c: mov to/from crN/drN: ignore mod bits

[Matthew Ogilvie]

Microport UNIX System V/386 v 2.1 (ca 1987) uses mod R/M bytes for
the control register mov instructions where the mod bits are 0,
even though the 80386 spec claims they are always 1's.  The fact
that it ran at all clearly indicates the real chips (at least 386
and 486) just ignores the bits and assumes they are 1's, rather
than trigger an illegal instruction if they aren't.

Also fixed: The dissassembled kernel also accesses debug
registers in a similar way, although other problems prevent
me verifiing that those instructions are reachable in UNIX.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

Alternatives?:

Potentially someone might want to make this dependent on some kind
of configuration option (what specific CPU it is emulating, or some
kind of quirks flag).

Or somehow log if it encounters unspecified instructions
like this, as a kind of warning mechanism for someone debugging
an OS.  (Although I'm not sure exactly what the qemu way to
log such a thing would be.)

But my initial thought is that neither of these are worth the effort.

--
Matthew Ogilvie   [mmogilvi_q...@miniinfo.net]
--

 target-i386/translate.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 1988dae..d056842 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7465,8 +7465,12 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 gen_exception(s, EXCP0D_GPF, pc_start - s-cs_base);
 } else {
 modrm = ldub_code(s-pc++);
-if ((modrm  0xc0) != 0xc0)
-goto illegal_op;
+/* Ignore the mod bits (assume (modrm0xc0)==0xc0).
+ * The 80386 reference manual says the bits are
+ * always 1, and doesn't say what happens if they aren't.
+ * But testing shows that the bits are just assumed to be
+ * 1s.
+ */
 rm = (modrm  7) | REX_B(s);
 reg = ((modrm  3)  7) | rex_r;
 if (CODE64(s))
@@ -7507,8 +7511,12 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 gen_exception(s, EXCP0D_GPF, pc_start - s-cs_base);
 } else {
 modrm = ldub_code(s-pc++);
-if ((modrm  0xc0) != 0xc0)
-goto illegal_op;
+/* Ignore the mod bits (assume (modrm0xc0)==0xc0).
+ * The 80386 reference manual says the bits are
+ * always 1, and doesn't say what happens if they aren't.
+ * But testing shows that the bits are just assumed to be
+ * 1s.
+ */
 rm = (modrm  7) | REX_B(s);
 reg = ((modrm  3)  7) | rex_r;
 if (CODE64(s))
-- 
1.7.10.2.484.gcd07cc5