Re: [Xen-devel] [PATCH v5 09/22] xen/arm: ITS: Export ITS info to Virtual ITS

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Vijay,

On 31/07/15 08:01, Vijay Kilari wrote:
>> The vITS will only expose 1 ITS to dom0. So there is no need to try to
>> allocate memory in order to expose all the ITS.
>>
>> We can extend vits_setup_hw if we ever want to support more ITS in the
>> future.
>>
>> Furthermore I'd like to see all the ITS data pass one by one rather than
>> in a structure. This would help to catch error if we ever extend the
>> structure.
>>
>> Something like:
>>
>> void vits_hw_setup(paddr_t its0_base, unsigned long its0_size,
>>                    uint32_t eventID_bits, uint32_t devID_bits);
>>
> 
>    Passing every value as argument makes this function take too many 
> arguments.
> Validation can be done member variable to catch errors.

Can you explain why you think that 4 arguments (based on my suggestion)
is too many? IHMO it's more readable to have the argument passed
explicitly rather than pack in a structure that would make the extension
more error-prone. FYI, we use 4 arguments for vgic_v3_hw_setup too.

Furthermore, you can't validate properly the field unless even if you
assume that the guest will zeroed the structure before passing it.

Regards,

-- 
Julien Grall

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