[Qemu-devel] Re: [PATCH v3 08/13] pcie root port: implement pcie root port.

[Isaku Yamahata]

On Mon, Sep 27, 2010 at 12:40:12PM +0200, Michael S. Tsirkin wrote:
> On Mon, Sep 27, 2010 at 03:22:43PM +0900, Isaku Yamahata wrote:
> > On Sun, Sep 26, 2010 at 02:50:42PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Sep 24, 2010 at 02:38:09PM +0900, Isaku Yamahata wrote:
> > > > On Wed, Sep 22, 2010 at 01:25:59PM +0200, Michael S. Tsirkin wrote:
> > > > 
> > > > > > +PCIESlot *pcie_root_init(PCIBus *bus, int devfn, bool 
> > > > > > multifunction,
> > > > > > +                         const char *bus_name, pci_map_irq_fn 
> > > > > > map_irq,
> > > > > > +                         uint8_t port, uint8_t chassis, uint16_t 
> > > > > > slot);
> > > > > > +
> > > > > 
> > > > > I am a bit unhappy about all these _init functions.
> > > > > Can devices be created with qdev? If they were
> > > > > it would be possible to configure the system completely
> > > > > from qemu command line.
> > > > 
> > > > That's very reasonable question.
> > > > Once machine configuration file is supported, those initialization
> > > > functions will go away.
> > > > I.e. when the initialization code like pc_init1() in pc_piix.c 
> > > > disappears,
> > > > those functions will also go away.
> > > > 
> > > > Until that, those initialization glues will stay like pci_create family
> > > > or other many initialization glues unfortunately.
> > > > This is the result of qdev missing a feature, not the cause.
> > > > It would be a long-term issue to add machine configuration file support.
> > > 
> > > Just to clarify, if I wanted to have a flag to make virtio-net
> > > a pci express device, how would I do this?
> > 
> > the following preparation is needed.
> > - register PCIDeviceInfo with name like "virtio-net-pci-xen"
> >   with PCIDeviceInfo::is_express = true.
> > - in initialization function, initialize express capability.
> > - in write config function, call related function.
> > 
> > And then,
> > if (express)
> >    create "virtio-net-pci-xen"
> > else
> >    create "virtio-net-pci"
> 
> Sounds pretty bad: we can't double the number of devices
> with each capability we add.
> Can we make it so setting is_express on command line
> will convert the device to PCI express?

I don't see your point. Capability isn't something that should be
genericly customizable.
If any, it would not be a generic option, but one specific to device.
There is no generic way to automatically convert pci device into
express device. It means designing a new express device.

Maybe what you want is
- set is_express = true
  This result in always allocating 4k-sized configuration space.
  (Possibly we need more fine-grained parameter in PCIDeviceInfo
   than is_express.)
- introduce device specific property that a user can turn on/off
- In initialization function/config write function and where necessary,
  check the property.
-- 
yamahata