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

Re: [Xen-devel] [PATCH for-next 07/16] xen/arm: Introduce copy_to_guest_phys_flush_dcache


On 06/12/17 12:27, Julien Grall wrote:
On 12/06/2017 01:26 AM, Stefano Stabellini wrote:
On Thu, 23 Nov 2017, Julien Grall wrote:
Hi Andrew,

On 23/11/17 18:49, Andrew Cooper wrote:
On 23/11/17 18:32, Julien Grall wrote:
This new function will be used in a follow-up patch to copy data to the
using the IPA (aka guest physical address) and then clean the cache.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
   xen/arch/arm/guestcopy.c           | 10 ++++++++++
   xen/include/asm-arm/guest_access.h |  6 ++++++
   2 files changed, 16 insertions(+)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index be53bee559..7958663970 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -110,6 +110,16 @@ unsigned long raw_copy_from_guest(void *to, const
void __user *from, unsigned le
                         COPY_from_guest | COPY_linear);
   +unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
+                                              paddr_t gpa,
+                                              void *buf,
+                                              unsigned int len)
+    /* P2M is shared between all vCPUs, so the vCPU used does not matter.

Be very careful with this line of thinking.  It is only works after
DOMCTL_max_vcpus has succeeded, and before that point, it is a latent
NULL pointer dereference.

I really don't expect that function been used before DOMCT_max_vcpus is set. It is only used for hardware emulation or Xen loading image into the hardware
domain memory. I could add a check d->vcpus to be safe.

Also, what about vcpus configured with alternative views?

It is not important because the underlying call is get_page_from_gfn that does
not care about the alternative view (that function take a domain in
parameter). I can update the comment.
Since this is a new function, would it make sense to take a struct
vcpu* as parameter, instead of a struct domain* ?

Well, I suggested this patch this way because likely everyone will use with d->vcpus[0]. And then you would have to wonder why d->vcpus[0] and not d->vcpus[1]...

Thinking a bit more to this, it might be better/safer to pass either a domain or a vCPU to copy_guest. I can see 2 solutions:
        1# Introduce a union that use the same parameter:
                                struct domain *d;
                        } ipa;
                                struct vcpu *v;
                        } gva;
The structure here would be to ensure that it is clear that only domain (resp. vcpu) should be used with ipa (resp. gva).

        2# Have 2 parameters, vcpu and domain.

Any opinions?


Julien Grall

Xen-devel mailing list



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