Re: [PATCH] xen: add privcmd ioctl to get p2pdma_distance

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.12.2024 09:18, Zhang, Julia wrote:
> On 2024/12/9 15:47, Jan Beulich wrote:
> On 07.12.2024 11:59, Julia Zhang wrote:

Yet another formality, sorry: Please send plain text emails. You'll note that 
what
I said and why you said is indistinguishably intermixed below.

> --- a/tools/include/xen-sys/Linux/privcmd.h
> 
> +++ b/tools/include/xen-sys/Linux/privcmd.h
> 
> @@ -110,6 +110,16 @@ typedef struct privcmd_map_hva_to_gpfns {
> 
>    int add_mapping;
> 
>  } privcmd_map_hva_to_gpfns_t;
> 
> 
> 
> +typedef struct privcmd_p2pdma_distance {
> 
> +  __u32 provider_bus;
> 
> +  __u32 provider_slot;
> 
> +  __u32 provider_func;
> 
> +  __u32 client_bus;
> 
> +  __u32 client_slot;
> 
> +  __u32 client_func;
> 
> +  __u32 distance;
> 
> +} privcmd_p2pdma_distance_t;
> 
> 
> 
> "Distance" typically is a symmetric thing. Why the asymmetry here? And
> 
> why __u32 when __u8 will be fine for most fields? And where's the segment
> 
> part of the device coordinates? Finally, with it being merely stub
> 
> implementations that you add here, all details on where the needed info
> 
> is to come from are missing.
> 
> "Distance" is p2pdma-distance between two PCI devices, it's calculated in 
> kernel driver.I don't get why it's symmetric?

Distance from A to B is usually the same as that from B to A. But yes,
not necessarily always (thinking of e.g. rings). Yet still I'm unclear
about the distinction between provide and client.

> I will use change __u32 to __u8.
> 
> By the segment part of the device coordinates, do you mean the domain number 
> of the device?

Some call it domain, yes. Since domain has an important-ish different
meaning in Xen, we prefer segment though.

> All the needed info(virtual bus/slot/function numbers) are from guest VM, 
> QEMU convert them to real physical info of two devices.
> 
> Do you mean that I should add more details in commit message or comment?

Perhaps. Or have the patch be more complete, in the sense of not only
putting in place stubs, thus actually making visible how the data is
produced / used.

Jan