Re: [PATCH V4 1/9] mshv: Provide a way to get partition ID if running in a VMM process

[Mukesh R <mrathor@linux.microsoft.com>]

On 7/24/26 15:08, Jacob Pan wrote:
> Hi Mukesh,
> 
> On Fri, 24 Jul 2026 12:14:08 -0700
> Mukesh R <mrathor@linux.microsoft.com> wrote:
> 
>> On 7/23/26 15:21, Jacob Pan wrote:
>>> Hi Mukesh,
>>
>> Hey Jacob, pl see inline..
>>
>>> On Fri, 17 Jul 2026 19:19:41 -0700
>>> Mukesh R <mrathor@linux.microsoft.com> wrote:
>>
>>     ... snip...
>>
>>>>    static int
>>>>    add_partition(struct mshv_partition *partition)
>>>>    {
>>>> @@ -2073,6 +2094,7 @@ mshv_ioctl_create_partition(void __user
>>>> *user_arg, struct device *module_dev) goto cleanup_irq_srcu;
>>>>    
>>>>    	partition->pt_id = pt_id;
>>>> +	partition->pt_vmm_tgid = current->tgid;
>>> I wonder how robust this mechanism is to identify target partition
>>> via tgid.
>>> 1) what prevents a VMM process create more than one partition? in
>>> that case each partition would have the same tgid.
>>
>> Currently, none of the VMMs we support do that, and doesn't look like
>> there is much of a demand for it.
>>
> My point is that from kernel UAPI pov, we cannot count on user behavior
> nor current VMM's implementation.

True, but as UAPI implementors, I thought we could list its
limitations. For example: an fwrite() implementation can say if you
don't fflush(), fwrite() can lose data!

>>> 2) IIUC, the lifetime of the partition is tied to FD, which is
>>> different than the lifetime of a PID. The partition FD can be
>>> inherited or passed to another process. The VMM tg can exit while
>>> the FDs can be alive. Then the tgid can be reused by another
>>> unrelated process, right?
>>
>> yeah, AI keeps telling me that, but not super accurate imo.
>>
>> we are using tgid and not pid. tgid is process group id, and that will
>> stay around as long as there is at least one process in it. if we used
>> pid, then that would be the case.
>>
> I understand it is tgid but don't think tgid makes difference in terms
> of FD lifetime.
> e.g. process A creates a partition, then passes the partition fd to
> process B over a Unix socket. Process A can then exit while process B
> still holds a reference to the partition file. At that point the tgid
> that was stored in pt_vmm_tgid can be reused by an unrelated process C,
> while the partition's file object remains alive.

Confused whether you are saying that C should still be able to manage
the VM without errors, or that C could cause harm because it has access
to possibly recycled pt_vmm_tgid. If latter, C cannot use pt_vmm_tgid
in harmful way because:

         if (pt->pt_vmm_tgid == current->tgid) {
                 ret_ptid = pt->pt_id;

         else return : HV_PARTITION_ID_INVALID

So, because C has different tgid, it will return HV_PARTITION_ID_INVALID
causing hypercalls to fail with invalid parameter.

>>> Would it be more robust to based this on the partition FD instead of
>>> tgid?
>>    
>> it might be, but problem with that is we need pt-id in other cases
>> where that is not available: for example in
>> hv_iommu_domain_alloc_paging and in irq remapping paths for direct
>> attached devices. if we can sort that out somehow, then we can do
>> that. but i suspect, it would take some time to figure that out, so i
>> hope we can make that a future enhancement.
>>
>> For now, i've been thinking of just putting a check and returning
>> ENOTSUPP if a vmm tries to create another partition.
>>
> IMHO, that only solves the uniqueness issue of partition ID but not the
> lifetime issue.
> maybe EEXIST instead of ENOTSUPP?

   EEXIST: A VM already exists for this process group
ENOTSUPP: more than one VM is not supported for a proc group

I think either are ok, but if you prefer EEXIST, that's fine too.


>> Thanks,
>> -Mukesh
>>
>>>>    	ret = add_partition(partition);
>>>>    	if (ret)
>>>> diff --git a/include/asm-generic/mshyperv.h
>>>> b/include/asm-generic/mshyperv.h index bf601d67cecb..e8cbc4e3f7ad
>>>> 100644 --- a/include/asm-generic/mshyperv.h
>>>> +++ b/include/asm-generic/mshyperv.h
>>>> @@ -350,6 +350,7 @@ int hv_call_add_logical_proc(int node, u32
>>>> lp_index, u32 acpi_id); int
>>>> hv_call_notify_all_processors_started(void); bool hv_lp_exists(u32
>>>> lp_index); int hv_call_create_vp(int node, u64 partition_id, u32
>>>> vp_index, u32 flags); +u64 mshv_current_partid(void);
>>>>    
>>>>    #else /* CONFIG_MSHV_ROOT */
>>>>    static inline bool hv_root_partition(void) { return false; }
>>>> @@ -380,6 +381,10 @@ static inline int hv_call_create_vp(int node,
>>>> u64 partition_id, u32 vp_index, u3 {
>>>>    	return -EOPNOTSUPP;
>>>>    }
>>>> +static inline u64 mshv_current_partid(void)
>>>> +{
>>>> +	return HV_PARTITION_ID_INVALID;
>>>> +}
>>>>    #endif /* CONFIG_MSHV_ROOT */
>>>>    
>>>>    static inline int hv_deposit_memory(u64 partition_id, u64
>>>> status)