Re: [PATCH 4/4] xen: Add V4V implementation

[Jean Guyader <jean.guyader@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

On 4 October 2012 16:12, Tim Deegan <tim@xen.org> wrote:
> At 16:03 +0100 on 04 Oct (1349366589), Jean Guyader wrote:
>> On 4 October 2012 13:11, Jan Beulich <JBeulich@suse.com> wrote:
>> >>>> On 04.10.12 at 14:03, Jean Guyader <jean.guyader@gmail.com> wrote:
>> >> On 20 September 2012 13:20, Jan Beulich <JBeulich@suse.com> wrote:
>> >>>>>> On 20.09.12 at 13:42, Jean Guyader <jean.guyader@citrix.com> wrote:
>> >>>>+        case V4VOP_register_ring:
>> >>>>+            {
>> >>>>+                XEN_GUEST_HANDLE(v4v_ring_t) ring_hnd =
>> >>>>+                    guest_handle_cast(arg1, v4v_ring_t);
>> >>>>+                XEN_GUEST_HANDLE(xen_pfn_t) pfn_hnd =
>> >>>>+                    guest_handle_cast(arg2, xen_pfn_t);
>> >>>>+                uint32_t npage = arg3;
>> >>>>+                if ( unlikely(!guest_handle_okay(ring_hnd, 1)) )
>> >>>>+                    goto out;
>> >>>>+                if ( unlikely(!guest_handle_okay(pfn_hnd, npage)) )
>> >>>>+                    goto out;
>> >>>
>> >>> Here and below - this isn't sufficient for compat guests, or else
>> >>> you give them a way to point into the compat m2p table.
>> >>>
>> >>
>> >> I'll probably switch to uint64_t for the v4v mfn list instead of using
>> >> xen_pfn_t which
>> >> are unsigned long. That way I can save the need for a compat wrapper.
>> >
>> > But that comment of yours doesn't address the problem I pointed
>> > out.
>> >
>>
>> [Resent, CCing everyone this time]
>>
>> I'm sorry, I don't really get what you mean them. I've tried to get
>> all my struct
>> layout such as all the offset for the field are the same for 64b and 32b, this
>> way I thought I could get away with doing a compat wrapper.
>
> Even if the args don't need translation, compat-mode guests have
> different VA layouts and need different range checks (though I'm not
> sure why these aren't automatically adjusted based on current).
>
> AIUI you need to use compat_handle_okay() instead of guest_handle_okay()
> to check the handles if is_pv_32on64_domain(current).
>

How about something like that?

Jean

[-- Attachment #2: guest_handle_okay_maybe_compat.patch --]
[-- Type: application/octet-stream, Size: 1085 bytes --]

diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h
index e3ac1d6..596d8a0 100644
--- a/xen/include/asm-x86/guest_access.h
+++ b/xen/include/asm-x86/guest_access.h
@@ -104,9 +104,12 @@
  * Pre-validate a guest handle.
  * Allows use of faster __copy_* functions.
  */
-#define guest_handle_okay(hnd, nr)                      \
-    (paging_mode_external(current->domain) ||           \
-     array_access_ok((hnd).p, (nr), sizeof(*(hnd).p)))
+#define guest_handle_okay(hnd, nr)                                      \
+    (is_pv_32on64_domain(current->domain) &&                            \
+     compat_array_access_ok((hnd).p, (nr), sizeof(*(hnd).p))) ||        \
+    (!is_pv_32on64_domain(current->domain) &&                           \
+     (paging_mode_external(current->domain) ||                          \
+      array_access_ok((hnd).p, (nr), sizeof(*(hnd).p))))
 #define guest_handle_subrange_okay(hnd, first, last)    \
     (paging_mode_external(current->domain) ||           \
      array_access_ok((hnd).p + (first),                 \

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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