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

["Zhang, Julia" <Julia.Zhang@xxxxxxx>]

On 2024/12/16 17:19, Jan Beulich wrote:
> 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.

Thanks for reminding.

> 
>> --- 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.

Provider - A driver which provides or publishes P2P resources.
Client - A driver which makes use of a resource.

In our case, we want to use passthrough dGPU render data, and virtio 
iGPU display data. So dGPU need to import display buffer of iGPU. iGPU 
is provider and dGPU is 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.
> 

I see, I will add this info in the struct.

>> 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.
> 

Sure, I will try to add more details.

Julia

> Jan