Re: [Xen-devel] [PATCH 2/7] Nested VMX: Allow to ack irq even virtual intr delivery is enabled

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

Jan Beulich wrote on 2013-08-09:
>>>> On 09.08.13 at 10:49, Yang Zhang <yang.z.zhang@xxxxxxxxx> wrote:
>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> 
>> In some special cases, we want to ack irq regardless of virtual
>> interrupt delivery.
> 
> Again, the whole change is meaningless. I can see reasons to break out
> such preparatory changes when otherwise the resulting patch would be
> huge and hard to review. That doesn't seem to be the case here; it
> rather looks like the splitting was done pretty arbitrarily here.
No, splitting the patch is not only for easy reviewing. It is useful for "git 
bisect" and debug purpose. Also, if the change is independent, we also should 
to split it into a separate patch. 
Here, though the change is minimal, it touches the key interrupt handle logic. 
It's better to put it into a single patch.
        
> Jan
> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> ---
>>  xen/arch/x86/hvm/irq.c           |    2 +-
>>  xen/arch/x86/hvm/vlapic.c        |    4 ++--
>>  xen/include/asm-x86/hvm/vlapic.h |    2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c index
>> 9eae5de..6a6fb68 100644
>> --- a/xen/arch/x86/hvm/irq.c
>> +++ b/xen/arch/x86/hvm/irq.c
>> @@ -437,7 +437,7 @@ struct hvm_intack hvm_vcpu_ack_pending_irq(
>>              intack.vector = (uint8_t)vector;
>>          break;
>>      case hvm_intsrc_lapic:
>> -        if ( !vlapic_ack_pending_irq(v, intack.vector) )
>> +        if ( !vlapic_ack_pending_irq(v, intack.vector, 0) )
>>              intack = hvm_intack_none;
>>          break;
>>      case hvm_intsrc_vector:
>> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
>> index 7a154f9..20a36a0 100644
>> --- a/xen/arch/x86/hvm/vlapic.c
>> +++ b/xen/arch/x86/hvm/vlapic.c
>> @@ -1048,11 +1048,11 @@ int vlapic_has_pending_irq(struct vcpu *v)
>>      return irr;
>>  }
>> -int vlapic_ack_pending_irq(struct vcpu *v, int vector)
>> +int vlapic_ack_pending_irq(struct vcpu *v, int vector, int
>> +force_ack)
>>  {
>>      struct vlapic *vlapic = vcpu_vlapic(v);
>> -    if ( vlapic_virtual_intr_delivery_enabled() )
>> +    if ( vlapic_virtual_intr_delivery_enabled() && !force_ack )
>>          return 1;
>>      vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]); diff
>> --git a/xen/include/asm-x86/hvm/vlapic.h
>> b/xen/include/asm-x86/hvm/vlapic.h
>> index 021a5f2..d8c9511 100644
>> --- a/xen/include/asm-x86/hvm/vlapic.h
>> +++ b/xen/include/asm-x86/hvm/vlapic.h
>> @@ -96,7 +96,7 @@ bool_t is_vlapic_lvtpc_enabled(struct vlapic
>> *vlapic);  void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec,
>> uint8_t trig);
>> 
>>  int vlapic_has_pending_irq(struct vcpu *v); -int
>> vlapic_ack_pending_irq(struct vcpu *v, int vector);
>> +int vlapic_ack_pending_irq(struct vcpu *v, int vector, int
>> +force_ack);
>> 
>>  int  vlapic_init(struct vcpu *v);
>>  void vlapic_destroy(struct vcpu *v);
>> --
>> 1.7.1
> 
>


Best regards,
Yang



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