[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen master] xen/version: Drop bogus return values for XENVER_platform_parameters



commit 273bde319fe039083a90f1aae211815d90a0a36e
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Tue Jan 3 13:45:48 2023 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Fri Jan 20 19:39:32 2023 +0000

    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@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 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 4f268d83e3..f7b1f65f37 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 0ff8bd9077..cbc4ef7a46 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;
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.