Re: [PATCH] pci: add enforce_slot_reserved_mask_manual property

[Chuck Zmudzinski]

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

Thanks, I will let you know if there is a patch to test on the
sun4u machine. For now, we are waiting to see if the xen
maintainers will accept a patch that uses slot_reserved_mask
to prevent other devices from using the slot that is required
by the Intel igd in the xenfv machine. That patch does not change
the way slot_reserved_mask works, but if that patch is added
some users might want to add a capability for a user to override
slot_reserved_mask, and that is what this patch attempts to
implement.

Kind regards,

Chuck