From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"Andrew Cooper" <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
Julien Grall <julien@xen.org>
Subject: Re: [PATCH 2/4] x86/ACPI: fix S3 wakeup vector mapping
Date: Mon, 23 Nov 2020 17:07:52 +0100 [thread overview]
Message-ID: <20201123160752.uzczcxnz5ytvtd46@Air-de-Roger> (raw)
In-Reply-To: <79776889-c566-5f07-abfe-2cb79cfa78fa@suse.com>
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.
next prev parent reply other threads:[~2020-11-23 16:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 12:38 [PATCH 0/4] x86: ACPI and DMI table mapping fixes Jan Beulich
2020-11-23 12:39 ` [PATCH 1/4] x86/ACPI: fix mapping of FACS Jan Beulich
2020-11-23 14:30 ` Roger Pau Monné
2020-12-29 10:56 ` Roger Pau Monné
2021-01-04 13:18 ` Jan Beulich
2020-11-23 12:40 ` [PATCH 2/4] x86/ACPI: fix S3 wakeup vector mapping Jan Beulich
2020-11-23 15:24 ` Roger Pau Monné
2020-11-23 15:30 ` Jan Beulich
2020-11-23 16:07 ` Roger Pau Monné [this message]
2020-11-23 16:14 ` Andrew Cooper
2020-11-24 11:04 ` Jan Beulich
2020-11-30 13:02 ` Jan Beulich
2020-12-23 15:09 ` Ping: " Jan Beulich
2020-12-29 10:54 ` Roger Pau Monné
2021-01-04 14:03 ` Jan Beulich
2021-01-04 15:13 ` Roger Pau Monné
2020-12-29 10:51 ` Roger Pau Monné
2021-01-04 14:10 ` Jan Beulich
2020-11-23 12:40 ` [PATCH 3/4] x86/DMI: fix table mapping when one lives above 1Mb Jan Beulich
2020-11-23 15:41 ` Roger Pau Monné
2020-11-23 12:41 ` [PATCH 4/4] x86/ACPI: don't invalidate S5 data when S3 wakeup vector cannot be determined Jan Beulich
2020-11-23 15:44 ` Roger Pau Monné
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201123160752.uzczcxnz5ytvtd46@Air-de-Roger \
--to=roger.pau@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).