it's ok with me. compilers are so odd sometimes..
I guess we can keep the patch under wraps until I validate on the following
AMD https://www.packet.net/bare-metal/servers/compute

FF

On 16 April 2018 at 20:28, Jan Kiszka <jan.kis...@siemens.com> wrote:

> On 2018-04-16 19:46, Francois-Frederic Ozog wrote:
> > From: Francois-Frederic Ozog <francois.o...@linaro.org>
> >
> > inmates cannot use X86_FEATURE_VMX from regular cpuid
> > as vcpu maks the bit explicitely on non-root cells.
> >
> > use cpuid leaf 0 to detect AuthenticAMD and change to VMMCALL
> >
> > use string compare for clearer code
> >
> > Signed-off-by: Francois-Frederic Ozog <francois.o...@linaro.org>
> > ---
> >  inmates/lib/x86/hypercall.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
> > index ac99f0c3..07529d4d 100644
> > --- a/inmates/lib/x86/hypercall.c
> > +++ b/inmates/lib/x86/hypercall.c
> > @@ -40,19 +40,26 @@
> >
> >  #define X86_FEATURE_VMX      (1 << 5)
> >
> > -bool jailhouse_use_vmcall;
> > +bool jailhouse_use_vmcall = true;
> > +
> > +static const char* AUTHENTIC_AMD = "AuthenticAMD";
> > +#define U32_SUBSTR(s, n) (((const u32*)s)[n])
> >
> >  void hypercall_init(void)
> >  {
> > -     u32 eax = 1, ecx = 0;
> > +     u32 ebx, ecx, edx;
> >
> >       asm volatile(
> > +             "xor %%eax, %%eax \n\t"
> >               "cpuid"
> > -             : "=c" (ecx)
> > -             : "a" (eax), "c" (ecx)
> > -             : "rbx", "rdx", "memory"
> > +             : "=b" (ebx), "=c" (ecx), "=d" (edx)
> > +             :
> > +             : "memory"
> >       );
> >
> > -     if (ecx & X86_FEATURE_VMX)
> > -             jailhouse_use_vmcall = true;
> > +     if (ebx == U32_SUBSTR(AUTHENTIC_AMD, 0)
> > +             && ecx == U32_SUBSTR(AUTHENTIC_AMD, 2)
> > +             && edx == U32_SUBSTR(AUTHENTIC_AMD, 1)
> > +             )
> > +             jailhouse_use_vmcall = false;
> >  }
> >
>
> I would go with this diff now unless you have concerns:
>
> diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
> index ac99f0c3e..fc5cd5ac8 100644
> --- a/inmates/lib/x86/hypercall.c
> +++ b/inmates/lib/x86/hypercall.c
> @@ -38,21 +38,24 @@
>
>  #include <inmate.h>
>
> -#define X86_FEATURE_VMX        (1 << 5)
> +#define X86_FEATURE_VMX                (1 << 5)
>
> -bool jailhouse_use_vmcall;
> +bool jailhouse_use_vmcall = true;
> +
> +#define AUTHENTIC_AMD(n)       (((const u32 *)"AuthenticAMD")[n])
>
>  void hypercall_init(void)
>  {
> -       u32 eax = 1, ecx = 0;
> +       u32 eax, ebx, ecx, edx;
>
> -       asm volatile(
> -               "cpuid"
> -               : "=c" (ecx)
> -               : "a" (eax), "c" (ecx)
> -               : "rbx", "rdx", "memory"
> +       asm volatile("cpuid"
> +               : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx)
> +               : "a" (0)
> +               : "memory"
>         );
>
> -       if (ecx & X86_FEATURE_VMX)
> -               jailhouse_use_vmcall = true;
> +       if (ebx == AUTHENTIC_AMD(0) &&
> +           edx == AUTHENTIC_AMD(1) &&
> +           ecx == AUTHENTIC_AMD(2))
> +               jailhouse_use_vmcall = false;
>  }
>
> We need to account for Intel cpuid clobbering eax. Moreover, I played a
> bit with the style. Newer gcc even avoids a static string.
>
> Still need to validate this on an AMD system, though.
>
> Thanks,
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
>



-- 
[image: Linaro] <http://www.linaro.org/>
François-Frédéric Ozog | *Director Linaro Networking Group*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jailhouse-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to