Re: [Xen-devel] [PATCH] arm64/xen: Add missing #address-cells and #size-cells properties

["Szyfelbein, Wojciech M" <wojciech.m.szyfelbein@xxxxxxxxx>]

-----Original Message-----
From: Grub-devel 
[mailto:grub-devel-bounces+wojciech.m.szyfelbein=intel.com@xxxxxxx] On Behalf 
Of Daniel Kiper
Sent: Thursday, November 30, 2017 10:23 PM
To: Julien Grall <julien.grall@xxxxxxxxxx>
Cc: grub-devel@xxxxxxx; fu.wei@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx
Subject: Re: [PATCH] arm64/xen: Add missing #address-cells and #size-cells 
properties

On Thu, Nov 30, 2017 at 01:22:37PM +0000, Julien Grall wrote:
> Hi Daniel,
>
> On 30/11/17 13:06, Daniel Kiper wrote:
> >On Wed, Nov 29, 2017 at 05:08:12PM +0000, Julien Grall wrote:
> >>The properties #address-cells and #size-cells are used to know the 
> >>number of cells for ranges provided by "regs". If they don't exist, 
> >>the value are resp. 2 and 1.
> >>
> >>Currently, when multiboot nodes are created it is assumed that 
> >>#address-cells
> >
> >IIRC ARM boot protocol is not related to Multiboot protocol in any way.
> >So, calling it in that way is very confusing. Could you invent a 
> >better not confusion name. Or at least provide a spec. I am happy to 
> >see it in GRUB2 tree.
>
> That's the name of the node see the MODULE_CUSTOM_COMPATIBLE in the 
> code... But this it not the Linux Arm boot protocol, it is an 
> extension currently only used by Xen. See [1].

Oh, boy! It is unfortunate. Sadly it looks that we have to live with this right 
now... Too late for change... I would just add the comment to this file that 
this is not related to the Multiboot protocol family in any way.

> >>and #size-cells are exactly 2. However, they are never set by GRUB 
> >>and will result to later failure when the device-tree is generated 
> >>by GRUB or contain different values.
> >>
> >>To prevent this failure, create the both properties in the chosen nodes.
> >>
> >>Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> >>---
> >>  grub-core/loader/arm64/xen_boot.c | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >>diff --git a/grub-core/loader/arm64/xen_boot.c 
> >>b/grub-core/loader/arm64/xen_boot.c
> >>index c95d6c5a8..6780b1f0c 100644
> >>--- a/grub-core/loader/arm64/xen_boot.c
> >>+++ b/grub-core/loader/arm64/xen_boot.c
> >>@@ -115,6 +115,17 @@ prepare_xen_hypervisor_params (void *xen_boot_fdt)
> >>    if (chosen_node < 1)
> >>      return grub_error (GRUB_ERR_IO, "failed to get chosen node in 
> >>FDT");
> >>
> >>+  /*
> >>+   * The address and size are always written using 64-bits value. 
> >>+ Set
> >
> >Here you say "64-bits value"...
> >
> >>+   * #address-cells and #size-cells accordingly.
> >>+   */
> >>+  retval = grub_fdt_set_prop32 (xen_boot_fdt, chosen_node, 
> >>+ "#address-cells", 2);
> >
> >...and then call grub_fdt_set_prop32(). I am confused...
>
> #address-cells and #size-cells are property to know the number of 
> cells per address/size.
>
> The address and size are 64-bit (i.e 2 cells) as you can see the call 
> to grub_fdt_set_reg64 in the prepare_xen_module_params.

Understood! I will apply this patch next week.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@xxxxxxx
https://lists.gnu.org/mailman/listinfo/grub-devel
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel