[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Fri, Jun 24, 2016 at 01:34:29PM -0400, 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:
> >>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!
> 
> The dependency on checkpolicy is called out in the Kconfig item that
> enables this option.  Are you suggesting I should add a mention below
> the instructions on running menuconfig for XSM in INSTALL?

No, I just thinking that the INSTALL has a list of packages one needs.
And it may be good to have checkpolicy on it.

> 
> I can remove the HAS_CHECKPOLICY check completely and make the call to
> checkpolicy only conditional on the Kconfig option.  I think this is
> less complicated than stopping the compile one step above the invocation
> of checkpolicy, and probably just as informative (and better, if the
> detection heuristic ever breaks).

I actually like the way you have it - with the checkpolicy check determining
whether the Kconfig option for XSM is shown or not.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.