[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V4 6/8] iommu/arm: Add lightweight iommu_fwspec support



On 19.09.2019 12:12, Julien Grall wrote:
> Hi,
> 
> On 17/09/2019 19:18, Oleksandr wrote:
>>
>> On 17.09.19 09:12, Jan Beulich wrote:
>>
>> Hi, Jan
>>
>>> On 16.09.2019 20:08, Oleksandr wrote:
>>>> On 16.09.19 13:40, Jan Beulich wrote:
>>>>>> +/* per-device IOMMU instance data */
>>>>>> +struct iommu_fwspec {
>>>>>> +    /* this device's IOMMU */
>>>>>> +    struct device *iommu_dev;
>>>>>> +    /* IOMMU driver private data for this device */
>>>>>> +    void *iommu_priv;
>>>>>> +    /* number of associated device IDs */
>>>>>> +    unsigned int num_ids;
>>>>>> +    /* IDs which this device may present to the IOMMU */
>>>>>> +    uint32_t ids[1];
>>>>>> +};
>>>>> Note that you abuse xrealloc_flex_struct() when using it with such
>>>>> a type: The last field is _not_ a flexible array member. Compilers
>>>>> might legitimately warn if they can prove that you access
>>>>> p->ids[1] anywhere, despite you (presumably) having allocated enough
>>>>> space. (I haven't been able to think of a way for the macro to
>>>>> actually detect and hence refuse such wrong uses.)
>>>> Indeed, you are right. I am in doubt, whether to retain ported from
>>>> Linux code (ids[1])
>>>>
>>>> and mention about such abuse or change it to deal with real flexible
>>>> array member (ids[]). Any thoughts?
>>> I'm of the strong opinion that you should switch to [] (or at
>>> least [0]) notation.
>>
>> I got it. Well, will switch to ids[] if there are no objections.
> 
> I suspect the rationale to use 1 rather than 0 is to avoid the re-allocation 
> in 
> the common case where a device has a single ID.
> 
> I would like to retain the similar behavior. The ids[1] is probably the most 
> pretty way to do it.
> 
> Another solution would to use xmalloc_bytes() for the initial allocation of 
> xmalloc_bytes().

Why not use xmalloc_flex_<whatever>() on the first allocation, too?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.