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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> +static void vmx_sync_pir_to_irr(struct vcpu *v)
> +{
> +    struct vlapic *vlapic = vcpu_vlapic(v);
> +    u32 val;
> +    int offset, group;

unsigned.

> +
> +    if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) )
> +        return;
> +
> +    for (group = 0; group < 8; group++ )

Formatting.

> +    {
> +        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


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