Re: RFC: [PATCH 1/3] Enhance platform support for PCI

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2015-02-27 at 14:33 +0000, Stefano Stabellini wrote:
> On Thu, 26 Feb 2015, Ian Campbell wrote:
> > On Thu, 2015-02-26 at 15:39 +0530, Manish Jaggi wrote:
> > > Have you reached a conclusion?
> > 
> > My current thinking on how PCI for Xen on ARM should look is thus:
> > 
> > xen/arch/arm/pci.c:
> > 
> >         New file, containing core PCI infrastructure for ARM. Includes:
> >         
> >         pci_hostbridge_register(), which registers a host bridge:
> >         
> >                 Registration includes:
> >                         DT node pointer
> >                         
> >                         CFG space address
> >                 
> >                         pci_hostbridge_ops function table, which
> >                         contains e.g. cfg space read/write ops, perhaps
> >                         other stuff).
> >         
> >         Function for setting the (segment,bus) for a given host bridge.
> >         Lets say pci_hostbridge_setup(), the host bridge must have been
> >         previously registered. Looks up the host bridge via CFG space
> >         address and maps that to (segment,bus).
> >         
> >         Functions for looking up host bridges by various keys as needed
> >         (cfg base address, DT node, etc)
> >         
> >         pci_init() function, called from somewhere appropriate in
> >         setup.c which calls device_init(node, DEVICE_PCIHOST, NULL) (see
> >         gic_init() for the shape of this)
> >         
> >         Any other common helper functions for managing PCI devices, e.g.
> >         for implementing PHYSDEVOP_*, which cannot be made properly
> >         common (i.e. shared with x86).
> > 
> > xen/drivers/pci/host-*.c (or pci/host/*.c):
> > 
> >         New files, one per supported PCI controller IP block. Each
> >         should use the normal DT_DEVICE infrastructure for probing,
> >         i.e.:
> >         DT_DEVICE_START(foo, "FOO", DEVICE_PCIHOST)
> >         
> >         Probe function should call pci_hostbridge_register() for each
> >         host bridge which the controller exposes.
> >         
> > xen/arch/arm/physdev.c:
> > 
> >         Implements do_physdev_op handling PHYSDEVOP_*. Includes:
> >         
> >         New hypercall subop PHYSDEVOP_pci_host_bridge_add:
> >         
> >                 As per 1424703761.27930.140.camel@citrix.com> which
> >                 calls pci_hostbridge_setup() to map the (segment,bus) to
> >                 a specific pci_hostbridge_ops (i.e. must have previously
> >                 been registered with pci_hostbridge_register(), else
> >                 error).
> 
> I think that the new hypercall is unnecessary. We know the MMCFG address
> ranges belonging to a given host bridge from DT and
> PHYSDEVOP_pci_mmcfg_reserved gives us segment, start_bus and end_bus for
> a specific MMCFG.

My understanding from discussion with Jan was that this is not what this
hypercall does, or at least that this would be an abuse of the existing
interface. See: <54E75D8702000078000621DD@mail.emea.novell.com>

Anyway, what happens for when there is no MMCFG table to drive dom0's
calls to pci_mmcfg_reserved? Or a given host-bridge doesn't have special
flags and so isn't mentioned there.

I think a dedicated hypercall is better.

Ian.