Re: [PATCH v11 4/4] Documentation: add debugfs description for vfio

[liulongfang]

On 2023/7/15 6:07, Alex Williamson write:
> On Fri, 30 Jun 2023 17:24:57 +0800
> liulongfang <liulongfang@xxxxxxxxxx> wrote:
> 
>> From: Longfang Liu <liulongfang@xxxxxxxxxx>
>>
>> 1.Add two debugfs document description file to help users understand
>> how to use the accelerator live migration driver's debugfs.
>> 2.Update the file paths that need to be maintained in MAINTAINERS
>>
>> Signed-off-by: Longfang Liu <liulongfang@xxxxxxxxxx>
>> ---
>>  .../ABI/testing/debugfs-hisi-migration        | 32 +++++++++++++++++++
>>  Documentation/ABI/testing/debugfs-vfio        | 25 +++++++++++++++
>>  MAINTAINERS                                   |  2 ++
>>  3 files changed, 59 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/debugfs-hisi-migration
>>  create mode 100644 Documentation/ABI/testing/debugfs-vfio
>>
>> diff --git a/Documentation/ABI/testing/debugfs-hisi-migration b/Documentation/ABI/testing/debugfs-hisi-migration
>> new file mode 100644
>> index 000000000000..0dfc7008a3e1
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/debugfs-hisi-migration
>> @@ -0,0 +1,32 @@
>> +What:		/sys/kernel/debug/vfio/<device>/migration/hisi_acc/data
>> +Date:		June 2023
>> +KernelVersion:  6.4
> 
> At best we'd be looking at introducing this for 6.6, where it might get
> into mainline in late August for all of these.
>
OK, so does this KernelVersion need to be marked as 6.6?

>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	Read the live migration data of the vfio device.
>> +		The output format of the data is defined by the live
>> +		migration driver.
> 
> This attribute is specific to the hisi_acc driver, so should the data
> be detailed here?
> 
OK I'll add a description of the role type of the data.

>> +
>> +What:		/sys/kernel/debug/vfio/<device>/migration/hisi_acc/attr
>> +Date:		June 2023
>> +KernelVersion:  6.4
>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	Read the live migration attributes of the vfio device.
>> +		The output format of the attributes is defined by the live
>> +		migration driver.
> 
> Same here.
> 
OK.

>> +
>> +What:		/sys/kernel/debug/vfio/<device>/migration/hisi_acc/io_test
>> +Date:		June 2023
>> +KernelVersion:  6.4
>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	Trigger the HiSilicon accelerator device to perform
>> +		the io test through the read operation, and directly output
>> +		the test result.
> 
> This has essentially been reduced to reading the mailbox value AIUI,
> should this simply document that and the resulting possible values?
>
OK,This can add information.

>> +
>> +What:		/sys/kernel/debug/vfio/<device>/migration/hisi_acc/save
>> +Date:		June 2023
>> +KernelVersion:  6.4
>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	Trigger the Hisilicon accelerator device to perform
>> +		the state saving operation of live migration through the read
>> +		operation, and directly output the operation result.
> 
> It's unclear to me why we need to parse a returned string from the
> debugfs seq file rather than read returns zero on success or errno on
> failure.
>
It is impossible to know the reason for the failure just by returning true or false.
Through the string obtained by seq_printf, the user can know the cause of the error.

>> +
>> diff --git a/Documentation/ABI/testing/debugfs-vfio b/Documentation/ABI/testing/debugfs-vfio
>> new file mode 100644
>> index 000000000000..e2132a2f602b
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/debugfs-vfio
>> @@ -0,0 +1,25 @@
>> +What:		/sys/kernel/debug/vfio
>> +Date:		June 2023
>> +KernelVersion:  6.4
>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	This debugfs file directory is used for debugging
>> +		of vfio devices.
>> +		Each device can create a device subdirectory under this
>> +		directory by referencing the public registration interface.
> 
> This suggests it's the device driver's responsibility to create those
> subdirectories, but we've delegated that to the core currently.  Maybe
> this is suggesting that each driver can create a driver subdirectory
> under the core created device directory?
>
OK This vfio directory file is added in vfio_main. All vfio devices share this
device directory.
Then each device creates its own device directory with its own dev_name.

Thanks.
Longfang.
> Thanks,
> Alex
> 
>> +
>> +What:		/sys/kernel/debug/vfio/<device>/migration
>> +Date:		June 2023
>> +KernelVersion:  6.4
>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	This debugfs file directory is used for debugging
>> +		of vfio devices that support live migration.
>> +		The debugfs of each vfio device that supports live migration
>> +		could be created under this directory.
>> +
>> +What:		/sys/kernel/debug/vfio/<device>/migration/state
>> +Date:		June 2023
>> +KernelVersion:  6.4
>> +Contact:	Longfang Liu <liulongfang@xxxxxxxxxx>
>> +Description:	Read the live migration status of the vfio device.
>> +		The status of these live migrations includes:
>> +		ERROR, RUNNING, STOP, STOP_COPY, RESUMING.
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ce6ac552d8f6..bdd6a5e7350f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -22098,6 +22098,7 @@ L:	kvm@xxxxxxxxxxxxxxx
>>  S:	Maintained
>>  T:	git https://github.com/awilliam/linux-vfio.git
>>  F:	Documentation/ABI/testing/sysfs-devices-vfio-dev
>> +F:	Documentation/ABI/testing/debugfs-vfio
>>  F:	Documentation/driver-api/vfio.rst
>>  F:	drivers/vfio/
>>  F:	include/linux/vfio.h
>> @@ -22115,6 +22116,7 @@ M:	Longfang Liu <liulongfang@xxxxxxxxxx>
>>  M:	Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
>>  L:	kvm@xxxxxxxxxxxxxxx
>>  S:	Maintained
>> +F:	Documentation/ABI/testing/debugfs-hisi-migration
>>  F:	drivers/vfio/pci/hisilicon/
>>  
>>  VFIO MEDIATED DEVICE DRIVERS
> 
> .
>