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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:
> Hello Daniel,
> 
> Please try to CC relevant maintainers on your patch. I would have missed it
> if Andrew did not ping me on IRC.
> 
> On 20/06/16 15:04, Daniel De Graaf wrote:
> >This adds a Kconfig option and support for including the XSM policy from
> >tools/flask/policy in the hypervisor so that the bootloader does not
> >need to provide a policy to get sane behavior from an XSM-enabled
> >hypervisor.  The policy provided by the bootloader, if present, will
> >override the built-in policy.
> >
> >Enabling this option only builds the policy if checkpolicy is available
> >during compilation of the hypervisor; otherwise, it does nothing.  The
> >XSM policy is not moved out of tools because that remains the primary
> >location for installing and configuring the policy.
> >
> >Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> >Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> For ARM bits:
> 
> Acked-by: Julien Grall <julien.grall@xxxxxxx>
> 
> Although, I one a question below.
> 
> [...]
> 
> >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!

> 
> >+endif
> 
> Regards,
> 
> -- 
> Julien Grall

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