qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4 of 4] [qemu ppc pci] Emulate the PCI(-legacy)


From: Aurelien Jarno
Subject: Re: [Qemu-devel] [PATCH 4 of 4] [qemu ppc pci] Emulate the PCI(-legacy) controller found on some 440 SoCs
Date: Mon, 5 May 2008 05:46:19 +0200
User-agent: Mutt/1.5.13 (2006-08-11)

Please find my comments inline.

On Wed, Apr 23, 2008 at 05:23:55PM -0500, Hollis Blanchard wrote:
> Signed-off-by: Hollis Blanchard <address@hidden>
> 
> diff --git a/qemu/hw/ppc4xx.h b/qemu/hw/ppc4xx.h
> --- a/qemu/hw/ppc4xx.h
> +++ b/qemu/hw/ppc4xx.h
> @@ -2,6 +2,9 @@
>   * QEMU PowerPC 4xx emulation shared definitions
>   *
>   * Copyright (c) 2007 Jocelyn Mayer
> + *
> + * Copyright 2008 IBM Corp.
> + * Authors: Hollis Blanchard <address@hidden>
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -46,4 +49,41 @@
>  qemu_irq *ppcuic_init (CPUState *env, qemu_irq *irqs,
>                         uint32_t dcr_base, int has_ssr, int has_vr);
>  
> +
> +struct pci_master_map {
> +    uint32_t la;
> +    uint32_t ma;
> +    uint32_t pcila;
> +    uint32_t pciha;
> +};
> +
> +struct pci_target_map {
> +    uint32_t ms;
> +    uint32_t la;
> +    uint32_t bar;
> +};
> +
> +#define PPC44x_PCI_NR_PMMS 3
> +#define PPC44x_PCI_NR_PTMS 2
> +
> +struct ppc4xx_pci_t {
> +    target_phys_addr_t config_space;
> +    target_phys_addr_t registers;
> +    struct pci_master_map pmm[PPC44x_PCI_NR_PMMS];
> +    struct pci_target_map ptm[PPC44x_PCI_NR_PTMS];
> +
> +    unsigned int pmm_offset_flags;
> +    qemu_irq *pic;
> +
> +    uint32_t pcic0_cfgaddr;
> +    PCIBus *bus;
> +};
> +typedef struct ppc4xx_pci_t ppc4xx_pci_t;

Looking at the other PCI controller implementation, that could probably
be PPC4xxState instead of ppc4xx_pci_t. But that's a detail.

> +
> +ppc4xx_pci_t *ppc4xx_pci_init(CPUState *env, qemu_irq *pic,
> +                              target_phys_addr_t config_space,
> +                              target_phys_addr_t int_ack,
> +                              target_phys_addr_t special_cycle,
> +                              target_phys_addr_t registers);
> +
>  #endif /* !defined(PPC_4XX_H) */
> diff --git a/qemu/hw/ppc4xx_devs.c b/qemu/hw/ppc4xx_devs.c
> --- a/qemu/hw/ppc4xx_devs.c
> +++ b/qemu/hw/ppc4xx_devs.c
> @@ -2,6 +2,9 @@
>   * QEMU PowerPC 4xx embedded processors shared devices emulation
>   *
>   * Copyright (c) 2007 Jocelyn Mayer
> + *
> + * Copyright 2008 IBM Corp.
> + * Authors: Hollis Blanchard <address@hidden>
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -25,6 +28,8 @@
>  #include "ppc.h"
>  #include "ppc4xx.h"
>  #include "sysemu.h"
> +#include "pci.h"
> +#include "bswap.h"
>  
>  extern int loglevel;
>  extern FILE *logfile;
> @@ -535,3 +540,369 @@
>  
>      return qemu_allocate_irqs(&ppcuic_set_irq, uic, UIC_MAX_IRQ);
>  }
> +
> +
> +
> +
> +#define PCIC0_CFGADDR       0x0
> +#define PCIC0_CFGDATA       0x4

Those values are never used.

> +
> +#define PCIL0_PMM0LA        0x0
> +#define PCIL0_PMM0MA        0x4
> +#define PCIL0_PMM0PCILA     0x8
> +#define PCIL0_PMM0PCIHA     0xc
> +#define PCIL0_PMM1LA        0x10
> +#define PCIL0_PMM1MA        0x14
> +#define PCIL0_PMM1PCILA     0x18
> +#define PCIL0_PMM1PCIHA     0x1c
> +#define PCIL0_PMM2LA        0x20
> +#define PCIL0_PMM2MA        0x24
> +#define PCIL0_PMM2PCILA     0x28
> +#define PCIL0_PMM2PCIHA     0x2c
> +#define PCIL0_PTM1MS        0x30
> +#define PCIL0_PTM1LA        0x34
> +#define PCIL0_PTM2MS        0x38
> +#define PCIL0_PTM2LA        0x3c
> +#define PCI_REG_SIZE        0x40
> +
> +#define PPC44x_PCI_MA_MASK   0xfffff000
> +#define PPC44x_PCI_MA_ENABLE 0x1
> +
> +
> +static uint32_t pci4xx_cfgaddr_read4(void *opaque, target_phys_addr_t addr)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    return cpu_to_le32(ppc4xx_pci->pcic0_cfgaddr);

This is wrong. QEMU works by passing values on the bus, and thus does
not need to byteswap depending and the *host*.

If you need swap here, it's probably because the PCI controller is
connected backward or does byteswapping. So you probably either want
an unconditional call to bswap32() instead, or depending on
TARGET_WORDS_BIGENDIAN if it is depending on the guest endianness.

> +}
> +
> +static CPUReadMemoryFunc *pci4xx_cfgaddr_read[] = {
> +    &pci4xx_cfgaddr_read4,
> +    &pci4xx_cfgaddr_read4,
> +    &pci4xx_cfgaddr_read4,
> +};

It's seem usual to call use 'l', 'w', 'b' instead of '4', '2', '1'. Not
really important though.

> +static void pci4xx_cfgaddr_write4(void *opaque, target_phys_addr_t addr,
> +                                  uint32_t value)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +
> +    value = le32_to_cpu(value);

endianness problem, as explained above

> +
> +    ppc4xx_pci->pcic0_cfgaddr = value & ~0x3;
> +}
> +
> +static CPUWriteMemoryFunc *pci4xx_cfgaddr_write[] = {
> +    &pci4xx_cfgaddr_write4,
> +    &pci4xx_cfgaddr_write4,
> +    &pci4xx_cfgaddr_write4,
> +};
> +
> +static uint32_t pci4xx_cfgdata_read1(void *opaque, target_phys_addr_t addr)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    int offset = addr & 0x3;
> +    uint32_t cfgaddr = ppc4xx_pci->pcic0_cfgaddr;
> +    uint32_t value;
> +
> +    if (!(cfgaddr & (1<<31)))
> +        return 0xffffffff;
> +
> +    value = pci_data_read(ppc4xx_pci->bus, cfgaddr | offset, 1);
> +
> +    return value;
> +}
> +
> +static uint32_t pci4xx_cfgdata_read2(void *opaque, target_phys_addr_t addr)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    int offset = addr & 0x3;
> +    uint32_t cfgaddr = ppc4xx_pci->pcic0_cfgaddr;
> +    uint32_t value;
> +
> +    if (!(cfgaddr & (1<<31)))
> +        return 0xffffffff;
> +
> +    value = pci_data_read(ppc4xx_pci->bus, cfgaddr | offset, 2);
> +
> +    return cpu_to_le16(value);

endianness problem, as explained above. Note that you can use functions
from pci_host.h which basically do the same, but correctly handle
endianness.

> +}
> +
> +static uint32_t pci4xx_cfgdata_read4(void *opaque, target_phys_addr_t addr)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    int offset = addr & 0x3;
> +    uint32_t cfgaddr = ppc4xx_pci->pcic0_cfgaddr;
> +    uint32_t value;
> +
> +    if (!(cfgaddr & (1<<31)))
> +        return 0xffffffff;
> +
> +    value = pci_data_read(ppc4xx_pci->bus, cfgaddr | offset, 4);
> +
> +    return cpu_to_le32(value);
endianness problem, as explained above

> +}
> +
> +static CPUReadMemoryFunc *pci4xx_cfgdata_read[] = {
> +    &pci4xx_cfgdata_read1,
> +    &pci4xx_cfgdata_read2,
> +    &pci4xx_cfgdata_read4,
> +};
> +
> +static void pci4xx_cfgdata_write1(void *opaque, target_phys_addr_t addr,
> +                                  uint32_t value)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    int offset = addr & 0x3;
> +
> +    pci_data_write(ppc4xx_pci->bus, ppc4xx_pci->pcic0_cfgaddr | offset,
> +                   value, 1);
> +}
> +
> +static void pci4xx_cfgdata_write2(void *opaque, target_phys_addr_t addr,
> +                                  uint32_t value)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    int offset = addr & 0x3;
> +
> +    value = le16_to_cpu(value);
endianness problem, as explained above

> +
> +    pci_data_write(ppc4xx_pci->bus, ppc4xx_pci->pcic0_cfgaddr | offset,
> +                   value, 2);
> +}
> +
> +static void pci4xx_cfgdata_write4(void *opaque, target_phys_addr_t addr,
> +                                  uint32_t value)
> +{
> +    ppc4xx_pci_t *ppc4xx_pci = opaque;
> +    int offset = addr & 0x3;
> +
> +    value = le32_to_cpu(value);
endianness problem, as explained above

> +
> +    pci_data_write(ppc4xx_pci->bus, ppc4xx_pci->pcic0_cfgaddr | offset,
> +                   value, 4);
> +}
> +
> +static CPUWriteMemoryFunc *pci4xx_cfgdata_write[] = {
> +    &pci4xx_cfgdata_write1,
> +    &pci4xx_cfgdata_write2,
> +    &pci4xx_cfgdata_write4,
> +};
> +
> +static void pci_reg_write4(void *opaque, target_phys_addr_t addr,
> +                           uint32_t value)
> +{
> +    struct ppc4xx_pci_t *pci = opaque;
> +    unsigned long offset = addr - pci->registers;
> +
> +    value = le32_to_cpu(value);
endianness problem, as explained above

> +
> +    switch (offset) {
> +    case PCIL0_PMM0LA:
> +        pci->pmm[0].la = value;
> +        break;
> +    case PCIL0_PMM1LA:
> +        pci->pmm[0].la = value;
> +        break;
> +    case PCIL0_PMM2LA:
> +        pci->pmm[0].la = value;
> +        break;
> +    default:
> +        //printf("  unhandled PCI internal register 0x%lx\n", offset);
> +        break;
> +    }
> +}
> +
> +static uint32_t pci_reg_read4(void *opaque, target_phys_addr_t addr)
> +{
> +    struct ppc4xx_pci_t *pci = opaque;
> +    unsigned long offset = addr - pci->registers;
> +    uint32_t value;
> +
> +    switch (offset) {
> +    case PCIL0_PMM0LA:
> +        value = pci->pmm[0].la;
> +        break;
> +    case PCIL0_PMM0MA:
> +        value = pci->pmm[0].ma;
> +        break;
> +    case PCIL0_PMM0PCIHA:
> +        value = pci->pmm[0].pciha;
> +        break;
> +    case PCIL0_PMM0PCILA:
> +        value = pci->pmm[0].pcila;
> +        break;
> +
> +    case PCIL0_PMM1LA:
> +        value = pci->pmm[1].la;
> +        break;
> +    case PCIL0_PMM1MA:
> +        value = pci->pmm[1].ma;
> +        break;
> +    case PCIL0_PMM1PCIHA:
> +        value = pci->pmm[1].pciha;
> +        break;
> +    case PCIL0_PMM1PCILA:
> +        value = pci->pmm[1].pcila;
> +        break;
> +
> +    case PCIL0_PMM2LA:
> +        value = pci->pmm[2].la;
> +        break;
> +    case PCIL0_PMM2MA:
> +        value = pci->pmm[2].ma;
> +        break;
> +    case PCIL0_PMM2PCIHA:
> +        value = pci->pmm[2].pciha;
> +        break;
> +    case PCIL0_PMM2PCILA:
> +        value = pci->pmm[2].pcila;
> +        break;
> +
> +    case PCIL0_PTM1MS:
> +        value = pci->ptm[0].ms;
> +        break;
> +    case PCIL0_PTM1LA:
> +        value = pci->ptm[0].la;
> +        break;
> +    case PCIL0_PTM2MS:
> +        value = pci->ptm[1].ms;
> +        break;
> +    case PCIL0_PTM2LA:
> +        value = pci->ptm[1].la;
> +        break;
> +
> +    default:
> +        //printf("  read from invalid PCI internal register 0x%lx\n", 
> offset);
> +        value = 0;
> +    }
> +
> +    value = cpu_to_le32(value);
endianness problem, as explained above

> +
> +    return value;
> +}
> +
> +static CPUReadMemoryFunc *pci_reg_read[] = {
> +    &pci_reg_read4,
> +    &pci_reg_read4,
> +    &pci_reg_read4,
> +};
> +
> +static CPUWriteMemoryFunc *pci_reg_write[] = {
> +    &pci_reg_write4,
> +    &pci_reg_write4,
> +    &pci_reg_write4,
> +};
> +
> +static uint32_t pci_int_ack_read4(void *opaque, target_phys_addr_t addr)
> +{
> +    printf("%s\n", __func__);

This should probably be inside a #ifdef #endif block. Also is it normal
that this function does nothing? Given its name, I guess it is supposed
to read the PIC controller to acknowledge an interrupt.

> +    return 0;
> +}
> +
> +static CPUReadMemoryFunc *pci_int_ack_read[] = {
> +    &pci_int_ack_read4,
> +    &pci_int_ack_read4,
> +    &pci_int_ack_read4,
> +};
> +
> +static void pci_special_write4(void *opaque, target_phys_addr_t addr,
> +                               uint32_t value)
> +{
> +    printf("%s\n", __func__);

Same here

> +}
> +
> +static CPUWriteMemoryFunc *pci_special_write[] = {
> +    &pci_special_write4,
> +    &pci_special_write4,
> +    &pci_special_write4,
> +};
> +
> +static int bamboo_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> +{
> +    int slot = pci_dev->devfn >> 3;
> +
> +#if 0
> +    printf("### %s: devfn %x irq %d -> %d\n", __func__,
> +           pci_dev->devfn, irq_num, slot+1);
> +#endif
> +
> +    /* All pins from each slot are tied to a single board IRQ (2-5) */
> +    return slot + 1;
> +}
> +
> +static void bamboo_pci_set_irq(qemu_irq *pic, int irq_num, int level)
> +{
> +#if 0
> +    printf("### %s: PCI irq %d, UIC irq %d\n", __func__, irq_num, 30 - 
> irq_num);
> +#endif
> +
> +    /* Board IRQs 2-5 are connected to UIC IRQs 28-25 */
> +    qemu_set_irq(pic[30-irq_num], level);
> +}
> +
> +/* XXX Needs some abstracting for boards other than Bamboo. */
> +ppc4xx_pci_t *ppc4xx_pci_init(CPUState *env, qemu_irq *pic,
> +                              target_phys_addr_t config_space,
> +                              target_phys_addr_t int_ack,
> +                              target_phys_addr_t special_cycle,
> +                              target_phys_addr_t registers)
> +{
> +    ppc4xx_pci_t *pci;
> +    PCIDevice *d;
> +    int index;
> +
> +    pci = qemu_mallocz(sizeof(ppc4xx_pci_t));
> +    if (!pci)
> +        return NULL;
> +
> +    pci->config_space = config_space;
> +    pci->registers = registers;
> +    pci->pic = pic;
> +
> +    pci->bus = pci_register_bus(bamboo_pci_set_irq, bamboo_pci_map_irq,
> +                                pic, 0, 4);
> +    d = pci_register_device(pci->bus, "host bridge", sizeof(PCIDevice),
> +                            0, NULL, NULL);
> +    d->config[0x00] = 0x14; // vendor_id
> +    d->config[0x01] = 0x10;
> +    d->config[0x02] = 0x7f; // device_id
> +    d->config[0x03] = 0x02;
> +    d->config[0x0a] = 0x80; // class_sub = other bridge type
> +    d->config[0x0b] = 0x06; // class_base = PCI_bridge
> +
> +    /* CFGADDR */
> +    index = cpu_register_io_memory(0, pci4xx_cfgaddr_read,
> +                                   pci4xx_cfgaddr_write, pci);
> +    if (index < 0)
> +        goto free;
> +    cpu_register_physical_memory(config_space, 4, index);
> +
> +    /* CFGDATA */
> +    index = cpu_register_io_memory(0, pci4xx_cfgdata_read,
> +                                   pci4xx_cfgdata_write, pci);
> +    if (index < 0)
> +        goto free;
> +    cpu_register_physical_memory(config_space + 4, 4, index);
> +
> +    /* "Special cycle" and interrupt acknowledge */
> +    index = cpu_register_io_memory(0, pci_int_ack_read,
> +                                   pci_special_write, pci);
> +    if (index < 0)
> +        goto free;
> +    cpu_register_physical_memory(int_ack, 4, index);
> +
> +    /* Internal registers */
> +    index = cpu_register_io_memory(0, pci_reg_read, pci_reg_write, pci);
> +    if (index < 0)
> +        goto free;
> +    cpu_register_physical_memory(registers, PCI_REG_SIZE, index);
> +
> +    /* XXX register_savevm() */
> +
> +    return pci;
> +
> +free:
> +    printf("%s error\n", __func__);
> +    qemu_free(pci);
> +    return NULL;
> +}
> 

-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net




reply via email to

[Prev in Thread] Current Thread [Next in Thread]