Re: [PATCH v1 1/2] xen/riscv: initialize bootinfo from dtb

[oleksii.kurochko@xxxxxxxxx]

On Tue, 2024-10-01 at 17:54 +0200, Jan Beulich wrote:
> On 30.09.2024 17:13, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/setup.c
> > +++ b/xen/arch/riscv/setup.c
> > @@ -28,6 +28,7 @@ void __init noreturn start_xen(unsigned long
> > bootcpu_id,
> >                                 paddr_t dtb_addr)
> >  {
> >      struct bootmodule *xen_bootmodule;
> > +    size_t fdt_size;
> >  
> >      remove_identity_mapping();
> >  
> > @@ -54,6 +55,9 @@ void __init noreturn start_xen(unsigned long
> > bootcpu_id,
> >  
> >      BUG_ON(!xen_bootmodule);
> >  
> > +    fdt_size = boot_fdt_info(device_tree_flattened, dtb_addr);
> > +    BUG_ON(!fdt_size);
> > +
> >      printk("All set up\n");
> >  
> >      machine_halt();
> 
> Looks plausible, and judging from Arm code there'll be an actual use
> of fdt_size
> down the road. Or else I would have asked what use the local variable
> is. In
> fact, again from looking at Arm code, I now question the need for the
> xen_bootmodule local var - that has no further use in Arm, and hence
> that's
> likely going to be the case for RISC-V, too.
fdt_size and xen_bootmodule are used only for BUG_ON() now and in
future ( the same as you mentioned as in Arm ). They could be dropped.
I'll do that in the next patch version.

> 
> However, may I ask that you limit the dependency trees in what you
> submit? This
> series depends on another series, which in turn depends on yet
> something else,
> all not yet committed. That's getting unwieldy, I'm afraid.
Sure, sorry for that. I will limit the depedency trees in future.

~ Oleksii