Re: [Xen-devel] [PATCH] docs/arm64: update the documention for loading XSM support

[Julien Grall <julien.grall@xxxxxxx>]

Hello Fu Wei,

On 14/04/16 18:06, fu.wei@xxxxxxxxxx wrote:
> From: Fu Wei <fu.wei@xxxxxxxxxx>
>
> This patch updates the documention for loading XSM by the module which
> lacks a specific compatible string.

s/documention/documentation/
s/which/that/

The sentence is not clear to me. I would rephrase:

"This patch updates the documentation for allowing detection of an XSM 
module that lacks a specific compatible string".
> This mechanism has been added by the
> commit ca32012341f3de7d3975407fb963e6028f0d0c8b
Missing full stop.

> Signed-off-by: Fu Wei <fu.wei@xxxxxxxxxx>
> ---
>   docs/misc/arm/device-tree/booting.txt | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index ad98bf3..f45a9c4 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -24,10 +24,19 @@ Each node contains the following properties:
>         string (which must always be present).
>
>         Xen will assume that the first module which lacks a more
> -       specific compatible string is a "multiboot,kernel" and that
> -       the second such is a "multiboot,ramdisk". Any subsequent
> -       modules which lack a specific compatiblity string will not
> -       receive any special treatment.
> +       specific compatible string is a "multiboot,kernel". Xen will
> +       detect the XSM magic from the second module which lacks of
> +       a specific compatiblity string:
s/compatiblity/

The sentence is not clear. You could read as: "Xen will check all the 
modules from the second module that lacks a specific compatible string".
I.e Xen will do the XSM magic check even if the module has a specific 
compatible string.
I would instead say "For the second module that lacks a specific 
compatible string, Xen will check if the module is a XSM policy:
> +       - if it's XSM, Xen will assume that the second such is a
"second such" is not clear.

> +       "xen,xsm-policy". and also assume user won't load ramdisk;
s/.//

I would drop the rest of the sentence after "and".

> +       - if it's not XSM, Xen will assume that the second such is a
"second such" is not clear.

> +       "multiboot,ramdisk".
> +       So if user want to load ramdisk without a specific compatiblity,
s/user/the user/
s/compatiblity/compatibility/

However this is a documentation for the user. So I would say "This means 
that if the ramdisk module is present and does not have a the compatible 
string "multiboot,ramdisk", then it must be always the second module".
> +       it must be the 2nd one.
> +       Xen will also detect the XSM Magic for the following modules
> +       which lack of a specific compatiblity, and assume that the module
s/compatiblity/compatibility/

> +       is a "xen,xsm-policy" or "multiboot,module", according to the
> +       result of detection.
s/or/or a/
s/detection/the detection/

I would also invert the two paragraphs. I.e inverting "So if user.." and 
"Xen will...".
Can you also mention that this behavior was introduced by Xen 4.7. I.e 
Xen 4.6 (and downwards) still requires the module to have the compatible 
string "xen,xsm-policy" and therefore XSM won't work with GRUB.
The latter bits may need to be documented in GRUB.

>         Xen 4.4 supported a different set of legacy compatible strings
>         which remain supported such that systems supporting both 4.4
Regards,

--
Julien Grall

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