[Qemu-devel] Re: [PATCH 6/8] ahci: make number of ports runtime determined

[Kevin Wolf <kwolf@redhat.com>]

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