Re: [PATCH V5 17/25] vfio-pci: cpr part 2

[Steven Sistare]

On 7/16/2021 4:51 PM, Alex Williamson wrote:
> On Wed,  7 Jul 2021 10:20:26 -0700
> Steve Sistare <steven.sistare@oracle.com> wrote:
> 
>> Finish cpr for vfio-pci by preserving eventfd's and vector state.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  hw/vfio/pci.c | 118 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 116 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 0f5c542..07bd360 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
> ...
>> @@ -3295,14 +3329,91 @@ static void vfio_merge_config(VFIOPCIDevice
> *vdev)
>>      g_free(phys_config);
>>  }
>>  
>> +static int vfio_pci_pre_save(void *opaque)
>> +{
>> +    VFIOPCIDevice *vdev = opaque;
>> +    PCIDevice *pdev = &vdev->pdev;
>> +    int i;
>> +
>> +    if (vfio_pci_read_config(pdev, PCI_INTERRUPT_PIN, 1)) {
>> +        error_report("%s: cpr does not support vfio-pci INTX",
>> +                     vdev->vbasedev.name);
>> +    }
> 
> You're not only not supporting INTx, but devices that support INTx, so
> this only works on VFs.  Why?  Is this just out of scope or is there
> something fundamentally difficult about it?
> 
> This makes me suspect there's a gap in INTx routing setup if it's more
> than just another eventfd to store and setup.  If we hot-add a device
> using INTx after cpr restart, are we going to find problems?  Thanks,

It could be supported, but requires more code (several event fd's plus other 
state in VFIOINTx
to save and restore) for a case that does not seem very useful (a directly 
assigned device that
only supports INTx ?). 

Hot add of such a device after cpr restart is allowed and works.  The next cpr 
restart operation
would fail with an error message without harming the guest.  However, I should 
add a check
to prevent the device from being added if only-cpr-capable is specified, in 
device_set_realized,
like check_only_migratable.

- Steve