2015-02-05 14:23+0800, Kai Huang:
> On 02/03/2015 11:18 PM, Radim Krčmář wrote:
> >You have it protected by CONFIG_X86_64, but use it unconditionally.
> Thanks for catching. This has been fixed by another patch, and the fix has
> also been merged by Paolo.
(And I haven't noticed the followup either ...)
I don't know of any present bugs in this patch and all questions have
been cleared,
Thanks.
> >>+ exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
> >What is the harm of enabling it here?
> >
> >(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES seems similar and does it.)
> Because the PML feature detection is unconditional (meaning
> SECONDARY_EXEC_ENABLE_PML is always in vmcs_config.cpu_based_2nd_exec_ctrl),
> but the PML buffer is only created when vcpu is created, and it is
> controlled by 'enable_pml' module parameter, if we always enable
> SECONDARY_EXEC_ENABLE_PML here, no PML buffer will be created if PML is
> disabled by 'enable_pml' parameter,
I meant
if (!enable_pml)
exec_control &= ~SECONDARY_EXEC_ENABLE_PML
here and no exec_control operations in vmx_create_vcpu().
> so it's better to enable it along with
> creating PML buffer.
I think the reason why KVM split the setup into vmx_create_vcpu() and
vmx_secondary_exec_control() is to simplify nested virtualization.
> >>+ if (!(to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
> >>+ cpu_has_virtual_nmis() &&
> >>+ (exit_qualification & INTR_INFO_UNBLOCK_NMI))
> >>+ vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
> >>+ GUEST_INTR_STATE_NMI);
> >Relevant part of the specification is pasted from 27.2.2 (Information
> >for VM Exits Due to Vectored Events), which makes me think that this bit
> >is mirrored to IDT-vectoring information field ...
> >
> >Isn't vmx_recover_nmi_blocking() enough?
> >
> >(I see the same code in handle_ept_violation(), but wasn't that needed
> > just because of a hardware error?)
> This needs to be handled in both EPT violation and PML. If you look at the
> PML specification (the link is in cover letter), you can see the definition
> of bit 12 of exit_qualification (section 1.5), which explains above code.
> The same definition of exit_qualification is in EPT violation part in
> Intel's SDM.
True ... IDT-vectoring doesn't set bit 12 because PML exit isn't a fault
and 27.2.2: "— For all other relevant VM exits, bit 12 is cleared to 0."
(This was humourously pasted to PML as well.)
> >>+ /* PML index always points to next available PML buffer entity */
> >>+ if (pml_idx >= PML_ENTITY_NUM)
> >>+ pml_idx = 0;
> >>+ else
> >>+ pml_idx++;
> >If the pml_idx is >= PML_ENTITY_NUM and < 0xffff, the log is empty,
> In this case, the log is full, actually. PML index is set to PML_ENTITY_NUM
> - 1 initially, and hardware decrease it after GPA is logged.
>
> Below is the pseudocode of PML hardware logic.
>
> IF (PML Index[15:9] ≠ 0)
> THEN VM exit;
> FI;
pml_idx >= PML_ENTITY_NUM exits without modifying the buffer,
> set accessed and dirty flags for EPT;
> IF (a dirty flag was updated from 0 to 1)
> THEN
> PML address[PML index] ← 4-KByte-aligned guest-physical address;
> PML index is decremented;
0xffff is the only value that specifies full buffer, the rest means
that we incorrectly set up the initial index and VMX exited without
changing the buffer => we shouldn't read it.
(I should have said "untouched" instead of "empty".)
> FI;
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
