Re: [Xen-devel] [PATCH v4 13/23] xen/arm: implement construct_domU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 20 Oct 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 10/19/18 11:53 PM, Stefano Stabellini wrote:
> > On Mon, 15 Oct 2018, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 05/10/2018 19:47, Stefano Stabellini wrote:
> > > > Similar to construct_dom0, construct_domU creates a barebone DomU guest.
> > > > 
> > > > The device tree node passed as argument is compatible "xen,domain", see
> > > > docs/misc/arm/device-tree/booting.txt.
> > > > 
> > > > Add const to kernel_probe dt_device_node parameter.
> > > 
> > > This likely belongs to patch #7 where the parameter was added.
> > 
> > OK
> > 
> > 
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > > > 
> > > > ---
> > > > Changes in v4:
> > > > - constify kernel_probe
> > > > - change title
> > > > - better error messages and printed info
> > > > - 64bit memory
> > > > 
> > > > Changes in v3:
> > > > - move setting type before allocate_memory
> > > > - add ifdef around it and a comment
> > > > 
> > > > Changes in v2:
> > > > - rename mem to memory
> > > > - make cpus and memory mandatory
> > > > - remove wront comment from commit message
> > > > - cpus and memory are read as integers
> > > > - read the vpl011 option
> > > > ---
> > > >    xen/arch/arm/domain_build.c | 37
> > > > ++++++++++++++++++++++++++++++++++---
> > > >    xen/arch/arm/kernel.c       |  3 ++-
> > > >    xen/arch/arm/kernel.h       |  2 +-
> > > >    3 files changed, 37 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > > index 547b624..efb530a 100644
> > > > --- a/xen/arch/arm/domain_build.c
> > > > +++ b/xen/arch/arm/domain_build.c
> > > > @@ -369,7 +369,6 @@ static void __init allocate_memory_11(struct domain
> > > > *d,
> > > >        }
> > > >    }
> > > >    -#if 0
> > > 
> > > Please add a word about this change in the commit message.
> > 
> > OK
> > 
> > 
> > > >    static bool __init allocate_bank_memory(struct domain *d,
> > > >                                            struct kernel_info *kinfo,
> > > >                                            gfn_t sgfn,
> > > > @@ -450,7 +449,6 @@ fail:
> > > >                (unsigned long)kinfo->unassigned_mem >> 10);
> > > >        BUG();
> > > >    }
> > > > -#endif
> > > >      static int __init write_properties(struct domain *d, struct
> > > > kernel_info
> > > > *kinfo,
> > > >                                       const struct dt_device_node *node)
> > > > @@ -2294,7 +2292,40 @@ static int __init __construct_domain(struct
> > > > domain
> > > > *d, struct kernel_info *kinfo
> > > >    static int __init construct_domU(struct domain *d,
> > > >                                     const struct dt_device_node *node)
> > > >    {
> > > > -    return -ENOSYS;
> > > > +    struct kernel_info kinfo = {};
> > > > +    int rc;
> > > > +    u64 mem;
> > > > +
> > > > +    rc = dt_property_read_u64(node, "memory", &mem);
> > > > +    if ( !rc )
> > > > +    {
> > > > +        printk("Error building DomU: cannot read \"memory\"
> > > > property\n");
> > > > +        return -EINVAL;
> > > > +    }
> > > > +    kinfo.unassigned_mem = (paddr_t)mem << 10;
> > > 
> > > I noticed I forgot to answer to:
> > > "KB() only works for numbers, it is defined as: (_AC(_kb, ULL) << 10)"
> > > 
> > > unsigned long long is always going to be bigger than paddr_t. Also, we
> > > already
> > > use MB(...) in similar situation. So I am not sure to understand your
> > > concern
> > > here.
> > 
> > I admit that my explanation was so bad that even I had to go back to
> > figure out what I meant :-)
> > 
> > What I wanted to say is that KB() only works for constants, not
> > variables. So KB(10) works, but KB(mem) does not.
> 
> Oh, that's annoying. Can you use mem * SZ_1K then?

OK

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