[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 2/4] ahci.c: Don't assume AHCIState's parent
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v6 2/4] ahci.c: Don't assume AHCIState's parent is AHCIPCIState |
Date: |
Mon, 31 Aug 2015 18:38:23 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 08/28/2015 08:04 PM, Alistair Francis wrote:
> The AHCIState struct can either have AHCIPCIState or SysbusAHCIState
> 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 <address@hidden>
> ---
> V5:
> - Remove the return checks when setting PCIDevice
> V4:
> - Remove unnesicary casts
> - Use object_dynamic_cast() instead of object_class_dynamic_cast()
>
> hw/ide/ahci.c | 13 +++++++------
> hw/ide/ahci.h | 2 ++
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 02d85fa..d83efa4 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -121,9 +121,9 @@ 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);
> + DeviceState *dev_state = s->container;
> + PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
> + TYPE_PCI_DEVICE);
>
> DPRINTF(0, "raise irq\n");
>
> @@ -136,9 +136,9 @@ 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 = s->container;
> + PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
> + TYPE_PCI_DEVICE);
>
> DPRINTF(0, "lower irq\n");
>
> @@ -1436,6 +1436,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;
>
This is kind of ugly ... but it works, and it doesn't impact migratability.
If someone abstracts MSI away from AHCI in the future, this can be
un-done and the state cleaned up again.
Doesn't break anything, so:
Acked-by: John Snow <address@hidden>