All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.