All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Jürgen Groß" <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org, Julien Grall <julien@xen.org>
Subject: Re: [PATCH v6 1/3] xen/arm: add support for run_in_exception_handler()
Date: Thu, 21 Jan 2021 09:56:09 +0100	[thread overview]
Message-ID: <23be37d3-49cf-ed9f-eb1b-3bd2b24e1018@suse.com> (raw)
In-Reply-To: <ea98e8d3-e28a-6bd3-d211-7d37741040cf@suse.com>

On 21.01.2021 09:00, Jürgen Groß wrote:
> On 21.01.21 08:55, Jan Beulich wrote:
>> On 20.01.2021 19:20, Julien Grall wrote:
>>> On 16/01/2021 10:33, Juergen Gross wrote:
>>>> Add support to run a function in an exception handler for Arm. Do it
>>>> as on x86 via a bug_frame, but pass the function pointer via a
>>>> register (this needs to be done that way, because inline asm support
>>>> for 32-bit Arm lacks the needed functionality to reference an
>>>> arbitrary function via the bugframe).
>>>
>>> I was going to commit the series, but then realized the commit message
>>> and comment needs some tweaking because technically GCC is supporting
>>> 'i' (I managed to get it working with -fno-pie).
>>>
>>> So how about:
>>>
>>> "This is needed to be done that way because GCC complains about invalid
>>> constraint when using a function pointer with "i" and PIE is enabled
>>> (default on most of GCC shipped with distro). Clang happily accepts it,
>>> so it may be a bug in GCC."
>>
>> May I ask why you think it's a bug in gcc? I'd rather consider it
>> a bug in clang. Taking the address of a symbol doesn't yield a
>> constant in PIC or PIE code, aiui.
> 
> Maybe we should just not add the suspect of a bug in the compiler to
> either commit message or a comment.
> 
> It could be a bug in gcc, or just a shortcoming (feature combination
> not supported).
> 
> It could be a bug in clang, or clang is clever enough to produce
> correct code for "i" + PIE.

I think the last option can be excluded. There simply is no room
for cleverness. If an insn requires an immediate operand, the
compiler has to find a compile time constant (possibly needing a
relocation, but only PC-relative ones are permitted then). The
compiler may in particular not inspect the insn(s) specified and
try to substitute them.

"i" (with a symbol ref) and PIE (or PIC) simply are incompatible
with one another. One could imagine trickery requiring agreement
between programmer and compiler, where the programmer would be
to specify the relocation to use (and then do the necessary
calculations to yield the actual symbol address), but that's not
applicable here.

I've experimented a little with gcc10 on x86-64. It behaves quite
strangely, accepting some symbol references but not others (see
example below). None of them get accepted with -fpic, and the ones
that do get accepted with -fpie don't result in position
independent code (or my understanding thereof is wrong), or to be
precise in relocations that will have to be carried into the final
executable because of being dependent upon the resulting program's
load address. I'm pondering whether to open a bug ...

Jan

void efn(void);
void(*efp)(void);

void*test(int i) {
	void*res;

	efn();
	efp();

	switch(i) {
	case 0:
		asm("mov %1,%0" : "=r" (res) : "i" (test));
		break;
#ifndef __PIE__
	case 1:
		asm("mov %1,%0" : "=r" (res) : "i" (efn));
		break;
#endif
	case 2:
		asm("mov %1,%0" : "=r" (res) : "i" (&efp));
		break;
	default:
		res = (void*)0;
		break;
	}

	return res;
}


  reply	other threads:[~2021-01-21  8:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-16 10:33 [PATCH v6 0/3] xen: add support for automatic debug key actions in case of crash Juergen Gross
2021-01-16 10:33 ` [PATCH v6 1/3] xen/arm: add support for run_in_exception_handler() Juergen Gross
2021-01-16 17:19   ` Julien Grall
2021-01-16 19:05     ` Jürgen Groß
2021-01-17 17:55       ` Julien Grall
2021-01-20 18:20   ` Julien Grall
2021-01-20 18:34     ` Jürgen Groß
2021-01-21  7:55     ` Jan Beulich
2021-01-21  8:00       ` Jürgen Groß
2021-01-21  8:56         ` Jan Beulich [this message]
2021-01-21  9:50       ` Julien Grall
2021-01-21 10:06         ` Jan Beulich
2021-01-23 11:28           ` Julien Grall
2021-01-16 10:33 ` [PATCH v6 2/3] xen: enable keyhandlers to work without register set specified Juergen Gross
2021-01-20 18:09   ` Julien Grall
2021-01-16 10:33 ` [PATCH v6 3/3] xen: add support for automatic debug key actions in case of crash Juergen Gross

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=23be37d3-49cf-ed9f-eb1b-3bd2b24e1018@suse.com \
    --to=jbeulich@suse.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.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 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.