On 26.09.20 16:21, Julien Grall wrote:
Hi Oleksandr,
Hi Julien.
On 24/09/2020 19:22, Oleksandr wrote:
On 24.09.20 20:25, Julien Grall wrote:
On 23/09/2020 21:16, Oleksandr wrote:
On 23.09.20 21:03, Julien Grall wrote:
On 10/09/2020 21:22, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Could you please clarify how this patch could be split in smaller one?
This patch is going to be reduced a fair bit if you make some of the
structure common. The next steps would be to move anything that is
not directly related to IOREQ out.
Thank you for the clarification.
Yes, however, I believed everything in this patch is directly related
to IOREQ...
From a quick look, there are few things that can be moved in
separate patches:
- The addition of the ASSERT_UNREACHABLE()
Did you mean the addition of the ASSERT_UNREACHABLE() to
arch_handle_hvm_io_completion/handle_pio can moved to separate patches?
Sorry, I don't quite understand, for what benefit?
Sorry I didn't realize there was multiple ASSERT_UNREACHABLE() in the
code. I was referring to the one in the follow chunk:
@@ -1955,9 +1959,14 @@ static void do_trap_stage2_abort_guest(struct
cpu_user_regs *regs,
case IO_HANDLED:
advance_pc(regs, hsr);
return;
+ case IO_RETRY:
+ /* finish later */
+ return;
case IO_UNHANDLED:
/* IO unhandled, try another way to handle it. */
break;
+ default:
+ ASSERT_UNREACHABLE();
}
}
While I understand the reason this was added, to me this doesn't seem
to be directly related to this patch.
In fact, the switch case will be done on an enum. So without the
default, the compiler will be able to notice if we are adding a new
field. With this new approach, you would only notice at runtime
(assuming the path is exercised).
So what do we gain?
Hmm, now I am in doubt whether we really need to put
ASSERT_UNREACHABLE() here. Also we would notice it at the runtime for
debug builds only.
[...]
I think Jan made some suggestion today. Let me know if you require
more input.
Yes. I am considering this now. I provided my thoughts on that a
little bit earlier. Could you please clarify there.
I have replied to it.
Thank you.
--
Regards,
Oleksandr Tyshchenko
