Re: [PATCH 1/3] KVM: VMX: Rename vmx_umip_emulated() to cpu_has_vmx_desc()

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

 



Sean Christopherson <seanjc@xxxxxxxxxx> 于2023年3月16日周四 01:50写道:
>
> Please fix your editor or whatever it is that is resulting your emails being
> wrapped at very bizarre boundaries.
>
(Sorry for late reply.)
Yes, I also noticed this. Just began using Gmail web portal for community mails.
I worried that it has no auto wrapping (didn't find the setting), so manually
wrapped; but now looks like it has some.
Give me some time, I'm going to switch to some mail client.
Welcome suggestions of mail clients which is suited for community
communications, on Windows platform.🙂

> On Sat, Mar 11, 2023, Robert Hoo wrote:
> > Also, vmx_umip_emulated() == true doesn't necessarily mean, as its name
> > indicates, UMIP-being-emulated, e.g. Host has UMIP capability, then UMIP
> > isn't emulated though vmx_umip_emulated() indicates true.
>
[...]
> there's no
> legitimate use for checking if UMIP _can_ be emulated.

Agree.
>
> Functionally, this should be a glorified nop,

Personally, I think it is more than this; good naming, not misleading
its user, saving their time to look into, is always good.

> but I agree it's worth changing.
> I'll formally post this after testing.

Thanks, looks good to me, it enriches the original patch, i.e. the
cr4_fixed1 part is beyond my original findings. Just don't quite
understand the last paragraph of the description. I ask inline below.
>
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> Date: Wed, 15 Mar 2023 10:27:40 -0700
> Subject: [PATCH] KVM: VMX: Treat UMIP as emulated if and only if the host
>  doesn't have UMIP
>
> Advertise UMIP as emulated if and only if the host doesn't natively
> support UMIP, otherwise vmx_umip_emulated() is misleading when the host
> _does_ support UMIP.  Of the four users of vmx_umip_emulated(), two
> already check for native support, and the logic in vmx_set_cpu_caps() is
> relevant if and only if UMIP isn't natively supported as UMIP is set in
> KVM's caps by kvm_set_cpu_caps() when UMIP is present in hardware.
>
Sorry I don't fully understand the paragraph below, though I believe
you're right.:)

> That leaves KVM's stuffing of X86_CR4_UMIP into the default cr4_fixed1
> value enumerated for nested VMX.  In that case, checking for (lack of)
> host support is actually a bug fix of sorts,

What bug?
By your assumption below:
    * host supports UMIP, host doesn't allow (nested?) vmx
    * UMIP enumerated to L1, L1 thinks it has UMIP capability and
enumerates to L2
    * L1 MSR_IA32_VMX_CR4_FIXED1.UMIP is set (meaning allow 1, not fixed to 1)

L2 can use UMIP, no matter L1's UMIP capability is backed by L0 host's
HW UMIP or L0's vmx emulation, I don't see any problem. Shed more
light?

> as enumerating UMIP support
> based solely on descriptor table

What "descriptor table" do you mean here?

> existing works only because KVM doesn't
> sanity check MSR_IA32_VMX_CR4_FIXED1.

Emm, nested_cr4_valid() should be applied to vmx_set_cr4()?

> E.g. if a (very theoretical) host
> supported UMIP in hardware but didn't allow UMIP+VMX, KVM would advertise
> UMIP but not actually emulate UMIP.  Of course, KVM would explode long
> before it could run a nested VM on said theoretical CPU, as KVM doesn't
> modify host CR4 when enabling VMX.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux