On 1/28/2023 4:58 PM, Mark Cave-Ayland wrote:
On 28/01/2023 03:39, Chuck Zmudzinski wrote:
On 1/27/2023 8:28 AM, Michael S. Tsirkin wrote:
On Sun, Jan 15, 2023 at 07:49:51PM -0500, Chuck Zmudzinski wrote:
The current reserved slot check in do_pci_register_device(), added with
commit 8b8849844fd6
add ("subject here") please
,is done even if the pci device being added is
configured manually for a particular slot. The new property, when set
to false, disables the check when the device is configured to request a
particular slot. This allows an administrator or management tool to
override slot_reserved_mask for a pci device by requesting a particular
slot for the device. The new property is initialized to true which
preserves the existing behavior of slot_reserved_mask by default.
Signed-off-by: Chuck Zmudzinski <brchuckz@aol.com>
Thanks!
I'm trying to think of the best default for this.
Hi Michael and Mark,
The Xen maintainers pulled up the commit that reserves slot 2
for the Intel IGD with the xenfv machine so we need to discuss
how slot_reserved_mask should work with this change. The original
version of the patch keeps the current default of always enforcing
slot_reserved_mask, even if the administrator tries to configure the
slots manually. So if we keep that behavior, we will need to patch
the xenfv machine again to implement the desired behavior that
slot_reserved_mask will only apply with automatic slot assignment
for the xenfv machine. That would be a fairly trivial one-line patch
to add on top of the patch the Xen maintainers just pulled up.
Another option is to change the default value of
enforce_slot_reserved_mask_manual to false so slot_reserved_mask
only affects automatic slot address assignment by default and then
preserve the current behavior for the sun4u machine by patching the
sun4u machine so the value of enforce_slot_reserved_mask_manual
is true for the sun4u machine.
I think it would be better for the default value of
enforce_slot_reserved_mask_manual to be false, so that a
user-specified slot will by default override slot_reserved_mask.
But doing that would change the current behavior of
slot_reserved_mask.
Currently, this is the only place where slot_reserved_mask is used in all
of the Qemu source (code from hw/sparc64/sun4u.c):
------ snip -------
/* Only in-built Simba APBs can exist on the root bus, slot 0 on busA is
reserved (leaving no slots free after on-board devices) however slots
0-3 are free on busB */
pci_bus->slot_reserved_mask = 0xfffffffc;
pci_busA->slot_reserved_mask = 0xfffffff1;
pci_busB->slot_reserved_mask = 0xfffffff0;
------ snip -------
I think we could safely change the default value of
enforce_slot_reserved_mask_manual to false but set
it to true for the sparc64 sun4u board here to preserve
the current behavior of the only existing board in Qemu
that uses slot_reserved_mask.
What do you think?
Users is trying to configure a specific device on a reserved
slot. Should we
CC a bunch more people for visibility. Input, anyone?
For a bit of background, slot_reserved_mask was added by me to solve a problem
with
the sun4u machine: on a real Ultra-5, the pci "A" bus has 2 free slots and the
pci
"B" bus has 4 free slots. Whilst it is possible to plug a PCI device into any
slot in
QEMU, the PCI bridges only have IRQ mapping registers for those 6 slots, so you
can
easily end up with an auto-allocated slot where it is impossible for the OS to
map
the IRQ.
Hence slot_reserved_mask was originally intended to mark slots as being
unavailable
for both manual and automatic allocation to ensure that devices plugged into
both PCI
buses would always work.
Here is a third option:
Mark, would it be acceptable to replace the error in the case of manual
allocation with a warning, so manual device assignment to a reserved slot
in the sun4u machine would succeed (automatic assignment would still
prevent the slot from being used) but there would be a warning message
to provide the administrator/user with a clue that the current manual
configuration of the device is not correct and could cause problems?
That would be a less aggressive change that could be implemented without
needing to add the enforce_slot_reserved_mask_manual property to PCIBus.
Also, Michael, I notice that the current usage of slot_reserved_mask violates
the comment at the beginning of pci_bus.h that asks that the properties of
PCIBus such as slot_reserved_mask be accessed via accessor functions in pci.h
instead of directly accessing them. Should I also write v2 of the patch to add
accessor functions for slot_reserved_mask so the accessor functions can be used
instead of accessing slot_reserved_mask directly?
I will wait a little while for some feedback before posting v2 of this patch.