Re: [Xen-devel] [PATCH 2/7] xen/iommu: Move dom0 setup code to __hwdom_init

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.03.14 at 15:06, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> On 03/19/2014 05:02 AM, Jan Beulich wrote:
>>>>> On 18.03.14 at 22:34, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -41,6 +41,10 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
>>>   ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
>>>   ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
>>>
>>> +ifeq ($(x86),y)
>>> +LATE_HWDOM_ENABLE ?= $(XSM_ENABLE)
>>> +endif
>>> +
>>>   CFLAGS += -fno-builtin -fno-common
>>>   CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
>>>   CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
>>> @@ -49,6 +53,7 @@ CFLAGS += -nostdinc
>>>   CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
>>>   CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
>>>   CFLAGS-$(FLASK_ENABLE)  += -DFLASK_DEVELOP -DFLASK_BOOTPARAM 
>>> -DFLASK_AVC_STATS
>>> +CFLAGS-$(LATE_HWDOM_ENABLE) += -DLATE_HWDOM_ENABLE
>>
>> You don't really need the make level definition, and it would seem
>> better to me to set CONFIG_LATE_HWDOM in asm/config.h instead
>> (we should really try to avoid adding further -D options here, and
>> instead see to remove some of what's there already - only options
>> needed at the make level _and_ at the source level are imo valid
>> candidates to go here).
> 
> OK. I think I can clean up some of the other XSM defines in that case,
> especially as it is no longer reasonable to enable XSM without FLASK.
> That change will be independent of this patch series, however.

Of course.

>>> --- a/xen/drivers/passthrough/iommu.c
>>> +++ b/xen/drivers/passthrough/iommu.c
>>> @@ -45,7 +45,7 @@ custom_param("iommu", parse_iommu_param);
>>>   bool_t __initdata iommu_enable = 1;
>>>   bool_t __read_mostly iommu_enabled;
>>>   bool_t __read_mostly force_iommu;
>>> -bool_t __initdata iommu_dom0_strict;
>>> +bool_t __read_mostly iommu_dom0_strict;
>>
>> Wouldn't this rather be __hwdom_initdata now?
> 
> I was trying to avoid making a __hwdom_initdata_or_read_mostly symbol to
> just save two bytes of theoretical memory (with zero actual savings due
> to alignment).  I can do that if it's preferred.

No need to - just use __hwdom_initdata. Depending on the other
uses of it, it might be appropriate to always make this an alias of
__read_mostly in the XSM-enabled case.

Jan


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