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

Re: [PATCH] x86/PV32: fix physdev_op_compat handling

  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 11 Oct 2021 08:22:18 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3cfSuhUjCR2LRhze5ASoTxf+xtQqvwf8VHRb+u3awzE=; b=Zhzr3kP3JJ0jdk4QDEYvaVzDujS03HBLiywOQYbOFWD4Nvt9OuyH9AdQG3869Um4Kh4JujCoWWRFyR59RYHuWI7WJqThGEG5i8CeL55OVxcg3PB71RQ1SR5tU1barcg56RNs18aj+iKrcryHDq+QWEUlFTEUx/uBxqvO/6IwQsfvMMcVOaNKLb/ww4c9IH5m/63oDZy11sfrOp6Mr4FlZ2TTAD0KGWSbNrEc7dnER382tbFvNuvmADDnN1HHVnj8ZWFZdIatHAEnbdUh3JWgG+kB+f+3m9sFBBW4UnfcHEpqMk11NfvFXkaMCUM8v9/h5ERqF9evOnqOaQ3JtVJ94A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=evgmQLC37Uo8GefsZn/HC0EDj4kiWJyACd9Kts0etg0+VSDKjFKGIawVOOi9O/Gks4HnJIV0JUHp5QHayiMkmILajbP1lOlOlxBpqN+DJkztL5Eo8UHSoUMWAqi1jHKABpSNwubC4Jh7NUYSUzOKRn+tqUhrpS9sSCbbdVhFaHlbRI6EprSYWyQDkTwRh10yGUHv/Bk+M8jHXjf8YDgKu+rs6c6u4jMbEk0b9DnLc9gx/S5GeI1HeSlOgCpC3n9xxDVitqd9W89Qy8FIkILEoErVExJ9dIxvZrZc/sqylbSD2a1patobxuGloUAPasp09OlSt4quBxA/ofp7XH5tmQ==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 11 Oct 2021 06:22:27 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 08.10.2021 19:47, Andrew Cooper wrote:
> On 08/10/2021 11:47, Jan Beulich wrote:
>> The conversion of the original code failed to recognize that the 32-bit
>> compat variant of this (sorry, two different meanings of "compat" here)
>> needs to continue to invoke the compat handler, not the native one.
>> Arrange for this and also remove the one #define that hasn't been
>> necessary anymore as of that change.
>> Affected functions (having existed prior to the introduction of the new
>> hypercall) are PHYSDEVOP_set_iobitmap and PHYSDEVOP_apic_{read,write}.
>> For all others the operand struct layout doesn't differ.
> :-/
> Neither of those ABI breakages would be subtle.  But why didn't XTF
> notice?  Edit: It appears as if my PHYSDEVOP_set_iobitmap tests never
> got completed.

But the XTF would have used the modern hypercall, wouldn't it? At
least the pv-iopl test does.

>> Additionally the XSA-344 fix causes guest register corruption afaict,
>> when EVTCHNOP_reset gets called through the compat function and needs a
>> continuation. While guests shouldn't invoke that function this way, I
>> think we would better have forced all pre-3.2-unavailable functions into
>> an error path, rather than forwarding them to the actual handler. I'm
>> not sure though how relevant we consider it to fix this (one way or
>> another).
> EVTCHNOP_reset{,_cont} are -ENOSYS'd in do_event_channel_op_compat()
> without being forwarded.  I can't see a problem.

You're right - I think I did look at do_physdev_op_compat() deriving
evtchn-compat behavior from there as well.

>> --- a/xen/arch/x86/x86_64/compat.c
>> +++ b/xen/arch/x86/x86_64/compat.c
>> @@ -10,8 +10,8 @@ EMIT_FILE;
>>  #define physdev_op                    compat_physdev_op
>>  #define physdev_op_t                  physdev_op_compat_t
>> -#define do_physdev_op                 compat_physdev_op
> This is still needed, technically.  It impacts the typeof() expression:
> typeof(do_physdev_op) *fn =
>     (void *)pv_hypercall_table[__HYPERVISOR_physdev_op].native;
> and the reason why everything compiles is because
> {do,compat}_physdev_op() have identical types.

Oh, indeed - thanks for pointing out.




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