From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59977) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLenn-0004rX-Jf for qemu-devel@nongnu.org; Tue, 09 Oct 2012 14:45:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLenj-0005XY-3m for qemu-devel@nongnu.org; Tue, 09 Oct 2012 14:45:11 -0400 Message-ID: <507470AD.2070703@suse.de> Date: Tue, 09 Oct 2012 20:45:01 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1349806750-17652-1-git-send-email-Bharat.Bhushan@freescale.com> <1349806750-17652-3-git-send-email-Bharat.Bhushan@freescale.com> In-Reply-To: <1349806750-17652-3-git-send-email-Bharat.Bhushan@freescale.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2 v2] Adding BAR0 for e500 PCI controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharat Bhushan Cc: Bharat Bhushan , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de Am 09.10.2012 20:19, schrieb Bharat Bhushan: > PCI Root complex have TYPE-1 configuration header while PCI endpoint > have type-0 configuration header. The type-1 configuration header have > a BAR (BAR0). In Freescale PCI controller BAR0 is used for mapping pci > address space to CCSR address space. This can used for 2 purposes: 1) > for MSI interrupt generation 2) Allow CCSR registers access when config= ured > as PCI endpoint, which I am not sure is a use case with QEMU-KVM guest. >=20 > What I observed is that when guest read the size of BAR0 of host contro= ller > configuration header (TYPE1 header) then it always reads it as 0. When > looking into the QEMU hw/ppce500_pci.c, I do not find the PCI controlle= r > device registering BAR0. I do not find any other controller also doing = so > may they do not use BAR0. >=20 > There are two issues when BAR0 is not there (which I can think of): > 1) There should be BAR0 emulated for PCI Root comaplex (TYPE1 header) a= nd "complex" > when reading the size of BAR0, it should give size as per real h/w. >=20 > 2) Do we need this BAR0 inbound address translation? > When BAR0 is of non-zero size then it will be configured for PC= I > address space to local address(CCSR) space translation on inbound acces= s. > The primary use case is for MSI interrupt generation. The device is > configured with a address offsets in PCI address space, which will be "with address offsets" or "with an address offset" > translated to MSI interrupt generation MPIC registers. Currently I do > not understand the MSI interrupt generation mechanism in QEMU and also > IIRC we do not use QEMU MSI interrupt mechanism on e500 guest machines. > But this BAR0 will be used when using MSI on e500. >=20 > I can see one more issue, There are ATMUs emulated in hw/ppce500_pci.c, > but i do not see these being used for address translation. > So far that works because pci address space and local address space are= 1:1 > mapped. BAR0 inbound translation + ATMU translation will complete the a= ddress > translation of inbound traffic. >=20 > Signed-off-by: Bharat Bhushan This looks perfect except for typos above and one line of code that maybe Alex can fix. Reviewed-by: Andreas F=E4rber > diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c > index 92b1dc0..58dbc1a 100644 > --- a/hw/ppce500_pci.c > +++ b/hw/ppce500_pci.c > @@ -307,6 +321,19 @@ static const VMStateDescription vmstate_ppce500_pc= i =3D { > =20 > #include "exec-memory.h" > =20 > +static int e500_pcihost_bridge_initfn(PCIDevice *d) > +{ > + PPCE500PCIBridgeState *b =3D PPC_E500_PCI_BRIDGE(d); > + PPCE500CCSRState *ccsr =3D CCSR(container_get(qdev_get_machine(), > + "/e500-ccsr")); > + > + b->bar0 =3D ccsr->ccsr_space; This copy-assignment is getting overwritten by the alias init in the next line, so it would seem cleaner to drop this line now. Andreas > + memory_region_init_alias(&b->bar0, "e500-pci-bar0", &ccsr->ccsr_sp= ace, > + 0, int128_get64(ccsr->ccsr_space.size)); > + pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0); > + return 0; > +} > + > static int e500_pcihost_initfn(SysBusDevice *dev) > { > PCIHostState *h; --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg