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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.17 at 15:13, <chao.gao@xxxxxxxxx> wrote:
> On Mon, Oct 16, 2017 at 07:15:16AM -0600, Jan Beulich wrote:
>>>>> On 13.10.17 at 07:10, <chao.gao@xxxxxxxxx> 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?

Why "also" - being the return value is the only purpose of "vector".
And as said - zero is a valid vector, and I wouldn't like to see the
function return a valid but meaningless vector number.

>>> --- 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.e. meaning you're fine with either variant, leaving it up to me
which one to use?

Jan


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