Re: [Qemu-devel] [PATCH] ahci: fix sysbus support

[Rob Herring]

On Mon, Feb 10, 2014 at 6:32 PM, Peter Maydell <address@hidden> wrote:
> On 10 February 2014 23:02, Rob Herring <address@hidden> wrote:
>> From: Rob Herring <address@hidden>
>>
>> Non-PCI AHCI support is broken due to assertion failures when trying
>> to convert AHCIState to a PCIDevice pointer as AHCIState can have
>> different container structs. Fix this by using the non-asserting object
>> cast and checking the returned pointer is not NULL.
>>
>> The AddressSpace pointer is also being initialized to NULL and causing
>> dma_memory_map call to fail. Fix this by initializing to
>> address_space_memory for sysbus instances.
>>
>> Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
>> for sysbus instances.
>>
>> Signed-off-by: Rob Herring <address@hidden>
>> ---
>>  hw/ide/ahci.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index fbea9e8..55f984e 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -118,11 +118,11 @@ static uint32_t  ahci_port_read(AHCIState *s, int 
>> port, int offset)
>>  static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
>>  {
>>      AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
>> -    PCIDevice *pci_dev = PCI_DEVICE(d);
>> +    PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), 
>> TYPE_PCI_DEVICE);
>>
>>      DPRINTF(0, "raise irq\n");
>>
>> -    if (msi_enabled(pci_dev)) {
>> +    if (pci_dev && msi_enabled(pci_dev)) {
>>          msi_notify(pci_dev, 0);
>>      } else {
>>          qemu_irq_raise(s->irq);
>> @@ -132,10 +132,11 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice 
>> *dev)
>>  static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
>>  {
>>      AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
>> +    PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), 
>> TYPE_PCI_DEVICE);
>>
>>      DPRINTF(0, "lower irq\n");
>>
>> -    if (!msi_enabled(PCI_DEVICE(d))) {
>> +    if (!pci_dev || !msi_enabled(pci_dev)) {
>>          qemu_irq_lower(s->irq);
>>      }
>>  }
>
> Cc'ing Andreas in case he knows a neater way of doing those casts.

Any comments on this? If not, can someone apply this?

>> @@ -1311,7 +1312,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
>>      .name = "sysbus-ahci",
>>      .unmigratable = 1, /* Still buggy under I/O load */
>>      .fields = (VMStateField []) {
>> -        VMSTATE_AHCI(ahci, AHCIPCIState),
>> +        VMSTATE_AHCI(ahci, SysbusAHCIState),
>>          VMSTATE_END_OF_LIST()
>>      },
>
> ...I wonder if that fixes the "buggy under I/O load" issue the
> comment is talking about. Jason, do you still have whatever
> testcase you were using when you added that comment?
>
> thanks
> -- PMM