libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch

PATCH Superseded Zhongze Liu 3 participants 7 messages Jun 16, 2017 – Jun 19, 2017

lore

Share & tools

lore

Zhongze Liu

author

2017-06-16 04:57 UTC #0 lore raw

PATCH 1/1 libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch

currently there is no wrapper for XENMEM_add_to_physmap_batch in libxc. add a wrapper to do that.

Signed-off-by Zhongze Liu <[email protected]>

---
Cc: Ian Jackson <[email protected]>,
Cc: Wei Liu <[email protected]>,
Cc: Stefano Stabellini <[email protected]>

---
 tools/libxc/include/xenctrl.h |  9 +++++++++
 tools/libxc/xc_domain.c       | 44 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

tools/libxc/include/xenctrl.h +9 -0

index 1629f412dd..4ea520b188 100644
@@ -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,

tools/libxc/xc_domain.c +44 -0

50 lines not on this page · raw message

Signature · 7 lines

2.13.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Jan Beulich

2017-06-16 08:45 UTC #1 lore raw

Quoted 3 lines

Quoted 1 line
Quoted 1 line · On 16.06.17 at 06:55, <[email protected]> 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.

tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1377

Quoted 9 lines

tools/libxc/include/xenctrl.h +3 -0

@@ -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, ...

tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1379

Quoted 2 lines

tools/libxc/include/xenctrl.h +2 -0

+                                   unsigned int space,
+                                   uint16_t size,

... this one to be unsigned int, ...

tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1381

Quoted 2 lines

tools/libxc/include/xenctrl.h +2 -0

+                                   xen_ulong_t *idxs,
+                                   xen_pfn_t *gfpns,

... and these two to be pointers to const.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Zhongze Liu

author

2017-06-16 09:36 UTC #2 lore raw

Hi Jan,

2017-06-16 16:45 GMT+08:00 Jan Beulich <[email protected]>:

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 16.06.17 at 06:55, <[email protected]> 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.

Quoted 7 lines

tools/libxc/include/xenctrl.h +0 -0

@@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
                              unsigned long idx,
                              xen_pfn_t gpfn);

Quoted 7 lines

tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1377
Quoted 3 lines
tools/libxc/include/xenctrl.h +3 -0
+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.

Quoted 4 lines

tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1379
Quoted 2 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   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?

Quoted 6 lines

tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1381
Quoted 3 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   xen_ulong_t *idxs,
+                                   xen_pfn_t *gfpns,
... and these two to be pointers to const.

Yes, indeed. Sorry for this.

Quoted 3 lines

Jan

Cheers,

Zhongze Liu

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Zhongze Liu

author

2017-06-16 09:44 UTC #3 lore raw

2017-06-16 17:36 GMT+08:00 Zhongze Liu <[email protected]>:

Quoted 48 lines

Hi Jan,
2017-06-16 16:45 GMT+08:00 Jan Beulich <[email protected]>:
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 16.06.17 at 06:55, <[email protected]> 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.
Quoted 7 lines
Quoted 7 lines
tools/libxc/include/xenctrl.h +0 -0
@@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
                              unsigned long idx,
                              xen_pfn_t gpfn);
 
Quoted 7 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1377
Quoted 3 lines
tools/libxc/include/xenctrl.h +3 -0
+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.
Quoted 4 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1379
Quoted 2 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   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?
Quoted 1 line

Quoted 8 lines

Quoted 5 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1381
Quoted 2 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   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
[email protected]
https://lists.xen.org/xen-devel

Zhongze Liu

author

2017-06-16 10:07 UTC #4 lore raw

2017-06-16 17:44 GMT+08:00 Zhongze Liu <[email protected]>:

Quoted 55 lines

2017-06-16 17:36 GMT+08:00 Zhongze Liu <[email protected]>:
Quoted 54 lines
Hi Jan,
2017-06-16 16:45 GMT+08:00 Jan Beulich <[email protected]>:
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 16.06.17 at 06:55, <[email protected]> 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.
Quoted 7 lines
Quoted 7 lines
tools/libxc/include/xenctrl.h +0 -0
@@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
                              unsigned long idx,
                              xen_pfn_t gpfn);
 
Quoted 7 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1377
Quoted 3 lines
tools/libxc/include/xenctrl.h +3 -0
+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.
Quoted 4 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1379
Quoted 2 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   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?
Quoted 6 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1381
Quoted 3 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   xen_ulong_t *idxs,
+                                   xen_pfn_t *gfpns,
... and these two to be pointers to const.

Quoted 7 lines

Quoted 2 lines
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
[email protected]
https://lists.xen.org/xen-devel

Jan Beulich

2017-06-16 10:33 UTC #5 lore raw

In reply to Zhongze Liu

Quoted 17 lines

Quoted 1 line
Quoted 1 line · On 16.06.17 at 11:36, <[email protected]> wrote:
2017-06-16 16:45 GMT+08:00 Jan Beulich <[email protected]>:
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line · On 16.06.17 at 06:55, <[email protected]> wrote:
tools/libxc/include/xenctrl.h +0 -0
@@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
                              unsigned long idx,
                              xen_pfn_t gpfn);
 
Quoted 7 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1377
Quoted 3 lines
tools/libxc/include/xenctrl.h +3 -0
+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, ...

Quoted 3 lines

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.

Quoted 10 lines

Quoted 4 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1379
Quoted 2 lines
tools/libxc/include/xenctrl.h +2 -0
+                                   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
[email protected]
https://lists.xen.org/xen-devel

Wei Liu

2017-06-19 11:12 UTC #6 lore raw

In reply to Zhongze Liu

Quoted 32 lines · Zhongze Liu

Hi Jan,
2017-06-16 16:45 GMT+08:00 Jan Beulich <[email protected]>:
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line · On 16.06.17 at 06:55, <[email protected]> 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.
Quoted 7 lines
Quoted 7 lines
tools/libxc/include/xenctrl.h +0 -0
@@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
                              unsigned long idx,
                              xen_pfn_t gpfn);
 
Quoted 7 lines
tools/libxc/include/xenctrl.h int xc_domain_add_to_physmap(xc_interface *xch, · line 1377
Quoted 3 lines
tools/libxc/include/xenctrl.h +3 -0
+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, ...

Quoted 3 lines

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.

Quoted 4 lines

Quoted 3 lines

tools/libxc/include/xenctrl.h +2 -0

+                                   unsigned int space,
+                                   uint16_t size,

Quoted 8 lines

Quoted 1 line
... 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
[email protected]
https://lists.xen.org/xen-devel