[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 12/21] xen/passthrough: iommu: Split generic IOMMU code
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |