On Tue, Apr 28, 2026 at 12:20:35AM +0000, Michael Kelley wrote: > From: Stanislav Kinsburskii <[email protected]> Sent: Monday, > April 27, 2026 7:44 AM > > > > Restore interrupt state before breaking out of the loop on error. > > > > The irq_flags are saved before entering the loop, but the early exit > > path on error fails to restore them. This leaves interrupts in an > > inconsistent state and can lead to lockdep warnings or other > > interrupt-related issues. > > > > Fixes: 621191d709b14 ("Drivers: hv: Introduce mshv_root module to expose > > /dev/mshv to VMMs") > > Signed-off-by: Stanislav Kinsburskii <[email protected]> > > --- > > drivers/hv/mshv_root_hv_call.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c > > index ab210a7fcb8c3..61291ec6f3468 100644 > > --- a/drivers/hv/mshv_root_hv_call.c > > +++ b/drivers/hv/mshv_root_hv_call.c > > @@ -229,8 +229,10 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 > > gfn, u64 page_struct_count, > > } else { > > pfnlist[i] = mmio_spa + done + i; > > } > > - if (ret) > > + if (ret) { > > + local_irq_restore(irq_flags); > > break; > > + } > > > > This looks good for fixing the immediate bug. > > But I'd note that this error path occurs solely based on the > if (index >= page_struct_count) test in the preceding 'for' loop. That test > is a > "can't happen" sanity test that never triggers if hv_do_map_gpa_hcall() > is coded correctly. At the beginning of the function there are validations of > the input arguments, which is reasonable. But this sanity test isn't based > on the input arguments, and it adds non-trivial complexity to the code > because of the nested loops and the need to figure out where the two > "break" statements go. I'd argue for dropping the sanity test entirely, > along with this test of 'ret' and the need to restore the interrupt state. >
Fair enough. Let me rework this function (and it's unmap peer). Thanks, Stanislav > Michael
