Re: [PATCH] pci: add enforce_slot_reserved_mask_manual property

[Mark Cave-Ayland]

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

If there is a need to change/refactor the logic then I can test the sun4u machine to 
ensure the original test case still works.


ATB,

Mark.