xen/version: Drop bogus return values for XENVER_platform_parameters

A split in virtual address space is only applicable for x86 PV guests.
Furthermore, the information returned for x86 64bit PV guests is wrong.

Explain the problem in version.h, stating the other information that PV guests
need to know.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 4f268d83e3cb41ddea7409a2cc3e146004deef59..f7b1f65f373c5eb6fb99976fe9fa20a9db5cc7cc 100644 (file)
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -518,11 +518,15 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     
     case XENVER_platform_parameters:
     {
+        const struct vcpu *curr = current;
+
 #ifdef CONFIG_COMPAT
-        if ( current->hcall_compat )
+        if ( curr->hcall_compat )
         {
             compat_platform_parameters_t params = {
-                .virt_start = HYPERVISOR_COMPAT_VIRT_START(current->domain),
+                .virt_start = is_pv_vcpu(curr)
+                            ? HYPERVISOR_COMPAT_VIRT_START(curr->domain)
+                            : 0,
             };
 
             if ( copy_to_guest(arg, &params, 1) )
@@ -532,7 +536,17 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 #endif
         {
             xen_platform_parameters_t params = {
-                .virt_start = HYPERVISOR_VIRT_START,
+                /*
+                 * Out of an abundance of caution, retain the useless return
+                 * value for 64bit PV guests, but in release builds only.
+                 *
+                 * This is not expected to cause any problems, but if it does,
+                 * the developer impacted will be the one best suited to fix
+                 * the caller not to issue this hypercall.
+                 */
+                .virt_start = !IS_ENABLED(CONFIG_DEBUG) && is_pv_vcpu(curr)
+                              ? HYPERVISOR_VIRT_START
+                              : 0,
             };
 
             if ( copy_to_guest(arg, &params, 1) )

diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 0ff8bd9077c63de05cc874f114e15f465aaad24b..cbc4ef7a46e6f1fc999e3a4b6496d8fce9814a9e 100644 (file)
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -42,6 +42,33 @@ typedef char xen_capabilities_info_t[1024];
 typedef char xen_changeset_info_t[64];
 #define XEN_CHANGESET_INFO_LEN (sizeof(xen_changeset_info_t))
 
+/*
+ * This API is problematic.
+ *
+ * It is only applicable to guests which share pagetables with Xen (x86 PV
+ * guests), but unfortunately has leaked into other guest types and
+ * architectures with an expectation of never failing.
+ *
+ * It is intended to identify the virtual address split between guest kernel
+ * and Xen.
+ *
+ * For 32bit PV guests, there is a split, and it is variable (between two
+ * fixed bounds), and this boundary is reported to guests.  The detail missing
+ * from the hypercall is that the second boundary is the 32bit architectural
+ * boundary at 4G.
+ *
+ * For 64bit PV guests, Xen lives at the bottom of the upper canonical range.
+ * This hypercall happens to report the architectural boundary, not the one
+ * which would be necessary to make a variable split work.  As such, this
+ * hypercall entirely useless for 64bit PV guests, and all inspected
+ * implementations at the time of writing were found to have compile time
+ * expectations about the split.
+ *
+ * For architectures where this hypercall is implemented, for backwards
+ * compatibility with the expectation of the hypercall never failing Xen will
+ * return 0 instead of failing with -ENOSYS in cases where the guest should
+ * not be making the hypercall.
+ */
 #define XENVER_platform_parameters 5
 struct xen_platform_parameters {
     xen_ulong_t virt_start;