On Tue, Apr 27, 2021 at 02:02:27PM -0400, John Snow wrote: > On 4/27/21 1:54 PM, 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. > > > > Regards, > > > > Phil. > > > > It sounds complicated. In the meantime, I think I am favor of taking > Thomas's patch because it merely adds some error routing to allow us to > avoid a crash. The core organizational issues of the IDE device(s) will > remain and can be solved later as needed.
The approach in this patch is okay but we should keep in mind it only solves piix3-ide. ISA provides a non-qdev backdoor API and there may be more instances of this type of bug. A qdev fix would address the root cause and make it possible to drop the backdoor API, but that's probably too much work for little benefit. Stefan
signature.asc
Description: PGP signature
