[PATCH v2 1/5] xen/version: Drop bogus return values for XENVER_platform_parameters

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Julien Grall <julien@xxxxxxx>

The only reason this does not get an XSA is because Xen does not have any form
of KALSR.

v2:
 * Retain the useless return value for 64bit PV guests in release builds only.
 * Rewrite comments.
---
 xen/common/kernel.c          | 20 +++++++++++++++++---
 xen/include/public/version.h | 27 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 4f268d83e3cb..f7b1f65f373c 100644
--- 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 0ff8bd9077c6..cbc4ef7a46e6 100644
--- 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;
-- 
2.11.0