[Qemu-devel] Re: May I use -device in qemu-system command to attach to PCIe bus?

[Isaku Yamahata <yamahata@valinux.co.jp>]

On Thu, Oct 28, 2010 at 03:37:27AM -0700, Wei Xu wrote:
> Isaku,
> 
> To make things clear, let me rephrase the problem. With q35/vPCIe, in VMM
> monitor, we can do hotplug like:
>  pci_add auto|<bus:dev> nic|storage
> to hotplug a device to PCIe/PCI bus.
> 
> But we have two problems  here:
> (1) command line for example, "-net nic,addr=<bus:dev>" always failed
> because it cannot find the bus.
> (2) If "pci_add auto" in monitor or no "addr=<bus:dev" in command line, the
> device will be attached to PCI bus, in stead of PCIe ports
> 
> I solved the first problem. The root cause is pcie_root_write_config (which
> init SECONDARY_BUS config) happened after pci_nic_init. The latter use
> pci_find_bus to find the parent bus but failed because config space for
> secondary bus still not initialized.

Okay, now the issue is clear to me.
When I tried to clean up pci bridge code, I included the similar lines
in bridge initialization function.
But Michael complained about it, so I dropped the lines for merge.

I think there are two ways to address it.
- initialize secondary/subordinate bus register by qemu
  i.e. something like the patch below.

- use qdev device path
  i.e. qbus_find() instead of pci_find_bus().
  I don't know if the corresponding command line option already exists or not.

My preference is the former, but discussion is necessary to get consensus.
My plan was to raise the issue when I address pv pci bus numbering issue.
I've expected that other developers wouldn't think the issue important
because multiple pci bus can't be used easily right now.

thanks,

> 
> The fix is like:
> 
> [wexu2@wexu2-lnx contents]$ git diff hw/pci_bridge.c
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index c048305..042c1f9 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -203,6 +203,7 @@ void pci_bridge_set_bus_number(PCIBridge *br,
>      uint16_t domain;
>      uint8_t primary_bus;
>      uint8_t devfn;
> +    uint8_t *conf;
>  
>      if (!pci_bus_number) {
>          pci_bus_number = qemu_mallocz(sizeof(*pci_bus_number));
> @@ -215,10 +216,15 @@ void pci_bridge_set_bus_number(PCIBridge *br,
>  
>      d = &br->dev;
>      devfn = d->devfn;
> +       conf = d->config;
>  
>      br->sec_bus = secondary;
>      br->sub_bus = subordinate;
>  
> +       //TODO wexu2 workaround of "-net nic,addr=<bus:dev>"
> +    pci_set_byte(conf + PCI_SECONDARY_BUS, secondary);
> +    pci_set_byte(conf + PCI_SUBORDINATE_BUS, subordinate);
> +
>      bus = d->bus;
>      primary_bus = 0;
>      if ((d = pci_bridge_get_device(bus)) != NULL){
> 
> The fix is a little bit hacky -- welcome any advice.
> 
> For 2nd issue, current code assume PCI bus is flat and not straight forward
> to change the behavior.
> 
> Wei Xu
> 
> 
> On 10/27/10 10:16 PM, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:
> 
> > Oh, now I'm seeing your point. Some codes of qemu assume that
> > the pci bus is flat. They need fixes.
> > 
> > Right now I've found
> > - parse_pci_devfn in qdev-properties.c
> >   This corresponds to (1)
> > - pci_get_bus_devfn() in pci.c
> >   This corresponds to (2)
> > 
> > thanks,
> > 
> > On Wed, Oct 27, 2010 at 08:51:29PM -0700, Wei Xu wrote:
> >> Isaku,
> >> 
> >> I found two issues:
> >> (1) command line option is not working: even with addr=<bus>, it complained
> >> bus cannot be found;
> >> (2) monitor: pci_add auto ... cannot work.
> >> 
> >> Wei
> >> 
> >> 
> >> On 10/27/10 8:03 PM, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:
> >> 
> >>> On Wed, Oct 27, 2010 at 09:51:47AM -0700, Wei Xu wrote:
> >>>> Isaku,
> >>>> 
> >>>> I have one question regarding EP and PCIe bus: now only way is to use
> >>>> "pci_add <bus:dev> .." to hot-plug the device to a PCIe port. Looks like
> >>>> command line with -device parameter automatically use PCI bus? Is it
> >>>> possible to support it? Have to provide the <bus:dev> in -device parameter?
> >>> 
> >>> I'm not sure I got your question right.
> >>> We can populate the device under an express port with -device command line
> >>> option. At least it should be possible.
> >>> 
> >>> For implementation detail, qemu doesn't distinguish pci express
> >>> from conventional pci so strictly.
> >>> The only difference is whether the bridge(pci host bridge or
> >>> pci-to-pci bridge) is just express port because I implemented express
> >>> ports as pci-to-pci bridge.
> >>> 
> >>> Or do you want to add express port with command line option?
> >>> If so, not possible at the moment.
> >> 
> 

-- 
yamahata