Re: [Xen-devel] [PATCH for-next 06/16] xen/arm: Extend copy_to_guest to support copying from/to guest physical address

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Stefano,

On 07/12/17 23:01, Stefano Stabellini wrote:
> On Wed, 6 Dec 2017, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 12/06/2017 01:22 AM, Stefano Stabellini wrote:
> > > On Thu, 23 Nov 2017, Julien Grall wrote:
> > > > The only differences between copy_to_guest and access_guest_memory_by_ipa
> > > > are:
> > > >       - The latter does not support copying data crossing page boundary
> > > >       - The former is copying from/to guest VA whilst the latter from
> > > >       guest PA
> > > >
> > > > copy_to_guest can easily be extended to support copying from/to guest
> > > > physical address. For that a new bit is used to tell whether linear
> > > > address or ipa is been used.
> > > >
> > > > Lastly access_guest_memory_by_ipa is reimplemented using copy_to_guest.
> > > > This also has the benefits to extend the use of it, it is now possible
> > > > to copy data crossing page boundary.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> > > Ah! This is the reason why previous patches were not using vaddr_t. It
> > > makes sense now. May I suggest we use something different from paddr_t
> > > in copy_guest for addr type? I don't think is correct to specify addr as
> > > paddr_t when it could be vaddr_t; in the future we could have type
> > > checks on them.
> > >
> > > I suggest we specify it as u64, but if you have a better idea go for it.
> > We should not use more u64 in the code. uint64_t could be a solution but even
> > that, I don't see the reason. How are you sure the physical address will
> > always fit in 64-bit?
> >
> > On the other side, very likely vaddr_t will fit in paddr_t. So paddr_t is the
> > right way to go for me.
> What about introducing xaddr_t?
I would prefer uint64_t in that case. xaddr_t is quite confusing to read 
and could be misused.
> Or at least:
>
>    static struct page_info *translate_get_page(struct vcpu *v, paddr_t /*or 
> vaddr_t */ addr
I can do that as well. What's your preference?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel