Re: [Xen-devel] [v2 06/11] vmx: add help functions to support PML

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.04.15 at 09:03, <kai.huang@xxxxxxxxxxxxxxx> wrote:
> +int vmx_vcpu_enable_pml(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +
> +    if ( vmx_vcpu_pml_enabled(v) )
> +        return 0;
> +
> +    v->arch.hvm_vmx.pml_pg = d->arch.paging.alloc_page(d);

So you latch v->domain into d for this invocation, ...

> +void vmx_vcpu_disable_pml(struct vcpu *v)
> +{
> +    if ( !vmx_vcpu_pml_enabled(v) )
> +        return;
> +
> +    /* Make sure we don't lose any logged GPAs */
> +    vmx_vcpu_flush_pml_buffer(v);
> +
> +    vmx_vmcs_enter(v);
> +
> +    v->arch.hvm_vmx.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
> +    __vmwrite(SECONDARY_VM_EXEC_CONTROL,
> +              v->arch.hvm_vmx.secondary_exec_control);
> +
> +    vmx_vmcs_exit(v);
> +
> +    v->domain->arch.paging.free_page(v->domain, v->arch.hvm_vmx.pml_pg);

... but not for this one. Please be consistent.

> +void vmx_vcpu_flush_pml_buffer(struct vcpu *v)
> +{
> +    uint64_t *pml_buf;
> +    unsigned long pml_idx;
> +
> +    ASSERT(vmx_vcpu_pml_enabled(v));
> +
> +    vmx_vmcs_enter(v);
> +
> +    __vmread(GUEST_PML_INDEX, &pml_idx);

Don't you require the vCPU to be non-running or current when you
get here? If so, perhaps add a respective ASSERT()?

> +
> +    /* Do nothing if PML buffer is empty */
> +    if ( pml_idx == (NR_PML_ENTRIES - 1) )
> +        goto out;
> +
> +    pml_buf = __map_domain_page(v->arch.hvm_vmx.pml_pg);
> +
> +    /*
> +     * PML index can be either 2^16-1 (buffer is full), or 0~511 (buffer is 
> not
> +     * full), and in latter case PML index always points to next available
> +     * entity.
> +     */
> +    if (pml_idx >= NR_PML_ENTRIES)
> +        pml_idx = 0;
> +    else
> +        pml_idx++;
> +
> +    for ( ; pml_idx < NR_PML_ENTRIES; pml_idx++ )
> +    {
> +        unsigned long gfn = pml_buf[pml_idx] >> PAGE_SHIFT;

Blank line here please.

> +        /*
> +         * Need to change type from log-dirty to normal memory for logged 
> GFN.
> +         * hap_track_dirty_vram depends on it to work. And we really only 
> need
> +         * to mark GFNs which hve been successfully changed from log-dirty to
> +         * normal memory to be dirty.
> +         */
> +        if ( !p2m_change_type_one(v->domain, gfn, p2m_ram_logdirty,
> +                    p2m_ram_rw) )

Indentation.

> +            paging_mark_gfn_dirty(v->domain, gfn);
> +    }
> +
> +    unmap_domain_page(pml_buf);
> +
> +    /* Reset PML index */
> +    __vmwrite(GUEST_PML_INDEX, NR_PML_ENTRIES - 1);
> +
> +out:

Labels indented by at least one space please.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel