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

Re: [PATCH v2 5/7] x86/hvm: Use __initdata_cf_clobber for hvm_funcs


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Mon, 14 Feb 2022 16:39:35 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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=TTsxX9IjVpKowN5pxW2wJvRpTcecF/0IS8QbS5eubNg=; b=SNVdQkEvnEceukvmJ3D9/eMnbU+yGurt60cn17DE8v1qF6X9rxWSaXGaawYzZ3VTbzN5vqXP7Z4TD4e57SZ5jkAT8Fo3DuQQU99LXoso8n7T8wJuMXak/8Xyhas916w477AKsEjKDva8yrERT/l1rNCChsko1i1rxiCXJz49VTqz0814PjMdtD7JrCB7KWVQDZkQm0QF1O0YzPxjhrzMsowgKx3lvYa/2LRqohL9cVvctNDUfAYCdbRtwmxEakwiM4fLhsBn3ttNTo/zL6Ulp95Y/PW4QUTGUXxpRc9k9sIINh0Dkr71S/Bf+Y1xuX0uWN/Ltbum3Xn2fLON8gQpkQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LFDt8Fmjzv7BMI5uFSOH+tMoIrYjh3lIRMUI/HslU0WdxD85shbkcc8nDOZFclGQrLVt958HZkfXRRF/wqJj/MhQo8PSUsPwJFgHo6/wzsfRJ7JIiFchVmstATqqadlHqSrME8GMzuU8mkVPZH8HK1gBnWI4wyeYtrksRmT2WWBKw8jfgSr7KxBim+M81UmzJfIOeet3oOZKajqz/tRBYAQW1EPOAeRm9SzM6j5DF+Hnd6sDjCKKiCEB9fAtMJC5uaoztCuTl6sZv1XuFk0mPPdi1o9bgqX0A/iCU4vD3ByKrKWQcAenJDvBBUvftSMV0tHToVwrkgf89rMPPNPzQA==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 14 Feb 2022 16:39:59 +0000
  • Ironport-data: A9a23:jPHy5qL2osV6lVYgFE+RBZIlxSXFcZb7ZxGr2PjKsXjdYENSgzxTm mUZW2rTaK2LNzT9c4xxa4zioEJX65PVmIVnGwJlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokcxIn5BC5C5xZVG/fjgqoHUVaiUakideSc+EH170Ug6x7Zj6mJVqYPR7z2l6 IuaT/L3YDdJ6xYsWo7Dw/vewP/HlK2aVAIw5jTSV9gS1LPtvyB94KYkDbOwNxPFrrx8RYZWc QphIIaRpQs19z91Yj+sfy2SnkciGtY+NiDW4pZatjTLbrGvaUXe345iXMfwZ3u7hB3Rvdop5 /BSpKC7YgApF7DdnekdWTdHRnQW0a1uoNcrIFC6uM2XiUbHb2Ht07NlC0Re0Y8wo7gtRzsUr LpBdW5LPkvra+GemdpXTsFFgMg5IdatF4QYonx6lhnSDOo8QICFSKLPjTNd9Glu3ZERTaeGD yYfQTNucjrtbS1dBlkWEIM6jL2Sn2DjeRQN/Tp5ooJoujOOnWSdyoPFINfTP9CHW8hRtkKZv X7duXT0BAkAM96SwibD9Wij7sfQmQvrVYRUE6e3ntZoilCOwm0YCDUNSEC25/K+jyaDt8l3c hJOvHB09O5rqRLtHoKVswCETGCs5jwWA/V1DPwG4yqKyoPJ8gOUBmIqUWsUADA5j/MeSTsv3 16PutrmAz1zrbGYIU6gGqeoQSCaYnZMczJbDcMQZU5cuoS4/tlv5v7aZos7SMaIYsvJ9SYcK txghAw3nP0tgMECzM1XFniX0mv39vAlouPYjzg7v15JDCskPuZJhKTysDA3CMqsy67DEDG8U IAswZT20Qz3JcjleNaxaOsMBqq1wP2OLSfRh1Vid7F4qWjxpC7zIN0IuWokTKuMDirjUWW3C HI/RCsLvMMDVJdURfMfj32N5zQCkvG7SIWNugH8ZdtSeJlhHDJrDwk1DXN8K1vFyRB2+YlmY M/zWZ/1UR4yVPQ2pBLrFrx1+eJ6mUgDKZb7GMmTI+KPiuHFOhZ4iN4tbTOzUwzOxP3Y8VuFq 44Fb6NnCXx3CYXDX8UeyqZKRXgiJnknH5Hm7ctRc++IOA19H289TfTWxNscl0ZNxsy5T8/Eo SOwXFF20l36iSGVIAmGcCk7OrjuQYx+vTQwOil1ZQSk3H0qYICO6qYDdsRoIel7pbI7lfMkH eMYf8igA+hUTmiV8ToqcpSg/pdpcw6mhFzSMnP9MiQ/ZZNpWyfA5sTgIln07CALAyfu7Zk+r rSs2xn1W50GQwg+Xs/aZOj2lwG6vGQHmfI0VEzNe4EBdELp+YlsCirwkv5ofJ1cdUSdnmOXj l/EDw0ZqO/Bp54O3OPI3a3U/Z20F+ZeH1ZBGzWJ57iBKiSHrHGoxpVNUbjUcGmFBn/04qire c5c0+r4bK8chF9PvodxT+RrwKY564e9rrNW1F05TnDCblDtAbJ8OHiWm8JIs/QVlLNevAK3X GOJ+8VbZurVaJ+0TgZJKVp3dPmH2NEVhiLWvKY8L0jN7SNq+KaKDBdJNB6WhS0BdLZ4PevJG wv6VBL6P+BnticXDw==
  • Ironport-hdrordr: A9a23:M/xzJaE7qbroUZvdpLqFTJHXdLJyesId70hD6qkvc3Nom52j+/ xGws536fatskdtZJkh8erwXZVp2RvnhNBICPoqTMuftW7dySqVxeBZnMTfKljbdREWmdQtrJ uIH5IOa+EYSGIK9/oSgzPIU+rIouP3iJxA7N22pxwGLGFXguNbnnxE426gYxdLrWJ9dP4E/e +nl6x6Tk2bCBMqh6qAdxs4dtmGg+eOuIPtYBYACRJiwhKJlymU5LnzFAXd9gsCUhtUqI1Ssl Ttokjc3OGOovu7whjT2yv49JJNgubszdNFGYilltUVEDPxkQylDb4RGYFq/QpF5d1H2mxa1+ UkkC1QefibLEmhJ11dlCGdnzUIFgxes0MKh2Xo2kcL6vaJOw7SQ/Ax+76xNCGptnbI9esMoJ 6ilQiixutqJAKFkyLn69fSURZ20kKyvHo5iOYWy2dSSI0EddZq3MYiFW5uYd899RjBmcsa+S hVfbXhzecTdUnfY2HSv2FpztDpVnMvHg2eSkxHvsCOyTBZkH1w0kNdnaUk7zs93YN4T4MB6/ XPM6xumr0LRsgKbbhlDONERcesEGTCTR/FLWrXK1X6E6MMPW7LtvfMkfgIzfDvfIZNwIo5mZ zHXl8dvWkue1j2AcnLx5FP+gClehT1Yd0s8LAp23FUgMyPeFPbC1z1dLl1qbrSnxw2OLyvZ8 qO
  • Ironport-sdr: ofsXPqblO46VrdGotSxv10KtYFzqIZJJxi2XD8ZDKELxFl/kLg5DiBmrmX4w0GeLsV11A6V6Hn P2JEWuFRDNmkLqG6zfCOIZkptajK/pwU2m5HrjboBmnbuydCw1ANH5UmOKElV7RMyIlkoWzb62 IVK+kPfZXsZLipb/HAC537cctrrd0V3Acm0Gv1YHrEsTBI6E2PmnPvybr2uEmUzHgiWjvVNyFi nN06SKpU5vPGtQoHdytOLhcTpL4YKiP5+XLZsOrNufUmbmCZh+tRY4l/UGGktnVEiG93dvDVi1 HiwZ5RCfQqE/pYZOHz5aziVK
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYIaJjsOAx5ctsj06lom2YGua3HKyTBU+AgAAG24CAADN0gA==
  • Thread-topic: [PATCH v2 5/7] x86/hvm: Use __initdata_cf_clobber for hvm_funcs

On 14/02/2022 13:35, Andrew Cooper wrote:
> On 14/02/2022 13:10, Jan Beulich wrote:
>> On 14.02.2022 13:56, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -88,7 +88,7 @@ unsigned int opt_hvm_debug_level __read_mostly;
>>>  integer_param("hvm_debug", opt_hvm_debug_level);
>>>  #endif
>>>  
>>> -struct hvm_function_table hvm_funcs __read_mostly;
>>> +struct hvm_function_table __ro_after_init hvm_funcs;
>> Strictly speaking this is an unrelated change. I'm fine with it living here,
>> but half a sentence would be nice in the description.
> I could split it out, but we could probably make 200 patches of
> "sprinkle some __ro_after_init around, now that it exists".
>
>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>> @@ -2513,7 +2513,7 @@ static void cf_check svm_set_reg(struct vcpu *v, 
>>> unsigned int reg, uint64_t val)
>>>      }
>>>  }
>>>  
>>> -static struct hvm_function_table __initdata svm_function_table = {
>>> +static struct hvm_function_table __initdata_cf_clobber svm_function_table 
>>> = {
>>>      .name                 = "SVM",
>>>      .cpu_up_prepare       = svm_cpu_up_prepare,
>>>      .cpu_dead             = svm_cpu_dead,
>>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>> index 41db538a9e3d..758df3321884 100644
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -2473,7 +2473,7 @@ static void cf_check vmx_set_reg(struct vcpu *v, 
>>> unsigned int reg, uint64_t val)
>>>      vmx_vmcs_exit(v);
>>>  }
>>>  
>>> -static struct hvm_function_table __initdata vmx_function_table = {
>>> +static struct hvm_function_table __initdata_cf_clobber vmx_function_table 
>>> = {
>>>      .name                 = "VMX",
>>>      .cpu_up_prepare       = vmx_cpu_up_prepare,
>>>      .cpu_dead             = vmx_cpu_dead,
>> While I'd like to re-raise my concern regarding the non-pointer fields
>> in these structure instances (just consider a sequence of enough bool
>> bitfields, which effectively can express any value, including ones
>> which would appear like pointers into .text), since for now all is okay
>> afaict:
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> I should probably put something in the commit message too.  It is a
> theoretical risk, but not (IMO) a practical one.

Updated commit message:

x86/hvm: Use __initdata_cf_clobber for hvm_funcs

Now that all calls through hvm_funcs are fully altcall'd, harden all the svm
and vmx function pointer targets.  This drops 106 endbr64 instructions.

Clobbering does come with a theoretical risk.  The non-pointer fields of
{svm,vmx}_function_table can in theory happen to form a bit pattern
matching a
pointer into .text at a legal endbr64 instruction, but this is expected
to be
implausible for anything liable to pass code review.

While at it, move hvm_funcs into __ro_after_init now that this exists.

~Andrew

 


Rackspace

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