For some machines it is impossible to plug devices into a particular PCI bus
slot, e.g. for a real Ultra 5 there are 2 PCI bridges attached to the root
bus behind which all devices must be plugged. Ignoring this rule will cause
problems with interrupt routing since the interrupt numbers are calculated
based upon PCI bridge id and secondary PCI bus slot id.
This patchset adds a new slot_reserved_mask property to PCIBus which is a
bitmask used to indicate whether PCI bus slots are reserved, i.e. they cannot
be used for hot or cold plugging on a particular PCI bus.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
v2:
- Rename dev_reserved_mask to slot_reserved_mask as suggested by Marcel
- Squash patches 2 and 3 together
Mark Cave-Ayland (2):
pci: move check for existing devfn into new pci_bus_devfn_available()
helper
pci: add reserved slot check to do_pci_register_device()
hw/pci/pci.c | 34 ++++++++++++++++++++++++++++++----
include/hw/pci/pci_bus.h | 1 +
2 files changed, 31 insertions(+), 4 deletions(-)
--
1.7.10.4
On 11/07/17 22:44, Mark Cave-Ayland wrote: > For some machines it is impossible to plug devices into a particular PCI bus > slot, e.g. for a real Ultra 5 there are 2 PCI bridges attached to the root > bus behind which all devices must be plugged. Ignoring this rule will cause > problems with interrupt routing since the interrupt numbers are calculated > based upon PCI bridge id and secondary PCI bus slot id. > > This patchset adds a new slot_reserved_mask property to PCIBus which is a > bitmask used to indicate whether PCI bus slots are reserved, i.e. they cannot > be used for hot or cold plugging on a particular PCI bus. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > v2: > - Rename dev_reserved_mask to slot_reserved_mask as suggested by Marcel > - Squash patches 2 and 3 together > > > Mark Cave-Ayland (2): > pci: move check for existing devfn into new pci_bus_devfn_available() > helper > pci: add reserved slot check to do_pci_register_device() > > hw/pci/pci.c | 34 ++++++++++++++++++++++++++++++---- > include/hw/pci/pci_bus.h | 1 + > 2 files changed, 31 insertions(+), 4 deletions(-) Ping? Any further feedback on the v2 version? My latest set of sun4u patches is dependent upon this patchset and it's freeze coming up next week! ATB, Mark.
On 14/07/2017 12:59, Mark Cave-Ayland wrote: > On 11/07/17 22:44, Mark Cave-Ayland wrote: > >> For some machines it is impossible to plug devices into a particular PCI bus >> slot, e.g. for a real Ultra 5 there are 2 PCI bridges attached to the root >> bus behind which all devices must be plugged. Ignoring this rule will cause >> problems with interrupt routing since the interrupt numbers are calculated >> based upon PCI bridge id and secondary PCI bus slot id. >> >> This patchset adds a new slot_reserved_mask property to PCIBus which is a >> bitmask used to indicate whether PCI bus slots are reserved, i.e. they cannot >> be used for hot or cold plugging on a particular PCI bus. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> >> v2: >> - Rename dev_reserved_mask to slot_reserved_mask as suggested by Marcel >> - Squash patches 2 and 3 together >> >> >> Mark Cave-Ayland (2): >> pci: move check for existing devfn into new pci_bus_devfn_available() >> helper >> pci: add reserved slot check to do_pci_register_device() >> >> hw/pci/pci.c | 34 ++++++++++++++++++++++++++++++---- >> include/hw/pci/pci_bus.h | 1 + >> 2 files changed, 31 insertions(+), 4 deletions(-) > > Ping? Any further feedback on the v2 version? My latest set of sun4u > patches is dependent upon this patchset and it's freeze coming up next week! > Hi, As in prev version, other than the minor comment on replacing "if (...) return true; else return false" with the actual value, I am OK with it. I believe Michael asked to see the series using this feature, can you add a link to it, or post it with the dependency on this one? Thanks, Marcel > > ATB, > > Mark. >
On 14/07/17 11:25, Marcel Apfelbaum wrote:
> On 14/07/2017 12:59, Mark Cave-Ayland wrote:
>> On 11/07/17 22:44, Mark Cave-Ayland wrote:
>>
>>> For some machines it is impossible to plug devices into a particular
>>> PCI bus
>>> slot, e.g. for a real Ultra 5 there are 2 PCI bridges attached to the
>>> root
>>> bus behind which all devices must be plugged. Ignoring this rule will
>>> cause
>>> problems with interrupt routing since the interrupt numbers are
>>> calculated
>>> based upon PCI bridge id and secondary PCI bus slot id.
>>>
>>> This patchset adds a new slot_reserved_mask property to PCIBus which
>>> is a
>>> bitmask used to indicate whether PCI bus slots are reserved, i.e.
>>> they cannot
>>> be used for hot or cold plugging on a particular PCI bus.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>
>>> v2:
>>> - Rename dev_reserved_mask to slot_reserved_mask as suggested by Marcel
>>> - Squash patches 2 and 3 together
>>>
>>>
>>> Mark Cave-Ayland (2):
>>> pci: move check for existing devfn into new pci_bus_devfn_available()
>>> helper
>>> pci: add reserved slot check to do_pci_register_device()
>>>
>>> hw/pci/pci.c | 34 ++++++++++++++++++++++++++++++----
>>> include/hw/pci/pci_bus.h | 1 +
>>> 2 files changed, 31 insertions(+), 4 deletions(-)
>>
>> Ping? Any further feedback on the v2 version? My latest set of sun4u
>> patches is dependent upon this patchset and it's freeze coming up next
>> week!
>>
>
> Hi,
>
> As in prev version, other than the minor comment
> on replacing "if (...) return true; else return false"
> with the actual value, I am OK with it.
Okay great! So change pci_bus_devfn_available() to something like this?
static bool pci_bus_devfn_available(PCIBus *bus, int devfn)
{
return !(bus->devices[devfn]);
}
> I believe Michael asked to see the series using this feature,
> can you add a link to it, or post it with the dependency on this one?
Sure, I posted the link earlier in the week:
https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03045.html.
Or more specifically:
https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03041.html
ATB,
Mark.
On 14/07/2017 13:37, Mark Cave-Ayland wrote:
> On 14/07/17 11:25, Marcel Apfelbaum wrote:
>
>> On 14/07/2017 12:59, Mark Cave-Ayland wrote:
>>> On 11/07/17 22:44, Mark Cave-Ayland wrote:
>>>
>>>> For some machines it is impossible to plug devices into a particular
>>>> PCI bus
>>>> slot, e.g. for a real Ultra 5 there are 2 PCI bridges attached to the
>>>> root
>>>> bus behind which all devices must be plugged. Ignoring this rule will
>>>> cause
>>>> problems with interrupt routing since the interrupt numbers are
>>>> calculated
>>>> based upon PCI bridge id and secondary PCI bus slot id.
>>>>
>>>> This patchset adds a new slot_reserved_mask property to PCIBus which
>>>> is a
>>>> bitmask used to indicate whether PCI bus slots are reserved, i.e.
>>>> they cannot
>>>> be used for hot or cold plugging on a particular PCI bus.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>
>>>> v2:
>>>> - Rename dev_reserved_mask to slot_reserved_mask as suggested by Marcel
>>>> - Squash patches 2 and 3 together
>>>>
>>>>
>>>> Mark Cave-Ayland (2):
>>>> pci: move check for existing devfn into new pci_bus_devfn_available()
>>>> helper
>>>> pci: add reserved slot check to do_pci_register_device()
>>>>
>>>> hw/pci/pci.c | 34 ++++++++++++++++++++++++++++++----
>>>> include/hw/pci/pci_bus.h | 1 +
>>>> 2 files changed, 31 insertions(+), 4 deletions(-)
>>>
>>> Ping? Any further feedback on the v2 version? My latest set of sun4u
>>> patches is dependent upon this patchset and it's freeze coming up next
>>> week!
>>>
>>
Hi Mark,
>> Hi,
>>
>> As in prev version, other than the minor comment
>> on replacing "if (...) return true; else return false"
>> with the actual value, I am OK with it.
>
> Okay great! So change pci_bus_devfn_available() to something like this?
>
> static bool pci_bus_devfn_available(PCIBus *bus, int devfn)
> {
> return !(bus->devices[devfn]);
> }
>
Yes, thanks.
>> I believe Michael asked to see the series using this feature,
>> can you add a link to it, or post it with the dependency on this one?
>
> Sure, I posted the link earlier in the week:
> https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03045.html.
> Or more specifically:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03041.html
>
Hi Michael,
Can you please have a look to the above patch using
the 'reserved slots' patch?
Thanks,
Marcel
>
> ATB,
>
> Mark.
>
On 16/07/17 08:28, Marcel Apfelbaum wrote:
>>> As in prev version, other than the minor comment
>>> on replacing "if (...) return true; else return false"
>>> with the actual value, I am OK with it.
>>
>> Okay great! So change pci_bus_devfn_available() to something like this?
>>
>> static bool pci_bus_devfn_available(PCIBus *bus, int devfn)
>> {
>> return !(bus->devices[devfn]);
>> }
>>
>
> Yes, thanks.
Great, I'll send a v3 with the above modification shortly.
>>> I believe Michael asked to see the series using this feature,
>>> can you add a link to it, or post it with the dependency on this one?
>>
>> Sure, I posted the link earlier in the week:
>> https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03045.html.
>> Or more specifically:
>>
>> https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03041.html
>>
>
> Hi Michael,
> Can you please have a look to the above patch using
> the 'reserved slots' patch?
I should add that if this patch isn't suitable for freeze then it's
likely I'll drop the sun4u patchset for 2.10 since otherwise using
-device places devices on the PCI root bus.
For existing users this means that the machine will start up with their
existing command lines but any extra devices added on the command line
via -device, e.g. virtio will be silently broken because the interrupt
routing is incorrect.
At least with this patch the user will get an error, and thus it's easy
to update the wiki to explain that moving forward it is necessary to add
"bus=pciB" for extra devices to function correctly.
ATB,
Mark.
© 2016 - 2024 Red Hat, Inc.