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

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

[Mark Cave-Ayland]

Hi all,

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.

Secondly, how does libvirt handle these type of restrictions? Is it able
to get the information from QEMU or is there some kind of libvirt
profile that needs to be updated? And do newer versions of libvirt have
the ability to attach devices behind PCI bridges using a GUI such as
virt-manager, or is that only something that can only be done by
directly editing the domain XML?


ATB,

Mark.

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

[Daniel P. Berrange]

On Tue, Jul 04, 2017 at 07:25:41PM +0100, Mark Cave-Ayland wrote:
> Hi all,
> 
> 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.
> 
> Secondly, how does libvirt handle these type of restrictions? Is it able
> to get the information from QEMU or is there some kind of libvirt
> profile that needs to be updated? And do newer versions of libvirt have
> the ability to attach devices behind PCI bridges using a GUI such as
> virt-manager, or is that only something that can only be done by
> directly editing the domain XML?

Libvirt has a bunch of code that provides different logic for various
machine types when doing addressing & setting up default PCI controllers.
Libvirt support for sparc machine types almost certainly broken since
I'm not aware of anyone having looked at it forever. Probably best to
re-ask your Q's on the libvir-list to get info from the people familiar
with this, since I'm fuzzy on precise details of what you'd need todo.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Markus Armbruster]

Copying Marcel for PCI expertise.

Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:

> Hi all,
>
> 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().

What device provides "the ebus", and how is it created?

Can you provide a list of all onboard PCI devices and how they are
connected?  Diagram would be best.

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

> Secondly, how does libvirt handle these type of restrictions? Is it able
> to get the information from QEMU or is there some kind of libvirt
> profile that needs to be updated? And do newer versions of libvirt have
> the ability to attach devices behind PCI bridges using a GUI such as
> virt-manager, or is that only something that can only be done by
> directly editing the domain XML?

Got nothing to add to Dan's reply.

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

[Mark Cave-Ayland]

On 04/07/17 21:44, Daniel P. Berrange wrote:

> On Tue, Jul 04, 2017 at 07:25:41PM +0100, Mark Cave-Ayland wrote:
>> Hi all,
>>
>> 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.
>>
>> Secondly, how does libvirt handle these type of restrictions? Is it able
>> to get the information from QEMU or is there some kind of libvirt
>> profile that needs to be updated? And do newer versions of libvirt have
>> the ability to attach devices behind PCI bridges using a GUI such as
>> virt-manager, or is that only something that can only be done by
>> directly editing the domain XML?
> 
> Libvirt has a bunch of code that provides different logic for various
> machine types when doing addressing & setting up default PCI controllers.
> Libvirt support for sparc machine types almost certainly broken since
> I'm not aware of anyone having looked at it forever. Probably best to
> re-ask your Q's on the libvir-list to get info from the people familiar
> with this, since I'm fuzzy on precise details of what you'd need todo.

Thanks for the information. From what you're saying above it seems like
we need to fix this on two levels: firstly in QEMU itself, and secondly
to determine some suitable logic for libvirt.

For me QEMU itself is the priority, however it does appear that some
people are using libvirt with SPARC, e.g. from this post on
debian-sparc: https://lists.debian.org/debian-sparc/2017/07/msg00000.html.

The other thing to bear in mind is that the machine model still needs at
least one more change (emulating a proper Sun HME nic) so it would be
good to get the model static before working on the libvirt logic.


ATB,

Mark.

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

[Mark Cave-Ayland]

On 05/07/17 06:38, Markus Armbruster wrote:

> Copying Marcel for PCI expertise.
> 
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> 
>> Hi all,
>>
>> 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().

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

> 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 /memory@0,0 (memory)
ffe2b370 /virtual-memory
ffe2d878 /pci@1fe,0 (pci)
ffe2e1a8 /pci@1fe,0/pci@1,1 (pci)
ffe2e960 /pci@1fe,0/pci@1,1/ebus@1
ffe2f1b0 /pci@1fe,0/pci@1,1/ebus@1/eeprom@0
ffe2f328 /pci@1fe,0/pci@1,1/ebus@1/fdthree@0 (block)
ffe2f878 /pci@1fe,0/pci@1,1/ebus@1/su@0 (serial)
ffe2fc08 /pci@1fe,0/pci@1,1/ebus@1/8042@0 (8042)
ffe2fe00 /pci@1fe,0/pci@1,1/ebus@1/8042@0/kb_ps2@0 (serial)
ffe301b0 /pci@1fe,0/pci@1,1/NE2000@1,1 (network)
ffe307c8 /pci@1fe,0/pci@1,1/QEMU,VGA@2 (display)
ffe31e40 /pci@1fe,0/pci@1,1/ide@3 (ide)
ffe32398 /pci@1fe,0/pci@1,1/ide@3/ide0@4100 (ide)
ffe32678 /pci@1fe,0/pci@1,1/ide@3/ide1@4200 (ide)
ffe32910 /pci@1fe,0/pci@1,1/ide@3/ide1@4200/cdrom@0 (block)
ffe32f98 /pci@1fe,0/pci@1 (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.

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.

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?


ATB,

Mark.

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

[Markus Armbruster]

Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:

> On 05/07/17 06:38, Markus Armbruster wrote:
>
>> Copying Marcel for PCI expertise.
>> 
>> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>> 
>>> Hi all,
>>>
>>> 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()".

>> 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()?

>> 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 /memory@0,0 (memory)
> ffe2b370 /virtual-memory
> ffe2d878 /pci@1fe,0 (pci)
> ffe2e1a8 /pci@1fe,0/pci@1,1 (pci)
> ffe2e960 /pci@1fe,0/pci@1,1/ebus@1
> ffe2f1b0 /pci@1fe,0/pci@1,1/ebus@1/eeprom@0
> ffe2f328 /pci@1fe,0/pci@1,1/ebus@1/fdthree@0 (block)
> ffe2f878 /pci@1fe,0/pci@1,1/ebus@1/su@0 (serial)
> ffe2fc08 /pci@1fe,0/pci@1,1/ebus@1/8042@0 (8042)
> ffe2fe00 /pci@1fe,0/pci@1,1/ebus@1/8042@0/kb_ps2@0 (serial)
> ffe301b0 /pci@1fe,0/pci@1,1/NE2000@1,1 (network)
> ffe307c8 /pci@1fe,0/pci@1,1/QEMU,VGA@2 (display)
> ffe31e40 /pci@1fe,0/pci@1,1/ide@3 (ide)
> ffe32398 /pci@1fe,0/pci@1,1/ide@3/ide0@4100 (ide)
> ffe32678 /pci@1fe,0/pci@1,1/ide@3/ide1@4200 (ide)
> ffe32910 /pci@1fe,0/pci@1,1/ide@3/ide1@4200/cdrom@0 (block)
> ffe32f98 /pci@1fe,0/pci@1 (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.

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

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

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

[Mark Cave-Ayland]

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 /memory@0,0 (memory)
>> ffe2b370 /virtual-memory
>> ffe2d878 /pci@1fe,0 (pci)
>> ffe2e1a8 /pci@1fe,0/pci@1,1 (pci)
>> ffe2e960 /pci@1fe,0/pci@1,1/ebus@1
>> ffe2f1b0 /pci@1fe,0/pci@1,1/ebus@1/eeprom@0
>> ffe2f328 /pci@1fe,0/pci@1,1/ebus@1/fdthree@0 (block)
>> ffe2f878 /pci@1fe,0/pci@1,1/ebus@1/su@0 (serial)
>> ffe2fc08 /pci@1fe,0/pci@1,1/ebus@1/8042@0 (8042)
>> ffe2fe00 /pci@1fe,0/pci@1,1/ebus@1/8042@0/kb_ps2@0 (serial)
>> ffe301b0 /pci@1fe,0/pci@1,1/NE2000@1,1 (network)
>> ffe307c8 /pci@1fe,0/pci@1,1/QEMU,VGA@2 (display)
>> ffe31e40 /pci@1fe,0/pci@1,1/ide@3 (ide)
>> ffe32398 /pci@1fe,0/pci@1,1/ide@3/ide0@4100 (ide)
>> ffe32678 /pci@1fe,0/pci@1,1/ide@3/ide1@4200 (ide)
>> ffe32910 /pci@1fe,0/pci@1,1/ide@3/ide1@4200/cdrom@0 (block)
>> ffe32f98 /pci@1fe,0/pci@1 (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?

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?

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

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


ATB,

Mark.

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

[Markus Armbruster]

Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> 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 /memory@0,0 (memory)
>>> ffe2b370 /virtual-memory
>>> ffe2d878 /pci@1fe,0 (pci)
>>> ffe2e1a8 /pci@1fe,0/pci@1,1 (pci)
>>> ffe2e960 /pci@1fe,0/pci@1,1/ebus@1
>>> ffe2f1b0 /pci@1fe,0/pci@1,1/ebus@1/eeprom@0
>>> ffe2f328 /pci@1fe,0/pci@1,1/ebus@1/fdthree@0 (block)
>>> ffe2f878 /pci@1fe,0/pci@1,1/ebus@1/su@0 (serial)
>>> ffe2fc08 /pci@1fe,0/pci@1,1/ebus@1/8042@0 (8042)
>>> ffe2fe00 /pci@1fe,0/pci@1,1/ebus@1/8042@0/kb_ps2@0 (serial)
>>> ffe301b0 /pci@1fe,0/pci@1,1/NE2000@1,1 (network)
>>> ffe307c8 /pci@1fe,0/pci@1,1/QEMU,VGA@2 (display)
>>> ffe31e40 /pci@1fe,0/pci@1,1/ide@3 (ide)
>>> ffe32398 /pci@1fe,0/pci@1,1/ide@3/ide0@4100 (ide)
>>> ffe32678 /pci@1fe,0/pci@1,1/ide@3/ide1@4200 (ide)
>>> ffe32910 /pci@1fe,0/pci@1,1/ide@3/ide1@4200/cdrom@0 (block)
>>> ffe32f98 /pci@1fe,0/pci@1 (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.

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

[Mark Cave-Ayland]

On 05/07/17 19:05, Markus Armbruster wrote:

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

Yes, I've managed to do that already. The issue is if a user tries to
add a device to bus A on the command line e.g. -device
virtio-blk-pci,bus=pciA then it appears that the QEMU code iterates over
the slots in order to find the first free slot, finds slot 0 and then
plugs it in before the ebus which breaks OpenBIOS :(

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

Agreed. My plan in terms of a migration strategy for existing users was
to say something along the lines of "just add bus=pciB to your -device
command line arguments" which isn't too bad, although there are people
who use a lot of legacy command line options for convenience. But that's
not insurmountable as I can write a guide on the wiki.

For my busA example above though, the only way I can see this working is
if a devfn bitmask can be specified as an optional PCIBus property to
control which slots/functions are available on the bus for hot-plugging
and cold-plugging PCI devices.


ATB,

Mark.

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

[Marcel Apfelbaum]

On 05/07/2017 21:05, Markus Armbruster wrote:
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> 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 /memory@0,0 (memory)
>>>> ffe2b370 /virtual-memory
>>>> ffe2d878 /pci@1fe,0 (pci)
>>>> ffe2e1a8 /pci@1fe,0/pci@1,1 (pci)
>>>> ffe2e960 /pci@1fe,0/pci@1,1/ebus@1
>>>> ffe2f1b0 /pci@1fe,0/pci@1,1/ebus@1/eeprom@0
>>>> ffe2f328 /pci@1fe,0/pci@1,1/ebus@1/fdthree@0 (block)
>>>> ffe2f878 /pci@1fe,0/pci@1,1/ebus@1/su@0 (serial)
>>>> ffe2fc08 /pci@1fe,0/pci@1,1/ebus@1/8042@0 (8042)
>>>> ffe2fe00 /pci@1fe,0/pci@1,1/ebus@1/8042@0/kb_ps2@0 (serial)
>>>> ffe301b0 /pci@1fe,0/pci@1,1/NE2000@1,1 (network)
>>>> ffe307c8 /pci@1fe,0/pci@1,1/QEMU,VGA@2 (display)
>>>> ffe31e40 /pci@1fe,0/pci@1,1/ide@3 (ide)
>>>> ffe32398 /pci@1fe,0/pci@1,1/ide@3/ide0@4100 (ide)
>>>> ffe32678 /pci@1fe,0/pci@1,1/ide@3/ide1@4200 (ide)
>>>> ffe32910 /pci@1fe,0/pci@1,1/ide@3/ide1@4200/cdrom@0 (block)
>>>> ffe32f98 /pci@1fe,0/pci@1 (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?
> 

Hi Markus,
Sorry for my late reply.

Indeed, we don't have currently a restriction on the number of usable
slots on a bus, however deriving from PCIBus class and implementing
the new policy should not be much trouble.


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

Right, hard-coding the device creation in machine init will solve that,
however it will be against our long-term goal to create the machine as
a puzzle, and for that, the devices should be created in some
order. I suspect the task would not be easy to integrate as
part of this project though.

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

I agree, but still the user will see an error. However the error would
be "slot x does not exist" which is clean.


I see two ways to continue:
  1. A new kind of pci-bridge should be created with a "special"
     secondary bus that has less slots. (harder to implement)
  2. Add the limitation of the number of slots to the PCIBus class,
     (simpler to implement, but since is not a widely used case maybe
     is not the best way to go.


Thanks,
Marcel

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

[Markus Armbruster]

Marcel Apfelbaum <marcel@redhat.com> writes:

> On 05/07/2017 21:05, Markus Armbruster wrote:
>> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
[...]
>>> 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?
>>
>
> Hi Markus,
> Sorry for my late reply.
>
> Indeed, we don't have currently a restriction on the number of usable
> slots on a bus, however deriving from PCIBus class and implementing
> the new policy should not be much trouble.
>
>
>>> 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.
>>
>
> Right, hard-coding the device creation in machine init will solve that,
> however it will be against our long-term goal to create the machine as
> a puzzle, and for that, the devices should be created in some
> order. I suspect the task would not be easy to integrate as
> part of this project though.

Even in a world where we start with an empty board, then plug in stuff
directed by configuration file, an onboard device's PCI address should
be explicit.  Having to count PCI devices to figure out their address
sucks.

>>>>> 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.
>>
>
> I agree, but still the user will see an error. However the error would
> be "slot x does not exist" which is clean.
>
>
> I see two ways to continue:
>  1. A new kind of pci-bridge should be created with a "special"
>     secondary bus that has less slots. (harder to implement)
>  2. Add the limitation of the number of slots to the PCIBus class,
>     (simpler to implement, but since is not a widely used case maybe
>     is not the best way to go.

I suspect (2) would be trivial.  I like trivial.

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

[Marcel Apfelbaum]

On 06/07/2017 14:25, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel@redhat.com> writes:
> 
>> On 05/07/2017 21:05, Markus Armbruster wrote:
>>> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> [...]
>>>> 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?
>>>
>>
>> Hi Markus,
>> Sorry for my late reply.
>>
>> Indeed, we don't have currently a restriction on the number of usable
>> slots on a bus, however deriving from PCIBus class and implementing
>> the new policy should not be much trouble.
>>
>>
>>>> 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.
>>>
>>

Hi Markus,

>> Right, hard-coding the device creation in machine init will solve that,
>> however it will be against our long-term goal to create the machine as
>> a puzzle, and for that, the devices should be created in some
>> order. I suspect the task would not be easy to integrate as
>> part of this project though.
> 
> Even in a world where we start with an empty board, then plug in stuff
> directed by configuration file, an onboard device's PCI address should
> be explicit.  Having to count PCI devices to figure out their address
> sucks.
> 

You are right, but this is not what I meant.
We have devices created from command line and devices "built-in".

Instead of manually/ad-hoc adding the devices wherever fits,
have two lists of devices, one from command line and one
per machine type (code or config file).
The init function should order them and take into account
addresses, of course.

>>>>>> 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.
>>>
>>
>> I agree, but still the user will see an error. However the error would
>> be "slot x does not exist" which is clean.
>>
>>
>> I see two ways to continue:
>>   1. A new kind of pci-bridge should be created with a "special"
>>      secondary bus that has less slots. (harder to implement)
>>   2. Add the limitation of the number of slots to the PCIBus class,
>>      (simpler to implement, but since is not a widely used case maybe
>>      is not the best way to go.
> 
> I suspect (2) would be trivial.  I like trivial.

I also like trivial, what might not be trivial is to convince Michael
a base PCIBus class needs a property that limits the number of slots.
But since the device registration code is generic, we may need that
property anyway.

Taking the idea a little farther, instead of limiting the slots number,
we can have a slot-available flag for each slot. In this way we can
cover more future requirements like "slot #5 is not never used".

Thanks,
Marcel


> 

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

[Markus Armbruster]

Marcel Apfelbaum <marcel@redhat.com> writes:

> On 06/07/2017 14:25, Markus Armbruster wrote:
>> Marcel Apfelbaum <marcel@redhat.com> writes:
>>
>>> On 05/07/2017 21:05, Markus Armbruster wrote:
>>>> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>> [...]
>>>>> 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?
>>>>
>>>
>>> Hi Markus,
>>> Sorry for my late reply.
>>>
>>> Indeed, we don't have currently a restriction on the number of usable
>>> slots on a bus, however deriving from PCIBus class and implementing
>>> the new policy should not be much trouble.
>>>
>>>
>>>>> 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.
>>>>
>>>
>
> Hi Markus,
>
>>> Right, hard-coding the device creation in machine init will solve that,
>>> however it will be against our long-term goal to create the machine as
>>> a puzzle, and for that, the devices should be created in some
>>> order. I suspect the task would not be easy to integrate as
>>> part of this project though.
>>
>> Even in a world where we start with an empty board, then plug in stuff
>> directed by configuration file, an onboard device's PCI address should
>> be explicit.  Having to count PCI devices to figure out their address
>> sucks.
>>
>
> You are right, but this is not what I meant.
> We have devices created from command line and devices "built-in".
>
> Instead of manually/ad-hoc adding the devices wherever fits,
> have two lists of devices, one from command line and one
> per machine type (code or config file).
> The init function should order them and take into account
> addresses, of course.

The board needs to be created first, so that when the user's additional
devices clash with the board's, we get decent error messages.  In that
sense, order matters.

We got sidetracked a bit, this stuff is unlikely to be of interest to
Mark :)

>>>>>>> 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.
>>>>
>>>
>>> I agree, but still the user will see an error. However the error would
>>> be "slot x does not exist" which is clean.
>>>
>>>
>>> I see two ways to continue:
>>>   1. A new kind of pci-bridge should be created with a "special"
>>>      secondary bus that has less slots. (harder to implement)
>>>   2. Add the limitation of the number of slots to the PCIBus class,
>>>      (simpler to implement, but since is not a widely used case maybe
>>>      is not the best way to go.
>>
>> I suspect (2) would be trivial.  I like trivial.
>
> I also like trivial, what might not be trivial is to convince Michael
> a base PCIBus class needs a property that limits the number of slots.
> But since the device registration code is generic, we may need that
> property anyway.
>
> Taking the idea a little farther, instead of limiting the slots number,
> we can have a slot-available flag for each slot. In this way we can
> cover more future requirements like "slot #5 is not never used".

That's strictly more general, and should still be trivial.

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

[Mark Cave-Ayland]

On 06/07/17 15:36, Markus Armbruster wrote:

>>>> I see two ways to continue:
>>>>   1. A new kind of pci-bridge should be created with a "special"
>>>>      secondary bus that has less slots. (harder to implement)
>>>>   2. Add the limitation of the number of slots to the PCIBus class,
>>>>      (simpler to implement, but since is not a widely used case maybe
>>>>      is not the best way to go.
>>>
>>> I suspect (2) would be trivial.  I like trivial.
>>
>> I also like trivial, what might not be trivial is to convince Michael
>> a base PCIBus class needs a property that limits the number of slots.
>> But since the device registration code is generic, we may need that
>> property anyway.
>>
>> Taking the idea a little farther, instead of limiting the slots number,
>> we can have a slot-available flag for each slot. In this way we can
>> cover more future requirements like "slot #5 is not never used".
> 
> That's strictly more general, and should still be trivial.

Indeed, having a bitmask is exactly what I suggested at the end of my
last email. I've quickly put something together which I'll send along
shortly...


ATB,

Mark.