Message-ID: <1bb599ee-fe28-409d-b430-2fc086268936@linux.microsoft.com>
Date: Fri, 29 Aug 2025 11:22:45 +0530
From: Naman Jain <namjain@...ux.microsoft.com>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "K . Y . Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org, Michael Kelley <mhklinux@...look.com>,
 Long Li <longli@...rosoft.com>, stable@...r.kernel.org
Subject: Re: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt
 mask



On 8/28/2025 8:32 PM, Stephen Hemminger wrote:
> On Thu, 28 Aug 2025 10:12:00 +0530
> Naman Jain <namjain@...ux.microsoft.com> wrote:
> 
>> Remove the logic to set interrupt mask by default in uio_hv_generic
>> driver as the interrupt mask value is supposed to be controlled
>> completely by the user space. If the mask bit gets changed
>> by the driver, concurrently with user mode operating on the ring,
>> the mask bit may be set when it is supposed to be clear, and the
>> user-mode driver will miss an interrupt which will cause a hang.
>>
>> For eg- when the driver sets inbound ring buffer interrupt mask to 1,
>> the host does not interrupt the guest on the UIO VMBus channel.
>> However, setting the mask does not prevent the host from putting a
>> message in the inbound ring buffer. So let’s assume that happens,
>> the host puts a message into the ring buffer but does not interrupt.
>>
>> Subsequently, the user space code in the guest sets the inbound ring
>> buffer interrupt mask to 0, saying “Hey, I’m ready for interrupts”.
>> User space code then calls pread() to wait for an interrupt.
>> Then one of two things happens:
>>
>> * The host never sends another message. So the pread() waits forever.
>> * The host does send another message. But because there’s already a
>>    message in the ring buffer, it doesn’t generate an interrupt.
>>    This is the correct behavior, because the host should only send an
>>    interrupt when the inbound ring buffer transitions from empty to
>>    not-empty. Adding an additional message to a ring buffer that is not
>>    empty is not supposed to generate an interrupt on the guest.
>>    Since the guest is waiting in pread() and not removing messages from
>>    the ring buffer, the pread() waits forever.
>>
>> This could be easily reproduced in hv_fcopy_uio_daemon if we delay
>> setting interrupt mask to 0.
>>
>> Similarly if hv_uio_channel_cb() sets the interrupt_mask to 1,
>> there’s a race condition. Once user space empties the inbound ring
>> buffer, but before user space sets interrupt_mask to 0, the host could
>> put another message in the ring buffer but it wouldn’t interrupt.
>> Then the next pread() would hang.
>>
>> Fix these by removing all instances where interrupt_mask is changed,
>> while keeping the one in set_event() unchanged to enable userspace
>> control the interrupt mask by writing 0/1 to /dev/uioX.
>>
>> Fixes: 95096f2fbd10 ("uio-hv-generic: new userspace i/o driver for VMBus")
>> Suggested-by: John Starks <jostarks@...rosoft.com>
>> Signed-off-by: Naman Jain <namjain@...ux.microsoft.com>
>> Cc: <stable@...r.kernel.org>
> 
> Makes sense. I think the logic got carried over from uio.
> Does it need to make sure interrupt is masked by default to avoid
> races at startup?

No, initially I also figured that this would be required, and that's why
this was added in the first place. But my experiments with userspace
told me otherwise and I don't think this is required.

Thanks.

Regards,
Naman

Powered by blists - more mailing lists