RE: [PATCH 3/5] qemu/kvm: MPIC and E500 PCI controller emualtion

2009-01-12 Thread Liu Yu


> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org 
> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
> Sent: Saturday, January 10, 2009 4:25 AM
> To: Liu Yu-B13201
> Cc: kvm-ppc@vger.kernel.org
> Subject: Re: [PATCH 3/5] qemu/kvm: MPIC and E500 PCI 
> controller emualtion
> 
> On Fri, 2009-01-09 at 15:56 +0800, Liu Yu wrote:
> > Signed-off-by: Liu Yu 
> > ---
> >  hw/mpic.c|  903 
> ++
> >  hw/ppce500_pci.c |  322 +++
> >  2 files changed, 1225 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/mpic.c
> >  create mode 100644 hw/ppce500_pci.c
> > 
> > diff --git a/hw/mpic.c b/hw/mpic.c
> > new file mode 100644
> > index 000..a68948a
> > --- /dev/null
> > +++ b/hw/mpic.c
> > @@ -0,0 +1,903 @@
> > +/*
> > + * MPIC emulation
> > + *
> > + * Copyright (C) 2009 Freescale Semiconductor, Inc. All 
> rights reserved.
> > + *
> > + * Author: Yu Liu, 
> > + *
> > + * This file is derived from hw/openpic.c,
> > + * the copyright for that material belongs to the original owners.
> 
> The Linux drivers for MPIC and OpenPIC share a lot of code. Can't you
> modify hw/openpic.c instead of copy/paste/hack?

I will try to do that.

> 
> The MPIC patch should be separate from the PCI patch.

Fixed. Thanks.

> 
> > diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> > new file mode 100644
> > index 000..9242639
> > --- /dev/null
> > +++ b/hw/ppce500_pci.c
> > @@ -0,0 +1,322 @@
> > +/*
> > + * QEMU PowerPC E500 embedded processors pci controller emulation
> > + *
> > + * Copyright (C) 2009 Freescale Semiconductor, Inc. All 
> rights reserved.
> > + *
> > + * Author: Yu Liu, 
> > + *
> > + * This file is derived from hw/ppc4xx_pci.c,
> > + * the copyright for that material belongs to the original owners.
> > + *
> > + * This is free software; you can redistribute it and/or modify
> > + * it under the terms of  the GNU General  Public License 
> as published by
> > + * the Free Software Foundation;  either version 2 of the  
> License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include "hw.h"
> > +#include "ppc.h"
> > +#include "ppce500.h"
> > +#include "sysemu.h"
> > +#include "pci.h"
> > +#include "bswap.h"
> > +#include "qemu-log.h"
> > +
> > +#ifdef DEBUG_PCI
> > +#define pci_debug(fmt, arg...) fprintf(stderr, fmt, ##arg)
> > +#else
> > +#define pci_debug(fmt, arg...)
> > +#endif
> > +
> > +#define PPCE500_PCI_REG_SIZE   0x1000
> > +
> > +#define PPCE500_PCI_CONFIG_ADDR0x0
> > +#define PPCE500_PCI_CONFIG_DATA0x4
> > +#define PPCE500_PCI_INTACK 0x8
> > +
> > +#define PPCE500_PCI_OW10xC20
> > +#define PPCE500_PCI_OW20xC40
> > +#define PPCE500_PCI_OW30xC60
> > +#define PPCE500_PCI_OW40xC80
> > +#define PPCE500_PCI_IW30xDA0
> > +#define PPCE500_PCI_IW20xDC0
> > +#define PPCE500_PCI_IW10xDE0
> > +
> > +#define PPCE500_PCI_GAS_TIMR   0xE20
> > +
> > +#define PCI_POTAR  0x0
> > +#define PCI_POTEAR 0x4
> > +#define PCI_POWBAR 0x8
> > +#define PCI_POWAR  0x10
> > +
> > +#define PCI_PITAR  0x0
> > +#define PCI_PIWBAR 0x8
> > +#define PCI_PIWBEAR0xC
> > +#define PCI_PIWAR  0x10
> > +
> > +#define PPCE500_PCI_NR_POBS 5
> > +#define PPCE500_PCI_NR_PIBS 3
> > +
> > +struct ppce500_pci_t {
> > +target_phys_addr_t registers;
> > +struct pci_outbound pob[PPCE500_PCI_NR_POBS];
> > +struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> > +uint32_t gas_time;
> > +
> > +qemu_irq *pic;
> > +
> > +uint32_t pcic0_cfgaddr;
> > +PCIBus *bus;
> > +};
> > +
> > +typedef struct ppce500_pci_t ppce500_pci_t;
> > +
> > +static uint32_t pci_reg_read(void *opaque, 
> target_phys_addr_t addr, int size)
> > +{
> > +ppce500_pci_t *pci = opaque;
> > +unsigned long offset = addr - pci->registers, win;
> 
> I don't think this could possibly work now, since a patch 
> went in a few
> weeks back that passes "

Re: [PATCH 3/5] qemu/kvm: MPIC and E500 PCI controller emualtion

2009-01-09 Thread Hollis Blanchard
On Fri, 2009-01-09 at 15:56 +0800, Liu Yu wrote:
> Signed-off-by: Liu Yu 
> ---
>  hw/mpic.c|  903 
> ++
>  hw/ppce500_pci.c |  322 +++
>  2 files changed, 1225 insertions(+), 0 deletions(-)
>  create mode 100644 hw/mpic.c
>  create mode 100644 hw/ppce500_pci.c
> 
> diff --git a/hw/mpic.c b/hw/mpic.c
> new file mode 100644
> index 000..a68948a
> --- /dev/null
> +++ b/hw/mpic.c
> @@ -0,0 +1,903 @@
> +/*
> + * MPIC emulation
> + *
> + * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: Yu Liu, 
> + *
> + * This file is derived from hw/openpic.c,
> + * the copyright for that material belongs to the original owners.

The Linux drivers for MPIC and OpenPIC share a lot of code. Can't you
modify hw/openpic.c instead of copy/paste/hack?

The MPIC patch should be separate from the PCI patch.

> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> new file mode 100644
> index 000..9242639
> --- /dev/null
> +++ b/hw/ppce500_pci.c
> @@ -0,0 +1,322 @@
> +/*
> + * QEMU PowerPC E500 embedded processors pci controller emulation
> + *
> + * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: Yu Liu, 
> + *
> + * This file is derived from hw/ppc4xx_pci.c,
> + * the copyright for that material belongs to the original owners.
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of  the GNU General  Public License as published by
> + * the Free Software Foundation;  either version 2 of the  License, or
> + * (at your option) any later version.
> + */
> +
> +#include "hw.h"
> +#include "ppc.h"
> +#include "ppce500.h"
> +#include "sysemu.h"
> +#include "pci.h"
> +#include "bswap.h"
> +#include "qemu-log.h"
> +
> +#ifdef DEBUG_PCI
> +#define pci_debug(fmt, arg...) fprintf(stderr, fmt, ##arg)
> +#else
> +#define pci_debug(fmt, arg...)
> +#endif
> +
> +#define PPCE500_PCI_REG_SIZE 0x1000
> +
> +#define PPCE500_PCI_CONFIG_ADDR  0x0
> +#define PPCE500_PCI_CONFIG_DATA  0x4
> +#define PPCE500_PCI_INTACK   0x8
> +
> +#define PPCE500_PCI_OW1  0xC20
> +#define PPCE500_PCI_OW2  0xC40
> +#define PPCE500_PCI_OW3  0xC60
> +#define PPCE500_PCI_OW4  0xC80
> +#define PPCE500_PCI_IW3  0xDA0
> +#define PPCE500_PCI_IW2  0xDC0
> +#define PPCE500_PCI_IW1  0xDE0
> +
> +#define PPCE500_PCI_GAS_TIMR 0xE20
> +
> +#define PCI_POTAR0x0
> +#define PCI_POTEAR   0x4
> +#define PCI_POWBAR   0x8
> +#define PCI_POWAR0x10
> +
> +#define PCI_PITAR0x0
> +#define PCI_PIWBAR   0x8
> +#define PCI_PIWBEAR  0xC
> +#define PCI_PIWAR0x10
> +
> +#define PPCE500_PCI_NR_POBS 5
> +#define PPCE500_PCI_NR_PIBS 3
> +
> +struct ppce500_pci_t {
> +target_phys_addr_t registers;
> +struct pci_outbound pob[PPCE500_PCI_NR_POBS];
> +struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> +uint32_t gas_time;
> +
> +qemu_irq *pic;
> +
> +uint32_t pcic0_cfgaddr;
> +PCIBus *bus;
> +};
> +
> +typedef struct ppce500_pci_t ppce500_pci_t;
> +
> +static uint32_t pci_reg_read(void *opaque, target_phys_addr_t addr, int size)
> +{
> +ppce500_pci_t *pci = opaque;
> +unsigned long offset = addr - pci->registers, win;

I don't think this could possibly work now, since a patch went in a few
weeks back that passes "offset" to MMIO callbacks, instead of "addr".

> +uint32_t value = 0;
> +
> +win = offset & 0xfe0;
> +
> +switch (win) {
> +case 0:
> + switch(offset & 0xC) {
> + case PPCE500_PCI_CONFIG_ADDR:
> + value = pci->pcic0_cfgaddr;
> + break;
> +
> + case PPCE500_PCI_CONFIG_DATA: {
> + uint32_t cfgaddr = pci->pcic0_cfgaddr;
> +
> + if (!(cfgaddr & (1<<31)))
> + return 0x;
> + 
> + value = pci_data_read(pci->bus, cfgaddr | (offset & 0x3), size);
> +
> + if (size == 2)
> + value = cpu_to_le16(value);
> + else if (size == 4)
> + value = cpu_to_le32(value);
> +
> + break;
> + }
> + default:;
> + }
> + break;
> +
> +case PPCE500_PCI_OW1:
> +case PPCE500_PCI_OW2:
> +case PPCE500_PCI_OW3:
> +case PPCE500_PCI_OW4:
> + switch (offset & 0xC) {
> + case PCI_POTAR: value = pci->pob[(offset >> 5) & 0x7].potar; break;
> + case PCI_POTEAR: value = pci->pob[(offset >> 5) & 0x7].potear; break;
> + case PCI_POWBAR: value = pci->pob[(offset >> 5) & 0x7].powbar; break;
> + case PCI_POWAR: value = pci->pob[(offset >> 5) & 0x7].powar; break;
> + default: break;
> + }
> + break;
> +
> +case PPCE500_PCI_IW3:
> +case PPCE500_PCI_IW2:
> +case PPCE500_PCI_IW1:
> + switch (offset & 0xC) {
> + case PCI_PI