Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page
On Fri, Feb 9, 2018 at 4:15 AM, KarimAllah Ahmedwrote: > Can you elaborate a bit? I do not really understand what is the concern. I can't find the posting that gave me this impression. The only thing I can find is the following in Documentation/vm/highmem.txt: (*) kmap(). This permits a short duration mapping of a single page. It needs global synchronization, but is amortized somewhat. It is also prone to deadlocks when using in a nested fashion, and so it is not recommended for new code. What constitutes a "short duration," and does this new usage qualify?
Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page
On Fri, Feb 9, 2018 at 4:15 AM, KarimAllah Ahmed wrote: > Can you elaborate a bit? I do not really understand what is the concern. I can't find the posting that gave me this impression. The only thing I can find is the following in Documentation/vm/highmem.txt: (*) kmap(). This permits a short duration mapping of a single page. It needs global synchronization, but is amortized somewhat. It is also prone to deadlocks when using in a nested fashion, and so it is not recommended for new code. What constitutes a "short duration," and does this new usage qualify?
Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page
On 02/05/2018 11:26 PM, Jim Mattson wrote: On Mon, Feb 5, 2018 at 10:48 AM KarimAllah Ahmedwrote: @@ -5264,9 +5264,8 @@ static void vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu) max_irr = find_last_bit((unsigned long *)vmx->nested.pi_desc->pir, 256); if (max_irr != 256) { - vapic_page = kmap(vmx->nested.virtual_apic_page); + vapic_page = vmx->nested.virtual_apic_mapping.kaddr; __kvm_apic_update_irr(vmx->nested.pi_desc->pir, vapic_page); - kunmap(vmx->nested.virtual_apic_page); Your kernel mapping now survives exits to userspace. I didn't think that was kosher, but I'd be happy to hear that I'm wrong. Can you elaborate a bit? I do not really understand what is the concern. @@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx) /* Unpin physical memory we referred to in the vmcs02 */ if (vmx->nested.apic_access_mapping.pfn) kvm_release_host_mapping(>nested.apic_access_mapping, true); - if (vmx->nested.virtual_apic_page) { - kvm_release_page_dirty(vmx->nested.virtual_apic_page); - vmx->nested.virtual_apic_page = NULL; - } + if (vmx->nested.virtual_apic_mapping.pfn) + kvm_release_host_mapping(>nested.virtual_apic_mapping, true); Is the host PFN guaranteed to be non-zero? Yeah, I should not have assumed that PFN zero is invalid. Will fix. Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page
On 02/05/2018 11:26 PM, Jim Mattson wrote: On Mon, Feb 5, 2018 at 10:48 AM KarimAllah Ahmed wrote: @@ -5264,9 +5264,8 @@ static void vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu) max_irr = find_last_bit((unsigned long *)vmx->nested.pi_desc->pir, 256); if (max_irr != 256) { - vapic_page = kmap(vmx->nested.virtual_apic_page); + vapic_page = vmx->nested.virtual_apic_mapping.kaddr; __kvm_apic_update_irr(vmx->nested.pi_desc->pir, vapic_page); - kunmap(vmx->nested.virtual_apic_page); Your kernel mapping now survives exits to userspace. I didn't think that was kosher, but I'd be happy to hear that I'm wrong. Can you elaborate a bit? I do not really understand what is the concern. @@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx) /* Unpin physical memory we referred to in the vmcs02 */ if (vmx->nested.apic_access_mapping.pfn) kvm_release_host_mapping(>nested.apic_access_mapping, true); - if (vmx->nested.virtual_apic_page) { - kvm_release_page_dirty(vmx->nested.virtual_apic_page); - vmx->nested.virtual_apic_page = NULL; - } + if (vmx->nested.virtual_apic_mapping.pfn) + kvm_release_host_mapping(>nested.virtual_apic_mapping, true); Is the host PFN guaranteed to be non-zero? Yeah, I should not have assumed that PFN zero is invalid. Will fix. Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page
On Mon, Feb 5, 2018 at 10:48 AM KarimAllah Ahmedwrote: > @@ -5264,9 +5264,8 @@ static void vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu) > max_irr = find_last_bit((unsigned long *)vmx->nested.pi_desc->pir, 256); > if (max_irr != 256) { > - vapic_page = kmap(vmx->nested.virtual_apic_page); > + vapic_page = vmx->nested.virtual_apic_mapping.kaddr; > __kvm_apic_update_irr(vmx->nested.pi_desc->pir, vapic_page); > - kunmap(vmx->nested.virtual_apic_page); Your kernel mapping now survives exits to userspace. I didn't think that was kosher, but I'd be happy to hear that I'm wrong. > @@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx) > /* Unpin physical memory we referred to in the vmcs02 */ > if (vmx->nested.apic_access_mapping.pfn) kvm_release_host_mapping(>nested.apic_access_mapping, true); > - if (vmx->nested.virtual_apic_page) { > - kvm_release_page_dirty(vmx->nested.virtual_apic_page); > - vmx->nested.virtual_apic_page = NULL; > - } > + if (vmx->nested.virtual_apic_mapping.pfn) > + kvm_release_host_mapping(>nested.virtual_apic_mapping, true); Is the host PFN guaranteed to be non-zero?
Re: [RFC 03/12] KVM/VMX: Use the new host mapping API for virtual_apic_page
On Mon, Feb 5, 2018 at 10:48 AM KarimAllah Ahmed wrote: > @@ -5264,9 +5264,8 @@ static void vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu) > max_irr = find_last_bit((unsigned long *)vmx->nested.pi_desc->pir, 256); > if (max_irr != 256) { > - vapic_page = kmap(vmx->nested.virtual_apic_page); > + vapic_page = vmx->nested.virtual_apic_mapping.kaddr; > __kvm_apic_update_irr(vmx->nested.pi_desc->pir, vapic_page); > - kunmap(vmx->nested.virtual_apic_page); Your kernel mapping now survives exits to userspace. I didn't think that was kosher, but I'd be happy to hear that I'm wrong. > @@ -7502,10 +7501,8 @@ static void free_nested(struct vcpu_vmx *vmx) > /* Unpin physical memory we referred to in the vmcs02 */ > if (vmx->nested.apic_access_mapping.pfn) kvm_release_host_mapping(>nested.apic_access_mapping, true); > - if (vmx->nested.virtual_apic_page) { > - kvm_release_page_dirty(vmx->nested.virtual_apic_page); > - vmx->nested.virtual_apic_page = NULL; > - } > + if (vmx->nested.virtual_apic_mapping.pfn) > + kvm_release_host_mapping(>nested.virtual_apic_mapping, true); Is the host PFN guaranteed to be non-zero?