qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI controller


From: Bhushan Bharat-R65777
Subject: Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI controller
Date: Thu, 4 Oct 2012 16:03:42 +0000


> -----Original Message-----
> From: Avi Kivity [mailto:address@hidden
> Sent: Thursday, October 04, 2012 8:28 PM
> To: Bhushan Bharat-R65777
> Cc: address@hidden; address@hidden; address@hidden
> Subject: Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI controller
> 
> On 10/04/2012 03:46 PM, Bhushan Bharat-R65777 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Avi Kivity [mailto:address@hidden
> >> Sent: Thursday, October 04, 2012 6:02 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: address@hidden; address@hidden; address@hidden;
> >> Bhushan Bharat-
> >> R65777
> >> Subject: Re: [Qemu-devel] [PATCH 2/2] Adding BAR0 for e500 PCI
> >> controller
> >>
> >> On 10/03/2012 01:50 PM, Bharat Bhushan wrote:
> >> >      sysbus_connect_irq(s, 0, mpic[pci_irq_nrs[0]]); diff --git
> >> > a/hw/ppce500_pci.c b/hw/ppce500_pci.c index 92b1dc0..16e4af2 100644
> >> > --- a/hw/ppce500_pci.c
> >> > +++ b/hw/ppce500_pci.c
> >> > @@ -87,6 +87,7 @@ struct PPCE500PCIState {
> >> >      /* mmio maps */
> >> >      MemoryRegion container;
> >> >      MemoryRegion iomem;
> >> > +    void *bar0;
> >> >  };
> >>
> >> void *?  Why?
> >
> > Passing parameter via qdev is either possible by value or by void *. That's
> why I used void *.
> 
> Then you shouldn't be using qdev for this.  But if you make the changes below,
> it will likely not be necessary.
> 
> >>
> >> However this is best done from the pci device's initialization
> >> function, not here (the MemoryRegion should be a member of the pci device 
> >> as
> well).
> >
> > We want to set BAR0 of PCI controller so we did this here. But yes, we also
> want that all devices under the PCI controller have this mapping, so when they
> access the MPIC register to send MSI then they can do that.
> 
> Please elaborate.  One way to describe what you want is to issue an 'info 
> mtree'
> and show what changes you want.

Setup is something like this:

  |-------------|
  | PCI device  |
  |             |
  ---------------
        |
        |
        |
        |
 |-------------------|
 |                   |
 | PCI controller    |
 |                   |
 |   --------------  |
 |   |  BAR0 in   |  |
 |   |   TYPE1    |  |
 |   | Config HDR |  |
 |   --------------  |
 |                   |
  -------------------

BAR0: it is an inbound window, and on e500 devices this maps the pci bus 
address to CCSR address.

My requirement are:
 1) when guest read pci controller BAR0, it is able to get proper size ( Size 
of CCSR space)
 2) Guest can program this to any address in pci address space. When PCI device 
access this address range then that address should be mapped to CCSR address 
space.

Though this patch only met the requirement number (1). 

> 
> >
> > Which device's initialization function you are talking about?
> 
> static const TypeInfo e500_host_bridge_info = {
>     .name          = "e500-host-bridge",
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(PCIDevice),
>     .class_init    = e500_host_bridge_class_init,
> };
> 
> This needs to describe a derived class of PCIDevice, hold the BAR as a data
> member, and register the BAR in the init function (which doesn't exist yet
> because you haven't subclassed it).  This way the BAR lives in the device 
> which
> exposes it, like BARs everywhere.

Do you mean that we add the "MemoryRegion bar0" in PCIDevice struct. Do the 
same thing that I was doing in e500_pcihost_initfn() in the k->init() (will add 
this) function of "e500-host-bridge"

This way I should be able to met both of my requirements.

Thanks
-Bharat

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





reply via email to

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