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

Re: [PATCH v2 01/10] xen: Implement xen/alternative-call.h for use in common code


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 13 Jul 2021 10:53:25 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=OgMfnghc11eKjaG5/zPcZfPSSKlkqnJnpmhblcTbWZ8=; b=Dbq+R8ZeKeZ382RABuWEgFvcKCzyWlPpVqpxYpK8SBc2A5smLRW3rzB7sFvm3om9ALkKeuM42EXvPhLyYD39WpeqTJvEKgbUtQVJg2necvhEtmzA1j87K5jJcAaFpT0MnYfPMHwisAQ99xrsUAbOUM4n5kfwuUoLZYs1R6QqBd3QXEBKKuUMNPnGlFmV4bK/+rtN7cDtz/kDsaP75keIW9RSH9uWk01QfXfqWtOdMca7t1HR17kfjXiEQCDq+cgv7kDH6xIS7U4S1BmhkPmARLGlUwaSeO0LpoY0z0joZArNSRufp1SDNECl4sQ4QJUjKFdMVOPUCD7JxDqaslyAZw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NK8/eoQ3cXS4BTSJLP15MxtmC8yV8h/CgA232e0s/Nb240pj9CnW5M42UewHsGqehzbCuGJjQ1OmhdI0q4Rld5tzqlLbqUr8XBQ96AnOF56yoWb9nMWygb9SOcH8PGpG7tOIu70/n/bSwFmdZqlBSZ+xesp4SXUS9eNKGL5e5AK+59RlIcR06SwKwMbpXSkrCE6aiZfnqovhSQpH/BtCWJQWUVMbNperGXg4nuw8L25JB394jQCo5SZIVhAMlvgL6Ljejc5U6pXF0wLhecZsIek5b7iLK8c1bK251Vo/SXb2L+r8Ju0rHhz5ukInxt+P1FuARThYLiMz4oEIs8xAvQ==
  • Authentication-results: apertussolutions.com; dkim=none (message not signed) header.d=none;apertussolutions.com; dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Wei Liu <wl@xxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 13 Jul 2021 08:53:35 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 13.07.2021 10:36, Andrew Cooper wrote:
> On 13/07/2021 07:28, Jan Beulich wrote:
>> On 13.07.2021 01:48, Andrew Cooper wrote:
>>> On 12/07/2021 21:32, Daniel P. Smith wrote:
>>>> diff --git a/xen/include/xen/alternative-call.h 
>>>> b/xen/include/xen/alternative-call.h
>>>> new file mode 100644
>>>> index 0000000000..11d1c26068
>>>> --- /dev/null
>>>> +++ b/xen/include/xen/alternative-call.h
>>>> @@ -0,0 +1,65 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +#ifndef XEN_ALTERNATIVE_CALL
>>>> +#define XEN_ALTERNATIVE_CALL
>>>> +
>>>> +/*
>>>> + * Some subsystems in Xen may have multiple implementions, which can be
>>>> + * resolved to a single implementation at boot time.  By default, this 
>>>> will
>>>> + * result in the use of function pointers.
>>>> + *
>>>> + * Some architectures may have mechanisms for dynamically modifying .text.
>>>> + * Using this mechnaism, function pointers can be converted to direct 
>>>> calls
>>>> + * which are typically more efficient at runtime.
>>>> + *
>>>> + * For architectures to support:
>>>> + *
>>>> + * - Implement alternative_{,v}call() in asm/alternative.h.  Code 
>>>> generation
>>>> + *   requirements are to emit a function pointer call at build time, and 
>>>> stash
>>>> + *   enough metadata to simplify the call at boot once the implementation 
>>>> has
>>>> + *   been resolved.
>>>> + * - Select ALTERNATIVE_CALL in Kconfig.
>>>> + *
>>>> + * To use:
>>>> + *
>>>> + * Consider the following simplified example.
>>>> + *
>>>> + *  1) struct foo_ops __alt_call_maybe_initdata ops;
>>>> + *
>>>> + *  2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
>>>> + *     struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
>>> It occurs to me after reviewing patch 2 that these want to be const
>>> struct foo_ops __initconst ...,
>> __initconstrel then, I suppose.
>>
>>> and there is no need for
>>> __alt_call_maybe_initconst at all.
>>>
>>> The only thing wanting a conditional annotation like this is the single
>>> ops object, and it needs to be initdata (or hopefully ro_after_init in
>>> the future).
>> ro_after_init and initdata can't be alternatives of one another; ops
>> (until be gain ro_after_init) need to remain in .bss (or .data).
> 
> Once alternatives have run, the ops structure is no longer referenced by
> anything at runtime, and can be reclaimed.

Oh, right - if all pointers have been consumed for patching, initdata
is of course fine.

Jan

> All the x86 examples are weird because we've either got extra data
> fields which are referenced at runtime, or we've not accelerated all
> function pointers.
> 
> In neither case does modifying an accelerated function pointer after the
> fact do what the programmer expected, and conditionally initdata makes
> this far more obvious.
> 
> ~Andrew
> 




 


Rackspace

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