> -----Original Message-----
> From: Julien Grall [mailto:julien.gr...@linaro.org]
> Sent: 30 October 2017 12:09
> To: Paul Durrant <paul.durr...@citrix.com>; Jan Beulich
> <jbeul...@suse.com>
> Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; Wei Liu <wei.l...@citrix.com>; George
> Dunlap <george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>;
> Stefano Stabellini <sstabell...@kernel.org>; xen-de...@lists.xenproject.org;
> Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; Daniel De Graaf
> <dgde...@tycho.nsa.gov>; Tim (Xen.org) <t...@xen.org>
> Subject: Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add
> HYPERVISOR_memory_op to acquire guest resources
>
> Hi Paul,
>
> On 27/10/17 16:19, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Julien Grall [mailto:julien.gr...@linaro.org]
> >> Sent: 27 October 2017 12:46
> >> To: Jan Beulich <jbeul...@suse.com>; Paul Durrant
> >> <paul.durr...@citrix.com>
> >> Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper
> >> <andrew.coop...@citrix.com>; Wei Liu <wei.l...@citrix.com>; George
> >> Dunlap <george.dun...@citrix.com>; Ian Jackson
> <ian.jack...@citrix.com>;
> >> Stefano Stabellini <sstabell...@kernel.org>; xen-
> de...@lists.xenproject.org;
> >> Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; Daniel De Graaf
> >> <dgde...@tycho.nsa.gov>; Tim (Xen.org) <t...@xen.org>
> >> Subject: Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add
> >> HYPERVISOR_memory_op to acquire guest resources
> >>
> >> Hi,
> >>
> >> On 26/10/17 16:39, Jan Beulich wrote:
> >>>>>> On 26.10.17 at 17:32, <julien.gr...@linaro.org> wrote:
> >>>> On 26/10/17 16:26, Jan Beulich wrote:
> >>>>>>>> On 17.10.17 at 15:24, <paul.durr...@citrix.com> wrote:
> >>>>>> + /* IN/OUT - If the tools domain is PV then, upon return,
> frame_list
> >>>>>> + * will be populated with the MFNs of the resource.
> >>>>>> + * If the tools domain is HVM then it is expected that,
> >>>>>> on
> >>>>>> + * entry, frame_list will be populated with a list of
> >>>>>> GFNs
> >>>>>> + * that will be mapped to the MFNs of the resource.
> >>>>>> + * If -EIO is returned then the frame_list has only been
> >>>>>> + * partially mapped and it is up to the caller to unmap
> >>>>>> all
> >>>>>> + * the GFNs.
> >>>>>> + * This parameter may be NULL if nr_frames is 0.
> >>>>>> + */
> >>>>>> + XEN_GUEST_HANDLE(xen_ulong_t) frame_list;
> >>>>>
> >>>>> This is still xen_ulong_t, which I can live with, but then you shouldn't
> >>>>> copy into / out of arrays of other types in acquire_resource() (the
> >>>>> more that this is common code, and iirc xen_ulong_t and
> >>>>> unsigned long aren't the same thing on ARM32).
> >>>>
> >>>> xen_ulong_t is always 64-bit on Arm (32-bit and 64-bit). But shouldn't
> >>>> we use xen_pfn_t here?
> >>>
> >>> I had put this question up earlier, but iirc Paul didn't like it.
> >>
> >> I'd like to understand why Paul doesn't like it. We should never assume
> >> that a frame fit in xen_ulong_t. xen_pfn_t was exactly introduced for
> >> that purpose.
> >
> > My reservation is whether xen_pfn_t is intended to hold either gfns or
> mfns, since this hypercall uses the same array for both. If it suitable then
> I am
> happy to change it, but Andrew led me to believe otherwise.
>
> Looking at the public hearders, xen_pfn_t is been used for both MFN (see
> xenpf_add_memtype) and GFN (see gnttab_setup_table).
>
> So I think it would be fine to do the same here.
Yes, I'm going to change it in the next version.
Cheers,
Paul
> Cheers,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
