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

[Shuo A Liu]

On Wed  4.Nov'20 at 19:51:57 +0100, Borislav Petkov wrote:
> 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."

Ok.

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

No, acrn HV is always return eax with maximum leaf number.

Let me add the document.

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

Yes.

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

OK.

Thanks
shuo