|
From: | Alexander Graf |
Subject: | Re: [Qemu-ppc] [PATCH: RFC] Adding BAR0 for e500 PCI controller |
Date: | Tue, 11 Sep 2012 13:33:16 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux i686 on x86_64; rv:14.0) Gecko/20120713 Thunderbird/14.0 |
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 <address@hidden> 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: address@hidden; address@hidden; address@hidden; 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-KVMguest.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
[Prev in Thread] | Current Thread | [Next in Thread] |