From: Purna Pavan Chandra Aekkaladevi <[email protected]>
[ Upstream commit 7538b80e5a4b473b73428d13b3a47ceaad9a8a7c ] Older versions of the hypervisor do not have a concept of separate SELF and PARENT stats areas. In this case, mapping the HV_STATS_AREA_SELF page is sufficient - it's the only page and it contains all available stats. Mapping HV_STATS_AREA_PARENT returns HV_STATUS_INVALID_PARAMETER which currently causes module init to fail on older hypevisor versions. Detect this case and gracefully fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the already-mapped SELF page. Add comments to clarify the behavior, including a clarification of why this isn't needed for hv_call_map_stats_page2() which always supports PARENT and SELF areas. Signed-off-by: Purna Pavan Chandra Aekkaladevi <[email protected]> Signed-off-by: Nuno Das Neves <[email protected]> Reviewed-by: Stanislav Kinsburskii <[email protected]> Acked-by: Stanislav Kinsburskii <[email protected]> Reviewed-by: Michael Kelley <[email protected]> Signed-off-by: Wei Liu <[email protected]> Signed-off-by: Sasha Levin <[email protected]> --- LLM Generated explanations, may be completely bogus: Now I have all the information needed for a thorough analysis. ## Analysis ### 1. Commit Message Analysis The commit clearly describes a backward compatibility bug: on older Hyper-V hypervisor versions that don't support separate SELF and PARENT stats areas, mapping `HV_STATS_AREA_PARENT` returns `HV_STATUS_INVALID_PARAMETER`, which causes **module initialization to fail entirely**. This is not a feature addition — it's fixing a regression/incompatibility where the driver doesn't work on older hypervisors. ### 2. Code Change Analysis The fix has three parts: **a) New helper `hv_stats_get_area_type()`** (~15 lines): Extracts the stats area type from the identity union based on the object type. This is needed to distinguish PARENT from SELF area mapping requests. **b) Modified `hv_call_map_stats_page()`** (~20 lines changed): When the hypercall returns `HV_STATUS_INVALID_PARAMETER` specifically for a PARENT area mapping, instead of failing with an error, it returns success but with `*addr = NULL`. This signals to the caller that PARENT isn't supported. **c) Modified `mshv_vp_stats_map()`** (+3 lines): After mapping PARENT, if the address is NULL (meaning older hypervisor), it falls back to using the already-mapped SELF page for both areas. This is safe because on older hypervisors, the SELF page contains all available stats. ### 3. Bug Impact - **Severity**: HIGH — the driver completely fails to create VPs (virtual processors), making it unusable on older hypervisor versions - **User impact**: Anyone running the mshv_root driver on an older Hyper-V hypervisor version cannot use the driver at all - **Trigger**: Deterministic — always fails on affected hypervisor versions, not a race or edge case ### 4. Scope and Risk - The change is small (~40 lines including comments) and well-contained to the stats page mapping path - It adds graceful degradation, not new behavior — the driver works the same on new hypervisors - Multiple reviewers: Reviewed-by and Acked-by from Stanislav Kinsburskii, Reviewed-by from Michael Kelley - The fallback logic (using SELF page for both) is explicitly described as correct by the authors since older hypervisors put all stats in the single SELF page ### 5. Stable Tree Applicability - The mshv_root driver was introduced in **v6.15** (commit `621191d709b14`) - The bug has been present **since the driver was first introduced** — the original `mshv_vp_stats_map()` function already mapped both SELF and PARENT areas with no fallback - In v6.19 the function was renamed from `hv_call_map_stat_page` to `hv_call_map_stats_page` and wrapped by `hv_map_stats_page` (commit `d62313bdf5961`), so backporting to pre-6.19 trees would require minor adaptation - The fix applies to all stable trees containing the mshv_root driver (6.15.y through 6.18.y) ### 6. Stable Criteria Assessment | Criterion | Assessment | |-----------|-----------| | Obviously correct and tested | YES — well-reviewed, clear logic | | Fixes a real bug | YES — module init failure | | Important issue | YES — driver completely non-functional on older hypervisors | | Small and contained | YES — ~40 lines, 2 files, single subsystem | | No new features | YES — adds backward compatibility/graceful degradation | ### Verification - `git show 621191d709b14 -- drivers/hv/mshv_root_main.c` confirmed `mshv_vp_stats_map()` with SELF+PARENT mapping was present in the original v6.15 driver introduction - `git show 621191d709b14 -- drivers/hv/mshv_root_hv_call.c` confirmed the original `hv_call_map_stat_page()` had no fallback for PARENT area failures - `git describe --contains 621191d709b14` returned `v6.15-rc1~193^2~2`, confirming v6.15 introduction - `git describe --contains d62313bdf5961` returned `v6.19-rc1~56^2~30`, confirming function rename happened in v6.19 - `git log -p` diff confirmed the rename from `hv_call_map_stat_page` to `hv_call_map_stats_page` occurred in `d62313bdf5961` - The bug mechanism is verified: older hypervisors return `HV_STATUS_INVALID_PARAMETER` for PARENT area mapping, which was not handled, causing the error to propagate up and fail VP creation - Could NOT independently verify which exact stable trees (6.15.y, 6.16.y, 6.17.y, 6.18.y) are still actively maintained (unverified) ### Conclusion This is a legitimate bug fix for a complete driver initialization failure on older Hyper-V hypervisor versions. The bug has existed since the driver was first introduced in v6.15. The fix is small, well- contained, well-reviewed, and poses minimal regression risk. Backporting to pre-6.19 stable trees would require minor adaptation due to function renames, but the logic is straightforward. It meets all stable kernel criteria. **YES** drivers/hv/mshv_root_hv_call.c | 52 +++++++++++++++++++++++++++++++--- drivers/hv/mshv_root_main.c | 3 ++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c index 598eaff4ff299..1f93b94d7580c 100644 --- a/drivers/hv/mshv_root_hv_call.c +++ b/drivers/hv/mshv_root_hv_call.c @@ -813,6 +813,13 @@ hv_call_notify_port_ring_empty(u32 sint_index) return hv_result_to_errno(status); } +/* + * Equivalent of hv_call_map_stats_page() for cases when the caller provides + * the map location. + * + * NOTE: This is a newer hypercall that always supports SELF and PARENT stats + * areas, unlike hv_call_map_stats_page(). + */ static int hv_call_map_stats_page2(enum hv_stats_object_type type, const union hv_stats_object_identity *identity, u64 map_location) @@ -855,6 +862,34 @@ static int hv_call_map_stats_page2(enum hv_stats_object_type type, return ret; } +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; +} + +/* + * Map a stats page, where the page location is provided by the hypervisor. + * + * NOTE: The concept of separate SELF and PARENT stats areas does not exist on + * older hypervisor versions. All the available stats information can be found + * on the SELF page. When attempting to map the PARENT area on a hypervisor + * that doesn't support it, return "success" but with a NULL address. The + * caller should check for this case and instead fallback to the SELF area + * alone. + */ static int hv_call_map_stats_page(enum hv_stats_object_type type, const union hv_stats_object_identity *identity, void **addr) @@ -863,7 +898,7 @@ static int hv_call_map_stats_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); @@ -878,11 +913,20 @@ static int hv_call_map_stats_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; + + if (hv_stats_get_area_type(type, identity) == HV_STATS_AREA_PARENT && + hv_status == HV_STATUS_INVALID_PARAMETER) { + *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 681b58154d5ea..d3e8a66443ad6 100644 --- a/drivers/hv/mshv_root_main.c +++ b/drivers/hv/mshv_root_main.c @@ -993,6 +993,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: -- 2.51.0
