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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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@xxxxxxxxxx> 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. We don't need anything else: we can simply match the
host bridge based on the MMCFG address that dom0 tells us via
PHYSDEVOP_pci_mmcfg_reserved with the addresses on DT.

But we do need to support PHYSDEVOP_pci_mmcfg_reserved on ARM.


>         PHYSDEVOP_pci_device_add/remove: Implement existing hypercall
>         interface used by x86 for ARM.
>         
>                 This requires that PHYSDEVOP_pci_host_bridge_add has
>                 been called for the (segment,bus) which it refers to,
>                 otherwise error.
>                 
>                 Looks up the host bridge and does whatever setup is
>                 required plus e.g. calling of pci_add_device().
> 
> No doubt various other existing interfaces will need wiring up, e.g.
> pci_conf_{read,write}* should lookup the host bridge ops struct and call
> the associated method.
> 
> I'm sure the above must be incomplete, but I hope the general shape
> makes sense?
 
I think it makes sense and it is along the lines of what I was thinking
too.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel