Mailing List Archive

[PATCH 2/4] docs/html/: Annotations for two hypercalls
Add annotations for a couple of the hypercalls:
HYPERVISOR_set_trap_table
HYPERVISOR_mmu_update

We do this by
* annotating the list of #defines for hypercall numbers
* annotating the list of error values
* providing a function prototype for the systematically-named functions
The header generator does the rest.

This exercise revealed a couple of infelicities:
* In the actual source code, do_mmu_update is defined to return an int
and do_set_trap_table a long. However both functions return either
-Efoo (on error) or 0 for success.
* The error numbers are defined only in the private header file
xen/include/xen/errno.h and then only with names which will
typically clash with other projects. It would be nice to include a
public version of this header which defines XEN_E*. But for now
we run xen-headers on errno.h too.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
docs/Makefile | 2 +-
xen/include/public/arch-x86/xen.h | 6 ++++++
xen/include/public/xen.h | 11 +++++++++--
xen/include/xen/errno.h | 5 +++++
4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/docs/Makefile b/docs/Makefile
index ad315cf..a2bbf2d 100644
--- a/docs/Makefile
+++ b/docs/Makefile
@@ -134,7 +134,7 @@ html/hypercall/stamp:
./xen-headers -O $(@D) \
-T 'arch-x86_64 - Xen public headers' \
-X arch-ia64 -X arch-x86_32 -X xen-x86_32 \
- ../xen include/public
+ ../xen include/public include/xen/errno.h
touch $@

txt/%.txt: %.txt
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 79ec633..d663cef 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -82,7 +82,13 @@ typedef unsigned long xen_pfn_t;
typedef unsigned long xen_ulong_t;

/*
+ * ` enum neg_errnoval
+ * ` HYPERVISOR_set_trap_table(const struct trap_info traps[]);
+ * `
+ */
+/*
* Send an array of these to HYPERVISOR_set_trap_table().
+ * Terminate the array with a sentinel entry, with traps[].address==0.
* The privilege level specifies which modes may enter a trap via a software
* interrupt. On x86/64, since rings 1 and 2 are unavailable, we allocate
* privilege levels as follows:
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index fde9fa5..f2c9e6f 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -55,6 +55,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
* HYPERCALLS
*/

+/* ` enum hypercall_num { // __HYPERVISOR_* => HYPERVISOR_*() */
+
#define __HYPERVISOR_set_trap_table 0
#define __HYPERVISOR_mmu_update 1
#define __HYPERVISOR_set_gdt 2
@@ -105,6 +107,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
#define __HYPERVISOR_arch_6 54
#define __HYPERVISOR_arch_7 55

+/* ` } */
+
/*
* HYPERCALL COMPATIBILITY.
*/
@@ -163,8 +167,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
#define NR_VIRQS 24

/*
- * HYPERVISOR_mmu_update(reqs, count, pdone, foreigndom)
- *
+ * ` enum neg_errnoval
+ * ` HYPERVISOR_mmu_update(const struct mmu_update reqs[],
+ * ` unsigned count, unsigned *done_out,
+ * ` unsigned foreigndom)
+ * `
* @reqs is an array of mmu_update_t structures ((ptr, val) pairs).
* @count is the length of the above array.
* @pdone is an output parameter indicating number of completed operations
diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
index 7cf599f..39147be 100644
--- a/xen/include/xen/errno.h
+++ b/xen/include/xen/errno.h
@@ -1,6 +1,9 @@
#ifndef _I386_ERRNO_H
#define _I386_ERRNO_H

+/* ` enum neg_errnoval { [ -Efoo for each Efoo in the list below ] } */
+/* ` enum errnoval { */
+
#define EPERM 1 /* Operation not permitted */
#define ENOENT 2 /* No such file or directory */
#define ESRCH 3 /* No such process */
@@ -129,4 +132,6 @@
#define ENOMEDIUM 123 /* No medium found */
#define EMEDIUMTYPE 124 /* Wrong medium type */

+/* ` } */
+
#endif
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 2/4] docs/html/: Annotations for two hypercalls [ In reply to ]
On Tue, 2011-11-29 at 15:01 +0000, Ian Jackson wrote:
> Add annotations for a couple of the hypercalls:
> HYPERVISOR_set_trap_table
> HYPERVISOR_mmu_update
>
> We do this by
> * annotating the list of #defines for hypercall numbers
> * annotating the list of error values
> * providing a function prototype for the systematically-named functions
> The header generator does the rest.
>
> This exercise revealed a couple of infelicities:
> * In the actual source code, do_mmu_update is defined to return an int
> and do_set_trap_table a long. However both functions return either
> -Efoo (on error) or 0 for success.

Should that be fixed or would that do more harm than good?

> * The error numbers are defined only in the private header file
> xen/include/xen/errno.h and then only with names which will
> typically clash with other projects. It would be nice to include a
> public version of this header which defines XEN_E*. But for now
> we run xen-headers on errno.h too.
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
> docs/Makefile | 2 +-
> xen/include/public/arch-x86/xen.h | 6 ++++++
> xen/include/public/xen.h | 11 +++++++++--
> xen/include/xen/errno.h | 5 +++++
> 4 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/docs/Makefile b/docs/Makefile
> index ad315cf..a2bbf2d 100644
> --- a/docs/Makefile
> +++ b/docs/Makefile
> @@ -134,7 +134,7 @@ html/hypercall/stamp:
> ./xen-headers -O $(@D) \
> -T 'arch-x86_64 - Xen public headers' \
> -X arch-ia64 -X arch-x86_32 -X xen-x86_32 \
> - ../xen include/public
> + ../xen include/public include/xen/errno.h
> touch $@
>
> txt/%.txt: %.txt
> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
> index 79ec633..d663cef 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -82,7 +82,13 @@ typedef unsigned long xen_pfn_t;
> typedef unsigned long xen_ulong_t;
>
> /*
> + * ` enum neg_errnoval
> + * ` HYPERVISOR_set_trap_table(const struct trap_info traps[]);
> + * `
> + */
> +/*
> * Send an array of these to HYPERVISOR_set_trap_table().
> + * Terminate the array with a sentinel entry, with traps[].address==0.
> * The privilege level specifies which modes may enter a trap via a software
> * interrupt. On x86/64, since rings 1 and 2 are unavailable, we allocate
> * privilege levels as follows:
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index fde9fa5..f2c9e6f 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -55,6 +55,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
> * HYPERCALLS
> */
>
> +/* ` enum hypercall_num { // __HYPERVISOR_* => HYPERVISOR_*() */
> +
> #define __HYPERVISOR_set_trap_table 0
> #define __HYPERVISOR_mmu_update 1
> #define __HYPERVISOR_set_gdt 2
> @@ -105,6 +107,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
> #define __HYPERVISOR_arch_6 54
> #define __HYPERVISOR_arch_7 55
>
> +/* ` } */
> +
> /*
> * HYPERCALL COMPATIBILITY.
> */
> @@ -163,8 +167,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
> #define NR_VIRQS 24
>
> /*
> - * HYPERVISOR_mmu_update(reqs, count, pdone, foreigndom)
> - *
> + * ` enum neg_errnoval
> + * ` HYPERVISOR_mmu_update(const struct mmu_update reqs[],
> + * ` unsigned count, unsigned *done_out,
> + * ` unsigned foreigndom)
> + * `
> * @reqs is an array of mmu_update_t structures ((ptr, val) pairs).
> * @count is the length of the above array.
> * @pdone is an output parameter indicating number of completed operations
> diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
> index 7cf599f..39147be 100644
> --- a/xen/include/xen/errno.h
> +++ b/xen/include/xen/errno.h
> @@ -1,6 +1,9 @@
> #ifndef _I386_ERRNO_H
> #define _I386_ERRNO_H
>
> +/* ` enum neg_errnoval { [ -Efoo for each Efoo in the list below ] } */
> +/* ` enum errnoval { */
> +
> #define EPERM 1 /* Operation not permitted */
> #define ENOENT 2 /* No such file or directory */
> #define ESRCH 3 /* No such process */
> @@ -129,4 +132,6 @@
> #define ENOMEDIUM 123 /* No medium found */
> #define EMEDIUMTYPE 124 /* Wrong medium type */
>
> +/* ` } */
> +
> #endif



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 2/4] docs/html/: Annotations for two hypercalls [ In reply to ]
Ian Campbell writes ("Re: [Xen-devel] [PATCH 2/4] docs/html/: Annotations for two hypercalls"):
> On Tue, 2011-11-29 at 15:01 +0000, Ian Jackson wrote:
> > This exercise revealed a couple of infelicities:
> > * In the actual source code, do_mmu_update is defined to return an int
> > and do_set_trap_table a long. However both functions return either
> > -Efoo (on error) or 0 for success.
>
> Should that be fixed or would that do more harm than good?

I wasn't sure whether I should go through all the hypercalls checking
and perhaps fixing up their return values.

> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 2/4] docs/html/: Annotations for two hypercalls [ In reply to ]
On Tue, 2011-11-29 at 15:29 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 2/4] docs/html/: Annotations for two hypercalls"):
> > On Tue, 2011-11-29 at 15:01 +0000, Ian Jackson wrote:
> > > This exercise revealed a couple of infelicities:
> > > * In the actual source code, do_mmu_update is defined to return an int
> > > and do_set_trap_table a long. However both functions return either
> > > -Efoo (on error) or 0 for success.
> >
> > Should that be fixed or would that do more harm than good?
>
> I wasn't sure whether I should go through all the hypercalls checking
> and perhaps fixing up their return values.

I'd definitely wait for Keir's OK before spending any time on it. Even
then it's likely to be a lot of fiddly work to convince yourself the
change is correct/safe and not an ABI wart we are stuck with...

>
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Thanks,
> Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel