Re: [Qemu-devel] [PATCH 2/3] e500: Adding CCSR memory region

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Avi Kivity [mailto:address@hidden
> Sent: Tuesday, October 09, 2012 2:35 PM
> To: Andreas Färber
> Cc: Bhushan Bharat-R65777; address@hidden; address@hidden; qemu-
> address@hidden; Bhushan Bharat-R65777
> Subject: Re: [PATCH 2/3] e500: Adding CCSR memory region
> 
> On 10/08/2012 07:21 PM, Andreas Färber wrote:
> > Am 08.10.2012 18:46, schrieb Bharat Bhushan:
> >> All devices are also placed under CCSR memory region.
> >> The CCSR memory region is exported to pci device. The MSI interrupt
> >> generation is the main reason to export the CCSR region to PCI device.
> >> This put the requirement to move mpic under CCSR region, but
> >> logically all devices should be under CCSR. So this patch places all
> >> emulated devices under ccsr region.
> >>
> >> +    sysbus_connect_irq(s, 0, mpic[pci_irq_nrs[0]]);
> >> +    sysbus_connect_irq(s, 1, mpic[pci_irq_nrs[1]]);
> >> +    sysbus_connect_irq(s, 2, mpic[pci_irq_nrs[2]]);
> >> +    sysbus_connect_irq(s, 3, mpic[pci_irq_nrs[3]]);
> >> +    memory_region_add_subregion(ccsr, MPC8544_PCI_REGS_OFFSET,
> >> + s->mmio[0].memory);
> >
> > ... I wonder if fiddling with SysBus MMIO is a good idea.
> > s->mmio[0].addr is not getting assigned this way, which is checked as
> > condition for deleting the subregion. But sysbus_mmio_map() only adds
> > to / deletes from get_system_memory().
> > The alternative would be using a custom field rather than the
> > SysBus-internal one. Avi/Alex?
> 
> IMO yes.  Or not use sysbus at all.

What about adding a API:
void sysbus_mmio_map_to_mr(SysBusDevice *dev, int n, target_phys_addr_t addr,
                           MemoryRegion *mr)
{
    assert(n >= 0 && n < dev->num_mmio);

    if (dev->mmio[n].addr == addr) {
        /* ??? region already mapped here.  */
        return;
    }
    if (dev->mmio[n].addr != (target_phys_addr_t)-1) {
        /* Unregister previous mapping.  */
        memory_region_del_subregion(mr, dev->mmio[n].memory);
    }
    dev->mmio[n].addr = addr;
    memory_region_add_subregion(mr, addr, dev->mmio[n].memory);
}

Thanks
-Bharat

> 
> 
> --
> error compiling committee.c: too many arguments to function