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

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 03/19/2014 05:02 AM, Jan Beulich wrote:
>>>> On 18.03.14 at 22:34, Daniel De Graaf <dgdegra@tycho.nsa.gov> 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.

>> --- 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.

>
>> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
>> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
>> @@ -36,7 +36,7 @@
>>    * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0
>>    * 1:1 iommu mappings except xen and unusable regions.
>>    */
>> -static bool_t __initdata iommu_inclusive_mapping = 1;
>> +static bool_t __read_mostly iommu_inclusive_mapping = 1;
>
> Same here?
>
> Jan
>
>
>


-- 
Daniel De Graaf
National Security Agency