Re: [PATCH] pci: add enforce_slot_reserved_mask_manual property

[Chuck Zmudzinski]

On 1/28/23 2:14 PM, Michael S. Tsirkin wrote:
> On Sat, Jan 28, 2023 at 08:20:55AM -0500, Chuck Zmudzinski wrote:
>> 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
> 
> Pls wait and see if that patch gets picked up. Let me know.
> 

A day or two ago Anthony said he would look at the xen patch soon. So we'll
just wait for him, and I'll let you know if he is going to pull it up.