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

Avi Kivity
2008-01-24T11:50:20Z

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.