Re: [Minios-devel] [UNIKRAFT PATCHv7 4/8] plat/common: Add fdt address translation support

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

Hi Sharan

> -----Original Message-----
> From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> Sent: 2019年7月8日 16:31
> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Cc: Florian Schmidt <florian.schmidt@xxxxxxxxx>; Felipe Huici
> <felipe.huici@xxxxxxxxx>; Julien Grall <Julien.Grall@xxxxxxx>;
> yuri.volchkov@xxxxxxxxx; Kaly Xin (Arm Technology China)
> <Kaly.Xin@xxxxxxx>
> Subject: Re: [UNIKRAFT PATCHv7 4/8] plat/common: Add fdt address
> translation support
>
> Hello Justin He,
>
>
> This patch is fine except for a minor comment, please find it inline.
>

Ok, will fix in v9. Thanks for the pointing

> This patch has compiler error because of the missing include path from
> patch 2.
Is this compiling error?
/root/hj/UK/unikraft_upstream/unikraft/plat/drivers/ofw/fdt.c:38:10: fatal 
error: ofw/fdt.h: No such file or directory
 #include <ofw/fdt.h>
          ^~~~~~~~~~~

If *yes*, I had changed it in v8 [1], add sth in plat/kvm/Makefile.uk
[1] 
https://lists.xenproject.org/archives/html/minios-devel/2019-07/msg00035.html


--
Cheers,
Justin (Jia He)


>
> Thanks & Regards
> Sharan
>
> On 6/27/19 9:55 AM, Jia He wrote:
> > The ranges property provides a means of defining a mapping or
> > translation between the address space of the bus (the child
> > address space) and the address space of the bus node's parent
> > (the parent address space).
> >
> > Currently only 1:1 mapping between parent and child address is
> > supported.
> >
> > Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> > Signed-off-by: Jia He <justin.he@xxxxxxx>
> > ---
> >   plat/drivers/include/ofw/fdt.h |  18 +++++
> >   plat/drivers/ofw/fdt.c         | 120 +++++++++++++++++++++++++++++++++
> >   2 files changed, 138 insertions(+)
> >
> > diff --git a/plat/drivers/include/ofw/fdt.h b/plat/drivers/include/ofw/fdt.h
> > index 7d40fba..b84c184 100644
> > --- a/plat/drivers/include/ofw/fdt.h
> > +++ b/plat/drivers/include/ofw/fdt.h
> > @@ -35,6 +35,8 @@
> >   #ifndef _PLAT_DRIVER_OFW_FDT_H
> >   #define _PLAT_DRIVER_OFW_FDT_H
> >
> > +#define FDT_BAD_ADDR (uint64_t)(-1)
> > +
> >   /**
> >    * fdt_getprop_u32_by_offset - retrieve u32 of a given property
> >    * @fdt: pointer to the device tree blob
> > @@ -76,4 +78,20 @@ int fdt_getprop_u32_by_offset(const void *fdt, int
> nodeoffset,
> >    */
> >   int fdt_interrupt_cells(const void *fdt, int nodeoffset);
> >
> > +/*
> > + * read and combine the big number of reg, caller needs to make sure size
> > + * is correct
> > + */
> > +static inline uint64_t fdt_reg_read_number(const fdt32_t *regs, uint32_t
> size)
> > +{
> > +uint64_t number = 0;
> > +
> > +for (uint32_t i = 0; i < size; i++) {
> > +number <<= 32;
> > +number |= fdt32_to_cpu(*regs);
> > +regs++;
> > +}
> > +
> > +return number;
> > +}
> >   #endif
> > diff --git a/plat/drivers/ofw/fdt.c b/plat/drivers/ofw/fdt.c
> > index a596df3..075e542 100644
> > --- a/plat/drivers/ofw/fdt.c
> > +++ b/plat/drivers/ofw/fdt.c
> > @@ -35,9 +35,14 @@
> >   #include <libfdt_env.h>
> >   #include <fdt.h>
> >   #include <libfdt.h>
> > +#include <ofw/fdt.h>
> >
> >   #include <uk/print.h>
> >
> > +#define FDT_MAX_ADDR_CELLS FDT_MAX_NCELLS
> > +#define FDT_CHECK_COUNTS(na, ns)  ((na) > 0 && (na) <=
> FDT_MAX_ADDR_CELLS && \
> > +(ns) > 0)
> > +
> >   int fdt_getprop_u32_by_offset(const void *fdt, int offset,
> >   const char *name, uint32_t *out)
> >   {
> > @@ -100,3 +105,118 @@ int fdt_interrupt_cells(const void *fdt, int offset)
> >
> >   return val;
> >   }
> > +
> > +/* Default translator (generic bus) */
> > +static void fdt_default_count_cells(const void *fdt, int parentoffset,
> > +       int *addrc, int *sizec)
> > +{
> > +if (addrc)
> > +*addrc = fdt_address_cells(fdt, parentoffset);
> > +
> > +if (sizec)
> > +*sizec = fdt_size_cells(fdt, parentoffset);
> > +}
> > +
> > +static int fdt_default_translate(fdt32_t *addr, uint64_t offset, int na)
> > +{
> > +uint64_t a = fdt_reg_read_number(addr, na);
> > +
> > +memset(addr, 0, na * sizeof(fdt32_t));
> > +a += offset;
> > +if (na > 1)
> > +addr[na - 2] = cpu_to_fdt32(a >> 32);
> > +addr[na - 1] = cpu_to_fdt32(a & 0xffffffffu);
> > +
> > +return 0;
> > +}
> > +
> > +static int fdt_translate_one(const void *fdt, int parent, fdt32_t *addr,
> > +    int na, int pna, const char *rprop)
> > +{
> > +const fdt32_t *ranges;
> > +int rlen;
> > +uint64_t offset = FDT_BAD_ADDR;
> > +
> > +ranges = fdt_getprop(fdt, parent, rprop, &rlen);
> > +if (!ranges)
> > +return 1;
> > +if (rlen == 0) {
> > +offset = fdt_reg_read_number(addr, na);
>
> We doing the memset in the "fdt_default_translate" function. So it may
> not be necessary to do it here as well.
> > +memset(addr, 0, pna * 4);
> > +uk_pr_debug("empty ranges, 1:1 translation\n");
> > +goto finish;
> > +}
> > +
> > +uk_pr_err("Error, only 1:1 translation is supported...\n");
> > +return 1;
> > + finish:
> > +uk_pr_debug("with offset: 0x%lx\n", offset);
> > +
> > +/* Translate it into parent bus space */
> > +return fdt_default_translate(addr, offset, pna);
> > +}
> > +
> > +/*
> > + * Translate an address from the device-tree into a CPU physical address,
> > + * this walks up the tree and applies the various bus mappings on the
> > + * way.
> > + */
> > +static uint64_t fdt_translate_address_by_ranges(const void *fdt,
> > +int node_offset, const fdt32_t *regs)
> > +{
> > +int parent;
> > +fdt32_t addr[FDT_MAX_ADDR_CELLS];
> > +int na, ns, pna, pns;
> > +uint64_t result = FDT_BAD_ADDR;
> > +
> > +/* Get parent */
> > +parent = fdt_parent_offset(fdt, node_offset);
> > +if (parent < 0)
> > +goto bail;
> > +
> > +/* Count address cells & copy address locally */
> > +fdt_default_count_cells(fdt, parent, &na, &ns);
> > +if (!FDT_CHECK_COUNTS(na, ns)) {
> > +uk_pr_err("Bad cell count for %s\n",
> > +       fdt_get_name(fdt, node_offset, NULL));
> > +goto bail;
> > +}
> > +memcpy(addr, regs, na * 4);
> > +
> > +/* Translate */
> > +for (;;) {
> > +/* Switch to parent bus */
> > +node_offset = parent;
> > +parent = fdt_parent_offset(fdt, node_offset);
> > +
> > +/* If root, we have finished */
> > +if (parent < 0) {
> > +uk_pr_debug("reached root node\n");
> > +result = fdt_reg_read_number(addr, na);
> > +break;
> > +}
> > +
> > +/* Get new parent bus and counts */
> > +fdt_default_count_cells(fdt, parent, &pna, &pns);
> > +if (!FDT_CHECK_COUNTS(pna, pns)) {
> > +uk_pr_err("Bad cell count for %s\n",
> > +fdt_get_name(fdt, node_offset, NULL));
> > +break;
> > +}
> > +
> > +uk_pr_debug("parent bus (na=%d, ns=%d) on %s\n",
> > + pna, pns, fdt_get_name(fdt, parent, NULL));
> > +
> > +/* Apply bus translation */
> > +if (fdt_translate_one(fdt, node_offset,
> > +addr, na, pna, "ranges"))
> > +break;
> > +
> > +/* Complete the move up one level */
> > +na = pna;
> > +ns = pns;
> > +}
> > +bail:
> > +return result;
> > +}
> > +
> >
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel