Re: [PATCH] KVM: VMX: Enable Virtual Processor Identification (VPID)

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Yang, Sheng wrote:
> To allow TLB entries to be retained across VM entry and VM exit, the VMM can 
> now identify distinct address spaces through a new virtual-processor ID 
> (VPID) field of the VMCS.
>
>   

> +#define VPID_BITMAP_SIZE			(1 << 16)*BITS_PER_BYTE
>   

This is strange; I think that just (1 << 16) would be simpler later on.

Please also rename to VMX_NR_VPIDS, since this header is generally 
implementation independent (just declares vmx constants).

>  static inline struct vcpu_vmx *to_vmx(struct kvm_vcpu *vcpu)
> @@ -86,6 +90,9 @@ static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
>  static struct page *vmx_io_bitmap_a;
>  static struct page *vmx_io_bitmap_b;
>  
> +static DECLARE_BITMAP(vmx_vpid_bitmap, VPID_BITMAP_SIZE / BITS_PER_BYTE);
>   

Like here, you could pass VPID_BITMAP_SIZE.


>  
> +static void allocate_vpid(struct vcpu_vmx *vmx)
> +{
> +	int vpid;
> +
> +	vmx->vpid = 0;
> +	if (!enable_vpid || !cpu_has_vmx_vpid())
> +		return;
> +	spin_lock(&vmx_vpid_lock);
> +	vpid = find_first_zero_bit(vmx_vpid_bitmap, VPID_BITMAP_SIZE);
>   

This can overflow since VPID_BITMAP_SIZE is 64K * 8.

> apic_access_page));
>  
> +	if (vmx->vpid != 0) {
> +		vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
> +		printk(KERN_DEBUG "VPID enabled, VPID is 0x%x\n", vmx->vpid);
> +	}
>   

Please drop the printk().

> ap_b);
>  
> +	set_bit(0, vmx_vpid_bitmap); /* 0 is reversed for host */
> +
>   

reversed -> reserved, some letters got reserved here.  See also Rik can 
Riel's signature.

Need to add vpid support to vmx_flush_tlb() (and perhaps call it from 
vmx_set_cr3).

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/