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

Re: [PATCH] x86: harden use of calc_ler_msr()


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 30 May 2022 18:16:33 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=JwLUE4SfM5xfy2ULcIw9uUs7OnSu2cPnd1ruBooiV1c=; b=JX7qbqVMrhysNQsoUVcMIR4aiCDwtHIGIA+VveIu0Zz4kv3SsfRlYG2A/399D+27kfhsPuGzYzeObpu2MDPUjdevq5gAE2YPYEphKw2Jm9/A9i52TCktxUDubNDK72NLuy61rfQ87xLoel1+KxwGwZnkBRygKMXPqA7UOVZpqz7/zDgPwMMMOT+AYOSczTWYkctM7vnlQwNylYKEze4Hy0kjWBy9Lpj97C+Vc82lY5sKJ+WNVrkxXM/VrdksHe43UTjIDKJ+rW7X277mB0++FaiME4xKjevZ2/3mUyNBN0xdGin/2H9Z2aktIP1+O+czg7J7F00ZgxymciNA/Iaxtw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=H6RP9KjUyvzf4pViP0tc/p1VAv7n3JhuhpbZwvSHPATCUGDMorPxwG0ZAUnrZ8UCXE04/mpzoe8FD/rFRnGXf6Cz6mD7GZNjpApl+5be4QRfRwyJGHMPRBCLnKuf8SG7pnf49babkDd7lV+rWuFC0kBxFTu3he+BjE3IEI9dkBcMPldkK0fdGSxXcAKwV6QopDePewR8rKPCFtnvXs4RYqqBcshQb0L2DvCg9iKow1GVmVFYmWfg7XZ0aXjFqFgpXO3wBKiawvnWfBOKA8YzANOc3E5lu/0oPQ1blgVs+wHC+QdmnNTJOeRAx9744scwYsSyGnkb8vshfI+wYDKWdw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 30 May 2022 16:16:50 +0000
  • Ironport-data: A9a23:pvCfGK+OJ2btAoAR/4wmDrUD9X+TJUtcMsCJ2f8bNWPcYEJGY0x3z mBLD2zVbPeDMTH1LYsgOtvk9E8AvMDdy94ySlNurC48E34SpcT7XtnIdU2Y0wF+jyHgoOCLy +1EN7Es+ehtFie0Si+Fa+Sn9T8mvU2xbuKU5NTsY0idfic5DnZ44f5fs7Rh2NQw3IPhW1nlV e7a+KUzBnf0g1aYDUpMg06zgEsHUCPa4W5wUvQWPJinjXeG/5UnJMt3yZKZdhMUdrJ8DO+iL 9sv+Znilo/vE7XBPfv++lrzWhVirrc/pmFigFIOM0SpqkAqSiDfTs/XnRfTAKtao2zhojx/9 DlCnaatSj4vHIj9pMEcdSQESA1XJ6hho7CSdBBTseTLp6HHW13F5q0ySW0TY8gf8OsxBnxS/ /sFLjxLdgqEm++93LO8TK9rm9gnK87oeogYvxmMzxmAVapgHc+FHP6MuY4wMDQY36iiGd7EY MUUc3x3ZQnoaBxTIFYHTpk5mY9Eg1GgKWEF+ArJ/cLb5UCQkSxf4KDkaOHla8WYHOJpnmmfg 0D/qjGR7hYycYb3JSC+2nCmi/LLnCj7cJkPD7D+/flv6HWDy2pWBBAIWF+TpfiillX4S99ZM 1YT+Cclse417kPDZsb5dw21pjiDpBF0ZjZLO+gz6QXIxq+K5Q+cXjgAVmQZNI1gs9IqTzs30 FPPh8nuGTFkrLySTzSa66uQqjSxfyMSKAfueBM5cOfM2PG7yKlbs/4FZowL/HKd5jEtJQzN/ g==
  • Ironport-hdrordr: A9a23:FkCrNKF3vTm0acw1pLqFepHXdLJyesId70hD6qkvc3Fom52j/f xGws5x6faVslkssb8b6LW90Y27MAvhHPlOkPIs1NaZLXDbUQ6TQL2KgrGD/9SNIVycygcZ79 YbT0EcMqyOMbEZt7ec3ODQKb9Jrri6GeKT9IHjJh9WPH1XgspbnmNE42igYy9LrF4sP+tFKH PQ3LsPmxOQPVAsKuirDHgMWObO4/XNiZLdeBYDQzoq8hOHgz+E4KPzV0Hw5GZUbxp/hZMZtU TVmQ3w4auu99m91x/nzmfWq7BbgsHoxNdvDNGFzuIVNjLvoAC1Y5kJYczLgBkF5MWUrHo6mt jFpBkte+x19nPqZ2mw5SDg3gHxuQxen0PK+Bu9uz/OsMb5TDU1B45qnoRCaCbU7EImoZVVzL 9L93jxjesZMTrw2ADGo/TYXRBjkUS55VA4l/QIsnBZWYwCLJdMsI0k+l9PGptoJlO31GkeKp guMCjg3ocXTbvDBEqp/VWHgebcE0jbJy32DHTr4aeuonprdHMQ9Tps+CVQpAZEyHsHceg02w 31CNUXqFhwdL5nUUtcPpZ3fSLlMB26ffrzWFjiUmjPJeUgB0/njaLRzfEc2NyKEaZ4vqfa3q 6xGm9liQ==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, May 30, 2022 at 06:07:23PM +0200, Jan Beulich wrote:
> On 30.05.2022 17:58, Roger Pau Monné wrote:
> > On Mon, May 30, 2022 at 05:48:51PM +0200, Jan Beulich wrote:
> >> Avoid calling the function more than once, thus making sure we won't,
> >> under any unusual circumstances, attempt to enable XEN_LER late (which
> >> can't work, for setup_force_cpu_cap() being __init. In turn this then
> >> allows making the function itself __init, too.
> >>
> >> While fiddling with section attributes in this area, also move the two
> >> involved variables to .data.ro_after_init.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>
> >> --- a/xen/arch/x86/traps.c
> >> +++ b/xen/arch/x86/traps.c
> >> @@ -126,11 +126,11 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct tss_p
> >>  static int debug_stack_lines = 20;
> >>  integer_param("debug_stack_lines", debug_stack_lines);
> >>  
> >> -static bool opt_ler;
> >> +static bool __ro_after_init opt_ler;
> >>  boolean_param("ler", opt_ler);
> >>  
> >>  /* LastExceptionFromIP on this hardware.  Zero if LER is not in use. */
> >> -unsigned int __read_mostly ler_msr;
> >> +unsigned int __ro_after_init ler_msr;
> >>  
> >>  const unsigned int nmi_cpu;
> >>  
> >> @@ -2133,7 +2133,7 @@ static void __init set_intr_gate(unsigne
> >>      __set_intr_gate(n, 0, addr);
> >>  }
> >>  
> >> -static unsigned int calc_ler_msr(void)
> >> +static unsigned int noinline __init calc_ler_msr(void)
> >>  {
> >>      switch ( boot_cpu_data.x86_vendor )
> >>      {
> >> @@ -2171,8 +2171,17 @@ void percpu_traps_init(void)
> >>      if ( !opt_ler )
> >>          return;
> >>  
> >> -    if ( !ler_msr && (ler_msr = calc_ler_msr()) )
> >> +    if ( !ler_msr )
> >> +    {
> >> +        ler_msr = calc_ler_msr();
> >> +        if ( !ler_msr )
> >> +        {
> > 
> > While doing this rework it might make sense to print some message
> > here, like: "LER option requested but no LBR support available" or
> > similar IMO.
> 
> Hmm, yes, but you look to do so in your series already. Could we
> leave things silent here (as it always was) until your adding of
> arch-LBR support, and then taking care of both failure conditions
> with a single log message? Of course I could add a message here
> just for you to then (likely) alter it again ...

Oh, so I do introduce one, sorry I didn't remember.  Then it's fine to
go as-is.

Thanks, Roger.



 


Rackspace

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