Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 4 Nov 2020 19:51:57 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     Shuo A Liu <shuo.a.liu@intel.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Yu Wang <yu1.wang@intel.com>,
        Reinette Chatre <reinette.chatre@intel.com>,
        Yin Fengwei <fengwei.yin@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Zhi Wang <zhi.a.wang@intel.com>,
        Zhenyu Wang <zhenyuw@linux.intel.com>
Subject: Re: [PATCH v5 03/17] x86/acrn: Introduce an API to check if a VM is
 privileged
Message-ID: <20201104185157.GE23298@zn.tnic>
References: <20201019061803.13298-1-shuo.a.liu@intel.com>
 <20201019061803.13298-4-shuo.a.liu@intel.com>
 <20201102143707.GC15392@zn.tnic>
 <20201103062718.GD12408@shuo-intel.sh.intel.com>
 <20201103102538.GB6310@zn.tnic>
 <20201104035027.GA17702@shuo-intel.sh.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20201104035027.GA17702@shuo-intel.sh.intel.com>
Precedence: bulk

On Wed, Nov 04, 2020 at 11:50:27AM +0800, Shuo A Liu wrote:
> On Tue  3.Nov'20 at 11:25:38 +0100, Borislav Petkov wrote:
> > On Tue, Nov 03, 2020 at 02:27:18PM +0800, Shuo A Liu wrote:
> > > The code just followed KVM style (see kvm_arch_para_features()).
> > 
> > Do you see Documentation/virt/kvm/cpuid.rst?
> 
> OK. It documents the leaf number.

It also says

"Note also that old hosts set eax value to 0x0. This should be
interpreted as if the value was 0x40000001."

Does this hold true for the acrn HV? The fact that I'm asking about
all those things should give you a hint that documenting the API is
important.

> > Now where is yours explaining what your hypervisor is doing?
> 
> Currently, it is in arch/x86/include/asm/acrn.h.

Yeah, you can't expect people to go scrape it from headers though - it
should be concentrated in a doc file.

> If the leaf numbers be documented explicitly (like kvm), i think i can
> use them as eax of cpuid_eax() directly (back to your first comment).

Which means, you don't need to do the logical OR-ing which kvm does
because of what I pasted above about eax being 0 on old hosts. Now we're
getting somewhere...

> 	cpuid_eax(ACRN_CPUID_FEATURES)...
> 
> If you looking at implementation of acrn-hypervisor, you will found the
> leaf number is hardcoded in the hypervisor. So, they also can be
> documented explicitly.

Ok.

> OK. I can add a similar cpuid.rst for acrn.

Yes please.

> Yes. Fix patches are always welcome.

Ok, good, the thing is open. You could put that in the doc too, along
with the link so that people can find it.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette