On Thu, Aug 27, 2015 at 1:47 PM, Alistair Francis <[email protected]> wrote: > The AHCIState strucut can either have AHCIPCIState or SysbusAHCIState
"struct" > as a parent. The ahci_irq_lower() and ahci_irq_raise() functions > assume that it is always AHCIPCIState, which is not always the > case, which causes a seg fault. Verify what the container of AHCIState > is before setting the PCIDevice struct. > > Signed-off-by: Alistair Francis <[email protected]> > --- > > hw/ide/ahci.c | 29 +++++++++++++++++++++++------ > hw/ide/ahci.h | 2 ++ > 2 files changed, 25 insertions(+), 6 deletions(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 02d85fa..b45b21d 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -121,9 +121,17 @@ 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 = > - (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE); Why can't you just convert this to use use s->container? ... > + DeviceState *dev_state = DEVICE(s->container); Unneeded cast. s->container is already DeviceState, but I'm not sure you need this local variable at all. > + PCIDevice *pci_dev = NULL; > + ObjectClass *ret; > + > + /* Check is AHCIState's parent is SysbusAHCIState or AHCIPCIState */ > + ret = object_class_dynamic_cast(object_get_class(OBJECT(dev_state)), > + TYPE_PCI_DEVICE); ... I don't understand why this needs to be done as a class cast, does the original object cast approach not work? Regards, Peter > + if (ret) { > + /* AHCIState parent is AHCIPCIState */ > + pci_dev = PCI_DEVICE(dev_state); > + } > > DPRINTF(0, "raise irq\n"); > > @@ -136,9 +144,17 @@ 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); > + DeviceState *dev_state = DEVICE(s->container); > + PCIDevice *pci_dev = NULL; > + ObjectClass *ret; > + > + /* Check is AHCIState's parent is SysbusAHCIState or AHCIPCIState */ > + ret = object_class_dynamic_cast(object_get_class(OBJECT(dev_state)), > + TYPE_PCI_DEVICE); > + if (ret) { > + /* AHCIState parent is AHCIPCIState */ > + pci_dev = PCI_DEVICE(dev_state); > + } > > DPRINTF(0, "lower irq\n"); > > @@ -1436,6 +1452,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev, > AddressSpace *as, int ports) > s->as = as; > s->ports = ports; > s->dev = g_new0(AHCIDevice, ports); > + s->container = qdev; > ahci_reg_init(s); > /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now > */ > memory_region_init_io(&s->mem, OBJECT(qdev), &ahci_mem_ops, s, > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index c055d6b..c9b3805 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -287,6 +287,8 @@ struct AHCIDevice { > }; > > typedef struct AHCIState { > + DeviceState *container; > + > AHCIDevice *dev; > AHCIControlRegs control_regs; > MemoryRegion mem; > -- > 1.7.1 >
