Re: [Qemu-devel] Managing architectural restrictions with -device and libvirt

[Markus Armbruster]

Mark Cave-Ayland <address@hidden> writes:

> On 05/07/17 16:46, Markus Armbruster wrote:
>
>>>>> I've been working on a patchset that brings the sun4u machine on
>>>>> qemu-system-sparc64 much closer to a real Ultra 5, however due to
>>>>> various design restrictions I need to be able to restrict how devices
>>>>> are added to the machine with -device.
>>>>>
>>>>> On a real Ultra 5, the root PCI bus (sabre) has 2 PCI bridges (simba A
>>>>> and simba B) with the onboard devices attached to simba A with 2 free
>>>>> slots, and an initially empty simba B.
>>>>>
>>>>> Firstly, is it possible to restrict the machine so that devices cannot
>>>>> be directly plugged into the root PCI bus, but only behind one of the
>>>>> PCI bridges? There is also an additional restriction in that slot 0
>>>>> behind simba A must be left empty to ensure that the ebus (containing
>>>>> the onboard devices) is the first device allocated.
>>>>
>>>> I figure sabre, simba A, simba B and the onboard devices attached to
>>>> simba A are all created by MachineClass init().
>>>
>>> Yes that is effectively correct, although the Simba devices are created
>>> as part of the PCI host bridge (apb) creation in pci_apb_init().
>> 
>> Anything that runs within init() counts as "created by init()".
>
> Okay, in that case we should be fine here.
>
>>>> What device provides "the ebus", and how is it created?
>>>
>>> It's actually just an ISA bus, so the ebus device is effectively a
>>> PCI-ISA bridge for legacy devices.
>> 
>> Is this bridge created by init()?
>
> Yes, it too is called via the machine init function.
>
>>>> Can you provide a list of all onboard PCI devices and how they are
>>>> connected?  Diagram would be best.
>>>
>>> I can try and come up with something more concise later, however I can
>>> quickly give you the OpenBIOS DT from my WIP patchset if that helps:
>>>
>>> 0 > show-devs
>>> ffe1bf38 /
>>> ffe1c110 /aliases
>>> ffe1c238 /openprom (BootROM)
>>> ffe26b50 /openprom/client-services
>>> ffe1c4f0 /options
>>> ffe1c5d0 /chosen
>>> ffe1c710 /builtin
>>> ffe1c838 /builtin/console
>>> ffe26618 /packages
>>> ffe28640 /packages/cmdline
>>> ffe28890 /packages/disk-label
>>> ffe2c8d8 /packages/deblocker
>>> ffe2cef0 /packages/grubfs-files
>>> ffe2d300 /packages/sun-parts
>>> ffe2d718 /packages/elf-loader
>>> ffe2b210 /address@hidden,0 (memory)
>>> ffe2b370 /virtual-memory
>>> ffe2d878 /address@hidden,0 (pci)
>>> ffe2e1a8 /address@hidden,0/address@hidden,1 (pci)
>>> ffe2e960 /address@hidden,0/address@hidden,1/address@hidden
>>> ffe2f1b0 /address@hidden,0/address@hidden,1/address@hidden/address@hidden
>>> ffe2f328 /address@hidden,0/address@hidden,1/address@hidden/address@hidden 
>>> (block)
>>> ffe2f878 /address@hidden,0/address@hidden,1/address@hidden/address@hidden 
>>> (serial)
>>> ffe2fc08 /address@hidden,0/address@hidden,1/address@hidden/address@hidden 
>>> (8042)
>>> ffe2fe00 
>>> /address@hidden,0/address@hidden,1/address@hidden/address@hidden/address@hidden
>>>  (serial)
>>> ffe301b0 /address@hidden,0/address@hidden,1/address@hidden,1 (network)
>>> ffe307c8 /address@hidden,0/address@hidden,1/QEMU,address@hidden (display)
>>> ffe31e40 /address@hidden,0/address@hidden,1/address@hidden (ide)
>>> ffe32398 /address@hidden,0/address@hidden,1/address@hidden/address@hidden 
>>> (ide)
>>> ffe32678 /address@hidden,0/address@hidden,1/address@hidden/address@hidden 
>>> (ide)
>>> ffe32910 
>>> /address@hidden,0/address@hidden,1/address@hidden/address@hidden/address@hidden
>>>  (block)
>>> ffe32f98 /address@hidden,0/address@hidden (pci)
>>> ffe336e8 /SUNW,UltraSPARC-IIi (cpu)
>>>  ok
>>>
>>> For comparison you can see the DT from a real Ultra 5 here:
>>> http://www.pearsonitcertification.com/articles/article.aspx?p=440286&seqNum=7
>>>
>>>> The real sabre has two slots, and doesn't support hot (un)plug.  Can we
>>>> simply model that?  If yes, the root PCI bus is full after init(), and
>>>> remains full.  Takes care of "cannot directly plugged into the root PCI
>>>> bus".
>>>
>>> Right. So what you're saying is that if we add the 2 simba devices to
>>> the sabre PCI host bridge during machine init and then mark the sabre
>>> PCI root bus as not hotplug-able then that will prevent people adding
>>> extra devices from the command line via -device? I will see if I can
>>> find time to try this later this evening.
>> 
>> No.  Marking the bus "not hotpluggable" only prevents *hotplug*,
>> i.e. plug/unplug after machine initialization completed, commonly with
>> device_add.  -device is *cold* plug; it happens during machine
>> initialization.
>> 
>> However, if you limit sabre's bus to two slots (modelling real hardware
>> faithfully), then you can't cold plug anything (there's no free slot).
>> If you additionally mark the bus or both simba devices not hotpluggable
>> (again modelling real hardware faithfully), you can't unplug the simbas.
>> I believe that's what you want.
>
> It seems like limiting the size of the bus would solve the majority of
> the problem. I've had a quick look around pci.c and while I can see that
> the PCIBus creation functions take a devfn_min parameter, I can't see
> anything that limits the number of slots available on the bus?

Marcel?

> And presumably if the user did try and coldplug something into a full
> bus then they would get the standard "PCI: no slot/function
> available..." error?

That's what I'd expect.

>>> My understanding from reading various bits of documentation is that the
>>> the empty simba bridge (bus B) can hold a maximum of 4 devices, whilst
>>> the non-empty simba bridge (bus A) can hold a maximum of 2 devices
>>> (presumably due to the on-board hardware). And in order to make sure
>>> OpenBIOS maps the PCI IO ranges correctly, the ebus must be the first
>>> on-board device found during a PCI bus scan which means slot 0 on bus A
>>> must be blacklisted.
>> 
>> Assuming init() plugs in the device providing ebus: plug it into slot 0,
>> mark it not hotpluggable, done.
>
> That is good solution in theory except that I'd like to keep the ebus in
> slot 1 so that it matches the real DT as much as possible. In the future
> it could be possible for people to boot using PROMs from a real Sun and
> I'm not yet convinced that there aren't hardcoded references to some of
> the onboard legacy devices in a real PROM.

Misunderstanding on my part!  You don't have to blacklist slot 0 to have
the PCI core put ebus in slot 1.  Simply ask for slot 1 by passing
PCI_DEVFN(1, 0) to pci_create() or similar.

>>> I guess what I'm looking for is some kind of hook that runs after both
>>> machine init and all the devices have been specified on the command
>>> line, which I can use to validate the configuration and provide a
>>> suitable error message/hint if the configuration is invalid?
>> 
>> You should be able to construct the machine you want, and protect the
>> parts the user shouldn't mess with from messing users.  No need to
>> validate the mess afterwards then.
>
> Unfortunately there would be issues if the user was allowed to construct
> a machine with more PCI devices than slots in real hardware, since the
> PCI interrupt number is limited to 4 bits - 2 bits for the PCI interrupt
> number (A to D), and 2 bits for the slot. So if a user tries to plug in
> more than 4 devices into each simba bus then the interrupts won't be
> mapped correctly.
>
> My feeling is that it makes more sense to error out if the user tries to
> add too many devices to the bus and/or in the wrong slots rather than
> let them carry on and wonder why the virtual devices don't work
> correctly, but I'm open to other options.

My advice is to model the physical hardware faithfully.  If it has four
PCI slots on a certain PCI bus, provide exactly four.  If it has onboard
devices hardwired into a certain slot, put them exactly there, and
disable unplug.  Make it impossible to plug too many devices into a bus,
or into the wrong slots.