From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
Stewart Hildebrand <stewart.hildebrand@amd.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>,
"Oleksii Kurochko" <oleksii.kurochko@gmail.com>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH] xen/x86: fix build with CONFIG_HVM=n and -Og
Date: Fri, 14 Feb 2025 09:25:15 +0100 [thread overview]
Message-ID: <7cbc513b-b98c-494d-9623-ba31a7a14360@suse.com> (raw)
In-Reply-To: <f5deca6a-313f-4daf-b774-cc05223ab034@citrix.com>
On 14.02.2025 02:05, Andrew Cooper wrote:
> On 13/02/2025 6:50 pm, Stewart Hildebrand wrote:
>> When building with CONFIG_HVM=n and -Og, we encounter:
>>
>> prelink.o: in function `pit_set_gate':
>> xen/xen/arch/x86/emul-i8254.c:195: undefined reference to `destroy_periodic_time'
>>
>> Add an IS_ENABLED(CONFIG_HVM) check to assist with dead code
>> elimination.
>>
>> Fixes: 14f42af3f52d ("x86/vPIT: account for "counter stopped" time")
While I don't mind this as a tag, you could equally blame the commit
having added support for EXTRA_CFLAGS_XEN_CORE, for not documenting
restrictions. As Andrew says further down, it's deemed known that -Og
doesn't work reliably. And who knows what other very special options
would cause havoc. I'm inclined to go as far as saying that quite
likely no Fixes: tag is appropriate here at all, as long as we have no
way to use -Og without making use of EXTRA_CFLAGS_XEN_CORE (or hacking
it in another way). People using EXTRA_CFLAGS_XEN_CORE are on their
own anyway, because the documenting of restrictions mentioned above
would be nice in theory, but is entirely impractical imo: We'd need to
exhaustively test all options, and then document which ones we've
found working (under what conditions).
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>
> While I appreciate the effort to get -Og working (I tried and gave up
> due to frustration), this is gnarly.
>
> PIT emulation is used by both PV and HVM guests. All other uses of
> {create,destroy}_periodic_time() are behind something that explicitly
> short-circuits in !HVM cases (usually an is_hvm_*() predicate).
>
> The PV path would normally passes 2 for the channel, which would
> normally get const-propagated and trigger DCE here.
>
> One option might be to make pit_set_gate() be __always_inline. It only
> has a single caller, and it's only because of -Og that it doesn't get
> inlined. Then again, this is arguably more subtle than the fix
> presented here.
Making it always_inline on the basis that there's just a single caller
would be equivalent to simply dropping the handling of channel 0 when
the sole caller passes channel 2. I don't like either. Instead ...
> A preferable fix (but one that really won't get into 4.20 at this point)
> would be to genuinely compile pit->pt0 out in !HVM builds. That would
> save structure space, but would also force the use of full #ifdef-ary
> across this file.
... I was about to also suggest this approach.
Jan
next prev parent reply other threads:[~2025-02-14 8:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-13 18:50 [PATCH] xen/x86: fix build with CONFIG_HVM=n and -Og Stewart Hildebrand
2025-02-13 20:17 ` Oleksii Kurochko
2025-02-14 1:05 ` Andrew Cooper
2025-02-14 8:25 ` Jan Beulich [this message]
2025-03-03 21:18 ` Stewart Hildebrand
2025-03-03 18:59 ` Stewart Hildebrand
2025-03-03 19:19 ` Andrew Cooper
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=7cbc513b-b98c-494d-9623-ba31a7a14360@suse.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=oleksii.kurochko@gmail.com \
--cc=roger.pau@citrix.com \
--cc=stewart.hildebrand@amd.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.