* [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
@ 2017-06-16 4:55 Zhongze Liu
2017-06-16 8:45 ` Jan Beulich
0 siblings, 1 reply; 7+ messages in thread
From: Zhongze Liu @ 2017-06-16 4:55 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, Zhongze Liu
currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc.
add a wrapper to do that.
Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
Cc: Wei Liu <wei.liu2@citrix.com>,
Cc: Stefano Stabellini <sstabellini@kernel.org>
---
tools/libxc/include/xenctrl.h | 9 +++++++++
tools/libxc/xc_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f412dd..4ea520b188 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
unsigned long idx,
xen_pfn_t gpfn);
+int xc_domain_add_to_physmap_batch(xc_interface *xch,
+ uint32_t domid,
+ uint32_t foreign_domid,
+ unsigned int space,
+ uint16_t size,
+ xen_ulong_t *idxs,
+ xen_pfn_t *gfpns,
+ int *errs);
+
int xc_domain_populate_physmap(xc_interface *xch,
uint32_t domid,
unsigned long nr_extents,
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 5d192ea0e4..0d34754821 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1032,6 +1032,50 @@ int xc_domain_add_to_physmap(xc_interface *xch,
return do_memory_op(xch, XENMEM_add_to_physmap, &xatp, sizeof(xatp));
}
+int xc_domain_add_to_physmap_batch(xc_interface *xch,
+ uint32_t domid,
+ uint32_t foreign_domid,
+ unsigned int space,
+ uint16_t size,
+ xen_ulong_t *idxs,
+ xen_pfn_t *gpfns,
+ int *errs)
+{
+ int rc;
+ DECLARE_HYPERCALL_BOUNCE(idxs, size * sizeof(*idxs), XC_HYPERCALL_BUFFER_BOUNCE_IN);
+ DECLARE_HYPERCALL_BOUNCE(gpfns, size * sizeof(*gpfns), XC_HYPERCALL_BUFFER_BOUNCE_IN);
+ DECLARE_HYPERCALL_BOUNCE(errs, size * sizeof(*errs), XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+ struct xen_add_to_physmap_batch xatp_batch = {
+ .domid = domid,
+ .space = space,
+ .size = size,
+ .u = {.foreign_domid = foreign_domid}
+ };
+
+ if ( xc_hypercall_bounce_pre(xch, idxs) ||
+ xc_hypercall_bounce_pre(xch, gpfns) ||
+ xc_hypercall_bounce_pre(xch, errs) )
+ {
+ PERROR("Could not bounce memory for XENMEM_add_to_physmap_batch");
+ goto out;
+ }
+
+ set_xen_guest_handle(xatp_batch.idxs, idxs);
+ set_xen_guest_handle(xatp_batch.gpfns, gpfns);
+ set_xen_guest_handle(xatp_batch.errs, errs);
+
+ rc = do_memory_op(xch, XENMEM_add_to_physmap_batch,
+ &xatp_batch, sizeof(xatp_batch));
+
+out:
+ xc_hypercall_bounce_post(xch, idxs);
+ xc_hypercall_bounce_post(xch, gpfns);
+ xc_hypercall_bounce_post(xch, errs);
+
+ return rc;
+}
+
int xc_domain_claim_pages(xc_interface *xch,
uint32_t domid,
unsigned long nr_pages)
--
2.13.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
2017-06-16 4:55 [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch Zhongze Liu
@ 2017-06-16 8:45 ` Jan Beulich
2017-06-16 9:36 ` Zhongze Liu
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-06-16 8:45 UTC (permalink / raw)
To: Zhongze Liu; +Cc: Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel
>>> On 16.06.17 at 06:55, <blackskygg@gmail.com> wrote:
> currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc.
> add a wrapper to do that.
It may help acceptance if you say why all of the sudden a wrapper
is needed.
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
> unsigned long idx,
> xen_pfn_t gpfn);
>
> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
> + uint32_t domid,
> + uint32_t foreign_domid,
I'm not exactly sure what the libxc coding rules are, but I'd expect
these both to be domid_t, ...
> + unsigned int space,
> + uint16_t size,
... this one to be unsigned int, ...
> + xen_ulong_t *idxs,
> + xen_pfn_t *gfpns,
... and these two to be pointers to const.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
2017-06-16 8:45 ` Jan Beulich
@ 2017-06-16 9:36 ` Zhongze Liu
2017-06-16 9:44 ` Zhongze Liu
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Zhongze Liu @ 2017-06-16 9:36 UTC (permalink / raw)
To: Jan Beulich; +Cc: Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel
Hi Jan,
2017-06-16 16:45 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>> On 16.06.17 at 06:55, <blackskygg@gmail.com> wrote:
>> currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc.
>> add a wrapper to do that.
>
> It may help acceptance if you say why all of the sudden a wrapper
> is needed.
>
It's indeed a preparation for my GSoC project:
https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
Thanks for the suggestion.
>
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
>> unsigned long idx,
>> xen_pfn_t gpfn);
>>
>> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
>> + uint32_t domid,
>> + uint32_t foreign_domid,
>
> I'm not exactly sure what the libxc coding rules are, but I'd expect
> these both to be domid_t, ...
>
I was planning to make them domid_t, but according to the other
domid-parameters' types
in the file, and they are all uint32_t, so I finally decided on uint32_t.
>> + unsigned int space,
>> + uint16_t size,
>
> ... this one to be unsigned int, ...
In the xen_add_to_physmap_batch struct, both @space and @size are
uint16_t, so I think
I should have made @space uint16_t, too. I'll fix this. Or do you have
any good reasons to
make both of them unsigned int?
>
>> + xen_ulong_t *idxs,
>> + xen_pfn_t *gfpns,
>
> ... and these two to be pointers to const.
>
Yes, indeed. Sorry for this.
>
> Jan
>
Cheers,
Zhongze Liu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
2017-06-16 9:36 ` Zhongze Liu
@ 2017-06-16 9:44 ` Zhongze Liu
2017-06-16 10:07 ` Zhongze Liu
2017-06-16 10:33 ` Jan Beulich
2017-06-19 11:12 ` Wei Liu
2 siblings, 1 reply; 7+ messages in thread
From: Zhongze Liu @ 2017-06-16 9:44 UTC (permalink / raw)
To: Jan Beulich; +Cc: Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel
2017-06-16 17:36 GMT+08:00 Zhongze Liu <blackskygg@gmail.com>:
> Hi Jan,
>
>
> 2017-06-16 16:45 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>> On 16.06.17 at 06:55, <blackskygg@gmail.com> wrote:
>>> currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc.
>>> add a wrapper to do that.
>>
>> It may help acceptance if you say why all of the sudden a wrapper
>> is needed.
>>
>
> It's indeed a preparation for my GSoC project:
> https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
>
> Thanks for the suggestion.
>
>>
>>> --- a/tools/libxc/include/xenctrl.h
>>> +++ b/tools/libxc/include/xenctrl.h
>>> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
>>> unsigned long idx,
>>> xen_pfn_t gpfn);
>>>
>>> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
>>> + uint32_t domid,
>>> + uint32_t foreign_domid,
>>
>> I'm not exactly sure what the libxc coding rules are, but I'd expect
>> these both to be domid_t, ...
>>
>
> I was planning to make them domid_t, but according to the other
> domid-parameters' types
> in the file, and they are all uint32_t, so I finally decided on uint32_t.
>
>>> + unsigned int space,
>>> + uint16_t size,
>>
>> ... this one to be unsigned int, ...
>
> In the xen_add_to_physmap_batch struct, both @space and @size are
> uint16_t, so I think
> I should have made @space uint16_t, too. I'll fix this. Or do you have
> any good reasons to
> make both of them unsigned int?
>
>>
>>> + xen_ulong_t *idxs,
>>> + xen_pfn_t *gfpns,
>>
>> ... and these two to be pointers to const.
>>
>
> Yes, indeed. Sorry for this.
>
But once again I didn't see any [IN] buffers that are made const in
the parameter
lists in the whole file.
Zhongze Liu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
2017-06-16 9:44 ` Zhongze Liu
@ 2017-06-16 10:07 ` Zhongze Liu
0 siblings, 0 replies; 7+ messages in thread
From: Zhongze Liu @ 2017-06-16 10:07 UTC (permalink / raw)
To: Jan Beulich; +Cc: Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel
2017-06-16 17:44 GMT+08:00 Zhongze Liu <blackskygg@gmail.com>:
> 2017-06-16 17:36 GMT+08:00 Zhongze Liu <blackskygg@gmail.com>:
>> Hi Jan,
>>
>>
>> 2017-06-16 16:45 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>>> On 16.06.17 at 06:55, <blackskygg@gmail.com> wrote:
>>>> currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc.
>>>> add a wrapper to do that.
>>>
>>> It may help acceptance if you say why all of the sudden a wrapper
>>> is needed.
>>>
>>
>> It's indeed a preparation for my GSoC project:
>> https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
>>
>> Thanks for the suggestion.
>>
>>>
>>>> --- a/tools/libxc/include/xenctrl.h
>>>> +++ b/tools/libxc/include/xenctrl.h
>>>> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
>>>> unsigned long idx,
>>>> xen_pfn_t gpfn);
>>>>
>>>> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
>>>> + uint32_t domid,
>>>> + uint32_t foreign_domid,
>>>
>>> I'm not exactly sure what the libxc coding rules are, but I'd expect
>>> these both to be domid_t, ...
>>>
>>
>> I was planning to make them domid_t, but according to the other
>> domid-parameters' types
>> in the file, and they are all uint32_t, so I finally decided on uint32_t.
>>
>>>> + unsigned int space,
>>>> + uint16_t size,
>>>
>>> ... this one to be unsigned int, ...
>>
>> In the xen_add_to_physmap_batch struct, both @space and @size are
>> uint16_t, so I think
>> I should have made @space uint16_t, too. I'll fix this. Or do you have
>> any good reasons to
>> make both of them unsigned int?
>>
>>>
>>>> + xen_ulong_t *idxs,
>>>> + xen_pfn_t *gfpns,
>>>
>>> ... and these two to be pointers to const.
>>>
>>
>> Yes, indeed. Sorry for this.
>>
>
> But once again I didn't see any [IN] buffers that are made const in
> the parameter
> lists in the whole file.
>
I guess this is probably because the hbuf and ubuf field of struct
xc_hypercall_buffer(
libxc/include/xenctrl.h) are not pointers to const. So we just can't
make a const-
buffer bounce.
Cheers,
Zhongze Liu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
2017-06-16 9:36 ` Zhongze Liu
2017-06-16 9:44 ` Zhongze Liu
@ 2017-06-16 10:33 ` Jan Beulich
2017-06-19 11:12 ` Wei Liu
2 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2017-06-16 10:33 UTC (permalink / raw)
To: Zhongze Liu; +Cc: Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel
>>> On 16.06.17 at 11:36, <blackskygg@gmail.com> wrote:
> 2017-06-16 16:45 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>> On 16.06.17 at 06:55, <blackskygg@gmail.com> wrote:
>>> --- a/tools/libxc/include/xenctrl.h
>>> +++ b/tools/libxc/include/xenctrl.h
>>> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
>>> unsigned long idx,
>>> xen_pfn_t gpfn);
>>>
>>> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
>>> + uint32_t domid,
>>> + uint32_t foreign_domid,
>>
>> I'm not exactly sure what the libxc coding rules are, but I'd expect
>> these both to be domid_t, ...
>>
>
> I was planning to make them domid_t, but according to the other
> domid-parameters' types
> in the file, and they are all uint32_t, so I finally decided on uint32_t.
You'll want to see what the maintainers of the code say, but my
general position on things like this is to try to avoid copying prior
mistakes.
>>> + unsigned int space,
>>> + uint16_t size,
>>
>> ... this one to be unsigned int, ...
>
> In the xen_add_to_physmap_batch struct, both @space and @size are
> uint16_t, so I think
> I should have made @space uint16_t, too. I'll fix this. Or do you have
> any good reasons to
> make both of them unsigned int?
Again, I'm not a maintainer of this code, but in the hypervisor we
view it the other way around: You need to have a good reason
to use fixed-width types.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch
2017-06-16 9:36 ` Zhongze Liu
2017-06-16 9:44 ` Zhongze Liu
2017-06-16 10:33 ` Jan Beulich
@ 2017-06-19 11:12 ` Wei Liu
2 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2017-06-19 11:12 UTC (permalink / raw)
To: Zhongze Liu
Cc: Ian Jackson, Stefano Stabellini, Wei Liu, Jan Beulich, xen-devel
On Fri, Jun 16, 2017 at 05:36:18PM +0800, Zhongze Liu wrote:
> Hi Jan,
>
>
> 2017-06-16 16:45 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> >>>> On 16.06.17 at 06:55, <blackskygg@gmail.com> wrote:
> >> currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc.
> >> add a wrapper to do that.
> >
> > It may help acceptance if you say why all of the sudden a wrapper
> > is needed.
> >
>
> It's indeed a preparation for my GSoC project:
> https://lists.xen.org/archives/html/xen-devel/2017-05/msg01288.html
>
> Thanks for the suggestion.
>
> >
> >> --- a/tools/libxc/include/xenctrl.h
> >> +++ b/tools/libxc/include/xenctrl.h
> >> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
> >> unsigned long idx,
> >> xen_pfn_t gpfn);
> >>
> >> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
> >> + uint32_t domid,
> >> + uint32_t foreign_domid,
> >
> > I'm not exactly sure what the libxc coding rules are, but I'd expect
> > these both to be domid_t, ...
> >
>
> I was planning to make them domid_t, but according to the other
> domid-parameters' types
> in the file, and they are all uint32_t, so I finally decided on uint32_t.
Both are used. We should use domid_t for new code.
>
> >> + unsigned int space,
> >> + uint16_t size,
> >
> > ... this one to be unsigned int, ...
>
> In the xen_add_to_physmap_batch struct, both @space and @size are
> uint16_t, so I think
> I should have made @space uint16_t, too. I'll fix this. Or do you have
> any good reasons to
> make both of them unsigned int?
>
I agree with what Jan said.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-19 11:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-16 4:55 [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch Zhongze Liu
2017-06-16 8:45 ` Jan Beulich
2017-06-16 9:36 ` Zhongze Liu
2017-06-16 9:44 ` Zhongze Liu
2017-06-16 10:07 ` Zhongze Liu
2017-06-16 10:33 ` Jan Beulich
2017-06-19 11:12 ` Wei Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.