Re: PCI Pass-through in Xen ARM: Draft 4

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

On Thu, 2015-08-13 at 09:29 -0600, Jan Beulich wrote:
>                >>> On 13.08.15 at 11:42, <mjaggi@caviumnetworks.com> 
> wrote:
> >   2.1    pci_hostbridge and pci_hostbridge_ops
> >   ---------------------------------------------------------------------
> > --------
> >   The init function in the PCI host driver calls to register hostbridge
> >   callbacks:
> > 
> >   int pci_hostbridge_register(pci_hostbridge_t *pcihb);
> > 
> >   struct pci_hostbridge_ops {
> >       u32 (*pci_conf_read)(struct pci_hostbridge*, u32 bus, u32 devfn,
> >                                   u32 reg, u32 bytes);
> >       void (*pci_conf_write)(struct pci_hostbridge*, u32 bus, u32 
> > devfn,
> >                                   u32 reg, u32 bytes, u32 val);
> >   };
> > 
> >   struct pci_hostbridge{
> >       u32 segno;
> >       paddr_t cfg_base;
> >       paddr_t cfg_size;
> >       struct dt_device_node *dt_node;
> >       struct pci_hostbridge_ops ops;
> >       struct list_head list;
> >   };
> > 
> >   A PCI conf_read function would internally be as follows:
> >   u32 pcihb_conf_read(u32 seg, u32 bus, u32 devfn,u32 reg, u32 bytes)
> >   {
> >       pci_hostbridge_t *pcihb;
> >       list_for_each_entry(pcihb, &pci_hostbridge_list, list)
> >       {
> >           if(pcihb-segno == seg)
> >               return pcihb-ops.pci_conf_read(pcihb, bus, devfn, reg, 
> > bytes);
> >       }
> >       return -1;
> >   }
> 
> Which implies 1 segment per host bridge, which doesn't seem too
> nice to me: I can't see why a bridge might not cover more than one
> segment, and I also can't see why you shouldn't be able to put
> multiple bridges in the same segment when the number of busses
> they have is small.

Does this imply that:
  #define PHYSDEVOP_pci_host_bridge_add    <<>>
  struct physdev_pci_host_bridge_add {
      /* IN */
      uint16_t seg;
      uint64_t cfg_base;
      uint64_t cfg_size;
  };
(as specified in this draft 4) ought to have a bus field (as proposed in 
http://article.gmane.org/gmane.comp.emulators.xen.devel/233386) or perhaps
even a bus and nr_buses field to give a span?

And then that the lookup functions ought to take those into account, of
course.

In the case where a single bridge covers multiple segments would we then
need to support calling this function multiple times i.e. once for each
segment the bridge contains each pointing to the same cfg region? Or
something similar.

Ian.