Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Wed, Mar 04, 2015 at 04:25:07PM +0100, Alexander Graf wrote:
> 
> 
> On 04.03.15 16:07, Frank Blaschka wrote:
> > On Wed, Mar 04, 2015 at 03:49:15PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 04.03.15 14:44, Frank Blaschka wrote:
> >>> On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 03.03.15 14:25, Frank Blaschka wrote:
> >>>>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <address@hidden>:
> >>>>>>>
> >>>>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
> >>>>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
> >>>>>>>>>>> This patch extends the current s390 pci implementation to
> >>>>>>>>>>> provide more flexibility in configuration of s390 specific
> >>>>>>>>>>> device handling. For this we had to introduce a new facility
> >>>>>>>>>>> (and bus) to hold devices representing information actually
> >>>>>>>>>>> provided by s390 firmware and I/O configuration.
> >>>>>>>>>>>
> >>>>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, 
> >>>>>>>>>>> slot)
> >>>>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
> >>>>>>>>>>> in qemu can also not be shown to the guest. The new zpci device 
> >>>>>>>>>>> class
> >>>>>>>>>>> represents this abstract view on the bare pci function and allows 
> >>>>>>>>>>> to
> >>>>>>>>>>> provide s390 specific configuration attributes for it.
> >>>>>>>>>>>
> >>>>>>>>>>> Sample qemu configuration:
> >>>>>>>>>>> -device e1000,id=zpci1
> >>>>>>>>>>> -device ne2k_pci,id=zpci2
> >>>>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> >>>>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> >>>>>>>>>>>
> >>>>>>>>>>> A zpci device references the corresponding PCI device via device 
> >>>>>>>>>>> id.
> >>>>>>>>>>> The new design allows to define multiple host bridges and support 
> >>>>>>>>>>> more
> >>>>>>>>>>> pci devices.
> >>>>>>>>>>
> >>>>>>>>>> Isn't this reverse? Shouldn't it rather be
> >>>>>>>>>>
> >>>>>>>>>>  -device zpci,...,id=zpci1
> >>>>>>>>>>  -device e1000,bus=zpci1.0
> >>>>>>>>>>
> >>>>>>>>>> with a limit on each virtual zpci bus to only support one device?
> >>>>>>>>>
> >>>>>>>>> Do you mean something like having multiple host bridges (providing 
> >>>>>>>>> a pci bus
> >>>>>>>>> each) and limit the bus to just one device?
> >>>>>>>>>
> >>>>>>>>> -device s390-pcihost,fid=16,uid=1234
> >>>>>>>>> -device s390-pcihost,fid=17,uid=5678
> >>>>>>>>> -device e1000,bus=pci.0
> >>>>>>>>> -device ne2k_pci,bus=pci.1
> >>>>>>>>>
> >>>>>>>>> We also discussed this option but we don't like the idea to put 
> >>>>>>>>> attributes
> >>>>>>>>> belong to the pci device to the host bridge.
> >>>>>>>>
> >>>>>>>> I guess I'm not grasping something obvious here :). What exactly are 
> >>>>>>>> the
> >>>>>>>> attributes again?
> >>>>>>> Sorry for the late response, I was on vacation the last couple days.
> >>>>>>>
> >>>>>>> The fid and uid values are provided by microcode/io layer on the real 
> >>>>>>> hardware.
> >>>>>>
> >>>>>> So they are arbitrary numbers? What uniqueness constraints do we have 
> >>>>>> on them?
> >>>>> fid and uid must be unique within the same qemu. At a first look the 
> >>>>> numbers are
> >>>>> arbitrary but our configuration folks want explicitly define a 
> >>>>> particular fid and uid
> >>>>> to better support migration and pass-through scenarios.
> >>>>
> >>>> Well, at the end of the day you want to make sure they're identical on
> >>>> both sides, yes.
> >>>>
> >>>>>> IIUC you can only have a single pcie device behind a virtual "bus" 
> >>>>>> anyway, so what if we just calculate uid and fid from the bus id?
> >>>>> I think this similar to the current implementation. We use the slot 
> >>>>> (idea for the future was
> >>>>> bus + slot) to generate uid and fid. But this is not flexible enough. 
> >>>>> As I said, our
> >>>>> configuration folks want to be able to specify fid and uid for the 
> >>>>> device.
> >>>>
> >>>> I don't see how this is different from what PPC does with its LIOBN
> >>>> which is a property of the PHB.
> >>>>
> >>>>
> >>>> Alex
> >>>>
> >>>
> >>> I played arround with the idea of having multiple host bridges and this 
> >>> worked well
> >>> at least for static (non hotplug) configuration. In case I want to 
> >>> hotplug a host
> >>> bridge I got following error:
> >>>
> >>> (qemu) device_add s390-pcihost,fid=8,uid=9
> >>> Bus 'main-system-bus' does not support hotplugging
> >>>
> >>> Is there anything I have to enable to support this?
> >>>
> >>> I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet 
> >>> = false
> >>> but this seems not to help for the hotplug case.
> >>
> >> Having s390 devices reside on sysbus is probably a bad idea. Instead,
> >> they should be on an s390 specific bus which then can implement hotplug
> >> easily.
> >>
> >>
> >> Alex
> >>
> > 
> > Hm now I get lost ...
> > 
> > Do you suggest we should implement a s390 specific device (which is not 
> > derived from
> > TYPE_PCI_HOST_BRIDGE) but implements a pci bus so we can attach a pci 
> > device to this
> > device?  
> 
> Ugh, PCI_HOST_BRIDGE is a sysbus device. Awesome.
> 
> Conceptually your PCI bridge is not a sysbus device, since it doesn't
> live on a flat MMIO + legacy IRQ routing bus. Instead, it lives on its
> own thing that handles MMIO and IRQs via special backdoor interfaces.
>
well spoken :-)

> How much of the PCI_HOST_BRIDGE device are you actually using? Would it
> be a lot of effort to have another s390 specific device that exposes a
> PCIBus, but is not of type PCI_HOST_BRIDGE (and thus sysbus)?
> 
I do not use much functionality of the PCI_HOST_BRIDGE but I was not able
to put a pci bus on a device != PCI_HOST_BRIDGE. If I recall it correctly
the pci bus code wants to collect all bridges in a list.
I have to do some more research to find out if it is possible to change
this ...

> 
> Alex
>