Re: [Xen-devel] [PATCH V3 26/29] x86/vvtd: Handle interrupt translation faults

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.10.17 at 12:08, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Oct 20, 2017 at 01:54:15PM +0800, Chao Gao wrote:
>> On Thu, Oct 19, 2017 at 05:31:37PM +0100, Roger Pau Monné wrote:
>> >On Thu, Sep 21, 2017 at 11:02:07PM -0400, Lan Tianyu wrote:
>> >> +static int vvtd_alloc_frcd(struct vvtd *vvtd)
>> >> +{
>> >> +    int prev;
>> >> +    uint64_t cap = vvtd_get_reg(vvtd, DMAR_CAP_REG);
>> >> +    unsigned int base = cap_fault_reg_offset(cap);
>> >> +
>> >> +    /* Set the F bit to indicate the FRCD is in use. */
>> >> +    if ( !vvtd_test_and_set_bit(vvtd,
>> >> +                                base + vvtd->status.fault_index * 
>> >> DMA_FRCD_LEN +
>> >> +                                DMA_FRCD3_OFFSET, DMA_FRCD_F_SHIFT) )
>> >> +    {
>> >> +        prev = vvtd->status.fault_index;
>> >> +        vvtd->status.fault_index = (prev + 1) % cap_num_fault_regs(cap);
>> >> +        return vvtd->status.fault_index;
>> >
>> >I would prefer that you return the index as an unsigned int parameter
>> >passed by reference rather than as the return value of the function,
>> >but that might not be the preference of others.
>> 
>> What are the pros and cons?
> 
> I personally don't like return values that have different meanings
> depending on it's sign. Here < 0 means error, while >= 0 is used to
> deliver some information.

We do this in various places, so as long as the (theoretical) value
range of valid positive values isn't too large, I'd be fine with this
approach to be used here as well.

Jan

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