Re: [PATCH v2] x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR

[Chao Gao <chao.gao@intel.com>]

On Mon, Oct 16, 2017 at 07:15:16AM -0600, Jan Beulich wrote:
>>>> On 13.10.17 at 07:10, <chao.gao@intel.com> wrote:
>> --- a/xen/arch/x86/hvm/irq.c
>> +++ b/xen/arch/x86/hvm/irq.c
>> @@ -168,11 +168,13 @@ void hvm_gsi_deassert(struct domain *d, unsigned int gsi)
>>      spin_unlock(&d->arch.hvm_domain.irq_lock);
>>  }
>>  
>> -void hvm_isa_irq_assert(
>> -    struct domain *d, unsigned int isa_irq)
>> +int hvm_isa_irq_assert(struct domain *d, unsigned int isa_irq,
>> +                       int (*get_vector)(const struct domain *d,
>> +                                         unsigned int gsi))
>>  {
>>      struct hvm_irq *hvm_irq = hvm_domain_irq(d);
>>      unsigned int gsi = hvm_isa_irq_to_gsi(isa_irq);
>> +    int vector = 0;
>
>Why zero (which is valid aiui) instead of e.g. -1?

vector also serves as the return value. I want to return 0 if no
callback is set.  And the callback, get_vector, can override the return
value. Do you think it is reasonable?

>
>> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
>> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
>> @@ -109,6 +109,11 @@ static inline int pi_test_and_set_pir(int vector, struct pi_desc *pi_desc)
>>      return test_and_set_bit(vector, pi_desc->pir);
>>  }
>>  
>> +static inline int pi_test_pir(int vector, const struct pi_desc *pi_desc)
>
>This should not be a signed quantity - uint8_t or unsigned int
>please.

Yes.

>
>I wouldn't mind making suitable adjustments while committing (and
>then adding my R-b), but that requires your feedback which way
>things should be.

Sure. I will appreciate it.
>
>Also please don't forget to Cc the release manager, unless you
>intend this fix only for after 4.10.

Hi, Julien.

This patch is to fix a possible cause of an assertion failure related to
periodic timer interrupt. OSSTEST reports regression occasionally when the bug
happens. I intend to merge this patch at first and then observe whether
the bug disappears or not. Since Jan said he could do some adjustments to the
patch when committing, Could you give acked-by on this patch?

Thanks
Chao.

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