Re: [PATCH 17/17] xsm: add a default policy to .init.data

[Julien Grall <julien.grall@arm.com>]

Hi Daniel,

On 24/06/16 18:34, Daniel De Graaf wrote:
> On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:
>> On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:
>>>> diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
>>>> index 12fc3a9..eefd37c 100644
>>>> --- a/xen/xsm/flask/Makefile
>>>> +++ b/xen/xsm/flask/Makefile
>>>> @@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
>>>>  $(AV_H_FILES): $(AV_H_DEPEND)
>>>>      $(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
>>>>
>>>> +ifeq ($(CONFIG_XSM_POLICY),y)
>>>> +HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen &&
>>>> echo y || echo n)
>>>> +
>>>> +obj-$(HAS_CHECKPOLICY) += policy.o
>>>
>>> I would have expect a warning (if not an error) here to tell the user
>>> that
>>> checkpolicy is not available. Otherwise it may take some time to the
>>> user to
>>> understand why the policy is not loaded/present. Because if you
>>> enable XSM,
>>> you don't necessarily check which other options have been enabled by
>>> default.
>>
>> Good point! And we should probably update the INSTALL document too to
>> mention
>> that you need checkpoint tool!
>
> The dependency on checkpolicy is called out in the Kconfig item that
> enables this option.

AFAICT, the dependency is only called out for CONFIG_XSM_POLICY. 
However, this option is enabled by default if CONFIG_XSM is selected.

User may not read what was enabled by default because of CONFIG_XSM 
selected.

In any case, silently disable an option is a bad idea and will likely 
cause trouble in the future if we ever decide to enable XSM by default. 
Another use case would be, a user post his .config on the ML asking why 
the policy is not loaded.

Regards,

-- 
Julien Grall

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