Re: [Xen-devel] [PATCH 05/10] VMX: add help functions to support PML

[Kai Huang <kaih.linux@xxxxxxxxx>]

On Mon, Mar 30, 2015 at 5:54 PM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
> On 30/03/15 07:43, Kai Huang wrote:
>>
>>
>> On 03/28/2015 05:09 AM, Andrew Cooper wrote:
>>> On 27/03/15 02:35, Kai Huang wrote:
>>>
>>>> +}
>>>> +
>>>> +int vmx_vcpu_enable_pml(struct vcpu *v)
>>>> +{
>>>> +    struct domain *d = v->domain;
>>>> +
>>>> +    ASSERT(!vmx_vcpu_pml_enabled(v));
>>>> +
>>>> +    v->arch.hvm_vmx.pml_pg = d->arch.paging.alloc_page(d);
>>>> +    if ( !v->arch.hvm_vmx.pml_pg )
>>>> +        return -ENOMEM;
>>>> +
>>>> +    vmx_vmcs_enter(v);
>>>> +
>>>> +    __vmwrite(PML_ADDRESS, page_to_mfn(v->arch.hvm_vmx.pml_pg) <<
>>>> PAGE_SHIFT);
>>>> +    __vmwrite(GUEST_PML_INDEX, PML_ENTITY_NUM - 1);
>>>> +
>>>> +    v->arch.hvm_vmx.secondary_exec_control |=
>>>> SECONDARY_EXEC_ENABLE_PML;
>>>> +
>>>> +    __vmwrite(SECONDARY_VM_EXEC_CONTROL,
>>>> +            v->arch.hvm_vmx.secondary_exec_control);
>>> Alignment.
>> Do you mean to put 'v->arch.hvm_vmx.secondary_exec_control' to the
>> same line with '__vmwrite(SECONDARY_VM_EXEC_CONTROL,'? In this case
>> the number of characters will be 81.
>
> Splitting the line is fine.  The v should be vertically in line with S
> from SECONDARY

Oh I got your point. Thanks. Will do.

>
>>
>>>
>>>> +        unsigned long gfn;
>>>> +        mfn_t mfn;
>>>> +        p2m_type_t t;
>>>> +        p2m_access_t a;
>>>> +
>>>> +        gfn = pml_buf[pml_idx] >> PAGE_SHIFT;
>>>> +        mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
>>>> +        if ( mfn_x(mfn) == INVALID_MFN )
>>>> +        {
>>>> +            /*
>>>> +             * Either EPT table entry for mapping the GFN has been
>>>> destroyed, or
>>>> +             * there's something wrong with hardware behavior, in
>>>> both cases we
>>>> +             * should report a warning.
>>>> +             */
>>>> +            dprintk(XENLOG_WARNING, "PML: vcpu %d: invalid GPA
>>>> 0x%lx logged\n",
>>>> +                    v->vcpu_id, pml_buf[pml_idx]);
>>> It would be shorter to log gfn rather than gpa.
>> Will do. And I'd also like to add the domain ID in the warning info.
>
> Ah of course - dprintk() doesn't identify current().  Use %pv with v.
>
>>
>>>
>>>> +{
>>>> +    return (d->arch.hvm_domain.vmx.status & VMX_DOMAIN_PML_ENABLED)
>>>> ? 1 : 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * This function enables PML for particular domain. It should be
>>>> called when
>>>> + * domain is paused.
>>> In which case assert that the domain is paused, or call domain_pause()
>>> yourself to take an extra pause refcount.
>> Which function should I use to assert domain is paused? I didn't find
>> a function like "domain_paused". Is below good enough?
>>
>> ASSERT(atomic_read(&d->pause_count));
>
> Hmm - we indeed don't have an appropriate helper.  That ASSERT() will do
> for now.
>
> ~Andrew
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



-- 
Thanks,
-Kai

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