Re: [Xen-devel] PCI Pass-through in Xen ARM: Draft 4

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.09.15 at 16:57, <ian.campbell@xxxxxxxxxx> wrote:
> On Thu, 2015-08-13 at 09:29 -0600, Jan Beulich wrote:
>>                >>> On 13.08.15 at 11:42, <mjaggi@xxxxxxxxxxxxxxxxxx>>> 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?

The latter, yes.

> 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.

Right.

Jan


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