Re: [PATCH] pci: add enforce_slot_reserved_mask_manual property

[Chuck Zmudzinski]

On 1/28/23 5:26 AM, Michael S. Tsirkin wrote:
> On Fri, Jan 27, 2023 at 10:39:28PM -0500, 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?
> 
> I guess first can you answer whether this is still needed
> with the latest Xen patches?
> 

It's not really needed except for experimental purposes to allow
an administrator to test experimental configurations with a device
other than the igd at slot 2. That might be useful in some cases,
but it is not really necessary unless someone asks for that capability.
If libvirt users who ordinarily like to manually specify all the
settings will be OK with the proposed patch to xen that prevents
an administrator from being able to override a new setting that
reserves slot 2 for the igd for type "xenfv" machines configured for
igd passthrough, then there is no need for this patch. I don't think
many users need the capability to insert a different device in slot 2 for
the "xenfv" machine type configured with igd-passthru=on, so I would be
OK if this patch is not included in qemu.

Chuck