lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <23d93b71-86cc-4c01-9264-b049cfec39e0@linux.microsoft.com>
Date: Mon, 8 Sep 2025 11:06:25 -0700
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
To: Easwar Hariharan <easwar.hariharan@...ux.microsoft.com>
Cc: linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
 kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
 mhklinux@...look.com, decui@...rosoft.com, paekkaladevi@...ux.microsoft.com
Subject: Re: [PATCH 2/6] mshv: Ignore second stats page map result failure

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@...ux.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)


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ