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

Re: [PATCH 2/4] x86/ACPI: fix S3 wakeup vector mapping


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 23 Nov 2020 17:07:52 +0100
  • 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-SenderADCheck; bh=/WhAvPdIgkF2fb97bY+SwGofAoF7RVa+5/6DK1whcMk=; b=FeFypgMjyb8C0kjTBojP+SEc68hQpui3w1qqIWTPLsgvlODUxWMZwr/2R/T2iwGXenHsKxIx5KkI+xBKfdGqajfMCFmpLX6w9FP6iXx6pU2JvrGrRrUdLrlukaKHbMS7po+/y7APFq2LNon7yE24pyfBnxuICMr+2KceBIsYxvP0+oXG98IjrpLLaveATNW+rFCH+ter5uxQ6+U7TkM3gXc7KAIclf4FFLJeCtejJxlXnPZgLN2KDp4WYoaJKeT6U6AQDBHtXHwT9TD9hei+WQdcAHBJ9q09aGfBGNaWOa+BRMnLjn1QCjni6rz3rxg6w7gzF7tts5A2K6G3RQ4/lw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZdXf9HL7hf813Qy82MUefN7erm2yjocB0cuYjwK1vv3BVsvPQNDbn2Myun6nlLTYrfbc+elIwjBjHLXxBdo1DYmudAhjcyw4RBoQV2hiCxmoiaEa5dAK98kgEy2B1VM2v75tCVqyi9fFKU2EBI4q4S77qfvzcxzYkFf4GR4aacrqZLv9tOgFXGDJ7uc8kuQpdT/FicFZUnneKPYvG9IardpZiCxN9jnM0cCWsjLVrFxWTJjL5shEfQIez+KeSNET+/kHO9YWCvVpaqRHLpDTreZv7DDJEKTHHr2R3BH5jnP9PDwZ37PAzm0BpBbzmhl/PxOaib2lZYsfVwrtsettQw==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Andrew Cooper" <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>
  • Delivery-date: Mon, 23 Nov 2020 16:08:07 +0000
  • Ironport-sdr: KlRta09svDEbrKFHohYIm9jEuN01XoIKs6mE2wbn6jj+wN8eIi3VDQ7fMmOxnug1UlERvzAQP0 0DePRvQ51Xk8i9GiQgib7DBoShroII9hCs47BnD2V0YYQEmHmjMBIC8u/ICs0u4a+zdSeRRrR/ w+ph35bb/QauBgYOqrNDj/u1NXEHa9K1hulBYJlbBwuEHEQCVblwk+8dvTaO5M4hbWiNlDjtPE G7DPV8aOY9TSzqIDFytj42LYUfJL5689M1jg8iTbV8LmqA//4Q2QAKrpoPyxLyf7g634cDfpwZ XNY=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Nov 23, 2020 at 04:30:05PM +0100, Jan Beulich wrote:
> On 23.11.2020 16:24, Roger Pau Monné wrote:
> > On Mon, Nov 23, 2020 at 01:40:12PM +0100, Jan Beulich wrote:
> >> --- a/xen/arch/x86/acpi/power.c
> >> +++ b/xen/arch/x86/acpi/power.c
> >> @@ -174,17 +174,20 @@ static void acpi_sleep_prepare(u32 state
> >>      if ( state != ACPI_STATE_S3 )
> >>          return;
> >>  
> >> -    wakeup_vector_va = __acpi_map_table(
> >> -        acpi_sinfo.wakeup_vector, sizeof(uint64_t));
> >> -
> >>      /* TBoot will set resume vector itself (when it is safe to do so). */
> >>      if ( tboot_in_measured_env() )
> >>          return;
> >>  
> >> +    set_fixmap(FIX_ACPI_END, acpi_sinfo.wakeup_vector);
> >> +    wakeup_vector_va = fix_to_virt(FIX_ACPI_END) +
> >> +                       PAGE_OFFSET(acpi_sinfo.wakeup_vector);
> >> +
> >>      if ( acpi_sinfo.vector_width == 32 )
> >>          *(uint32_t *)wakeup_vector_va = bootsym_phys(wakeup_start);
> >>      else
> >>          *(uint64_t *)wakeup_vector_va = bootsym_phys(wakeup_start);
> >> +
> >> +    clear_fixmap(FIX_ACPI_END);
> > 
> > Why not use vmap here instead of the fixmap?
> 
> Considering the S3 path is relatively fragile (as in: we end up
> breaking it more often than about anything else) I wanted to
> make as little of a change as possible. Hence I decided to stick
> to the fixmap use that was (indirectly) used before as well.

Unless there's a restriction to use the ACPI fixmap entry I would just
switch to use vmap, as it's used extensively in the code and less
likely to trigger issues in the future, or else a bunch of other stuff
would also be broken.

IMO doing the mapping differently here when it's not required will end
up turning this code more fragile in the long run.

Thanks, Roger.



 


Rackspace

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