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

Re: [PATCH 11/16] x86: add a boot option to enable and disable the direct map


On 01/05/2020 14:11, Wei Liu wrote:
On Fri, May 01, 2020 at 01:59:24PM +0100, Hongyan Xia wrote:
On Fri, 2020-05-01 at 12:11 +0000, Wei Liu wrote:
On Thu, Apr 30, 2020 at 09:44:20PM +0100, Hongyan Xia wrote:
From: Hongyan Xia <hongyxia@xxxxxxxxxx>

Also add a helper function to retrieve it. Change
to check this option before returning.

This is added as a boot command line option, not a Kconfig. We do
produce different builds for EC2 so this is not introduced as a
compile-time configuration.

Having a Kconfig will probably allow the compiler to eliminate dead

This is not asking you to do the work, someone can come along and
arch_has_directmap easily.

My original code added this as a CONFIG option, but I converted it into
a boot-time switch, so I can just dig out history and convert it back.
I wonder if we should get more opinions on this to make a decision.

Form my perspective, you as a contributor has done the work to scratch
your own itch, hence I said "not asking you to do the work". I don't
want to turn every comment into a formal ask and eventually lead to
feature creep.

I would love Xen to have static key support though so that a boot-time
switch costs no run-time performance.

Yes that would be great.

From my understanding static key is very powerful as you could modify the value even at runtime.

On Arm, I wrote a version that I would call static key for the poor. We are using alternative to select between 0 and 1 as an immediate value.

#define CHECK_WORKAROUND_HELPER(erratum, feature, arch)         \
static inline bool check_workaround_##erratum(void)             \
{                                                               \
    if ( !IS_ENABLED(arch) )                                    \
        return false;                                           \
    else                                                        \
    {                                                           \
        register_t ret;                                         \
        asm volatile (ALTERNATIVE("mov %0, #0",                 \
                                  "mov %0, #1",                 \
                                  feature)                      \
                      : "=r" (ret));                            \
        return unlikely(ret);                                   \
    }                                                           \

The code generated will still be using a conditional branch, but the processor should be able to always infer correctly whether the condition is true or not.

The implementation is also very tailored to Arm as we consider workaround are not enabled by default. But I think this can be improved and made more generic.


Julien Grall



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