From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <stefanos@xilinx.com>,
Stefano Stabellini <sstabellini@kernel.org>,
andrii_anisov@epam.com, Achin.Gupta@arm.com,
Ian Jackson <ian.jackson@eu.citrix.com>,
xen-devel@lists.xen.org, Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH 1/5] xen/arm: copy dtb fragment to guest dtb
Date: Wed, 2 Jan 2019 15:25:06 -0800 (PST) [thread overview]
Message-ID: <alpine.DEB.2.10.1901021519210.800@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <bb30fde0-8870-d433-c0fe-5c7cad5aade7@arm.com>
On Mon, 24 Dec 2018, Julien Grall wrote:
> (+ Wei and Ian)
>
> Hi Stefano,
>
> On 12/5/18 5:28 PM, Stefano Stabellini wrote:
> > Read the dtb fragment corresponding to a passthrough device from memory
> > at the location referred to by the "multiboot,dtb" compatible node.
> >
> > Copy the fragment to the guest dtb.
> >
> > Add a dtb_bootmodule field to struct kernel_info to find the dtb
> > fragment for a guest.
>
> The code below is basically a copy from libxl, right? If so, this should be
> specified in the commit message.
>
> Also, the license is different in libxl compare to the hypervisor (LGPLv2.1 vs
> GPLv2). So is there any issue to copy that code in the hypervisor?
Wei confirmed that it is OK. I'll add a note about this in the commit
message.
> >
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > ---
> > xen/arch/arm/domain_build.c | 77
> > ++++++++++++++++++++++++++++++++++++++++++++
> > xen/include/asm-arm/kernel.h | 2 +-
> > 2 files changed, 78 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index b0ec3f0..cc6b464 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -14,6 +14,7 @@
> > #include <xen/guest_access.h>
> > #include <xen/iocap.h>
> > #include <xen/acpi.h>
> > +#include <xen/vmap.h>
> > #include <xen/warning.h>
> > #include <acpi/actables.h>
> > #include <asm/device.h>
> > @@ -1669,6 +1670,59 @@ static int __init make_vpl011_uart_node(const struct
> > domain *d, void *fdt)
> > }
> > #endif
> > +static int __init copy_properties(void *fdt, void *pfdt, int nodeoff)
> > +{
> > + int propoff, nameoff, r;
> > + const struct fdt_property *prop;
> > +
> > + for ( propoff = fdt_first_property_offset(pfdt, nodeoff);
> > + propoff >= 0;
> > + propoff = fdt_next_property_offset(pfdt, propoff) ) {
>
> Coding style:
>
> for ( ... )
> {
OK
> > +
> > + if ( !(prop = fdt_get_property_by_offset(pfdt, propoff, NULL)) )
> > + return -FDT_ERR_INTERNAL;
> > +
> > + nameoff = fdt32_to_cpu(prop->nameoff);
> > + r = fdt_property(fdt, fdt_string(pfdt, nameoff),
> > + prop->data, fdt32_to_cpu(prop->len));
> > + if ( r )
> > + return r;
> > + }
> > +
> > + /* FDT_ERR_NOTFOUND => There is no more properties for this node */
> > + return ( propoff != -FDT_ERR_NOTFOUND ) ? propoff : 0;
> > +}
> > +
> > +static int __init copy_node(void *fdt, void *pfdt, int nodeoff, int depth)
> > +{
> > + int r;
> > +
> > + r = fdt_begin_node(fdt, fdt_get_name(pfdt, nodeoff, NULL));
> > + if ( r )
> > + return r;
> > +
> > + r = copy_properties(fdt, pfdt, nodeoff);
> > + if ( r )
> > + return r;
> > +
> > + for ( nodeoff = fdt_first_subnode(pfdt, nodeoff);
> > + nodeoff >= 0;
> > + nodeoff = fdt_next_subnode(pfdt, nodeoff) ) {
>
> Same here.
OK
> > + r = copy_node(fdt, pfdt, nodeoff, depth + 1);
> > + if ( r )
> > + return r;
> > + }
> > +
> > + if ( nodeoff != -FDT_ERR_NOTFOUND )
> > + return nodeoff;
> > +
> > + r = fdt_end_node(fdt);
> > + if ( r )
> > + return r;
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * The max size for DT is 2MB. However, the generated DT is small, 4KB
> > * are enough for now, but we might have to increase it in the future.
> > @@ -1740,6 +1794,29 @@ static int __init prepare_dtb_domU(struct domain *d,
> > struct kernel_info *kinfo)
> > goto err;
> > }
> > + if ( kinfo->dtb_bootmodule ) {
> > + int nodeoff, res;
> > + void *pfdt;
> > +
> > + pfdt = ioremap_cache(kinfo->dtb_bootmodule->start,
> > + kinfo->dtb_bootmodule->size);
>
> This suggests the DTB fragment should be cacheable. Can this be written in the
> documentation?
OK
> > + if ( pfdt == NULL )
> > + return -EFAULT;
> > +
> > + if ( fdt_magic(pfdt) != FDT_MAGIC )
> > + return -EINVAL;
> > +
> > + nodeoff = fdt_path_offset(pfdt, "/passthrough");
> > + if (nodeoff < 0)
> > + return nodeoff;
> > +
> > + res = copy_node(kinfo->fdt, pfdt, nodeoff, 0);
> > + if ( res )
> > + return res;
>
> I would also copy /aliases as it may be used by the users to refer easily a
> node.
OK
> > +
> > + iounmap(pfdt);
> > + }
> > +
> > ret = fdt_end_node(kinfo->fdt);
> > if ( ret < 0 )
> > goto err;
> > diff --git a/xen/include/asm-arm/kernel.h b/xen/include/asm-arm/kernel.h
> > index 33f3e72..720dec4 100644
> > --- a/xen/include/asm-arm/kernel.h
> > +++ b/xen/include/asm-arm/kernel.h
> > @@ -28,7 +28,7 @@ struct kernel_info {
> > paddr_t gnttab_size;
> > /* boot blob load addresses */
> > - const struct bootmodule *kernel_bootmodule, *initrd_bootmodule;
> > + const struct bootmodule *kernel_bootmodule, *initrd_bootmodule,
> > *dtb_bootmodule;
> > const char* cmdline;
> > paddr_t dtb_paddr;
> > paddr_t initrd_paddr;
> >
>
> Cheers,
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-01-02 23:25 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-05 17:28 [PATCH 0/5] dom0less device assignment Stefano Stabellini
2018-12-05 17:28 ` [PATCH 1/5] xen/arm: copy dtb fragment to guest dtb Stefano Stabellini
2018-12-24 11:17 ` Julien Grall
2019-01-02 12:22 ` Wei Liu
2019-01-02 23:25 ` Stefano Stabellini [this message]
2018-12-05 17:28 ` [PATCH 2/5] xen/arm: assign devices to boot domains Stefano Stabellini
2018-12-24 13:55 ` Julien Grall
2019-01-03 22:07 ` Stefano Stabellini
2019-01-15 12:50 ` Julien Grall
2018-12-05 17:28 ` [PATCH 3/5] xen/arm: handle "multiboot, dtb" compatible nodes Stefano Stabellini
2018-12-24 13:58 ` Julien Grall
2019-01-02 23:28 ` Stefano Stabellini
2018-12-05 17:28 ` [PATCH 4/5] xen/arm: use the physical number of gic lines for boot domains Stefano Stabellini
2018-12-24 14:01 ` Julien Grall
2019-01-03 19:07 ` Stefano Stabellini
2019-01-15 12:56 ` Julien Grall
2018-12-05 17:28 ` [PATCH 5/5] xen/arm: add dom0less device assignment info to docs Stefano Stabellini
2018-12-24 14:06 ` Julien Grall
2019-01-03 22:07 ` Stefano Stabellini
2019-01-03 23:31 ` Stefano Stabellini
2019-01-15 13:03 ` Julien Grall
2019-01-15 13:02 ` Julien Grall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.10.1901021519210.800@sstabellini-ThinkPad-X260 \
--to=sstabellini@kernel.org \
--cc=Achin.Gupta@arm.com \
--cc=andrii_anisov@epam.com \
--cc=ian.jackson@eu.citrix.com \
--cc=julien.grall@arm.com \
--cc=stefanos@xilinx.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).