Patchwork [6/8] ahci: make number of ports runtime determined

login
register
mail settings
Submitter Alexander Graf
Date Dec. 20, 2010, 9:13 p.m.
Message ID <1292879604-22268-7-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/76229/
State New
Headers show

Comments

Alexander Graf - Dec. 20, 2010, 9:13 p.m.
Different AHCI controllers have a different number of ports, so the core
shouldn't care about the amount of ports available.

This patch makes the number of ports available to the AHCI core runtime
configurable, allowing us to have multiple different AHCI implementations
with different amounts of ports.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/ahci.c |   29 +++++++++++++++++++----------
 hw/ide/ahci.h |   10 +++++-----
 hw/ide/ich.c  |    3 ++-
 3 files changed, 26 insertions(+), 16 deletions(-)
Kevin Wolf - Jan. 18, 2011, 12:40 p.m.
Am 20.12.2010 22:13, schrieb Alexander Graf:
> Different AHCI controllers have a different number of ports, so the core
> shouldn't care about the amount of ports available.
> 
> This patch makes the number of ports available to the AHCI core runtime
> configurable, allowing us to have multiple different AHCI implementations
> with different amounts of ports.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/ide/ahci.c |   29 +++++++++++++++++++----------
>  hw/ide/ahci.h |   10 +++++-----
>  hw/ide/ich.c  |    3 ++-
>  3 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index bd4f8a4..c0bc5ff 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -146,7 +146,7 @@ static void ahci_check_irq(AHCIState *s)
>  
>      DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
>  
> -    for (i = 0; i < SATA_PORTS; i++) {
> +    for (i = 0; i < s->ports; i++) {
>          AHCIPortRegs *pr = &s->dev[i].port_regs;
>          if (pr->irq_stat & pr->irq_mask) {
>              s->control_regs.irqstatus |= (1 << i);
> @@ -300,7 +300,8 @@ static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr)
>  
>          DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
>      } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
> -               (addr < AHCI_PORT_REGS_END_ADDR)) {
> +               (addr < (AHCI_PORT_REGS_START_ADDR +
> +                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
>          val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
>                               addr & AHCI_PORT_ADDR_OFFSET_MASK);
>      }
> @@ -352,7 +353,8 @@ static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
>                  DPRINTF(-1, "write to unknown register 0x%x\n", (unsigned)addr);
>          }
>      } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
> -               (addr < AHCI_PORT_REGS_END_ADDR)) {
> +               (addr < (AHCI_PORT_REGS_START_ADDR +
> +                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
>          ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
>                          addr & AHCI_PORT_ADDR_OFFSET_MASK, val);
>      }
> @@ -375,16 +377,16 @@ static void ahci_reg_init(AHCIState *s)
>  {
>      int i;
>  
> -    s->control_regs.cap = (SATA_PORTS - 1) |
> +    s->control_regs.cap = (s->ports - 1) |
>                            (AHCI_NUM_COMMAND_SLOTS << 8) |
>                            (AHCI_SUPPORTED_SPEED_GEN1 << AHCI_SUPPORTED_SPEED) |
>                            HOST_CAP_NCQ | HOST_CAP_AHCI;
>  
> -    s->control_regs.impl = (1 << SATA_PORTS) - 1;
> +    s->control_regs.impl = (1 << s->ports) - 1;
>  
>      s->control_regs.version = AHCI_VERSION_1_0;
>  
> -    for (i = 0; i < SATA_PORTS; i++) {
> +    for (i = 0; i < s->ports; i++) {
>          s->dev[i].port_state = STATE_RUN;
>      }
>  }
> @@ -1177,17 +1179,19 @@ static const IDEDMAOps ahci_dma_ops = {
>      .reset = ahci_dma_reset,
>  };
>  
> -void ahci_init(AHCIState *s, DeviceState *qdev)
> +void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
>  {
>      qemu_irq *irqs;
>      int i;
>  
> +    s->ports = ports;
> +    s->dev = qemu_mallocz(sizeof(AHCIDevice) * ports);
>      ahci_reg_init(s);
>      s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s,
>                                      DEVICE_LITTLE_ENDIAN);
> -    irqs = qemu_allocate_irqs(ahci_irq_set, s, SATA_PORTS);
> +    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
>  
> -    for (i = 0; i < SATA_PORTS; i++) {
> +    for (i = 0; i < s->ports; i++) {
>          AHCIDevice *ad = &s->dev[i];
>  
>          ide_bus_new(&ad->port, qdev, i);
> @@ -1201,6 +1205,11 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
>      }
>  }
>  
> +void ahci_uninit(AHCIState *s)
> +{
> +    qemu_free(s->dev);
> +}
> +
>  void ahci_pci_map(PCIDevice *pci_dev, int region_num,
>          pcibus_t addr, pcibus_t size, int type)
>  {
> @@ -1218,7 +1227,7 @@ void ahci_reset(void *opaque)
>      d->ahci.control_regs.irqstatus = 0;
>      d->ahci.control_regs.ghc = 0;
>  
> -    for (i = 0; i < SATA_PORTS; i++) {
> +    for (i = 0; i < d->ahci.ports; i++) {
>          ahci_reset_port(&d->ahci, i);
>      }
>  }
> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> index 0824064..eb87dcd 100644
> --- a/hw/ide/ahci.h
> +++ b/hw/ide/ahci.h
> @@ -165,11 +165,9 @@
>  #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
>                                              /* Shouldn't this be 0x2c? */
>  
> -#define SATA_PORTS                         4
> -
>  #define AHCI_PORT_REGS_START_ADDR          0x100
> -#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
>  #define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
> +#define AHCI_PORT_ADDR_OFFSET_LEN          0x80
>  
>  #define AHCI_NUM_COMMAND_SLOTS             31
>  #define AHCI_SUPPORTED_SPEED               20
> @@ -269,9 +267,10 @@ struct AHCIDevice {
>  };
>  
>  typedef struct AHCIState {
> -    AHCIDevice dev[SATA_PORTS];
> +    AHCIDevice *dev;
>      AHCIControlRegs control_regs;
>      int mem;
> +    int ports;
>      qemu_irq irq;
>  } AHCIState;
>  
> @@ -303,7 +302,8 @@ typedef struct NCQFrame {
>      uint8_t reserved10;
>  } __attribute__ ((packed)) NCQFrame;
>  
> -void ahci_init(AHCIState *s, DeviceState *qdev);
> +void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
> +void ahci_uninit(AHCIState *s);
>  
>  void ahci_pci_map(PCIDevice *pci_dev, int region_num,
>          pcibus_t addr, pcibus_t size, int type);
> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
> index 70cb766..f242d7a 100644
> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -100,7 +100,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>  
>      msi_init(dev, 0x50, 1, true, false);
>  
> -    ahci_init(&d->ahci, &dev->qdev);
> +    ahci_init(&d->ahci, &dev->qdev, 6);
>      d->ahci.irq = d->card.irq[0];

What about a qdev property instead of just hardcoding the value
somewhere else?

Kevin
Alexander Graf - Jan. 18, 2011, 12:46 p.m.
On 18.01.2011, at 13:40, Kevin Wolf wrote:

> Am 20.12.2010 22:13, schrieb Alexander Graf:
>> Different AHCI controllers have a different number of ports, so the core
>> shouldn't care about the amount of ports available.
>> 
>> This patch makes the number of ports available to the AHCI core runtime
>> configurable, allowing us to have multiple different AHCI implementations
>> with different amounts of ports.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/ide/ahci.c |   29 +++++++++++++++++++----------
>> hw/ide/ahci.h |   10 +++++-----
>> hw/ide/ich.c  |    3 ++-
>> 3 files changed, 26 insertions(+), 16 deletions(-)
>> 
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index bd4f8a4..c0bc5ff 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -146,7 +146,7 @@ static void ahci_check_irq(AHCIState *s)
>> 
>>     DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
>> 
>> -    for (i = 0; i < SATA_PORTS; i++) {
>> +    for (i = 0; i < s->ports; i++) {
>>         AHCIPortRegs *pr = &s->dev[i].port_regs;
>>         if (pr->irq_stat & pr->irq_mask) {
>>             s->control_regs.irqstatus |= (1 << i);
>> @@ -300,7 +300,8 @@ static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr)
>> 
>>         DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
>>     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
>> -               (addr < AHCI_PORT_REGS_END_ADDR)) {
>> +               (addr < (AHCI_PORT_REGS_START_ADDR +
>> +                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
>>         val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
>>                              addr & AHCI_PORT_ADDR_OFFSET_MASK);
>>     }
>> @@ -352,7 +353,8 @@ static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
>>                 DPRINTF(-1, "write to unknown register 0x%x\n", (unsigned)addr);
>>         }
>>     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
>> -               (addr < AHCI_PORT_REGS_END_ADDR)) {
>> +               (addr < (AHCI_PORT_REGS_START_ADDR +
>> +                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
>>         ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
>>                         addr & AHCI_PORT_ADDR_OFFSET_MASK, val);
>>     }
>> @@ -375,16 +377,16 @@ static void ahci_reg_init(AHCIState *s)
>> {
>>     int i;
>> 
>> -    s->control_regs.cap = (SATA_PORTS - 1) |
>> +    s->control_regs.cap = (s->ports - 1) |
>>                           (AHCI_NUM_COMMAND_SLOTS << 8) |
>>                           (AHCI_SUPPORTED_SPEED_GEN1 << AHCI_SUPPORTED_SPEED) |
>>                           HOST_CAP_NCQ | HOST_CAP_AHCI;
>> 
>> -    s->control_regs.impl = (1 << SATA_PORTS) - 1;
>> +    s->control_regs.impl = (1 << s->ports) - 1;
>> 
>>     s->control_regs.version = AHCI_VERSION_1_0;
>> 
>> -    for (i = 0; i < SATA_PORTS; i++) {
>> +    for (i = 0; i < s->ports; i++) {
>>         s->dev[i].port_state = STATE_RUN;
>>     }
>> }
>> @@ -1177,17 +1179,19 @@ static const IDEDMAOps ahci_dma_ops = {
>>     .reset = ahci_dma_reset,
>> };
>> 
>> -void ahci_init(AHCIState *s, DeviceState *qdev)
>> +void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
>> {
>>     qemu_irq *irqs;
>>     int i;
>> 
>> +    s->ports = ports;
>> +    s->dev = qemu_mallocz(sizeof(AHCIDevice) * ports);
>>     ahci_reg_init(s);
>>     s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s,
>>                                     DEVICE_LITTLE_ENDIAN);
>> -    irqs = qemu_allocate_irqs(ahci_irq_set, s, SATA_PORTS);
>> +    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
>> 
>> -    for (i = 0; i < SATA_PORTS; i++) {
>> +    for (i = 0; i < s->ports; i++) {
>>         AHCIDevice *ad = &s->dev[i];
>> 
>>         ide_bus_new(&ad->port, qdev, i);
>> @@ -1201,6 +1205,11 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
>>     }
>> }
>> 
>> +void ahci_uninit(AHCIState *s)
>> +{
>> +    qemu_free(s->dev);
>> +}
>> +
>> void ahci_pci_map(PCIDevice *pci_dev, int region_num,
>>         pcibus_t addr, pcibus_t size, int type)
>> {
>> @@ -1218,7 +1227,7 @@ void ahci_reset(void *opaque)
>>     d->ahci.control_regs.irqstatus = 0;
>>     d->ahci.control_regs.ghc = 0;
>> 
>> -    for (i = 0; i < SATA_PORTS; i++) {
>> +    for (i = 0; i < d->ahci.ports; i++) {
>>         ahci_reset_port(&d->ahci, i);
>>     }
>> }
>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>> index 0824064..eb87dcd 100644
>> --- a/hw/ide/ahci.h
>> +++ b/hw/ide/ahci.h
>> @@ -165,11 +165,9 @@
>> #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
>>                                             /* Shouldn't this be 0x2c? */
>> 
>> -#define SATA_PORTS                         4
>> -
>> #define AHCI_PORT_REGS_START_ADDR          0x100
>> -#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
>> #define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
>> +#define AHCI_PORT_ADDR_OFFSET_LEN          0x80
>> 
>> #define AHCI_NUM_COMMAND_SLOTS             31
>> #define AHCI_SUPPORTED_SPEED               20
>> @@ -269,9 +267,10 @@ struct AHCIDevice {
>> };
>> 
>> typedef struct AHCIState {
>> -    AHCIDevice dev[SATA_PORTS];
>> +    AHCIDevice *dev;
>>     AHCIControlRegs control_regs;
>>     int mem;
>> +    int ports;
>>     qemu_irq irq;
>> } AHCIState;
>> 
>> @@ -303,7 +302,8 @@ typedef struct NCQFrame {
>>     uint8_t reserved10;
>> } __attribute__ ((packed)) NCQFrame;
>> 
>> -void ahci_init(AHCIState *s, DeviceState *qdev);
>> +void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
>> +void ahci_uninit(AHCIState *s);
>> 
>> void ahci_pci_map(PCIDevice *pci_dev, int region_num,
>>         pcibus_t addr, pcibus_t size, int type);
>> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
>> index 70cb766..f242d7a 100644
>> --- a/hw/ide/ich.c
>> +++ b/hw/ide/ich.c
>> @@ -100,7 +100,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>> 
>>     msi_init(dev, 0x50, 1, true, false);
>> 
>> -    ahci_init(&d->ahci, &dev->qdev);
>> +    ahci_init(&d->ahci, &dev->qdev, 6);
>>     d->ahci.irq = d->card.irq[0];
> 
> What about a qdev property instead of just hardcoding the value
> somewhere else?

I'm not sure. That specific PCI ID simply has 6 ports. So it somehow belongs there :).


Alex
Gerd Hoffmann - Jan. 18, 2011, 1:09 p.m.
Hi,

>> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
>> index 70cb766..f242d7a 100644
>> --- a/hw/ide/ich.c
>> +++ b/hw/ide/ich.c
>> @@ -100,7 +100,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>>
>>       msi_init(dev, 0x50, 1, true, false);
>>
>> -    ahci_init(&d->ahci,&dev->qdev);
>> +    ahci_init(&d->ahci,&dev->qdev, 6);
>>       d->ahci.irq = d->card.irq[0];
>
> What about a qdev property instead of just hardcoding the value
> somewhere else?

That particular piece of emulated hardware (ich9-ahci) actually has 6 
ports.  ich7 has 4 ports.  A thin hardware-specific wrapper which 
hardcodes this stuff (and pci ids and maybe some other minor 
differences) looks fine to me.  Maybe alex should add a ich7 variant 
just to prove the point ;)

cheers,
   Gerd
Kevin Wolf - Jan. 18, 2011, 1:33 p.m.
Am 18.01.2011 13:46, schrieb Alexander Graf:
> 
> On 18.01.2011, at 13:40, Kevin Wolf wrote:
> 
>> Am 20.12.2010 22:13, schrieb Alexander Graf:
>>> Different AHCI controllers have a different number of ports, so the core
>>> shouldn't care about the amount of ports available.
>>>
>>> This patch makes the number of ports available to the AHCI core runtime
>>> configurable, allowing us to have multiple different AHCI implementations
>>> with different amounts of ports.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> hw/ide/ahci.c |   29 +++++++++++++++++++----------
>>> hw/ide/ahci.h |   10 +++++-----
>>> hw/ide/ich.c  |    3 ++-
>>> 3 files changed, 26 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>> index bd4f8a4..c0bc5ff 100644
>>> --- a/hw/ide/ahci.c
>>> +++ b/hw/ide/ahci.c
>>> @@ -146,7 +146,7 @@ static void ahci_check_irq(AHCIState *s)
>>>
>>>     DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
>>>
>>> -    for (i = 0; i < SATA_PORTS; i++) {
>>> +    for (i = 0; i < s->ports; i++) {
>>>         AHCIPortRegs *pr = &s->dev[i].port_regs;
>>>         if (pr->irq_stat & pr->irq_mask) {
>>>             s->control_regs.irqstatus |= (1 << i);
>>> @@ -300,7 +300,8 @@ static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr)
>>>
>>>         DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
>>>     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
>>> -               (addr < AHCI_PORT_REGS_END_ADDR)) {
>>> +               (addr < (AHCI_PORT_REGS_START_ADDR +
>>> +                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
>>>         val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
>>>                              addr & AHCI_PORT_ADDR_OFFSET_MASK);
>>>     }
>>> @@ -352,7 +353,8 @@ static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
>>>                 DPRINTF(-1, "write to unknown register 0x%x\n", (unsigned)addr);
>>>         }
>>>     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
>>> -               (addr < AHCI_PORT_REGS_END_ADDR)) {
>>> +               (addr < (AHCI_PORT_REGS_START_ADDR +
>>> +                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
>>>         ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
>>>                         addr & AHCI_PORT_ADDR_OFFSET_MASK, val);
>>>     }
>>> @@ -375,16 +377,16 @@ static void ahci_reg_init(AHCIState *s)
>>> {
>>>     int i;
>>>
>>> -    s->control_regs.cap = (SATA_PORTS - 1) |
>>> +    s->control_regs.cap = (s->ports - 1) |
>>>                           (AHCI_NUM_COMMAND_SLOTS << 8) |
>>>                           (AHCI_SUPPORTED_SPEED_GEN1 << AHCI_SUPPORTED_SPEED) |
>>>                           HOST_CAP_NCQ | HOST_CAP_AHCI;
>>>
>>> -    s->control_regs.impl = (1 << SATA_PORTS) - 1;
>>> +    s->control_regs.impl = (1 << s->ports) - 1;
>>>
>>>     s->control_regs.version = AHCI_VERSION_1_0;
>>>
>>> -    for (i = 0; i < SATA_PORTS; i++) {
>>> +    for (i = 0; i < s->ports; i++) {
>>>         s->dev[i].port_state = STATE_RUN;
>>>     }
>>> }
>>> @@ -1177,17 +1179,19 @@ static const IDEDMAOps ahci_dma_ops = {
>>>     .reset = ahci_dma_reset,
>>> };
>>>
>>> -void ahci_init(AHCIState *s, DeviceState *qdev)
>>> +void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
>>> {
>>>     qemu_irq *irqs;
>>>     int i;
>>>
>>> +    s->ports = ports;
>>> +    s->dev = qemu_mallocz(sizeof(AHCIDevice) * ports);
>>>     ahci_reg_init(s);
>>>     s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s,
>>>                                     DEVICE_LITTLE_ENDIAN);
>>> -    irqs = qemu_allocate_irqs(ahci_irq_set, s, SATA_PORTS);
>>> +    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
>>>
>>> -    for (i = 0; i < SATA_PORTS; i++) {
>>> +    for (i = 0; i < s->ports; i++) {
>>>         AHCIDevice *ad = &s->dev[i];
>>>
>>>         ide_bus_new(&ad->port, qdev, i);
>>> @@ -1201,6 +1205,11 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
>>>     }
>>> }
>>>
>>> +void ahci_uninit(AHCIState *s)
>>> +{
>>> +    qemu_free(s->dev);
>>> +}
>>> +
>>> void ahci_pci_map(PCIDevice *pci_dev, int region_num,
>>>         pcibus_t addr, pcibus_t size, int type)
>>> {
>>> @@ -1218,7 +1227,7 @@ void ahci_reset(void *opaque)
>>>     d->ahci.control_regs.irqstatus = 0;
>>>     d->ahci.control_regs.ghc = 0;
>>>
>>> -    for (i = 0; i < SATA_PORTS; i++) {
>>> +    for (i = 0; i < d->ahci.ports; i++) {
>>>         ahci_reset_port(&d->ahci, i);
>>>     }
>>> }
>>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>>> index 0824064..eb87dcd 100644
>>> --- a/hw/ide/ahci.h
>>> +++ b/hw/ide/ahci.h
>>> @@ -165,11 +165,9 @@
>>> #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
>>>                                             /* Shouldn't this be 0x2c? */
>>>
>>> -#define SATA_PORTS                         4
>>> -
>>> #define AHCI_PORT_REGS_START_ADDR          0x100
>>> -#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
>>> #define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
>>> +#define AHCI_PORT_ADDR_OFFSET_LEN          0x80
>>>
>>> #define AHCI_NUM_COMMAND_SLOTS             31
>>> #define AHCI_SUPPORTED_SPEED               20
>>> @@ -269,9 +267,10 @@ struct AHCIDevice {
>>> };
>>>
>>> typedef struct AHCIState {
>>> -    AHCIDevice dev[SATA_PORTS];
>>> +    AHCIDevice *dev;
>>>     AHCIControlRegs control_regs;
>>>     int mem;
>>> +    int ports;
>>>     qemu_irq irq;
>>> } AHCIState;
>>>
>>> @@ -303,7 +302,8 @@ typedef struct NCQFrame {
>>>     uint8_t reserved10;
>>> } __attribute__ ((packed)) NCQFrame;
>>>
>>> -void ahci_init(AHCIState *s, DeviceState *qdev);
>>> +void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
>>> +void ahci_uninit(AHCIState *s);
>>>
>>> void ahci_pci_map(PCIDevice *pci_dev, int region_num,
>>>         pcibus_t addr, pcibus_t size, int type);
>>> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
>>> index 70cb766..f242d7a 100644
>>> --- a/hw/ide/ich.c
>>> +++ b/hw/ide/ich.c
>>> @@ -100,7 +100,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>>>
>>>     msi_init(dev, 0x50, 1, true, false);
>>>
>>> -    ahci_init(&d->ahci, &dev->qdev);
>>> +    ahci_init(&d->ahci, &dev->qdev, 6);
>>>     d->ahci.irq = d->card.irq[0];
>>
>> What about a qdev property instead of just hardcoding the value
>> somewhere else?
> 
> I'm not sure. That specific PCI ID simply has 6 ports. So it somehow belongs there :).

Okay, makes sense.

Kevin

Patch

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index bd4f8a4..c0bc5ff 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -146,7 +146,7 @@  static void ahci_check_irq(AHCIState *s)
 
     DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < s->ports; i++) {
         AHCIPortRegs *pr = &s->dev[i].port_regs;
         if (pr->irq_stat & pr->irq_mask) {
             s->control_regs.irqstatus |= (1 << i);
@@ -300,7 +300,8 @@  static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr)
 
         DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
-               (addr < AHCI_PORT_REGS_END_ADDR)) {
+               (addr < (AHCI_PORT_REGS_START_ADDR +
+                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
         val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
                              addr & AHCI_PORT_ADDR_OFFSET_MASK);
     }
@@ -352,7 +353,8 @@  static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
                 DPRINTF(-1, "write to unknown register 0x%x\n", (unsigned)addr);
         }
     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
-               (addr < AHCI_PORT_REGS_END_ADDR)) {
+               (addr < (AHCI_PORT_REGS_START_ADDR +
+                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
         ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
                         addr & AHCI_PORT_ADDR_OFFSET_MASK, val);
     }
@@ -375,16 +377,16 @@  static void ahci_reg_init(AHCIState *s)
 {
     int i;
 
-    s->control_regs.cap = (SATA_PORTS - 1) |
+    s->control_regs.cap = (s->ports - 1) |
                           (AHCI_NUM_COMMAND_SLOTS << 8) |
                           (AHCI_SUPPORTED_SPEED_GEN1 << AHCI_SUPPORTED_SPEED) |
                           HOST_CAP_NCQ | HOST_CAP_AHCI;
 
-    s->control_regs.impl = (1 << SATA_PORTS) - 1;
+    s->control_regs.impl = (1 << s->ports) - 1;
 
     s->control_regs.version = AHCI_VERSION_1_0;
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < s->ports; i++) {
         s->dev[i].port_state = STATE_RUN;
     }
 }
@@ -1177,17 +1179,19 @@  static const IDEDMAOps ahci_dma_ops = {
     .reset = ahci_dma_reset,
 };
 
-void ahci_init(AHCIState *s, DeviceState *qdev)
+void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
 {
     qemu_irq *irqs;
     int i;
 
+    s->ports = ports;
+    s->dev = qemu_mallocz(sizeof(AHCIDevice) * ports);
     ahci_reg_init(s);
     s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s,
                                     DEVICE_LITTLE_ENDIAN);
-    irqs = qemu_allocate_irqs(ahci_irq_set, s, SATA_PORTS);
+    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < s->ports; i++) {
         AHCIDevice *ad = &s->dev[i];
 
         ide_bus_new(&ad->port, qdev, i);
@@ -1201,6 +1205,11 @@  void ahci_init(AHCIState *s, DeviceState *qdev)
     }
 }
 
+void ahci_uninit(AHCIState *s)
+{
+    qemu_free(s->dev);
+}
+
 void ahci_pci_map(PCIDevice *pci_dev, int region_num,
         pcibus_t addr, pcibus_t size, int type)
 {
@@ -1218,7 +1227,7 @@  void ahci_reset(void *opaque)
     d->ahci.control_regs.irqstatus = 0;
     d->ahci.control_regs.ghc = 0;
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < d->ahci.ports; i++) {
         ahci_reset_port(&d->ahci, i);
     }
 }
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 0824064..eb87dcd 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -165,11 +165,9 @@ 
 #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
                                             /* Shouldn't this be 0x2c? */
 
-#define SATA_PORTS                         4
-
 #define AHCI_PORT_REGS_START_ADDR          0x100
-#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
 #define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
+#define AHCI_PORT_ADDR_OFFSET_LEN          0x80
 
 #define AHCI_NUM_COMMAND_SLOTS             31
 #define AHCI_SUPPORTED_SPEED               20
@@ -269,9 +267,10 @@  struct AHCIDevice {
 };
 
 typedef struct AHCIState {
-    AHCIDevice dev[SATA_PORTS];
+    AHCIDevice *dev;
     AHCIControlRegs control_regs;
     int mem;
+    int ports;
     qemu_irq irq;
 } AHCIState;
 
@@ -303,7 +302,8 @@  typedef struct NCQFrame {
     uint8_t reserved10;
 } __attribute__ ((packed)) NCQFrame;
 
-void ahci_init(AHCIState *s, DeviceState *qdev);
+void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
+void ahci_uninit(AHCIState *s);
 
 void ahci_pci_map(PCIDevice *pci_dev, int region_num,
         pcibus_t addr, pcibus_t size, int type);
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 70cb766..f242d7a 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -100,7 +100,7 @@  static int pci_ich9_ahci_init(PCIDevice *dev)
 
     msi_init(dev, 0x50, 1, true, false);
 
-    ahci_init(&d->ahci, &dev->qdev);
+    ahci_init(&d->ahci, &dev->qdev, 6);
     d->ahci.irq = d->card.irq[0];
 
     return 0;
@@ -116,6 +116,7 @@  static int pci_ich9_uninit(PCIDevice *dev)
     }
 
     qemu_unregister_reset(ahci_reset, d);
+    ahci_uninit(&d->ahci);
 
     return 0;
 }