Re: [PATCH 2/6] mshv: Ignore second stats page map result failure

[Praveen K Paladugu <praveenkpaladugu@gmail.com>]

On 9/8/2025 1:06 PM, Nuno Das Neves wrote:
> On 9/8/2025 10:22 AM, Easwar Hariharan wrote:
>> On 9/8/2025 10:04 AM, Nuno Das Neves wrote:
>>> On 9/5/2025 12:21 PM, Easwar Hariharan wrote:
>>>> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>>>>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>>>>
>>>>> Some versions of the hypervisor do not support HV_STATUS_AREA_PARENT and
>>>>> return HV_STATUS_INVALID_PARAMETER for the second stats page mapping
>>>>> request.
>>>>>
>>>>> This results a failure in module init. Instead of failing, gracefully
>>>>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>>>>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>>>
>>>> What's the impact of this graceful fallback? It occurs to me that if a stats
>>>> accumulator, in userspace perhaps, expected to get stats from the 2 pages,
>>>> it'd get incorrect values.
>>>>
>>> This is going out of scope of this series a bit but I'll explain briefly.
>>>
>>> When we do add the code to expose these stats to userspace, the SELF and
>>> PARENT pages won't be exposed separately, there is no duplication.
>>>
>>> For each stat counter in the page, we'll expose either the SELF or PARENT
>>> value, depending on whether there is anything in that slot (whether it's zero
>>> or not).
>>>
>>> Some stats are available via the SELF page, and some via the PARENT page, but
>>> the counters in the page have the same layout. So some counters in the SELF
>>> page will all stay zero while on the PARENT page they are updated, and vice
>>> versa.
>>>
>>> I believe the hypervisor takes this strange approach for the purpose of
>>> backward compatibility. Introducing L1VH created the need for this SELF/PARENT
>>> distinction.
>>>
>>> Hope that makes some kind of sense...it will be clearer when we post the mshv
>>> debugfs code itself.
>>>
>>> To put it another way, falling back to the SELF page won't cause any impact
>>> to userspace because the distinction between the pages is all handled in the
>>> driver, and we only read each stat value from either SELF or PARENT.
>>>
>>> Nuno
>>
>> Thank you for that explanation, it sorta makes sense.
>>
>> I think it'd be better if this patch is part of the series that exposes the stats
>> to userspace, so that it can be reviewed in context with the rest of the code in
>> the driver that manages the pick-and-choose of a stat value from the SELF/PARENT
>> page.
>>
> Good idea, I think I'll do that. Thanks!
> 
>> Unless there's an active problem now in the upstream kernel that this patch solves?
>> i.e. are the versions of the hypervisor that don't support the PARENT stats
>> page available in the wild?
>>
> I thought there was, but on reflection, no it doesn't solve a problem that exists in
> the code today.
> 
> Nuno
>

The usecases for stats exposed by the hypervisor are:
1) used within the kernel by root scheduler
2) exposed to userspace via debugfs.

I thought we are addressing the first use-case here (patch1 in this 
series). If root scheduler support was upstreamed then this patchset 
does solve a problem in upstream code.


>> Thanks,
>> Easwar (he/him)
> 
> 

-- 
Regards,
Praveen K Paladugu