Hi Julien
+
PROGRESS(xen):
ret = relinquish_memory(d, &d->xenpage_list);
if ( ret )
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index ae7ef96..9814481 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -16,6 +16,7 @@
* GNU General Public License for more details.
*/
+#include <xen/ioreq.h>
#include <xen/lib.h>
#include <xen/spinlock.h>
#include <xen/sched.h>
@@ -23,6 +24,7 @@
#include <asm/cpuerrata.h>
#include <asm/current.h>
#include <asm/mmio.h>
+#include <asm/hvm/ioreq.h>
Shouldn't this have been included by "xen/ioreq.h"?
Well, for V1 asm/hvm/ioreq.h was included by xen/ioreq.h. But, it
turned out that there was nothing inside common header required
arch one to be included and
I was asked to include arch header where it was indeed needed
(several *.c files).
Fair enough.
[...]
If you return IO_HANDLED here, then it means the we will take care
of previous I/O but the current one is going to be ignored.
Which current one? As I understand, if try_fwd_ioserv() gets called
with vio->req.state == STATE_IORESP_READY then this is a second
round after emulator completes the emulation (the first round was when
we returned IO_RETRY down the function and claimed that we would
need a completion), so we are still dealing with previous I/O.
vcpu_ioreq_handle_completion() -> arch_ioreq_complete_mmio() ->
try_handle_mmio() -> try_fwd_ioserv() -> handle_ioserv()
And after we return IO_HANDLED here, handle_ioserv() will be called
to complete the handling of this previous I/O emulation.
Or I really missed something?
Hmmm... I somehow thought try_fw_ioserv() would only be called the
first time. Do you have a branch with your code applied? This would
help to follow the different paths.
Yes, I mentioned about it in cover letter.
Please see
https://github.com/otyshchenko1/xen/commits/ioreq_4.14_ml5
why 5 - because I started counting from the RFC)
Oh, I looked at the cover letter and didn't find it. Hence why I
asked. I should have looked more carefully. Thanks!
I have looked closer at the question and I am not sure to understand
why arch_ioreq_complete_mmio() is going to call try_handle_mmio().
This looks pretty innefficient to me because we already now the IO was
handled by the IOREQ server.
I realize that x86 is calling handle_mmio() again. However, I don't
think we need the same on Arm because the instruction for accessing
device memory are a lot simpler (you can only read or store at most a
64-bit value).
I think, I agree.
So I would like to keep our emulation simple and not rely on
try_ioserv_fw() to always return true when call from completion
(AFAICT it is not possible to return false then).
So what you are proposing is just a replacement try_ioserv_fw() by
handle_ioserv() technically?
diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index 40b9e59..0508bd8 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -101,12 +101,10 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs
*regs,
bool arch_ioreq_complete_mmio(void)
{
- struct vcpu *v = current;
struct cpu_user_regs *regs = guest_cpu_user_regs();
const union hsr hsr = { .bits = regs->hsr };
- paddr_t addr = v->io.req.addr;
- if ( try_handle_mmio(regs, hsr, addr) == IO_HANDLED )
+ if ( handle_ioserv(regs, current) == IO_HANDLED )
{
advance_pc(regs, hsr);
return true;
I will answer to the rest separately.
Thank you.
Cheers,
--
Regards,
Oleksandr Tyshchenko