Re: [Xen-devel] [PATCH v4 12/21] xen/passthrough: iommu: Split generic IOMMU code

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 04/23/2014 09:43 AM, Jan Beulich wrote:
>>>> On 22.04.14 at 20:02, <julien.grall@xxxxxxxxxx> wrote:
>> On 04/22/2014 05:59 PM, Jan Beulich wrote:
>>>>>> On 22.04.14 at 18:45, <julien.grall@xxxxxxxxxx> wrote:
>>>> On 04/22/2014 05:33 PM, Jan Beulich wrote:
>>>>>>>> On 22.04.14 at 16:58, <julien.grall@xxxxxxxxxx> wrote:
>>>>>>>> +void __hwdom_init arch_iommu_check_hwdom_reqs(struct domain *d)
>>>>>>>> +{
>>>>>>>> +    if ( !iommu_enabled )
>>>>>>>> +        panic("Presently, iommu must be enabled for pvh dom0\n");
>>>>>>>> +}
>>>>>>>
>>>>>>> Message text (containing PVH) and function name (not containing
>>>>>>> PVH) don't fit together, nor does the conditional really establish a
>>>>>>> connection.
>>>>>>
>>>>>> Do you prefer a comment, or an explicit check to is_pvh_domain(d)?
>>>>>
>>>>> That depends on where it would go: If the caller checks for PVH, then
>>>>> the function name should change. If the caller doesn't, then I don't
>>>>> see how you'd avoid getting here for non-PVH.
>>>>
>>>> The caller will go there when the DOM0 is auto-translated (i.e PVH as
>>>> dom0 can't be an HVM).
>>>>
>>>> I can remove PVH from the log, but for the user it's not accurate.
>>>
>>> In which case the function name should reflect this.
>>
>> What about arch_iommu_check_autotranslate_hwdom_reqs?
> 
> Getting quite long, but seems okay. Perhaps drop the _reqs?

Sounds good to me. I will drop it for the next version.

Regards,

-- 
Julien Grall

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