qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: "Elena Ufimtseva" <elena.ufimtseva@oracle.com>,
	"John G Johnson" <john.g.johnson@oracle.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Jagannathan Raman" <jag.raman@oracle.com>,
	qemu-block@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
	"John Snow" <jsnow@redhat.com>,
	qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine
Date: Wed, 28 Apr 2021 16:21:13 +0200	[thread overview]
Message-ID: <87im461odi.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <YIknpUAyIn9hTzpl@stefanha-x1.localdomain> (Stefan Hajnoczi's message of "Wed, 28 Apr 2021 10:15:17 +0100")

Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Tue, Apr 27, 2021 at 07:54:21PM +0200, Philippe Mathieu-Daudé wrote:
>> On 4/27/21 7:16 PM, John Snow wrote:
>> > On 4/27/21 9:54 AM, Stefan Hajnoczi wrote:
>> >> I suggest fixing this at the qdev level. Make piix3-ide have a
>> >> sub-device that inherits from ISA_DEVICE so it can only be instantiated
>> >> when there's an ISA bus.
>> >>
>> >> Stefan
>> > 
>> > My qdev knowledge is shaky. Does this imply that you agree with the
>> > direction of Thomas's patch, or do you just mean to disagree with Phil
>> > on his preferred course of action?
>> 
>> My understanding is a disagreement to both, with a 3rd direction :)
>> 
>> I agree with Stefan direction but I'm not sure (yet) that a sub-device
>> is the best (long-term) solution. I guess there is a design issue with
>> this device, and would like to understanding it first.
>> 
>> IIUC Stefan says the piix3-ide is both a PCI and IDE device, but QOM
>> only allow a single parent. Multiple QOM inheritance is resolved as
>> interfaces, but PCI/IDE qdev aren't interfaces, rather abstract objects.
>> So he suggests to embed an IDE device within the PCI piix3-ide device.
>> 
>> My view is the PIIX is a chipset that share stuffs between components,
>> and the IDE bus belongs to the chipset PCI root (or eventually the
>> PCI-ISA bridge, function #0). The IDE function would use the IDE bus
>> from its root parent as a linked property.
>> My problem is currently this device is user-creatable as a Frankenstein
>> single PCI function, out of its chipset. I'm not sure yet this is a
>> dead end or I could work something out.
>
> Kevin and Paolo previously pointed out that piix3-ide is sometimes used
> with the Q35 machine type. The user-creatable piix3-ide device needs to
> be deprecated before it can be dropped. That's a long process that
> cannot fix the current crash any time soon.
>
> I do support deprecating the user-creatable piix3-ide device in favor of
> a proper Q35 Legacy IDE implementation. The main problem is this
> involves a bunch of work and I'm not sure who would do it (the payoff is
> not very high).

In my opinion, letting users plug device models for PCI *functions* as
if they were *devices* was a mistake.  Compounding the mistake of not
modelling the difference between PCI function and PCI device.

The more of them we can deprecate, the better.



  reply	other threads:[~2021-04-28 14:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 12:52 [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine Thomas Huth
2021-04-27 13:27 ` Philippe Mathieu-Daudé
2021-04-27 13:54   ` Stefan Hajnoczi
2021-04-27 17:16     ` John Snow
2021-04-27 17:54       ` Philippe Mathieu-Daudé
2021-04-27 18:02         ` John Snow
2021-04-28  9:22           ` Stefan Hajnoczi
2021-04-28 14:18             ` Markus Armbruster
2021-04-28 18:43               ` Stefan Hajnoczi
2021-04-29  6:08                 ` Markus Armbruster
2021-05-03 17:53                   ` Philippe Mathieu-Daudé
2021-04-28  9:15         ` Stefan Hajnoczi
2021-04-28 14:21           ` Markus Armbruster [this message]
2021-04-27 18:06 ` John Snow
2021-04-28  9:24 ` Stefan Hajnoczi
2021-04-28  9:32   ` Thomas Huth
2021-04-28 10:53     ` Philippe Mathieu-Daudé
2021-05-18 19:21     ` John Snow
2021-05-18 21:07     ` John Snow
2021-07-06  8:24 ` Thomas Huth
2021-07-06  8:37   ` Philippe Mathieu-Daudé
2021-07-07  9:30     ` Stefan Hajnoczi

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=87im461odi.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=elena.ufimtseva@oracle.com \
    --cc=jag.raman@oracle.com \
    --cc=john.g.johnson@oracle.com \
    --cc=jsnow@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    /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).