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 <address@hidden> 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=ogilvie&group=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, 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
> >
> > Jan