[Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north

[Michael S. Tsirkin]

On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> As stated in the previous patch, the Uninorth PCI bridge requires different
> layouts in its PCI config space accessors.
> 
> This patch introduces a conversion function that makes it compatible with
> the way Linux accesses it.
> 
> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
> take small steps here and do the config space access rework in OpenBIOS
> later on. When that's done we can remove that hack.
> 
> Signed-off-by: Alexander Graf <address@hidden>
> ---
>  hw/unin_pci.c |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index fdb9401..1c49008 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
>  {
>  }
>  
> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> +{
> +    uint32_t retval;
> +    uint32_t reg = s->config_reg;
> +
> +    if (reg & (1u << 31)) {
> +        /* XXX OpenBIOS compatibility hack */
> +        retval = reg;
> +        addr |= reg & 7;
> +    } else if (reg & 1) {
> +        /* Set upper valid bit and remove lower one */
> +        retval = (reg & ~3u) | (1u << 31);
> +    } else {
> +        uint32_t slot, func;
> +        uint32_t devfn;
> +
> +        /* Grab CFA0 style values */
> +        slot = ffs(reg & 0xfffff800) - 1;
> +        func = (reg >> 8) & 7;
> +        devfn = PCI_DEVFN(slot, func);
> +
> +        /* ... and then convert them to x86 format */
> +        retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);

Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
number?  This way this encoding can be changed down the road.

> +    }
> +
> +    retval &= ~3u;
> +    retval |= (addr & 7);
> +
> +    UNIN_DPRINTF("Converted config space accessor %08x/%08x -> %08x\n",
> +                 reg, addr, retval);
> +
> +    return retval;
> +}
> +
>  static int pci_unin_main_init_device(SysBusDevice *dev)
>  {
>      UNINState *s;
> @@ -85,6 +119,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev)
>      s = FROM_SYSBUS(UNINState, dev);
>  
>      pci_host_init(&s->host_state);
> +    s->host_state.get_config_reg = unin_get_config_reg;

This looks slightly ugly: let's make pci_host_init accept
the conversion function instead?

>      pci_mem_config = pci_host_conf_register_mmio(&s->host_state);
>      pci_mem_data = pci_host_data_register_mmio(&s->host_state);
>      sysbus_init_mmio(dev, 0x1000, pci_mem_config);
> -- 
> 1.6.0.2
> 
>