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 <paekkalad...@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 > Thanks, > Easwar (he/him)