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

[Nuno Das Neves <nunodasneves@linux.microsoft.com>]

On 9/5/2025 8:31 AM, Praveen K Paladugu wrote:
> 
> 
> On 8/28/2025 7: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.
>>
> Is this behavior limited to VP stats? Or does it extend to other
> stats (hypervisor, partition, etc) as well?
> 
In practice we will only need to worry about partition and VP.

In the current code in hyperv-next, it's only VP stats. Upcoming patches
to add debugfs code will also need it for partition stats.

>> This results a failure in module init. Instead of failing, gracefully
> nit: s/This results in a failure during module init/

Thanks, I'll change it for v2

Nuno

>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>
>> Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>>   drivers/hv/mshv_root_hv_call.c | 43 ++++++++++++++++++++++++++++++----
>>   drivers/hv/mshv_root_main.c    |  3 +++
>>   2 files changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>> index c9c274f29c3c..1c38576a673c 100644
>> --- a/drivers/hv/mshv_root_hv_call.c
>> +++ b/drivers/hv/mshv_root_hv_call.c
>> @@ -724,6 +724,24 @@ hv_call_notify_port_ring_empty(u32 sint_index)
>>       return hv_result_to_errno(status);
>>   }
>>   +static int
>> +hv_stats_get_area_type(enum hv_stats_object_type type,
>> +               const union hv_stats_object_identity *identity)
>> +{
>> +    switch (type) {
>> +    case HV_STATS_OBJECT_HYPERVISOR:
>> +        return identity->hv.stats_area_type;
>> +    case HV_STATS_OBJECT_LOGICAL_PROCESSOR:
>> +        return identity->lp.stats_area_type;
>> +    case HV_STATS_OBJECT_PARTITION:
>> +        return identity->partition.stats_area_type;
>> +    case HV_STATS_OBJECT_VP:
>> +        return identity->vp.stats_area_type;
>> +    }
>> +
>> +    return -EINVAL;
>> +}
>> +
>>   int hv_call_map_stat_page(enum hv_stats_object_type type,
>>                 const union hv_stats_object_identity *identity,
>>                 void **addr)
>> @@ -732,7 +750,7 @@ int hv_call_map_stat_page(enum hv_stats_object_type type,
>>       struct hv_input_map_stats_page *input;
>>       struct hv_output_map_stats_page *output;
>>       u64 status, pfn;
>> -    int ret = 0;
>> +    int hv_status, ret = 0;
>>         do {
>>           local_irq_save(flags);
>> @@ -747,11 +765,28 @@ int hv_call_map_stat_page(enum hv_stats_object_type type,
>>           pfn = output->map_location;
>>             local_irq_restore(flags);
>> -        if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
>> -            ret = hv_result_to_errno(status);
>> +
>> +        hv_status = hv_result(status);
>> +        if (hv_status != HV_STATUS_INSUFFICIENT_MEMORY) {
>>               if (hv_result_success(status))
>>                   break;
>> -            return ret;
>> +
>> +            /*
>> +             * Some versions of the hypervisor do not support the
>> +             * PARENT stats area. In this case return "success" but
>> +             * set the page to NULL. The caller checks for this
>> +             * case instead just uses the SELF area.
>> +             */
>> +            if (hv_stats_get_area_type(type, identity) == HV_STATS_AREA_PARENT &&
>> +                hv_status == HV_STATUS_INVALID_PARAMETER) {
>> +                pr_debug_once("%s: PARENT area type is unsupported\n",
>> +                          __func__);
>> +                *addr = NULL;
>> +                return 0;
>> +            }
>> +
>> +            hv_status_debug(status, "\n");
>> +            return hv_result_to_errno(status);
>>           }
>>             ret = hv_call_deposit_pages(NUMA_NO_NODE,
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index bbdefe8a2e9c..56ababab57ce 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -929,6 +929,9 @@ static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
>>       if (err)
>>           goto unmap_self;
>>   +    if (!stats_pages[HV_STATS_AREA_PARENT])
>> +        stats_pages[HV_STATS_AREA_PARENT] = stats_pages[HV_STATS_AREA_SELF];
>> +
>>       return 0;
>>     unmap_self:
>