Re: [PATCH v5 03/17] x86/acrn: Introduce an API to check if a VM is privileged

[Shuo A Liu]

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.

> Now where is yours explaining what your hypervisor is doing?

Currently, it is in arch/x86/include/asm/acrn.h.

> > I can change to use
> > 	cpuid_eax(acrn_cpuid_base() + 1)...
> > If you prefer to.
>
> Yes please.

Sure.

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).
	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.

> > hypervisor_cpuid_base() implies the base is variable, no? We use
> > this function to detect the base.
>
> Yes, but you need to document all that and make it clear and
> understandable. If Linux is supposed to run as an acrn guest, that
> interface better be documented just like KVM does.

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

> Also, if there's a bug in the KVM guest/host interface, we might be able
> to fix it modulo ABI. Is that possible with acrn?
>
> I'm guessing the answer to that is yes if I'm looking at
>
> https://github.com/projectacrn/acrn-hypervisor
>
> ?

Yes. Fix patches are always welcome.

> > OK. Then i will define acrn_cpuid_base() as a static inline function
> > in asm/acrn.h for callers.
>
> Yah, that function is simple enough.


Thanks
shuo