qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: BALATON Zoltan <balaton@eik.bme.hu>
To: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
Cc: qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Anthony Liguori" <aliguori@amazon.com>,
	"Markus Armbruster" <armbru@redhat.com>
Subject: Re: [Qemu-devel] fix clearing i8259 IRQ lines (Was: Should the i8259 devices remain no-user?)
Date: Tue, 29 Oct 2013 17:27:15 +0100 (CET)	[thread overview]
Message-ID: <alpine.GSO.2.01.1310291656010.2709@mono> (raw)
In-Reply-To: <20131026200024.GA27159@comcast.net>

On Sat, 26 Oct 2013, Matthew Ogilvie wrote:
> 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:

There's at least one more guest that cares I know about which is less 
ancient but maybe just as unimportant: OPENSTEP for Mach. But nevertheless 
it still is a now known bug which just seems to be tolerated by the OS-es 
that are most commonly run under Qemu. What was not clear to me is how 
significant are the risks of the fix and if they were considered or the 
patch was just forgotten without ever getting the thought about merging 
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.

I had the impression from previous discussion that the main risk was a 
potential lost timer interrupt in some circumstances at migration which 
may affect some guests but it was not clear (to me at least) how big of a 
risk is this. IMO if other models depend on a bug they are also buggy and 
should be fixed but I don't know how many models could that affect.

> 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.)

I think you have already provided detailed analysis, test cases and 
multiple options and patch versions so it is not you who should spend more 
time on this now. What I think would be needed is that people who have the 
knowledge and insight to analyse and decide about the patches give it some 
time to think about it and come to a decision then tell what to do or why 
it's better to leave it unfixed. Can this be done in this thread? Or maybe 
on one of the upcoming phone conferences where the right people are 
together anyway to discuss it?

Regards,
BALATON Zoltan

  reply	other threads:[~2013-10-29 16:27 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-10 14:42 [Qemu-devel] [PATCH RFC 0/9] Clean up and fix no_user armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 1/9] qdev: Replace no_user by cannot_instantiate_with_device_add_yet armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 2/9] sysbus: Set cannot_instantiate_with_device_add_yet armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 3/9] apic: Document why cannot_instantiate_with_device_add_yet armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 4/9] pci-host: Consistently set cannot_instantiate_with_device_add_yet armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 5/9] ich9: Document why cannot_instantiate_with_device_add_yet armbru
2013-10-10 16:01   ` Paolo Bonzini
2013-10-10 16:03     ` Paolo Bonzini
2013-10-11  6:13       ` Markus Armbruster
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 6/9] piix3 piix4: " armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 7/9] vt82c686: " armbru
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 8/9] isa: Clean up use of cannot_instantiate_with_device_add_yet armbru
2013-10-15 12:43   ` [Qemu-devel] Should the i8259 devices remain no-user? (was: [PATCH RFC 8/9] isa: Clean up use of cannot_instantiate_with_device_add_yet) Markus Armbruster
2013-10-15 12:54     ` [Qemu-devel] Should the i8259 devices remain no-user? Paolo Bonzini
2013-10-16  9:51       ` Markus Armbruster
2013-10-16 11:06         ` Paolo Bonzini
2013-10-16 16:12           ` Markus Armbruster
2013-10-16 16:21           ` BALATON Zoltan
2013-10-16 16:23             ` Paolo Bonzini
2013-10-26 20:00               ` [Qemu-devel] fix clearing i8259 IRQ lines (Was: Should the i8259 devices remain no-user?) Matthew Ogilvie
2013-10-29 16:27                 ` BALATON Zoltan [this message]
2013-10-10 14:42 ` [Qemu-devel] [PATCH RFC 9/9] qdev: Do not let the user try to device_add when it cannot work armbru
2013-10-15 12:24 ` [Qemu-devel] Why is TYPE_CPU no-user? (was: [PATCH RFC 0/9] Clean up and fix no_user) Markus Armbruster
2013-10-15 12:37   ` Peter Maydell
2013-10-15 14:01     ` [Qemu-devel] Why is TYPE_CPU no-user? Markus Armbruster
2013-10-15 13:08   ` Andreas Färber
2013-10-16  9:55     ` Markus Armbruster
2013-10-15 13:21 ` [Qemu-devel] Which functions of southbridges should be no-user? (was: [PATCH RFC 0/9] Clean up and fix no_user) Markus Armbruster
2013-10-15 13:31   ` [Qemu-devel] Which functions of southbridges should be no-user? Andreas Färber
2013-10-15 14:41     ` Kevin Wolf
2013-10-15 14:53       ` Andreas Färber
2013-10-16  9:58         ` Markus Armbruster
2013-10-15 15:09       ` Anthony Liguori
2013-10-16 10:00         ` Markus Armbruster
2013-10-16 11:02           ` Andreas Färber
2013-10-17  9:47             ` Markus Armbruster
2013-10-16 17:53           ` Anthony Liguori
2013-10-17  9:49             ` Markus Armbruster

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=alpine.GSO.2.01.1310291656010.2709@mono \
    --to=balaton@eik.bme.hu \
    --cc=afaerber@suse.de \
    --cc=aliguori@amazon.com \
    --cc=armbru@redhat.com \
    --cc=mmogilvi_qemu@miniinfo.net \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

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

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