From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42845) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBOiY-00088i-Bp for qemu-devel@nongnu.org; Tue, 11 Sep 2012 07:33:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TBOiW-0002Oe-Id for qemu-devel@nongnu.org; Tue, 11 Sep 2012 07:33:22 -0400 Message-ID: <504F217C.80408@suse.de> Date: Tue, 11 Sep 2012 13:33:16 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1344948607-23291-1-git-send-email-Bharat.Bhushan@freescale.com> <502AFB70.2070101@freescale.com> <6A3DF150A5B70D4F9B66A25E3F7C888D06453361@039-SN2MPN1-022.039d.mgd.msft.net> <50492EA9.3050605@freescale.com> <504A43EC.6030004@freescale.com> In-Reply-To: <504A43EC.6030004@freescale.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH: RFC] Adding BAR0 for e500 PCI controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Scott Wood Cc: Wood Scott-B07421 , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Bhushan Bharat-R65777 On 09/07/2012 08:58 PM, Scott Wood wrote: > On 09/07/2012 03:08 AM, Alexander Graf wrote: >> >> On 07.09.2012, at 01:15, Scott Wood wrote: >> >>> On 09/03/2012 01:44 AM, Bhushan Bharat-R65777 wrote: >>>> >>>>> -----Original Message----- From: Wood Scott-B07421 Sent: Wednesday, >>>>> August 15, 2012 6:59 AM To: Bhushan Bharat-R65777 Cc: >>>>> qemu-devel@nongnu.org; qemu-ppc@nongnu.org; agraf@suse.de; Bhushan >>>>> Bharat- R65777 Subject: Re: [Qemu-ppc] [PATCH: RFC] Adding BAR0 for >>>>> e500 PCI controller >>>>> >>>>> On 08/14/2012 07:50 AM, Bharat Bhushan wrote: >>>>>> 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 configured as PCI >>>>>> endpoint, which I am not sure is a use case with QEMU-KVM >>>>> guest. >>>>>> What I observed is that when guest read the size of BAR0 of host >>>>>> controller 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 controller device registering BAR0. I do not >>>>>> find any other controller also doing so may they do not use >>>>>> BAR0. >>>>>> >>>>>> 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) and when reading the size of BAR0, it should give >>>>>> size as per real h/w. >>>>>> >>>>>> 2) Do we need this BAR0 inbound address translation? When BAR0 is >>>>>> of non-zero size then it will be configured for PCI address space >>>>>> to local address(CCSR) space translation on inbound access. The >>>>>> primary use case is for MSI interrupt generation. The device is >>>>>> configured with a address offsets in PCI address space, which >>>>>> will be 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. >>>>> This patch is only trying to address #1, right? I don't see any >>>>> connection from this BAR to CCSR. >>>>> >>>>>> + memory_region_init_io(&h->bar0, &pci_host_conf_be_ops, h, + >>>>>> "PCIHOST-bar0", 0x1000000); >>>>> 0x01000000 is correct for e500mc-based systems, but it should be >>>>> 0x00100000 for e500v2-based systems. >>>> Scott, >>>> >>>> Currently we have a generic e500 machine which have CCSR size >>>> 0x00100000 (MPC8544_CCSRBAR_SIZE). We do not have e500mc and e500v2 >>>> machines. So should we make this 0x00100000 as per generic e500 >>>> machine? >>> Yes, but structure it so that board code decides the size, not the PCI code. >>> >>>> Can we somehow pass this via qdev/varargs from machine emulation code >>>> (hw/ppc/e500.c) ? >>> Possibly, though it may not be the best idea to express every single >>> aspect of intercomponent integration via qdev -- maybe that's best left >>> for things that are reasonably user-tweakable. If CCSR size is user >>> tweakable, it would be somewhere other than the PCI controller. >> It depends. Qdev properties are basically object constructor >> parameters. So if you were weiting C++ code and would have a >> constructor that gets the size as argument, it would end up being >> modeled as qdev property. >> >> If however actual functionality differs, thus you would in OO speech >> create a subclass / child class, then you are better off creating a >> new device struct. >> >> In this case, I'm not sure. They are different devices really, but >> are close enough that the differences could be expressed through qdev >> properties. > I wasn't suggesting that they be different devices. I was suggesting > that this isn't a property of the PCI controller, but rather of some > other entity to which the PCI controller connects. So maybe a reference > to the associated CCSR object would be a qdev parameter, but not the > size of that CCSR. The first common place of information we get is the machine description. So here we can do: create_device(e500_ccsr, CCSR_SIZE); create_device(e500_pci_host_controller, CCSR_SIZE); Obviously code-wise this would look quite different from above, as object constructor parameters go through qdev properties. Alex