diff mbox series

[V4,07/11] vfio/pci: Update stale comment

Message ID f16872889aed9a7b042a7e53722b9ae83430a126.1682615447.git.reinette.chatre@intel.com
State New
Headers
Series vfio/pci: Support dynamic allocation of MSI-X interrupts |

Commit Message

Reinette Chatre April 27, 2023, 5:36 p.m. UTC 
  In preparation for surrounding code change it is helpful to
ensure that existing comments are accurate.

Remove inaccurate comment about direct access and update
the rest of the comment to reflect the purpose of writing
the cached MSI message to the device.

Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Link: https://lore.kernel.org/lkml/20230330164050.0069e2a5.alex.williamson@redhat.com/
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
No changes since V3.

Changes since V2:
- New patch.

 drivers/vfio/pci/vfio_pci_intrs.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Tian, Kevin April 28, 2023, 6:42 a.m. UTC | #1 
> From: Chatre, Reinette <reinette.chatre@intel.com>
> Sent: Friday, April 28, 2023 1:36 AM
> 
> @@ -419,11 +419,9 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_core_device *vdev,
>  	}
> 
>  	/*
> -	 * The MSIx vector table resides in device memory which may be
> cleared
> -	 * via backdoor resets. We don't allow direct access to the vector
> -	 * table so even if a userspace driver attempts to save/restore
> around
> -	 * such a reset it would be unsuccessful. To avoid this, restore the
> -	 * cached value of the message prior to enabling.
> +	 * If the vector was previously allocated, refresh the on-device
> +	 * message data before enabling in case it had been cleared or
> +	 * corrupted since writing.
>  	 */
>  	cmd = vfio_pci_memory_lock_and_enable(vdev);
>  	if (msix) {

What about keeping backdoor reset as an example, e.g.

"in case it had been cleared or corrupted (e.g. due to backdoor resets) ..."

otherwise one may doubt whether it is a more severe problem causing
the corruption inadvertently w/o userspace driver's awareness and then
whether just restoring the data is sufficient to move on.
Reinette Chatre April 28, 2023, 6:24 p.m. UTC | #2 
Hi Kevin,

On 4/27/2023 11:42 PM, Tian, Kevin wrote:
>> From: Chatre, Reinette <reinette.chatre@intel.com>
>> Sent: Friday, April 28, 2023 1:36 AM
>>
>> @@ -419,11 +419,9 @@ static int vfio_msi_set_vector_signal(struct
>> vfio_pci_core_device *vdev,
>>  	}
>>
>>  	/*
>> -	 * The MSIx vector table resides in device memory which may be
>> cleared
>> -	 * via backdoor resets. We don't allow direct access to the vector
>> -	 * table so even if a userspace driver attempts to save/restore
>> around
>> -	 * such a reset it would be unsuccessful. To avoid this, restore the
>> -	 * cached value of the message prior to enabling.
>> +	 * If the vector was previously allocated, refresh the on-device
>> +	 * message data before enabling in case it had been cleared or
>> +	 * corrupted since writing.
>>  	 */
>>  	cmd = vfio_pci_memory_lock_and_enable(vdev);
>>  	if (msix) {
> 
> What about keeping backdoor reset as an example, e.g.
> 
> "in case it had been cleared or corrupted (e.g. due to backdoor resets) ..."
> 
> otherwise one may doubt whether it is a more severe problem causing
> the corruption inadvertently w/o userspace driver's awareness and then
> whether just restoring the data is sufficient to move on.

Will do. Thank you very much.

Reinette
diff mbox series

Patch

diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 5e3de004f4cb..bdda7f46c2be 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -419,11 +419,9 @@  static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev,
 	}
 
 	/*
-	 * The MSIx vector table resides in device memory which may be cleared
-	 * via backdoor resets. We don't allow direct access to the vector
-	 * table so even if a userspace driver attempts to save/restore around
-	 * such a reset it would be unsuccessful. To avoid this, restore the
-	 * cached value of the message prior to enabling.
+	 * If the vector was previously allocated, refresh the on-device
+	 * message data before enabling in case it had been cleared or
+	 * corrupted since writing.
 	 */
 	cmd = vfio_pci_memory_lock_and_enable(vdev);
 	if (msix) {