[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V3] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

On 06/09/16 12:36, Razvan Cojocaru wrote:
On 09/06/2016 02:20 PM, Julien Grall wrote:

On 06/09/16 12:05, George Dunlap wrote:
On 06/09/16 11:51, Julien Grall wrote:

On 06/09/16 11:45, Razvan Cojocaru wrote:

On 06/09/16 11:00, Razvan Cojocaru wrote:
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b648a9d..e65a9b8 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1836,6 +1836,15 @@ long p2m_set_mem_access(struct domain *d,
gfn, uint32_t nr,
     return 0;

+long p2m_set_mem_access_multi(struct domain *d,
+                              const XEN_GUEST_HANDLE(const_uint64)
+                              const XEN_GUEST_HANDLE(const_uint8)
+                              uint32_t nr, uint32_t start, uint32_t
+                              unsigned int altp2m_idx)
+    return -ENOTSUP;

Why didn't you implement this function for ARM?

Because unfortunately I don't have an ARM setup to test it on and I
thought it would be unfair to publish the patch with untestable ARM

So what's the plan? Who will implement the ARM solution?

I don't think there is a technical challenge to implement the ARM one.

Are we going to require that all new functionality be implemented both
on x86 and on ARM?  That seems like a bit of a lot to ask of someone
who's not going to use it (and as Razvan points out, can't even test
it).  The mem_access functionality is still fairly technical -- anyone
who decides they want to use it and runs across the same problem Razvan
did should have no trouble implementing this hypercall for ARM when they
need it.

I am not saying we should every time implement the ARM version when the
x86 version is added and vice-versa.

However, I don't think this is a lot to ask when an hypercall benefits
both architecture. Note that nobody would have complained that the code
was not tested on ARM if the hypercall was implemented in the common
code. Even if it could break the platform...

Yes, but in that case the code would actually have been tested, because
the common code runs (or should run) in the same manner for both ARM and
x86. So x86 testing would have implicitly tested the common code.

That's quite a big assumption. I have seen common code patch touching memory subsystem breaking ARM because the way memory is handled is different.

For our case, it is unfortunately impossible to implement this in the
common part of the code. The main reason for this is that it's important
to avoid a TLB flush until the very end, so we can't just iterate over
the arrays and call the old p2m_set_mem_access() for each element, since
p2m_set_mem_access() flushes the TLB on each call (the common code
approach was my first thought going in).

I certainly want everyone to be happy, but even assuming I "blindly"
write some passable ARM code, it's always possible that a problem will
pop up, at which point it will be rightly expected of me to remedy the
situation as quickly as possible - but I may not even be able to
investigate the issue.

There is a free ARM model [1] available that can boot Xen easily.


[1] http://www.arm.com/products/tools/models/foundation-model.php

Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.