On Wed, Aug 26, 2015 at 4:11 PM, John Snow <[email protected]> wrote: > > > On 08/26/2015 06:15 PM, Peter Crosthwaite wrote: >> On Wed, Aug 26, 2015 at 2:46 PM, John Snow <[email protected]> wrote: >>> >>> >>> On 08/26/2015 05:02 PM, Peter Crosthwaite wrote: > > [snip] > >>>> >>>> So there a few problems in the way of a correct solution. The caller >>>> for ahci_lower_irq does not have access to the QOM object pointer, >>>> it's been abstracted away by AHCIState (which is not a QOM object). So >>>> you would need to replumb the call path to ahci_lower_irq to pass the >>>> QOM object. This would let you drop the container_of completely.
Ok, I think I have figured it out. It took me a while to get my head around what is going on here. I'm just testing then I'll send a new version of the series. Thanks for your help Peter. Thanks, Alistair >>>> >>>> The next step would be to virtualise ahci_lower_irq, as this is >>>> implementation dependent (assume specific devices really do need to >>>> control the use of PCI MSI?), one implementation for sysbus, one for >>>> PCI. This is blocked by the re-plumbing described above as the >>>> virtualised called itself will need a ptr to the QOM object. >>>> >>>> But I think the back ptr is an acceptable solution for the meantime, >>>> this is a clear bug in Sysbus AHCI and should probably even go to >>>> qemu-stable. >>>> >>> >>> I'm not intricately familiar with how the QOM plumbing works, but I can >>> definitely see how assuming all AHCIState pointers come from >>> AHCIPCIState is a problem... >>> >>> For the uninitiated, how does MSI work with Sysbus? >> >> No such thing :) Interrupts in Sysbus shouldn't really exist and those >> that do are just a thin wrapper around raw pins. The short answer is >> MSI is a PCI specific concept and sysbus is an alternative to PCI. >> Long answer, is any form of MSI requires some sort of transport layer >> capable of messaging an interrupt controller from a device with the >> device as the initiator. This is not possible in most real busses >> which we use the sysbus abstraction for. >> > > Ah, OK ... I seemed to recall there being some MSI bits in the HBA > registers, so I was confused. However, I think it's just a read-only > status bit, which allows it to just always be off for your device. > >>> What does a Sysbus >>> AHCI device look like to a guest, and what happens if it tries to >> >> Sysbus AHCI is basically the raw MMIO regions mapped onto the "system >> bus" by the machine model. Usually this is done as static mapping. The >> most common user of this is ARM (and other embedded) SoCs. The >> downside is there is no standard for remapping the device or self >> identification of the device (as provided by PCI standard). The >> existance of the device is made known to a kernel usually by a device >> tree blob. >> >> In the Xilinx MPSoC case, the AHCI controller is on the SoC (with the >> ARM processor and many other IO peripherals). There are no cards or >> even inter-chip connectivites in Alistairs AHCI connection. >> >>> utilize the functionality? >>> >> >> If you rework an exsiting PCI driver to use raw MMIO regions and >> interrupts rather than bars and MSIs it usually works with minor edits >> only. SDHCI, AHCI, EHCI, xHCI at least all have both PCI >> implementations and real world sysbus implementations. >> >> What I would be interested in, is it possible to push all MSI code up >> to the PCI core to completely remove the need for all PCIisms in AHCI? >> This seems to be the only thing that is PCI specific. > > Fields that reference MSI in the AHCI spec are: > > GHC.MRSM (MSI Revert To Single Message, read-only, is 0 when MSI is off) > CCC_CTL.INT (Advertises MSI vector) > > You're save leaving both of these to zero, so I think there's nothing > else PCI-specific in the HBA region. > > Anyway, apologies for not noticing this while I was reworking the AHCI > device! > >> >> Regards, >> Peter >> >>> (Well, segfault, I guess.) >>> >>> If someone wants to clue in the device model newbie and send a patch my >>> way, I'll take it. >>> >>> --js >>> > > [snip] >
