Re: [PATCH 1/1] pcie: Do not set power state for some hot-plugged devices

["Annie.li" <annie.li@oracle.com>]

Hello Gerd,

On 12/16/2021 1:11 AM, Gerd Hoffmann wrote:
>    Hi,
>
>>> Maybe we should just not set DeviceState->hotplugged = true for devices
>>> added in VM_STATE_PRELAUNCH?  It's not actual hotplug (i.e. device added
>>> while the system is running) after all ...
>> Simply not setting "DeviceState->hotplugged" doesn't work. Devices created
>> in
>> PHASE_MACHINE_READY phase are treated as hot-plugged devices. So I just
>> tried
>> following change for the quick test, the device is still invisible to the
>> firmware with
>> this change.
> Looking again, the difference is probably the reset handling.
> pcie_cap_slot_reset() will turn on power (via PCI_EXP_SLTCTL_PCC) in
> case some device is plugged into the slot.
If the VM is booting from the system disk in the qemu command line(no 
hot-plug),
pcie_cap_slot_reset turns on the power for this device. And this happens 
before
the VM runs into VM_STATE_PRELAUNCH state.(I add '-S' option in this case
for comparison)
>
> So I suspect when plugging devices during VM_STATE_PRELAUNCH they are
> resetted individually (specifically before the device is plugged),
When the device is hot-plugged in VM_STATE_PRELAUNCH state, there is no
reset for the device during this state. Before entering this state,
pcie_cap_slot_reset  does get called for the device(like general VM 
booting).
However, it doesn't turn on the power. I think this is due to that the 
device isn't
hot-plugged yet, and "populated" value is false.
> whereas otherwise they are resetted when all devices are plugged in.
>
> Does resetting devices when leaving RUN_STATE_PRELAUNCH fix this?
I suppose only calling pcie_cap_slot_reset  isn't sufficient? maybe 
rp_reset?
However, the place of state transition is in runstate_set. I don't know
hot to hook this function to trigger the reset yet.
Just thinking of the implementation, if the patch is deployed in this way,
isn't the change is more complicated than the current one? :) Maybe I've
missed something here.

Thanks
Annie
>
> take care,
>    Gerd
>