Re: [Xen-devel] [PATCH 3/4] VMX: Add posted interrupt supporting

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2013-04-09:
>>>> On 09.04.13 at 08:01, Yang Zhang <yang.z.zhang@xxxxxxxxx> wrote:
>> +static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector, u8 trig_mode)
>> +{
>> +    vlapic_set_tmr(vcpu_vlapic(v), vector, trig_mode);
>> +
>> +    vmx_update_eoi_exit_bitmap(v, vector, trig_mode);
>> +
>> +    if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
>> +        return;
>> +
>> +    if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) )
>> +    {
>> +        /* If EOI exitbitmap needs to changed or notification vector
>> +         * can't be allocated, interrupt will not be injected till
>> +         * VMEntry as it used to be
>> +         */
>> +        pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);
> 
> Why is this a test-and-set when the result is unused?
>
> 
>> +        goto out;
>> +    }
>> +
>> +    if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
> 
> And if it's just because you don't want to have a simple "set" only
> operation, then the two instances should be folded, and the result
> stored in a local variable.
Yes. Only want to set on bit. Perhaps it is better to define a separate 
function pi_set_on() for this to make the logic more clear.

> 
>> +static void vmx_sync_pir_to_irr(struct vcpu *v)
>> +{
>> +    struct vlapic *vlapic = vcpu_vlapic(v);
>> +    u32 val;
>> +    int offset, group;
> 
> unsigned.
Sure.
 
>> +
>> +    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
>> +        return;
>> +
>> +    for (group = 0; group < 8; group++ )
> 
> Formatting.
Sure.

> 
>> +    {
>> +        val = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);
>> +        offset = APIC_IRR + 0x10 * group;
>> +        *((uint32_t *)(&vlapic->regs->data[offset])) |= val;
> 
> Can't you use vlapic_set_vector() here (even if that means
> looping over vectors individually rather than groups), to add the
> necessary atomicity (I don't see how you avoid races with other
> updates) and to avoid the ugly cast?
> 
> Jan


Best regards,
Yang



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