On 9/21/2026 5:14 PM, Michael Kelley wrote:
> From: Hardik Garg <[email protected]>
>> Register-page setup is optional for each CPU. If allocation or
>> registration fails, mshv_vtl_configure_reg_page() warns and leaves that
>> CPU's reg_page NULL, but initial context setup continues successfully.
>> Meanwhile, successful setup on another CPU sets the global
>> mshv_has_reg_page flag.
>>
>> mshv_vtl_fault() checks this global flag before selecting the requested
>> CPU's register page. With mixed setup results across online CPUs, the
>> check passes even for a CPU with no register page, reaching
>> get_page(NULL) when userspace faults in that mapping.
> I'm a bit late in reviewing this because I was travelling all last week.
>
> This patch seems like it is just papering over the real problem, which
> is that a global variable like mshv_has_reg_page can't represent the
> status of an operation that may succeed or fail on a per-cpu basis.
> I pointed this out (as did Sashiko) in review comments back in
> April [1].
>
> The only other place mshv_has_reg_page is used is in
> mshv_ioctl_check_extensions() where its value is returned from an
> ioctl() system call made by user space. That's a questionable practice
> since ioctl() usually returns 0 on success, though the man page for
> ioctl() does admit that some ioctls use the return value as an output
> parameter and return a non-negative value on success. But even
> then, a single value can't accurately reflect the status of an
> operation that may succeed or fail on a per-cpu basis. User
> space would presumably have a similar problem to what is
> being fixed by this patch.
>
> Is there any reason that the real problem couldn't be fixed
> instead of doing this fix on top of something that is fundamentally
> broken?
>
> Michael
>
> [1]
> https://lore.kernel.org/linux-hyperv/sn6pr02mb4157cf364da2c0cc657a6dcbd4...@sn6pr02mb4157.namprd02.prod.outlook.com/
Hi Michael,
Thanks for the feedback and for pointing me to your April review.
After going through it, I understand that the NULL check only addresses
the immediate fault, while the global capability still misrepresents
per-CPU availability.
I'll work on addressing the underlying issue and send a proposed fix
soon.
Thanks,
Hardik
>> Check the selected per-CPU register page before taking its reference and
>> return VM_FAULT_SIGBUS if it is absent.
>>
>> Fixes: 7bfe3b8ea6e3 ("Drivers: hv: Introduce mshv_vtl driver")
>> Cc: [email protected]
>> Signed-off-by: Hardik Garg <[email protected]>
>> ---
>> drivers/hv/mshv_vtl_main.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
>> index fc993560a45c..3296ab5c0243 100644
>> --- a/drivers/hv/mshv_vtl_main.c
>> +++ b/drivers/hv/mshv_vtl_main.c
>> @@ -912,6 +912,9 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
>> return VM_FAULT_NOPAGE;
>> }
>>
>> + if (!page)
>> + return VM_FAULT_SIGBUS;
>> +
>> get_page(page);
>> vmf->page = page;
>>
