All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 01/26] xen: allow console_io hypercalls from certain DomUs
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 02/26] xen/arm: extend device tree based multiboot protocol Stefano Stabellini
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, sstabellini, andrii_anisov, konrad.wilk,
	George.Dunlap, andrew.cooper3, Achin.Gupta, ian.jackson,
	xen-devel, tim, jbeulich, wei.liu2, dgdegra

Introduce an is_console option to allow certain classes of domUs to use
the Xen console. Specifically, it will be used to give console access to
all domUs started from Xen from information on device tree.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: andrew.cooper3@citrix.com
CC: George.Dunlap@eu.citrix.com
CC: ian.jackson@eu.citrix.com
CC: jbeulich@suse.com
CC: konrad.wilk@oracle.com
CC: tim@xen.org
CC: wei.liu2@citrix.com
CC: dgdegra@tycho.nsa.gov
---
Changes in v3:
- remove changes to hooks.c

Changes in v2:
- introduce is_console
- remove #ifdefs
---
 xen/include/xen/sched.h | 2 ++
 xen/include/xsm/dummy.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 0ba80cb..abcc74e 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -379,6 +379,8 @@ struct domain
     bool             auto_node_affinity;
     /* Is this guest fully privileged (aka dom0)? */
     bool             is_privileged;
+    /* Can this guest access the Xen console? */
+    bool             is_console;
     /* Is this a xenstore domain (not dom0)? */
     bool             is_xenstore;
     /* Domain's VCPUs are pinned 1:1 to physical CPUs? */
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b0ac1f6..29d7b50 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -230,6 +230,8 @@ static XSM_INLINE int xsm_memory_stat_reservation(XSM_DEFAULT_ARG struct domain
 static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain *d, int cmd)
 {
     XSM_ASSERT_ACTION(XSM_OTHER);
+    if ( d->is_console )
+        return xsm_default_action(XSM_HOOK, d, NULL);
 #ifdef CONFIG_VERBOSE_DEBUG
     if ( cmd == CONSOLEIO_write )
         return xsm_default_action(XSM_HOOK, d, NULL);
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 02/26] xen/arm: extend device tree based multiboot protocol
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 01/26] xen: allow console_io hypercalls from certain DomUs Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 03/26] xen/arm: document dom0less Stefano Stabellini
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Extend the existing device tree based multiboot protocol to include
information regarding multiple domains to boot.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---
Changes in v4:
- memory is 64bit

Changes in v3:
- remove "xen,initial-domain" for now
- make vpl011 an empty property
- memory in KBs

Changes in v2:
- lower case kernel
- rename mem to memory
- mandate cpus and memory
- replace domU-kernel with kernel and domU-ramdisk with ramdisk
- rename xen,domU with xen,domain
- add info about dom0
- switch memory and cpus to integers
- remove defaults
- add vpl011
---
 docs/misc/arm/device-tree/booting.txt | 107 ++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index ce2d0dc..317a9e9 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -119,3 +119,110 @@ For those you would hardcode the Xen commandline in the DTB under
 line by writing bootargs (as for native Linux).
 A Xen-aware bootloader would set xen,xen-bootargs for Xen, xen,dom0-bootargs
 for Dom0 and bootargs for native Linux.
+
+
+Creating Multiple Domains directly from Xen
+===========================================
+
+It is possible to have Xen create other domains, in addition to dom0,
+out of the information provided via device tree. A kernel and initrd
+(optional) need to be specified for each guest.
+
+For each domain to be created there needs to be one node under /chosen
+with the following properties:
+
+- compatible
+
+    For domUs: "xen,domain"
+
+- memory
+
+	A 64-bit integer specifying the amount of kilobytes of RAM to
+    allocate to the guest.
+
+- cpus
+
+    An integer specifying the number of vcpus to allocate to the guest.
+
+- vpl011
+
+    An empty property to enable/disable a virtual pl011 for the guest to use.
+
+- #address-cells and #size-cells
+
+    Both #address-cells and #size-cells need to be specified because
+    both sub-nodes (described shortly) have reg properties.
+
+Under the "xen,domain" compatible node, one or more sub-nodes are present
+for the DomU kernel and ramdisk.
+
+The kernel sub-node has the following properties:
+
+- compatible
+
+    "multiboot,kernel"
+
+- reg
+
+    Specifies the physical address of the kernel in RAM and its
+    length.
+
+- bootargs (optional)
+
+    Command line parameters for the guest kernel.
+
+The ramdisk sub-node has the following properties:
+
+- compatible
+
+    "multiboot,ramdisk"
+
+- reg
+
+    Specifies the physical address of the ramdisk in RAM and its
+    length.
+
+
+Example
+=======
+
+chosen {
+    domU1 {
+        compatible = "xen,domain";
+        #address-cells = <0x2>;
+        #size-cells = <0x1>;
+        memory = <0 131072>;
+        cpus = <2>;
+        vpl011;
+
+        module@0x4a000000 {
+            compatible = "multiboot,kernel";
+            reg = <0x0 0x4a000000 0xffffff>;
+            bootargs = "console=ttyAMA0 init=/bin/sh";
+        };
+
+        module@0x4b000000 {
+            compatible = "multiboot,ramdisk";
+            reg = <0x0 0x4b000000 0xffffff>;
+        };
+    };
+
+    domU2 {
+        compatible = "xen,domain";
+        #address-cells = <0x2>;
+        #size-cells = <0x1>;
+        memory = <0 65536>;
+        cpus = <1>;
+
+        module@0x4c000000 {
+            compatible = "multiboot,kernel";
+            reg = <0x0 0x4c000000 0xffffff>;
+            bootargs = "console=ttyAMA0 init=/bin/sh";
+        };
+
+        module@0x4d000000 {
+            compatible = "multiboot,ramdisk";
+            reg = <0x0 0x4d000000 0xffffff>;
+        };
+    };
+};
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 03/26] xen/arm: document dom0less
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 01/26] xen: allow console_io hypercalls from certain DomUs Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 02/26] xen/arm: extend device tree based multiboot protocol Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 04/26] xen/arm: increase MAX_MODULES Stefano Stabellini
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Add a new document to provide information on how to use dom0less related
features and their current limitations.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---
Changes in v5:
- convert to markdown
- move to docs/features
- add entry to docs/INDEX

Changes in v4:
- rename to .txt
- improve wording

Changes in v3:
- add patch
---
 docs/INDEX                      |  1 +
 docs/features/dom0less.markdown | 49 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 docs/features/dom0less.markdown

diff --git a/docs/INDEX b/docs/INDEX
index 868ab1f..e673edd 100644
--- a/docs/INDEX
+++ b/docs/INDEX
@@ -25,3 +25,4 @@ misc/arm/early-printk		Enabling early printk on ARM
 misc/arm/passthrough		Passthrough a device described in the Device Tree to a guest
 misc/arm/device-tree/booting	Device tree bindings to boot Xen
 misc/arm/device-tree/passthrough	Device tree binding to passthrough a device
+features/dom0less.markdown	Boot multiple domains from Xen in parallel
diff --git a/docs/features/dom0less.markdown b/docs/features/dom0less.markdown
new file mode 100644
index 0000000..4e342b7
--- /dev/null
+++ b/docs/features/dom0less.markdown
@@ -0,0 +1,49 @@
+Dom0less
+========
+
+"Dom0less" is a set of Xen features that enable the deployment of a Xen
+system without an control domain (often referred to as "dom0"). Each
+feature can be used independently from the others, unless otherwise
+stated.
+
+Booting Multiple Domains from Device Tree
+-----------------------------------------
+
+This feature enables Xen to create a set of DomUs at boot time.
+Information about the DomUs to be created by Xen is passed to the
+hypervisor via Device Tree. Specifically, the existing Device Tree based
+Multiboot specification has been extended to allow for multiple domains
+to be passed to Xen. See docs/misc/arm/device-tree/booting.txt for more
+information about the Multiboot specification and how to use it.
+
+Currently, a control domain ("dom0") is still required, but in the
+future it will become unnecessary when all domains are created
+directly from Xen. Instead of waiting for the control domain to be fully
+booted and the Xen tools to become available, domains created by Xen
+this way are started right away in parallel. Hence, their boot time is
+typically much shorter.
+
+Domains started by Xen at boot time currently have the following
+limitations:
+
+- They cannot be properly shutdown or rebooted using xl. If one of them
+  crashes, the whole platform should be rebooted.
+
+- Some xl operations might not work as expected. xl is meant to be used
+  with domains that have been created by it. Using xl with domains
+  started by Xen at boot might not work as expected.
+
+- The GIC version is the native version. In absence of other
+  information, the GIC version exposed to the domains started by Xen at
+  boot is the same as the native GIC version.
+
+- No PV drivers. There is no support for PV devices at the moment. All
+  devices need to be statically assigned to guests.
+
+- Pinning vCPUs of domains started by Xen at boot can be
+  done from the control domain, using `xl vcpu-pin` as usual. It is not
+  currently possible to configure vCPU pinning without a control domain.
+  However, the NULL scheduler can be selected by passing `sched=null` to
+  the Xen command line. The NULL scheduler automatically assigns and
+  pins vCPUs to pCPUs, but the vCPU-pCPU assignments cannot be
+  configured.
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 04/26] xen/arm: increase MAX_MODULES
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (2 preceding siblings ...)
  2018-11-02 23:44 ` [PATCH v6 03/26] xen/arm: document dom0less Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen Stefano Stabellini
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Xen boot modules need to account not just for Dom0 but also for a few
potential DomUs, each of them coming with their own kernel and initrd.
Increase MAX_MODULES to 32 to allow for more DomUs.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
---
 xen/include/asm-arm/setup.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 0cc3330..f1e4a3f 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -8,7 +8,7 @@
 
 #define NR_MEM_BANKS 128
 
-#define MAX_MODULES 5 /* Current maximum useful modules */
+#define MAX_MODULES 32 /* Current maximum useful modules */
 
 typedef enum {
     BOOTMOD_XEN,
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (3 preceding siblings ...)
  2018-11-02 23:44 ` [PATCH v6 04/26] xen/arm: increase MAX_MODULES Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-09 14:01   ` Julien Grall
  2018-11-02 23:44 ` [PATCH v6 06/26] xen/arm: introduce bootcmdlines Stefano Stabellini
                   ` (20 subsequent siblings)
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Make sure to only look for multiboot compatible nodes only under
/chosen, not under any other paths (depth <= 3).

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---

Changes in v6:
- do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
- remove sizeof, use hardcoded value

Changes in v5:
- add patch
- add check on return value of fdt_get_path
---
 xen/arch/arm/bootfdt.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 8eba42c..a42fe87 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -173,7 +173,15 @@ static void __init process_multiboot_node(const void *fdt, int node,
     bootmodule_kind kind;
     paddr_t start, size;
     const char *cmdline;
-    int len;
+    int len = 8; /* sizeof "/chosen" */
+    char path[8];
+    int ret;
+
+    /* Check that the node is under "chosen" */
+    ret = fdt_get_path(fdt, node, path, len);
+    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||
+         strncmp(path, "/chosen", len - 1) )
+        return;
 
     prop = fdt_get_property(fdt, node, "reg", &len);
     if ( !prop )
@@ -286,8 +294,8 @@ static int __init early_scan_node(const void *fdt,
 {
     if ( device_tree_node_matches(fdt, node, "memory") )
         process_memory_node(fdt, node, name, address_cells, size_cells);
-    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
-              device_tree_node_compatible(fdt, node, "multiboot,module" ))
+    else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
+              device_tree_node_compatible(fdt, node, "multiboot,module" )))
         process_multiboot_node(fdt, node, name, address_cells, size_cells);
     else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
         process_chosen_node(fdt, node, name, address_cells, size_cells);
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 06/26] xen/arm: introduce bootcmdlines
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (4 preceding siblings ...)
  2018-11-02 23:44 ` [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 07/26] xen/arm: don't add duplicate boot modules, introduce domU flag Stefano Stabellini
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Introduce a new array to store the cmdline of each boot module. It is
separate from struct bootmodules. Remove the cmdline field from struct
boot_module. This way, kernels and initrds with the same address in
memory can share struct bootmodule (important because we want them to be
free'd only once), but they can still have their separate bootcmdline
entries.

Add a dt_name field to struct bootcmdline to make it easier to find the
correct entry. Store the name of the "xen,domain" compatible node (for
example "Dom1"). This is a better choice compared to the name of the
"multiboot,kernel" compatible node, because their names are not unique.
For instance there can be more than one "module@0x4c000000" in the
system, but there can only be one "/chosen/Dom1".

Add a pointer to struct kernel_info to point to the cmdline for a given
kernel.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v6:
- code style
- add ack

Changes in v5:
- remove leftover DEBUG message
- improve commit message
- use kinfo->cmdline when possible
- move add_bood_cmdline to setup.c

Changes in v4:
- check that the multiboot node is under /chosen
- use cmd and cmds as variable names for struct bootcmdline and struct
  bootcmdline*
- fix printk
- use ASSERT instea of panic
- do not add empty cmdline entries
- add more debug printks to early_print_info
- code style fixes
- add comment on DT_MAX_NAME
- increase DT_MAX_NAME to 41
- make nr_mods unsigned int

Changes in v3:
- introduce bootcmdlines
- do not modify boot_fdt_cmdline
- add comments

Changes in v2:
- new patch
---
 xen/arch/arm/bootfdt.c      | 43 ++++++++++++++++++++++-------------------
 xen/arch/arm/domain_build.c | 12 ++++++------
 xen/arch/arm/kernel.h       |  1 +
 xen/arch/arm/setup.c        | 47 ++++++++++++++++++++++++++++++++++++++-------
 xen/include/asm-arm/setup.h | 19 ++++++++++++++++--
 5 files changed, 87 insertions(+), 35 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index a42fe87..07a9037 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -172,10 +172,12 @@ static void __init process_multiboot_node(const void *fdt, int node,
     const __be32 *cell;
     bootmodule_kind kind;
     paddr_t start, size;
-    const char *cmdline;
     int len = 8; /* sizeof "/chosen" */
     char path[8];
-    int ret;
+    int parent_node, ret;
+
+    parent_node = fdt_parent_offset(fdt, node);
+    ASSERT(parent_node >= 0);
 
     /* Check that the node is under "chosen" */
     ret = fdt_get_path(fdt, node, path, len);
@@ -228,17 +230,12 @@ static void __init process_multiboot_node(const void *fdt, int node,
             kind = BOOTMOD_XSM;
     }
 
-    prop = fdt_get_property(fdt, node, "bootargs", &len);
-    if ( prop )
-    {
-        if ( len > BOOTMOD_MAX_CMDLINE )
-            panic("module %s command line too long\n", name);
-        cmdline = prop->data;
-    }
-    else
-        cmdline = NULL;
+    add_boot_module(kind, start, size);
 
-    add_boot_module(kind, start, size, cmdline);
+    prop = fdt_get_property(fdt, node, "bootargs", &len);
+    if ( !prop )
+        return;
+    add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data, kind);
 }
 
 static void __init process_chosen_node(const void *fdt, int node,
@@ -284,7 +281,7 @@ static void __init process_chosen_node(const void *fdt, int node,
 
     printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end);
 
-    add_boot_module(BOOTMOD_RAMDISK, start, end-start, NULL);
+    add_boot_module(BOOTMOD_RAMDISK, start, end-start);
 }
 
 static int __init early_scan_node(const void *fdt,
@@ -307,6 +304,7 @@ static void __init early_print_info(void)
 {
     struct meminfo *mi = &bootinfo.mem;
     struct bootmodules *mods = &bootinfo.modules;
+    struct bootcmdlines *cmds = &bootinfo.cmdlines;
     int i, nr_rsvd;
 
     for ( i = 0; i < mi->nr_banks; i++ )
@@ -315,12 +313,12 @@ static void __init early_print_info(void)
                      mi->bank[i].start + mi->bank[i].size - 1);
     printk("\n");
     for ( i = 0 ; i < mods->nr_mods; i++ )
-        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s %s\n",
+        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s\n",
                      i,
                      mods->module[i].start,
                      mods->module[i].start + mods->module[i].size,
-                     boot_module_kind_as_string(mods->module[i].kind),
-                     mods->module[i].cmdline);
+                     boot_module_kind_as_string(mods->module[i].kind));
+
     nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
     for ( i = 0; i < nr_rsvd; i++ )
     {
@@ -333,6 +331,11 @@ static void __init early_print_info(void)
                      i, s, e);
     }
     printk("\n");
+    for ( i = 0 ; i < cmds->nr_mods; i++ )
+        printk("CMDLINE[%d]:%s %s\n", i,
+               cmds->cmdline[i].dt_name,
+               &cmds->cmdline[i].cmdline[0]);
+    printk("\n");
 }
 
 /**
@@ -349,7 +352,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
     if ( ret < 0 )
         panic("No valid device tree\n");
 
-    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), NULL);
+    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt));
 
     device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
     early_print_info();
@@ -369,11 +372,11 @@ const char *boot_fdt_cmdline(const void *fdt)
     prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
     if ( prop == NULL )
     {
-        struct bootmodule *dom0_mod =
-            boot_module_find_by_kind(BOOTMOD_KERNEL);
+        struct bootcmdline *dom0_cmdline =
+            boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
 
         if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
-            ( dom0_mod && dom0_mod->cmdline[0] ) )
+            ( dom0_cmdline && dom0_cmdline->cmdline[0] ) )
             prop = fdt_get_property(fdt, node, "bootargs", NULL);
     }
     if ( prop == NULL )
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f552154..6b15bc7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -375,10 +375,8 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
     int res = 0;
     int had_dom0_bootargs = 0;
 
-    const struct bootmodule *kernel = kinfo->kernel_bootmodule;
-
-    if ( kernel && kernel->cmdline[0] )
-        bootargs = &kernel->cmdline[0];
+    if ( kinfo->cmdline && kinfo->cmdline[0] )
+        bootargs = &kinfo->cmdline[0];
 
     dt_for_each_property_node (node, prop)
     {
@@ -952,9 +950,9 @@ static int __init make_chosen_node(const struct kernel_info *kinfo)
     if ( res )
         return res;
 
-    if ( mod && mod->cmdline[0] )
+    if ( kinfo->cmdline && kinfo->cmdline[0] )
     {
-        bootargs = &mod->cmdline[0];
+        bootargs = &kinfo->cmdline[0];
         res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
         if ( res )
            return res;
@@ -2109,6 +2107,7 @@ static void __init find_gnttab_region(struct domain *d,
 
 int __init construct_dom0(struct domain *d)
 {
+    const struct bootcmdline *kernel = boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
     struct kernel_info kinfo = {};
     struct vcpu *saved_current;
     int rc, i, cpu;
@@ -2154,6 +2153,7 @@ int __init construct_dom0(struct domain *d)
 
 #endif
 
+    kinfo.cmdline = (kernel != NULL) ? &kernel->cmdline[0] : NULL;
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 47eacb5..39b7828 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -29,6 +29,7 @@ struct kernel_info {
 
     /* boot blob load addresses */
     const struct bootmodule *kernel_bootmodule, *initrd_bootmodule;
+    const char* cmdline;
     paddr_t dtb_paddr;
     paddr_t initrd_paddr;
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ea2495a..2098591 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -201,8 +201,7 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e,
 }
 
 struct bootmodule __init *add_boot_module(bootmodule_kind kind,
-                                          paddr_t start, paddr_t size,
-                                          const char *cmdline)
+                                          paddr_t start, paddr_t size)
 {
     struct bootmodules *mods = &bootinfo.modules;
     struct bootmodule *mod;
@@ -218,10 +217,6 @@ struct bootmodule __init *add_boot_module(bootmodule_kind kind,
     mod->kind = kind;
     mod->start = start;
     mod->size = size;
-    if ( cmdline )
-        safe_strcpy(mod->cmdline, cmdline);
-    else
-        mod->cmdline[0] = 0;
 
     return mod;
 }
@@ -240,6 +235,44 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
     return NULL;
 }
 
+void __init add_boot_cmdline(const char *name, const char *cmdline,
+                             bootmodule_kind kind)
+{
+    struct bootcmdlines *cmds = &bootinfo.cmdlines;
+    struct bootcmdline *cmd;
+
+    if ( cmds->nr_mods == MAX_MODULES )
+    {
+        printk("Ignoring %s cmdline (too many)\n", name);
+        return;
+    }
+
+    cmd = &cmds->cmdline[cmds->nr_mods++];
+    cmd->kind = kind;
+
+    ASSERT(strlen(name) <= DT_MAX_NAME);
+    safe_strcpy(cmd->dt_name, name);
+
+    if ( strlen(cmdline) > BOOTMOD_MAX_CMDLINE )
+        panic("module %s command line too long\n", name);
+    safe_strcpy(cmd->cmdline, cmdline);
+}
+
+struct bootcmdline * __init boot_cmdline_find_by_kind(bootmodule_kind kind)
+{
+    struct bootcmdlines *cmds = &bootinfo.cmdlines;
+    struct bootcmdline *cmd;
+    int i;
+
+    for ( i = 0 ; i < cmds->nr_mods ; i++ )
+    {
+        cmd = &cmds->cmdline[i];
+        if ( cmd->kind == kind )
+            return cmd;
+    }
+    return NULL;
+}
+
 const char * __init boot_module_kind_as_string(bootmodule_kind kind)
 {
     switch ( kind )
@@ -728,7 +761,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     /* Register Xen's load address as a boot module. */
     xen_bootmodule = add_boot_module(BOOTMOD_XEN,
                              (paddr_t)(uintptr_t)(_start + boot_phys_offset),
-                             (paddr_t)(uintptr_t)(_end - _start + 1), NULL);
+                             (paddr_t)(uintptr_t)(_end - _start + 1));
     BUG_ON(!xen_bootmodule);
 
     xen_paddr = get_xen_paddr();
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index f1e4a3f..7580007 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -35,6 +35,13 @@ struct bootmodule {
     bootmodule_kind kind;
     paddr_t start;
     paddr_t size;
+};
+
+/* DT_MAX_NAME is the node name max length according the DT spec */
+#define DT_MAX_NAME 41
+struct bootcmdline {
+    bootmodule_kind kind;
+    char dt_name[DT_MAX_NAME];
     char cmdline[BOOTMOD_MAX_CMDLINE];
 };
 
@@ -43,9 +50,15 @@ struct bootmodules {
     struct bootmodule module[MAX_MODULES];
 };
 
+struct bootcmdlines {
+    unsigned int nr_mods;
+    struct bootcmdline cmdline[MAX_MODULES];
+};
+
 struct bootinfo {
     struct meminfo mem;
     struct bootmodules modules;
+    struct bootcmdlines cmdlines;
 #ifdef CONFIG_ACPI
     struct meminfo acpi;
 #endif
@@ -78,9 +91,11 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
 const char __init *boot_fdt_cmdline(const void *fdt);
 
 struct bootmodule *add_boot_module(bootmodule_kind kind,
-                                   paddr_t start, paddr_t size,
-                                   const char *cmdline);
+                                   paddr_t start, paddr_t size);
 struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+void add_boot_cmdline(const char *name, const char *cmdline,
+                      bootmodule_kind kind);
+struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
 const char * __init boot_module_kind_as_string(bootmodule_kind kind);
 
 #endif
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 07/26] xen/arm: don't add duplicate boot modules, introduce domU flag
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (5 preceding siblings ...)
  2018-11-02 23:44 ` [PATCH v6 06/26] xen/arm: introduce bootcmdlines Stefano Stabellini
@ 2018-11-02 23:44 ` Stefano Stabellini
  2018-11-09 14:06   ` Julien Grall
  2018-11-02 23:45 ` [PATCH v6 08/26] xen/arm: probe domU kernels and initrds Stefano Stabellini
                   ` (18 subsequent siblings)
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:44 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Don't add duplicate boot modules (same kind and same start address),
they are freed later, we don't want to introduce double-free errors.

Introduce a domU flag in struct bootmodule and struct bootcmdline. Set
it for kernels and ramdisks of "xen,domain" nodes to avoid getting
confused in kernel_probe, where we try to guess which is the dom0 kernel
and initrd to be compatible with all versions of the multiboot spec.

boot_module_find_by_kind and boot_cmdline_find_by_kind automatically
check for !domU entries (they are only used for non-domU modules).

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---
Changes in v6:
- update comments

Changes in v5:
- improve commit message
- add in-code comments

Changes in v4:
- use unsigned int
- better commit message
- introduce domU flag and usage

Changes in v2:
- new patch
---
 xen/arch/arm/bootfdt.c      | 11 +++++++----
 xen/arch/arm/setup.c        | 34 +++++++++++++++++++++++++++++-----
 xen/include/asm-arm/setup.h | 12 ++++++++++--
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 07a9037..8d9ba47 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -175,6 +175,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
     int len = 8; /* sizeof "/chosen" */
     char path[8];
     int parent_node, ret;
+    bool domU;
 
     parent_node = fdt_parent_offset(fdt, node);
     ASSERT(parent_node >= 0);
@@ -230,12 +231,14 @@ static void __init process_multiboot_node(const void *fdt, int node,
             kind = BOOTMOD_XSM;
     }
 
-    add_boot_module(kind, start, size);
+    domU = fdt_node_check_compatible(fdt, parent_node, "xen,domain") == 0;
+    add_boot_module(kind, start, size, domU);
 
     prop = fdt_get_property(fdt, node, "bootargs", &len);
     if ( !prop )
         return;
-    add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data, kind);
+    add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data,
+                     kind, domU);
 }
 
 static void __init process_chosen_node(const void *fdt, int node,
@@ -281,7 +284,7 @@ static void __init process_chosen_node(const void *fdt, int node,
 
     printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end);
 
-    add_boot_module(BOOTMOD_RAMDISK, start, end-start);
+    add_boot_module(BOOTMOD_RAMDISK, start, end-start, false);
 }
 
 static int __init early_scan_node(const void *fdt,
@@ -352,7 +355,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
     if ( ret < 0 )
         panic("No valid device tree\n");
 
-    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt));
+    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
 
     device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
     early_print_info();
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 2098591..c69e872 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -201,10 +201,12 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e,
 }
 
 struct bootmodule __init *add_boot_module(bootmodule_kind kind,
-                                          paddr_t start, paddr_t size)
+                                          paddr_t start, paddr_t size,
+                                          bool domU)
 {
     struct bootmodules *mods = &bootinfo.modules;
     struct bootmodule *mod;
+    unsigned int i;
 
     if ( mods->nr_mods == MAX_MODULES )
     {
@@ -212,15 +214,31 @@ struct bootmodule __init *add_boot_module(bootmodule_kind kind,
                boot_module_kind_as_string(kind), start, start + size);
         return NULL;
     }
+    for ( i = 0 ; i < mods->nr_mods ; i++ )
+    {
+        mod = &mods->module[i];
+        if ( mod->kind == kind && mod->start == start )
+        {
+            if ( !domU )
+                mod->domU = false;
+            return mod;
+        }
+    }
 
     mod = &mods->module[mods->nr_mods++];
     mod->kind = kind;
     mod->start = start;
     mod->size = size;
+    mod->domU = domU;
 
     return mod;
 }
 
+/*
+ * boot_module_find_by_kind can only be used to return Xen modules (e.g
+ * XSM, DTB) or Dom0 modules. This is not suitable for looking up guest
+ * modules.
+ */
 struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
 {
     struct bootmodules *mods = &bootinfo.modules;
@@ -229,14 +247,14 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
     for (i = 0 ; i < mods->nr_mods ; i++ )
     {
         mod = &mods->module[i];
-        if ( mod->kind == kind )
+        if ( mod->kind == kind && !mod->domU )
             return mod;
     }
     return NULL;
 }
 
 void __init add_boot_cmdline(const char *name, const char *cmdline,
-                             bootmodule_kind kind)
+                             bootmodule_kind kind, bool domU)
 {
     struct bootcmdlines *cmds = &bootinfo.cmdlines;
     struct bootcmdline *cmd;
@@ -249,6 +267,7 @@ void __init add_boot_cmdline(const char *name, const char *cmdline,
 
     cmd = &cmds->cmdline[cmds->nr_mods++];
     cmd->kind = kind;
+    cmd->domU = domU;
 
     ASSERT(strlen(name) <= DT_MAX_NAME);
     safe_strcpy(cmd->dt_name, name);
@@ -258,6 +277,11 @@ void __init add_boot_cmdline(const char *name, const char *cmdline,
     safe_strcpy(cmd->cmdline, cmdline);
 }
 
+/*
+ * boot_cmdline_find_by_kind can only be used to return Xen modules (e.g
+ * XSM, DTB) or Dom0 modules. This is not suitable for looking up guest
+ * modules.
+ */
 struct bootcmdline * __init boot_cmdline_find_by_kind(bootmodule_kind kind)
 {
     struct bootcmdlines *cmds = &bootinfo.cmdlines;
@@ -267,7 +291,7 @@ struct bootcmdline * __init boot_cmdline_find_by_kind(bootmodule_kind kind)
     for ( i = 0 ; i < cmds->nr_mods ; i++ )
     {
         cmd = &cmds->cmdline[i];
-        if ( cmd->kind == kind )
+        if ( cmd->kind == kind && !cmd->domU )
             return cmd;
     }
     return NULL;
@@ -761,7 +785,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     /* Register Xen's load address as a boot module. */
     xen_bootmodule = add_boot_module(BOOTMOD_XEN,
                              (paddr_t)(uintptr_t)(_start + boot_phys_offset),
-                             (paddr_t)(uintptr_t)(_end - _start + 1));
+                             (paddr_t)(uintptr_t)(_end - _start + 1), false);
     BUG_ON(!xen_bootmodule);
 
     xen_paddr = get_xen_paddr();
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 7580007..3a30329 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -30,9 +30,16 @@ struct meminfo {
     struct membank bank[NR_MEM_BANKS];
 };
 
+/*
+ * The domU flag is set for kernels and ramdisks of "xen,domain" nodes.
+ * The purpose of the domU flag is to avoid getting confused in
+ * kernel_probe, where we try to guess which is the dom0 kernel and
+ * initrd to be compatible with all versions of the multiboot spec. 
+ */
 #define BOOTMOD_MAX_CMDLINE 1024
 struct bootmodule {
     bootmodule_kind kind;
+    bool domU;
     paddr_t start;
     paddr_t size;
 };
@@ -41,6 +48,7 @@ struct bootmodule {
 #define DT_MAX_NAME 41
 struct bootcmdline {
     bootmodule_kind kind;
+    bool domU;
     char dt_name[DT_MAX_NAME];
     char cmdline[BOOTMOD_MAX_CMDLINE];
 };
@@ -91,10 +99,10 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
 const char __init *boot_fdt_cmdline(const void *fdt);
 
 struct bootmodule *add_boot_module(bootmodule_kind kind,
-                                   paddr_t start, paddr_t size);
+                                   paddr_t start, paddr_t size, bool domU);
 struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
 void add_boot_cmdline(const char *name, const char *cmdline,
-                      bootmodule_kind kind);
+                      bootmodule_kind kind, bool domU);
 struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
 const char * __init boot_module_kind_as_string(bootmodule_kind kind);
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 08/26] xen/arm: probe domU kernels and initrds
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (6 preceding siblings ...)
  2018-11-02 23:44 ` [PATCH v6 07/26] xen/arm: don't add duplicate boot modules, introduce domU flag Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-09 14:09   ` Julien Grall
  2018-11-02 23:45 ` [PATCH v6 09/26] xen/arm: add start to struct bootcmdline Stefano Stabellini
                   ` (17 subsequent siblings)
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Find addresses, sizes on device tree from kernel_probe.
Find the cmdline from the bootcmdlines array.

Introduce a new boot_module_find_by_addr_and_kind function to match not
just on boot module kind, but also by address so that we can support
multiple domains.

Introduce a boot_cmdline_find_by_name function to find the right struct
cmdline based on the device tree node name of the "xen,domain"
compatible node.

Set command line for dom0 in kernel_probe for consistency.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
---
Changes in v6:
- style improvement in comment
- remove redundant cmdline assignment in construct_dom0

Changes in v5:
- constify kernel_probe
- add ASSERT and comment in kernel_probe
- limit variable scope
- fix printk message
- int/unsigned int
- set cmdline for dom0 too
- improve code readability

Changes in v3:
- retrieve cmdline from bootcmdlines array

Changes in v2:
- fix indentation
- unify kernel_probe with kernel_probe_domU
- find cmdline on device_tree from kernel_probe
---
 xen/arch/arm/domain_build.c |  4 +--
 xen/arch/arm/kernel.c       | 64 ++++++++++++++++++++++++++++++++++++++++-----
 xen/arch/arm/kernel.h       |  2 +-
 xen/arch/arm/setup.c        | 31 ++++++++++++++++++++++
 xen/include/asm-arm/setup.h |  3 +++
 5 files changed, 93 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6b15bc7..59c9f34 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2107,7 +2107,6 @@ static void __init find_gnttab_region(struct domain *d,
 
 int __init construct_dom0(struct domain *d)
 {
-    const struct bootcmdline *kernel = boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
     struct kernel_info kinfo = {};
     struct vcpu *saved_current;
     int rc, i, cpu;
@@ -2135,7 +2134,7 @@ int __init construct_dom0(struct domain *d)
     kinfo.unassigned_mem = dom0_mem;
     kinfo.d = d;
 
-    rc = kernel_probe(&kinfo);
+    rc = kernel_probe(&kinfo, NULL);
     if ( rc < 0 )
         return rc;
 
@@ -2153,7 +2152,6 @@ int __init construct_dom0(struct domain *d)
 
 #endif
 
-    kinfo.cmdline = (kernel != NULL) ? &kernel->cmdline[0] : NULL;
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index da8410e..d6c810b 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -421,22 +421,72 @@ static int __init kernel_zimage32_probe(struct kernel_info *info,
     return 0;
 }
 
-int __init kernel_probe(struct kernel_info *info)
+int __init kernel_probe(struct kernel_info *info,
+                        const struct dt_device_node *domain)
 {
-    struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
+    struct bootmodule *mod = NULL;
+    struct bootcmdline *cmd = NULL;
+    struct dt_device_node *node;
+    u64 kernel_addr, initrd_addr, size;
     int rc;
 
+    /* domain is NULL only for the hardware domain */
+    if ( domain == NULL )
+    {
+        ASSERT(is_hardware_domain(info->d));
+
+        mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
+
+        info->kernel_bootmodule = mod;
+        info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+
+        cmd = boot_cmdline_find_by_kind(BOOTMOD_KERNEL);
+        if ( cmd )
+            info->cmdline = &cmd->cmdline[0];
+    }
+    else
+    {
+        const char *name = NULL;
+
+        dt_for_each_child_node(domain, node)
+        {
+            if ( dt_device_is_compatible(node, "multiboot,kernel") )
+            {
+                u32 len;
+                const __be32 *val;
+
+                val = dt_get_property(node, "reg", &len);
+                dt_get_range(&val, node, &kernel_addr, &size);
+                mod = boot_module_find_by_addr_and_kind(
+                        BOOTMOD_KERNEL, kernel_addr);
+                info->kernel_bootmodule = mod;
+            }
+            else if ( dt_device_is_compatible(node, "multiboot,ramdisk") )
+            {
+                u32 len;
+                const __be32 *val;
+
+                val = dt_get_property(node, "reg", &len);
+                dt_get_range(&val, node, &initrd_addr, &size);
+                info->initrd_bootmodule = boot_module_find_by_addr_and_kind(
+                        BOOTMOD_RAMDISK, initrd_addr);
+            }
+            else
+                continue;
+        }
+        name = dt_node_name(domain);
+        cmd = boot_cmdline_find_by_name(name);
+        if ( cmd )
+            info->cmdline = &cmd->cmdline[0];
+    }
     if ( !mod || !mod->size )
     {
         printk(XENLOG_ERR "Missing kernel boot module?\n");
         return -ENOENT;
     }
 
-    info->kernel_bootmodule = mod;
-
-    printk("Loading kernel from boot module @ %"PRIpaddr"\n", mod->start);
-
-    info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
+    printk("Loading Dom%d kernel from boot module @ %"PRIpaddr"\n",
+           info->d->domain_id, info->kernel_bootmodule->start);
     if ( info->initrd_bootmodule )
         printk("Loading ramdisk from boot module @ %"PRIpaddr"\n",
                info->initrd_bootmodule->start);
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 39b7828..4320f72 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -55,7 +55,7 @@ struct kernel_info {
  *  ->type
  *  ->load hook, and sets loader specific variables ->zimage
  */
-int kernel_probe(struct kernel_info *info);
+int kernel_probe(struct kernel_info *info, const struct dt_device_node *domain);
 
 /*
  * Loads the kernel into guest RAM.
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index c69e872..dbba8f3 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -297,6 +297,37 @@ struct bootcmdline * __init boot_cmdline_find_by_kind(bootmodule_kind kind)
     return NULL;
 }
 
+struct bootcmdline * __init boot_cmdline_find_by_name(const char *name)
+{
+    struct bootcmdlines *mods = &bootinfo.cmdlines;
+    struct bootcmdline *mod;
+    unsigned int i;
+
+    for (i = 0 ; i < mods->nr_mods ; i++ )
+    {
+        mod = &mods->cmdline[i];
+        if ( strcmp(mod->dt_name, name) == 0 )
+            return mod;
+    }
+    return NULL;
+}
+
+struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind kind,
+                                                             paddr_t start)
+{
+    struct bootmodules *mods = &bootinfo.modules;
+    struct bootmodule *mod;
+    unsigned int i;
+
+    for (i = 0 ; i < mods->nr_mods ; i++ )
+    {
+        mod = &mods->module[i];
+        if ( mod->kind == kind && mod->start == start )
+            return mod;
+    }
+    return NULL;
+}
+
 const char * __init boot_module_kind_as_string(bootmodule_kind kind)
 {
     switch ( kind )
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 3a30329..33fb04e 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -101,9 +101,12 @@ const char __init *boot_fdt_cmdline(const void *fdt);
 struct bootmodule *add_boot_module(bootmodule_kind kind,
                                    paddr_t start, paddr_t size, bool domU);
 struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind kind,
+                                                             paddr_t start);
 void add_boot_cmdline(const char *name, const char *cmdline,
                       bootmodule_kind kind, bool domU);
 struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
+struct bootcmdline * __init boot_cmdline_find_by_name(const char *name);
 const char * __init boot_module_kind_as_string(bootmodule_kind kind);
 
 #endif
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 09/26] xen/arm: add start to struct bootcmdline
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (7 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 08/26] xen/arm: probe domU kernels and initrds Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-09 14:10   ` Julien Grall
  2018-11-02 23:45 ` [PATCH v6 10/26] xen/arm: rename get_11_allocation_size to get_allocation_size Stefano Stabellini
                   ` (16 subsequent siblings)
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Add a new start address field to struct bootcmdline to easily match a
cmdline to the corresponding bootmodule. This is useful for debugging
(not actually needed for functionalities today, but could be.)

Instead of printing the index in the cmdline array, print the start
address of the corresponding bootmodule for each cmdline in
early_print_info.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
---
 xen/arch/arm/bootfdt.c      | 4 ++--
 xen/arch/arm/setup.c        | 3 ++-
 xen/include/asm-arm/setup.h | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 8d9ba47..4f0712a 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -238,7 +238,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
     if ( !prop )
         return;
     add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data,
-                     kind, domU);
+                     kind, start, domU);
 }
 
 static void __init process_chosen_node(const void *fdt, int node,
@@ -335,7 +335,7 @@ static void __init early_print_info(void)
     }
     printk("\n");
     for ( i = 0 ; i < cmds->nr_mods; i++ )
-        printk("CMDLINE[%d]:%s %s\n", i,
+        printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start,
                cmds->cmdline[i].dt_name,
                &cmds->cmdline[i].cmdline[0]);
     printk("\n");
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index dbba8f3..a819953 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -254,7 +254,7 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
 }
 
 void __init add_boot_cmdline(const char *name, const char *cmdline,
-                             bootmodule_kind kind, bool domU)
+                             bootmodule_kind kind, paddr_t start, bool domU)
 {
     struct bootcmdlines *cmds = &bootinfo.cmdlines;
     struct bootcmdline *cmd;
@@ -268,6 +268,7 @@ void __init add_boot_cmdline(const char *name, const char *cmdline,
     cmd = &cmds->cmdline[cmds->nr_mods++];
     cmd->kind = kind;
     cmd->domU = domU;
+    cmd->start = start;
 
     ASSERT(strlen(name) <= DT_MAX_NAME);
     safe_strcpy(cmd->dt_name, name);
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 33fb04e..0d787e6 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -49,6 +49,7 @@ struct bootmodule {
 struct bootcmdline {
     bootmodule_kind kind;
     bool domU;
+    paddr_t start;
     char dt_name[DT_MAX_NAME];
     char cmdline[BOOTMOD_MAX_CMDLINE];
 };
@@ -104,7 +105,7 @@ struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
 struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind kind,
                                                              paddr_t start);
 void add_boot_cmdline(const char *name, const char *cmdline,
-                      bootmodule_kind kind, bool domU);
+                      bootmodule_kind kind, paddr_t start, bool domU);
 struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
 struct bootcmdline * __init boot_cmdline_find_by_name(const char *name);
 const char * __init boot_module_kind_as_string(bootmodule_kind kind);
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 10/26] xen/arm: rename get_11_allocation_size to get_allocation_size
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (8 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 09/26] xen/arm: add start to struct bootcmdline Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 11/26] xen/arm: rename allocate_memory to allocate_memory_11 Stefano Stabellini
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

No functional changes.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---

Changes in v3:
- no change in print messages
- do not remove BUG_ON

Changes in v2:
- new patch
---
 xen/arch/arm/domain_build.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 59c9f34..ca0c4f7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -77,7 +77,7 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
     return vcpu_create(dom0, 0, 0);
 }
 
-static unsigned int __init get_11_allocation_size(paddr_t size)
+static unsigned int __init get_allocation_size(paddr_t size)
 {
     /*
      * get_order_from_bytes returns the order greater than or equal to
@@ -249,7 +249,7 @@ static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
         get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
     const unsigned int min_order = get_order_from_bytes(MB(4));
     struct page_info *pg;
-    unsigned int order = get_11_allocation_size(kinfo->unassigned_mem);
+    unsigned int order = get_allocation_size(kinfo->unassigned_mem);
     int i;
 
     bool lowmem = true;
@@ -301,7 +301,7 @@ static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
      * If we failed to allocate bank0 under 4GB, continue allocating
      * memory from above 4GB and fill in banks.
      */
-    order = get_11_allocation_size(kinfo->unassigned_mem);
+    order = get_allocation_size(kinfo->unassigned_mem);
     while ( kinfo->unassigned_mem && kinfo->mem.nr_banks < NR_MEM_BANKS )
     {
         pg = alloc_domheap_pages(d, order, lowmem ? MEMF_bits(32) : 0);
@@ -312,7 +312,7 @@ static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
             if ( lowmem && order < min_low_order)
             {
                 D11PRINT("Failed at min_low_order, allow high allocations\n");
-                order = get_11_allocation_size(kinfo->unassigned_mem);
+                order = get_allocation_size(kinfo->unassigned_mem);
                 lowmem = false;
                 continue;
             }
@@ -332,7 +332,7 @@ static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
             if ( lowmem )
             {
                 D11PRINT("Allocation below bank 0, allow high allocations\n");
-                order = get_11_allocation_size(kinfo->unassigned_mem);
+                order = get_allocation_size(kinfo->unassigned_mem);
                 lowmem = false;
                 continue;
             }
@@ -347,7 +347,7 @@ static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
          * Success, next time around try again to get the largest order
          * allocation possible.
          */
-        order = get_11_allocation_size(kinfo->unassigned_mem);
+        order = get_allocation_size(kinfo->unassigned_mem);
     }
 
     if ( kinfo->unassigned_mem )
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 11/26] xen/arm: rename allocate_memory to allocate_memory_11
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (9 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 10/26] xen/arm: rename get_11_allocation_size to get_allocation_size Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 12/26] xen/arm: introduce allocate_memory Stefano Stabellini
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

allocate_memory only deals with directly mapped memory. Rename it to
allocate_memory_11.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
Changes in v3:
- add patch
---
 xen/arch/arm/domain_build.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ca0c4f7..66a258a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -243,7 +243,8 @@ fail:
  * (as described above) we allow higher allocations and continue until
  * that runs out (or we have allocated sufficient dom0 memory).
  */
-static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
+static void __init allocate_memory_11(struct domain *d,
+                                      struct kernel_info *kinfo)
 {
     const unsigned int min_low_order =
         get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
@@ -2152,7 +2153,7 @@ int __init construct_dom0(struct domain *d)
 
 #endif
 
-    allocate_memory(d, &kinfo);
+    allocate_memory_11(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
     /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 12/26] xen/arm: introduce allocate_memory
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (10 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 11/26] xen/arm: rename allocate_memory to allocate_memory_11 Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-09 14:19   ` Julien Grall
  2018-11-02 23:45 ` [PATCH v6 13/26] xen/arm: refactor construct_dom0 Stefano Stabellini
                   ` (13 subsequent siblings)
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Introduce an allocate_memory function able to allocate memory for DomUs
and map it at the right guest addresses, according to the guest memory
map: GUEST_RAM0_BASE and GUEST_RAM1_BASE.

This is under #if 0 as not used for now.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
---
Changes in v6:
- turn dprintks into printk
- use panic instead of printk+BUG_ON
- use Julien's implementation of allocate_bank_memory

Changes in v5:
- improve commit message
- code style
- remove unneeded local var
- while loop in allocate_bank_memory to allocate memory so that it
  doesn't have to be contiguos
- combile while loops in allocate_memory

Changes in v4:
- move earlier, add #if 0
- introduce allocate_bank_memory, remove insert_bank
- allocate_bank_memory allocate memory and inserts the bank, while
  allocate_memory specifies where to do that

Changes in v3:
- new patch
---
 xen/arch/arm/domain_build.c | 102 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 66a258a..86abcc6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -368,6 +368,108 @@ static void __init allocate_memory_11(struct domain *d,
     }
 }
 
+#if 0
+static bool __init allocate_bank_memory(struct domain *d,
+                                       struct kernel_info *kinfo,
+                                       gfn_t sgfn,
+                                       unsigned long tot_size)
+{
+    int res;
+    struct page_info *pg;
+    struct membank *bank;
+    gfn_t gfn = sgfn;
+    unsigned long size = tot_size;
+    unsigned int max_order = ~0;
+
+    while ( size > 0 )
+    {
+        unsigned int order = get_allocation_size(size);
+
+        order = min(max_order, order);
+
+        pg = alloc_domheap_pages(d, order, 0);
+        if ( !pg )
+        {
+            /*
+             * If we can't allocate one page, then it is unlikely to
+             * succeed in the next iteration. So bail out.
+             */
+            if ( !order )
+                return false;
+
+            /*
+             * If we can't allocate memory with order, then it is
+             * unlikely to succeed in the next iteration.
+             * Record the order - 1 to avoid re-trying.
+             */
+            max_order = order - 1;
+            continue;
+        }
+
+        res = guest_physmap_add_page(d, gfn, page_to_mfn(pg), order);
+        if ( res )
+        {
+            dprintk(XENLOG_ERR, "Failed map pages to DOMU: %d", res);
+            return false;
+        }
+
+        gfn = gfn_add(gfn, 1UL << order);
+        size -= (1ULL << (PAGE_SHIFT + order));
+    }
+
+    bank = &kinfo->mem.bank[kinfo->mem.nr_banks];
+    bank->start = gfn_to_gaddr(sgfn);
+    bank->size = tot_size;
+    kinfo->mem.nr_banks++;
+    kinfo->unassigned_mem -= tot_size;
+
+    return true;
+}
+
+static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
+{
+    unsigned int i;
+    unsigned long bank_size;
+
+    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for dom%d:\n",
+           /* Don't want format this as PRIpaddr (16 digit hex) */
+           (unsigned long)(kinfo->unassigned_mem >> 20), d->domain_id);
+
+    kinfo->mem.nr_banks = 0;
+    bank_size = MIN(GUEST_RAM0_SIZE, kinfo->unassigned_mem);
+    if ( !allocate_bank_memory(d, kinfo, gaddr_to_gfn(GUEST_RAM0_BASE),
+                               bank_size) )
+        goto fail;
+
+    bank_size = MIN(GUEST_RAM1_SIZE, kinfo->unassigned_mem);
+    if ( !allocate_bank_memory(d, kinfo, gaddr_to_gfn(GUEST_RAM1_BASE),
+                               bank_size) )
+        goto fail;
+
+    if ( kinfo->unassigned_mem )
+        goto fail;
+
+    for( i = 0; i < kinfo->mem.nr_banks; i++ )
+    {
+        printk(XENLOG_INFO "Dom%d BANK[%d] %#"PRIpaddr"-%#"PRIpaddr" (%ldMB)\n",
+               d->domain_id,
+               i,
+               kinfo->mem.bank[i].start,
+               kinfo->mem.bank[i].start + kinfo->mem.bank[i].size,
+               /* Don't want format this as PRIpaddr (16 digit hex) */
+               (unsigned long)(kinfo->mem.bank[i].size >> 20));
+    }
+
+    return;
+
+fail:
+    panic("Failed to allocate requested domain memory."
+          /* Don't want format this as PRIpaddr (16 digit hex) */
+          " %ldKB unallocated. Fix the VMs configurations.\n",
+          (unsigned long)kinfo->unassigned_mem >> 10);
+}
+#endif
+
 static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
                                    const struct dt_device_node *node)
 {
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree
@ 2018-11-02 23:45 Stefano Stabellini
  2018-11-02 23:44 ` [PATCH v6 01/26] xen: allow console_io hypercalls from certain DomUs Stefano Stabellini
                   ` (25 more replies)
  0 siblings, 26 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall; +Cc: Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Hi all,

This is first step toward "dom0less" as discussed in the various
certifications related threads and discussions.

The goal of this series is to enable Xen to boot multiple domains in
parallel, in addition to dom0, out of information found on device tree.

The device tree based boot protocol is extended to carry information
about DomUs. Based on that information, Xen creates and starts one or
more DomUs. DomUs created this way don't have access to xenstore for the
moment. This is actually OK, because this is meant for mission critical
applications that typically only access directly assigned devices. They
cannot tolerate interference or increased IRQ latency due to PV
protocols. Device assignment is not actually covered by this series, it
will be added later.

DomUs can print to the Xen serial using the CONSOLEIO hypercalls. A
virtual PL011 is also emulated for them so that they can use their
regular PL011 driver. This allows unmodified guests to run as Xen on ARM
guests -- no Xen support needed at all. Console input also comes from
the Xen serial: the Ctrl-AAA switching mechanism is extended to switch
among domUs, dom0, and Xen.

In this version of the series, I reordered the patches to make sure they
are all bisectable.


Cheers,

Stefano



The following changes since commit 359970fd8b781fac2ddcbc84dd5b890075fa08ef:

  tools/libxl: Switch Arm guest type to PVH (2018-10-03 15:58:02 +0100)

are available in the git repository at:

  http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git dom0less-v6

for you to fetch changes up to 403ae582e24b30e4e78b015ce2ddba8de07686a9:

  xen/arm: split domain_build.c (2018-11-02 15:21:46 -0700)

----------------------------------------------------------------
Stefano Stabellini (26):
      xen: allow console_io hypercalls from certain DomUs
      xen/arm: extend device tree based multiboot protocol
      xen/arm: document dom0less
      xen/arm: increase MAX_MODULES
      xen/arm: check for multiboot nodes only under /chosen
      xen/arm: introduce bootcmdlines
      xen/arm: don't add duplicate boot modules, introduce domU flag
      xen/arm: probe domU kernels and initrds
      xen/arm: add start to struct bootcmdline
      xen/arm: rename get_11_allocation_size to get_allocation_size
      xen/arm: rename allocate_memory to allocate_memory_11
      xen/arm: introduce allocate_memory
      xen/arm: refactor construct_dom0
      xen/arm: move unregister_init_virtual_region to init_done
      xen/arm: introduce create_domUs
      xen/arm: implement construct_domU
      xen/arm: generate a simple device tree for domUs
      xen/arm: make set_interrupt_ppi able to handle non-PPI
      xen/arm: generate vpl011 node on device tree for domU
      xen/arm: introduce a union in vpl011
      xen/arm: refactor vpl011_data_avail
      xen: support console_switching between Dom0 and DomUs on ARM
      xen/arm: Allow vpl011 to be used by DomU
      xen/vpl011: buffer out chars when the backend is xen
      xen/arm: move kernel.h to asm-arm/
      xen/arm: split domain_build.c

 docs/INDEX                                 |    1 +
 docs/features/dom0less.markdown            |   49 ++
 docs/misc/arm/device-tree/booting.txt      |  107 +++
 xen/arch/arm/acpi/Makefile                 |    1 +
 xen/arch/arm/acpi/domain_build.c           |  591 +++++++++++++++
 xen/arch/arm/bootfdt.c                     |   58 +-
 xen/arch/arm/domain_build.c                | 1092 +++++++++++++---------------
 xen/arch/arm/kernel.c                      |   67 +-
 xen/arch/arm/setup.c                       |  112 ++-
 xen/arch/arm/vpl011.c                      |  297 ++++++--
 xen/drivers/char/console.c                 |   90 ++-
 xen/include/asm-arm/domain_build.h         |   31 +
 xen/{arch/arm => include/asm-arm}/kernel.h |    6 +-
 xen/include/asm-arm/setup.h                |   36 +-
 xen/include/asm-arm/vpl011.h               |   20 +-
 xen/include/asm-x86/setup.h                |    2 +
 xen/include/xen/console.h                  |    2 +
 xen/include/xen/sched.h                    |    2 +
 xen/include/xsm/dummy.h                    |    2 +
 19 files changed, 1861 insertions(+), 705 deletions(-)
 create mode 100644 docs/features/dom0less.markdown
 create mode 100644 xen/arch/arm/acpi/domain_build.c
 create mode 100644 xen/include/asm-arm/domain_build.h
 rename xen/{arch/arm => include/asm-arm}/kernel.h (91%)

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v6 13/26] xen/arm: refactor construct_dom0
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (11 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 12/26] xen/arm: introduce allocate_memory Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 14/26] xen/arm: move unregister_init_virtual_region to init_done Stefano Stabellini
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Move generic initializations out of construct_dom0 so that they can be
reused.

Rename prepare_dtb to prepare_dtb_hwdom to avoid confusion.

No functional changes in this patch.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
Changes in v5:
- rename __construct_domain to construct_domain

Changes in v4:
- newline and style changes

Changes in v3:
- move setting type before allocate_memory
- add ifdef around it and a comment

Changes in v2:
- move discard_initial_modules() after __construct_domain()
- remove useless blank line
- leave safety BUG_ONs in __construct_domain
- rename prepare_dtb to prepare_dtb_hwdom
---
 xen/arch/arm/domain_build.c | 122 ++++++++++++++++++++++++--------------------
 1 file changed, 66 insertions(+), 56 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 86abcc6..3a9c989 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1473,7 +1473,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
     return res;
 }
 
-static int __init prepare_dtb(struct domain *d, struct kernel_info *kinfo)
+static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info *kinfo)
 {
     const p2m_type_t default_p2mt = p2m_mmio_direct_c;
     const void *fdt;
@@ -2208,73 +2208,29 @@ static void __init find_gnttab_region(struct domain *d,
            kinfo->gnttab_start, kinfo->gnttab_start + kinfo->gnttab_size);
 }
 
-int __init construct_dom0(struct domain *d)
+static int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
 {
-    struct kernel_info kinfo = {};
     struct vcpu *saved_current;
-    int rc, i, cpu;
-
+    int i, cpu;
     struct vcpu *v = d->vcpu[0];
     struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;
 
-    /* Sanity! */
-    BUG_ON(d->domain_id != 0);
     BUG_ON(d->vcpu[0] == NULL);
     BUG_ON(v->is_initialised);
 
-    printk("*** LOADING DOMAIN 0 ***\n");
-    if ( dom0_mem <= 0 )
-    {
-        warning_add("PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW\n");
-        dom0_mem = MB(512);
-    }
-
-
-    iommu_hwdom_init(d);
-
-    d->max_pages = ~0U;
-
-    kinfo.unassigned_mem = dom0_mem;
-    kinfo.d = d;
-
-    rc = kernel_probe(&kinfo, NULL);
-    if ( rc < 0 )
-        return rc;
-
 #ifdef CONFIG_ARM_64
     /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
-    if ( !(cpu_has_el1_32) && kinfo.type == DOMAIN_32BIT )
+    if ( !(cpu_has_el1_32) && kinfo->type == DOMAIN_32BIT )
     {
         printk("Platform does not support 32-bit domain\n");
         return -EINVAL;
     }
-    d->arch.type = kinfo.type;
 
     if ( is_64bit_domain(d) )
         vcpu_switch_to_aarch64_mode(v);
 
 #endif
 
-    allocate_memory_11(d, &kinfo);
-    find_gnttab_region(d, &kinfo);
-
-    /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
-    rc = gic_map_hwdom_extra_mappings(d);
-    if ( rc < 0 )
-        return rc;
-
-    rc = platform_specific_mapping(d);
-    if ( rc < 0 )
-        return rc;
-
-    if ( acpi_disabled )
-        rc = prepare_dtb(d, &kinfo);
-    else
-        rc = prepare_acpi(d, &kinfo);
-
-    if ( rc < 0 )
-        return rc;
-
     /*
      * The following loads use the domain's p2m and require current to
      * be a vcpu of the domain, temporarily switch
@@ -2287,20 +2243,18 @@ int __init construct_dom0(struct domain *d)
      * kernel_load will determine the placement of the kernel as well
      * as the initrd & fdt in RAM, so call it first.
      */
-    kernel_load(&kinfo);
+    kernel_load(kinfo);
     /* initrd_load will fix up the fdt, so call it before dtb_load */
-    initrd_load(&kinfo);
-    dtb_load(&kinfo);
+    initrd_load(kinfo);
+    dtb_load(kinfo);
 
     /* Now that we are done restore the original p2m and current. */
     set_current(saved_current);
     p2m_restore_state(saved_current);
 
-    discard_initial_modules();
-
     memset(regs, 0, sizeof(*regs));
 
-    regs->pc = (register_t)kinfo.entry;
+    regs->pc = (register_t)kinfo->entry;
 
     if ( is_32bit_domain(d) )
     {
@@ -2318,14 +2272,14 @@ int __init construct_dom0(struct domain *d)
          */
         regs->r0 = 0; /* SBZ */
         regs->r1 = 0xffffffff; /* We use DTB therefore no machine id */
-        regs->r2 = kinfo.dtb_paddr;
+        regs->r2 = kinfo->dtb_paddr;
     }
 #ifdef CONFIG_ARM_64
     else
     {
         regs->cpsr = PSR_GUEST64_INIT;
         /* From linux/Documentation/arm64/booting.txt */
-        regs->x0 = kinfo.dtb_paddr;
+        regs->x0 = kinfo->dtb_paddr;
         regs->x1 = 0; /* Reserved for future use */
         regs->x2 = 0; /* Reserved for future use */
         regs->x3 = 0; /* Reserved for future use */
@@ -2353,6 +2307,62 @@ int __init construct_dom0(struct domain *d)
     return 0;
 }
 
+int __init construct_dom0(struct domain *d)
+{
+    struct kernel_info kinfo = {};
+    int rc;
+
+    /* Sanity! */
+    BUG_ON(d->domain_id != 0);
+
+    printk("*** LOADING DOMAIN 0 ***\n");
+    if ( dom0_mem <= 0 )
+    {
+        warning_add("PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW\n");
+        dom0_mem = MB(512);
+    }
+
+    iommu_hwdom_init(d);
+
+    d->max_pages = ~0U;
+
+    kinfo.unassigned_mem = dom0_mem;
+    kinfo.d = d;
+
+    rc = kernel_probe(&kinfo, NULL);
+    if ( rc < 0 )
+        return rc;
+
+#ifdef CONFIG_ARM_64
+    /* type must be set before allocate_memory */
+    d->arch.type = kinfo.type;
+#endif
+    allocate_memory_11(d, &kinfo);
+    find_gnttab_region(d, &kinfo);
+
+    /* Map extra GIC MMIO, irqs and other hw stuffs to dom0. */
+    rc = gic_map_hwdom_extra_mappings(d);
+    if ( rc < 0 )
+        return rc;
+
+    rc = platform_specific_mapping(d);
+    if ( rc < 0 )
+        return rc;
+
+    if ( acpi_disabled )
+        rc = prepare_dtb_hwdom(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
+    if ( rc < 0 )
+        return rc;
+
+    rc = construct_domain(d, &kinfo);
+    discard_initial_modules();
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 14/26] xen/arm: move unregister_init_virtual_region to init_done
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (12 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 13/26] xen/arm: refactor construct_dom0 Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 15/26] xen/arm: introduce create_domUs Stefano Stabellini
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Move unregister_init_virtual_region to init_done. Follow the same path
as x86. It is also useful to move it later so that create_domUs can be
called before that in following patches.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index a819953..b0a5e35 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -66,6 +66,9 @@ integer_param("xenheap_megabytes", opt_xenheap_megabytes);
 
 static __used void init_done(void)
 {
+    /* Must be done past setting system_state. */
+    unregister_init_virtual_region();
+
     free_init_memory();
     startup_cpu_idle_loop();
 }
@@ -960,9 +963,6 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     system_state = SYS_STATE_active;
 
-    /* Must be done past setting system_state. */
-    unregister_init_virtual_region();
-
     domain_unpause_by_systemcontroller(dom0);
 
     /* Switch on to the dynamically allocated stack for the idle vcpu
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 15/26] xen/arm: introduce create_domUs
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (13 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 14/26] xen/arm: move unregister_init_virtual_region to init_done Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 16/26] xen/arm: implement construct_domU Stefano Stabellini
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, sstabellini, andrii_anisov, andrew.cooper3,
	Achin.Gupta, xen-devel, jbeulich

Call a new function, "create_domUs", from setup_xen to start DomU VMs.

Introduce support for the "xen,domain" compatible node on device tree.
Create new DomU VMs based on the information found on device tree under
"xen,domain". Call construct_domU for each domain.

Introduce a simple global variable named max_init_domid to keep track of
the initial allocated domids. It holds the max domid among the initial
domains.

Move the discard_initial_modules after DomUs have been built.

First create domUs, then start dom0 -- no point in trying to start dom0
when the cpu is busy.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
CC: andrew.cooper3@citrix.com
CC: jbeulich@suse.com
---
Changes in v5:
- use dt_property_read_bool
- improve commit message
- code style
- use dt_find_node_by_path instead of dt_find_node_by_name
- use true with is_console

Changes in v4:
- constify parameters
- nr_spis to 0 or  GUEST_VPL011_SPI - 32 + 1 depending on vpl011
- remove pointless initializer
- remove change to domain_create for dom0 (useless)
- make construct_domU return error

Changes in v3:
- move patch earlier and introduce empty construct_domU to fix bisection
  builds
- fix max_init_domid to actually hold the max domid among initial
  domains (instead of max_domid + 1)
- move domain_unpause_by_systemcontroller(dom0) after creating domUs

Changes in v2:
- coding style
- set nr_spis to 32
- introduce create_domUs
---
 xen/arch/arm/domain_build.c | 50 +++++++++++++++++++++++++++++++++++++++++----
 xen/arch/arm/setup.c        |  5 +++++
 xen/include/asm-arm/setup.h |  3 +++
 xen/include/asm-x86/setup.h |  2 ++
 4 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 3a9c989..d94540e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -7,6 +7,7 @@
 #include <asm/irq.h>
 #include <asm/regs.h>
 #include <xen/errno.h>
+#include <xen/err.h>
 #include <xen/device_tree.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
@@ -2307,6 +2308,50 @@ static int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
     return 0;
 }
 
+static int __init construct_domU(struct domain *d,
+                                 const struct dt_device_node *node)
+{
+    return -ENOSYS;
+}
+
+void __init create_domUs(void)
+{
+    struct dt_device_node *node;
+    const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
+
+    BUG_ON(chosen == NULL);
+    dt_for_each_child_node(chosen, node)
+    {
+        struct domain *d;
+        struct xen_domctl_createdomain d_cfg = {
+            .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
+            .arch.nr_spis = 0,
+            .max_vcpus = 1,
+            .max_evtchn_port = -1,
+            .max_grant_frames = 64,
+            .max_maptrack_frames = 1024,
+        };
+
+        if ( !dt_device_is_compatible(node, "xen,domain") )
+            continue;
+
+        if ( dt_property_read_bool(node, "vpl011") )
+            d_cfg.arch.nr_spis = GUEST_VPL011_SPI - 32 + 1;
+        dt_property_read_u32(node, "cpus", &d_cfg.max_vcpus);
+
+        d = domain_create(++max_init_domid, &d_cfg, false);
+        if ( IS_ERR(d) )
+            panic("Error creating domain %s", dt_node_name(node));
+
+        d->is_console = true;
+
+        if ( construct_domU(d, node) != 0 )
+            panic("Could not set up domain %s", dt_node_name(node));
+
+        domain_unpause_by_systemcontroller(d);
+    }
+}
+
 int __init construct_dom0(struct domain *d)
 {
     struct kernel_info kinfo = {};
@@ -2357,10 +2402,7 @@ int __init construct_dom0(struct domain *d)
     if ( rc < 0 )
         return rc;
 
-    rc = construct_domain(d, &kinfo);
-    discard_initial_modules();
-
-    return rc;
+    return construct_domain(d, &kinfo);
 }
 
 /*
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index b0a5e35..3940982 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -64,11 +64,14 @@ static unsigned long opt_xenheap_megabytes __initdata;
 integer_param("xenheap_megabytes", opt_xenheap_megabytes);
 #endif
 
+domid_t __read_mostly max_init_domid;
+
 static __used void init_done(void)
 {
     /* Must be done past setting system_state. */
     unregister_init_virtual_region();
 
+    discard_initial_modules();
     free_init_memory();
     startup_cpu_idle_loop();
 }
@@ -963,6 +966,8 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     system_state = SYS_STATE_active;
 
+    create_domUs();
+
     domain_unpause_by_systemcontroller(dom0);
 
     /* Switch on to the dynamically allocated stack for the idle vcpu
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 0d787e6..eb0c95f 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -75,6 +75,8 @@ struct bootinfo {
 
 extern struct bootinfo bootinfo;
 
+extern domid_t max_init_domid;
+
 void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
@@ -91,6 +93,7 @@ void acpi_create_efi_mmap_table(struct domain *d,
 int acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
 
 int construct_dom0(struct domain *d);
+void __init create_domUs(void);
 
 void discard_initial_modules(void);
 void dt_unreserved_regions(paddr_t s, paddr_t e,
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 42fddeb..1c80783 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -66,4 +66,6 @@ extern bool opt_dom0_shadow;
 #endif
 extern bool dom0_pvh;
 
+#define max_init_domid (0)
+
 #endif
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 16/26] xen/arm: implement construct_domU
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (14 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 15/26] xen/arm: introduce create_domUs Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 17/26] xen/arm: generate a simple device tree for domUs Stefano Stabellini
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

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.

Remove #if 0 from allocate_memory as this patch will start using it.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
Changes in v5:
- move changes to kernel_probe prototype to previous patch
- improve commit message
- remove superflous allocation of d->vcpu
- use mem * SZ_1K

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 | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d94540e..7b4e34e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -4,6 +4,7 @@
 #include <xen/mm.h>
 #include <xen/domain_page.h>
 #include <xen/sched.h>
+#include <xen/sizes.h>
 #include <asm/irq.h>
 #include <asm/regs.h>
 #include <xen/errno.h>
@@ -369,7 +370,6 @@ static void __init allocate_memory_11(struct domain *d,
     }
 }
 
-#if 0
 static bool __init allocate_bank_memory(struct domain *d,
                                        struct kernel_info *kinfo,
                                        gfn_t sgfn,
@@ -469,7 +469,6 @@ fail:
           " %ldKB unallocated. Fix the VMs configurations.\n",
           (unsigned long)kinfo->unassigned_mem >> 10);
 }
-#endif
 
 static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
                                    const struct dt_device_node *node)
@@ -2311,7 +2310,37 @@ 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 * SZ_1K;
+
+    printk("*** LOADING DOMU cpus=%u memory=%luKB ***\n", d->max_vcpus, mem);
+
+    if ( vcpu_create(d, 0, 0) == NULL )
+        return -ENOMEM;
+    d->max_pages = ~0U;
+
+    kinfo.d = d;
+
+    rc = kernel_probe(&kinfo, node);
+    if ( rc < 0 )
+        return rc;
+
+#ifdef CONFIG_ARM_64
+    /* type must be set before allocate memory */
+    d->arch.type = kinfo.type;
+#endif
+    allocate_memory(d, &kinfo);
+
+    return construct_domain(d, &kinfo);
 }
 
 void __init create_domUs(void)
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 17/26] xen/arm: generate a simple device tree for domUs
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (15 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 16/26] xen/arm: implement construct_domU Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 18/26] xen/arm: make set_interrupt_ppi able to handle non-PPI Stefano Stabellini
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Introduce functions to generate a basic domU device tree, similar to the
existing functions in tools/libxl/libxl_arm.c.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v5:
- use d->arch.vgic.version

Changes in v4:
- code style
- two separate functions for gicv2 and gicv3
- remove useless local variables
- fix typos
- do not use host address and size cells for the guest DT
- use #define for addrcells and sizecells

Changes in v3:
- remove CONFIG_ACPI for make_chosen_node
- remove make_hypervisor_node until all Xen related functionalities
  (evtchns, grant table, etc.) work correctly

Changes in v2:
- move prepare_dtb rename to previous patch
- use switch for the gic version
- use arm,gic-400 instead of arm,cortex-a15-gic
- add @unit-address in the gic node name
- add comment on DOMU_DTB_SIZE
---
 xen/arch/arm/domain_build.c | 235 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 233 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7b4e34e..aee92d3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1035,7 +1035,6 @@ static int __init make_timer_node(const struct domain *d, void *fdt,
     return res;
 }
 
-#ifdef CONFIG_ACPI
 /*
  * This function is used as part of the device tree generation for Dom0
  * on ACPI systems, and DomUs started directly from Xen based on device
@@ -1081,7 +1080,6 @@ static int __init make_chosen_node(const struct kernel_info *kinfo)
 
     return res;
 }
-#endif
 
 static int __init map_irq_to_domain(struct domain *d, unsigned int irq,
                                     bool need_mapping, const char *devname)
@@ -1473,6 +1471,235 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
     return res;
 }
 
+static int __init make_gicv2_domU_node(const struct domain *d, void *fdt)
+{
+    int res = 0;
+    __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
+    __be32 *cells;
+
+    res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICD_BASE));
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#address-cells", 0);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#interrupt-cells", 3);
+    if ( res )
+        return res;
+
+    res = fdt_property(fdt, "interrupt-controller", NULL, 0);
+    if ( res )
+        return res;
+
+    res = fdt_property_string(fdt, "compatible", "arm,gic-400");
+    if ( res )
+        return res;
+
+    cells = &reg[0];
+    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                       GUEST_GICD_BASE, GUEST_GICD_SIZE);
+    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                       GUEST_GICC_BASE, GUEST_GICC_SIZE);
+
+    res = fdt_property(fdt, "reg", reg, sizeof(reg));
+    if (res)
+        return res;
+
+    res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
+    if (res)
+        return res;
+
+    res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
+    if (res)
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int __init make_gicv3_domU_node(const struct domain *d, void *fdt)
+{
+    int res = 0;
+    __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
+    __be32 *cells;
+
+    res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE));
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#address-cells", 0);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "#interrupt-cells", 3);
+    if ( res )
+        return res;
+
+    res = fdt_property(fdt, "interrupt-controller", NULL, 0);
+    if ( res )
+        return res;
+
+    res = fdt_property_string(fdt, "compatible", "arm,gic-v3");
+    if ( res )
+        return res;
+
+    cells = &reg[0];
+    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                       GUEST_GICV3_GICD_BASE, GUEST_GICV3_GICD_SIZE);
+    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+                       GUEST_GICV3_GICR0_BASE, GUEST_GICV3_GICR0_SIZE);
+
+    res = fdt_property(fdt, "reg", reg, sizeof(reg));
+    if (res)
+        return res;
+
+    res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
+    if (res)
+        return res;
+
+    res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
+    if (res)
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int __init make_gic_domU_node(const struct domain *d, void *fdt)
+{
+    switch ( d->arch.vgic.version )
+    {
+    case GIC_V3:
+        return make_gicv3_domU_node(d, fdt);
+    case GIC_V2:
+        return make_gicv2_domU_node(d, fdt);
+    default:
+        panic("Unsupported GIC version");
+    }
+}
+
+static int __init make_timer_domU_node(const struct domain *d, void *fdt)
+{
+    int res;
+    gic_interrupt_t intrs[3];
+
+    res = fdt_begin_node(fdt, "timer");
+    if ( res )
+        return res;
+
+    if ( !is_64bit_domain(d) )
+    {
+        res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
+        if ( res )
+            return res;
+    }
+    else
+    {
+        res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
+        if ( res )
+            return res;
+    }
+
+    set_interrupt_ppi(intrs[0], GUEST_TIMER_PHYS_S_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt_ppi(intrs[1], GUEST_TIMER_PHYS_NS_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt_ppi(intrs[2], GUEST_TIMER_VIRT_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+
+    res = fdt_property(fdt, "interrupts", intrs, sizeof (intrs[0]) * 3);
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "interrupt-parent",
+                            GUEST_PHANDLE_GIC);
+    if (res)
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+/*
+ * 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.
+ */
+#define DOMU_DTB_SIZE 4096
+static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
+{
+    int addrcells, sizecells;
+    int ret;
+
+    addrcells = GUEST_ROOT_ADDRESS_CELLS;
+    sizecells = GUEST_ROOT_SIZE_CELLS;
+
+    kinfo->fdt = xmalloc_bytes(DOMU_DTB_SIZE);
+    if ( kinfo->fdt == NULL )
+        return -ENOMEM;
+
+    ret = fdt_create(kinfo->fdt, DOMU_DTB_SIZE);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_finish_reservemap(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_begin_node(kinfo->fdt, "/");
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_property_cell(kinfo->fdt, "#address-cells", addrcells);
+    if ( ret )
+        goto err;
+
+    ret = fdt_property_cell(kinfo->fdt, "#size-cells", sizecells);
+    if ( ret )
+        goto err;
+
+    ret = make_chosen_node(kinfo);
+    if ( ret )
+        goto err;
+
+    ret = make_psci_node(kinfo->fdt, NULL);
+    if ( ret )
+        goto err;
+
+    ret = make_cpus_node(d, kinfo->fdt, NULL);
+    if ( ret )
+        goto err;
+
+    ret = make_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
+    if ( ret )
+        goto err;
+
+    ret = make_gic_domU_node(d, kinfo->fdt);
+    if ( ret )
+        goto err;
+
+    ret = make_timer_domU_node(d, kinfo->fdt);
+    if ( ret )
+        goto err;
+
+    ret = fdt_end_node(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_finish(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    return 0;
+
+  err:
+    printk("Device tree generation failed (%d).\n", ret);
+    xfree(kinfo->fdt);
+
+    return -EINVAL;
+}
+
 static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info *kinfo)
 {
     const p2m_type_t default_p2mt = p2m_mmio_direct_c;
@@ -2340,6 +2567,10 @@ static int __init construct_domU(struct domain *d,
 #endif
     allocate_memory(d, &kinfo);
 
+    rc = prepare_dtb_domU(d, &kinfo);
+    if ( rc < 0 )
+        return rc;
+
     return construct_domain(d, &kinfo);
 }
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 18/26] xen/arm: make set_interrupt_ppi able to handle non-PPI
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (16 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 17/26] xen/arm: generate a simple device tree for domUs Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 19/26] xen/arm: generate vpl011 node on device tree for domU Stefano Stabellini
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

also rename it to set_interrupt.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index aee92d3..44c7861 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -595,19 +595,20 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
 
 typedef __be32 gic_interrupt_t[3];
 
-static void __init set_interrupt_ppi(gic_interrupt_t interrupt,
-                                     unsigned int irq,
-                                     unsigned int cpumask,
-                                     unsigned int level)
+static void __init set_interrupt(gic_interrupt_t interrupt,
+                                 unsigned int irq,
+                                 unsigned int cpumask,
+                                 unsigned int level)
 {
     __be32 *cells = interrupt;
+    bool is_ppi = !!(irq < 32);
 
     BUG_ON(irq < 16);
-    BUG_ON(irq >= 32);
+    irq -= (is_ppi) ? 16: 32; /* PPIs start at 16, SPIs at 32 */
 
     /* See linux Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
-    dt_set_cell(&cells, 1, 1); /* is a PPI */
-    dt_set_cell(&cells, 1, irq - 16); /* PPIs start at 16 */
+    dt_set_cell(&cells, 1, is_ppi); /* is a PPI? */
+    dt_set_cell(&cells, 1, irq);
     dt_set_cell(&cells, 1, (cpumask << 8) | level);
 }
 
@@ -730,7 +731,7 @@ static int __init make_hypervisor_node(struct domain *d,
      *  - All CPUs
      *  TODO: Handle properly the cpumask;
      */
-    set_interrupt_ppi(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
     res = fdt_property_interrupts(fdt, &intr, 1);
     if ( res )
         return res;
@@ -1007,15 +1008,15 @@ static int __init make_timer_node(const struct domain *d, void *fdt,
 
     irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
     dt_dprintk("  Secure interrupt %u\n", irq);
-    set_interrupt_ppi(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
     irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
     dt_dprintk("  Non secure interrupt %u\n", irq);
-    set_interrupt_ppi(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
     irq = timer_get_irq(TIMER_VIRT_PPI);
     dt_dprintk("  Virt interrupt %u\n", irq);
-    set_interrupt_ppi(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
     res = fdt_property_interrupts(fdt, intrs, 3);
     if ( res )
@@ -1604,9 +1605,9 @@ static int __init make_timer_domU_node(const struct domain *d, void *fdt)
             return res;
     }
 
-    set_interrupt_ppi(intrs[0], GUEST_TIMER_PHYS_S_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
-    set_interrupt_ppi(intrs[1], GUEST_TIMER_PHYS_NS_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
-    set_interrupt_ppi(intrs[2], GUEST_TIMER_VIRT_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intrs[0], GUEST_TIMER_PHYS_S_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intrs[1], GUEST_TIMER_PHYS_NS_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+    set_interrupt(intrs[2], GUEST_TIMER_VIRT_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
     res = fdt_property(fdt, "interrupts", intrs, sizeof (intrs[0]) * 3);
     if ( res )
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 19/26] xen/arm: generate vpl011 node on device tree for domU
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (17 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 18/26] xen/arm: make set_interrupt_ppi able to handle non-PPI Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 20/26] xen/arm: introduce a union in vpl011 Stefano Stabellini
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Introduce vpl011 support to guests started from Xen: it provides a
simple way to print output from a guest, as most guests come with a
pl011 driver. It is also able to provide a working console with
interrupt support.

The UART exposed to the guest is a SBSA compatible UART and not a PL011.
SBSA UART is a subset of PL011 r1p5. A full PL011 implementation in Xen
would just be too difficult, so guests may require some drivers changes.

Enable vpl011 conditionally if the user requested it.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v5:
- use dt_property_read_bool

Changes in v4:
- move rename set_interrupt_ppi and making set_interrupt_ppi generic to
  a separate patch
- properly name the vpl011 device node name
- code style
- use #define for addrcells and sizecells

Changes in v3:
- use bool
- retain BUG_ON(irq < 16)
- add vpl011 bool to kinfo
- return error of vpl011 is required but CONFIG_SBSA_VUART_CONSOLE is
  missing

Changes in v2:
- code style fixes
- make set_interrupt_ppi generic
- rename set_interrupt_ppi to set_interrupt
- only make the vpl011 node if the option was enabled
---
 xen/arch/arm/domain_build.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/kernel.h       |  3 +++
 2 files changed, 63 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 44c7861..d81ec53 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1623,6 +1623,54 @@ static int __init make_timer_domU_node(const struct domain *d, void *fdt)
     return res;
 }
 
+#ifdef CONFIG_SBSA_VUART_CONSOLE
+static int __init make_vpl011_uart_node(const struct domain *d, void *fdt)
+{
+    int res;
+    gic_interrupt_t intr;
+    __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
+    __be32 *cells;
+
+    res = fdt_begin_node(fdt, "sbsa-uart@"__stringify(GUEST_PL011_BASE));
+    if ( res )
+        return res;
+
+    res = fdt_property_string(fdt, "compatible", "arm,sbsa-uart");
+    if ( res )
+        return res;
+
+    cells = &reg[0];
+    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS,
+                       GUEST_ROOT_SIZE_CELLS, GUEST_PL011_BASE,
+                       GUEST_PL011_SIZE);
+    if ( res )
+        return res;
+    res = fdt_property(fdt, "reg", reg, sizeof(reg));
+    if ( res )
+        return res;
+
+    set_interrupt(intr, GUEST_VPL011_SPI, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
+
+    res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
+    if ( res )
+        return res;
+
+    res = fdt_property_cell(fdt, "interrupt-parent",
+                            GUEST_PHANDLE_GIC);
+    if ( res )
+        return res;
+
+    /* Use a default baud rate of 115200. */
+    fdt_property_u32(fdt, "current-speed", 115200);
+
+    res = fdt_end_node(fdt);
+    if ( res )
+        return res;
+
+    return 0;
+}
+#endif
+
 /*
  * 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.
@@ -1684,6 +1732,16 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo)
     if ( ret )
         goto err;
 
+    if ( kinfo->vpl011 )
+    {
+        ret = -EINVAL;
+#ifdef CONFIG_SBSA_VUART_CONSOLE
+        ret = make_vpl011_uart_node(d, kinfo->fdt);
+#endif
+        if ( ret )
+            goto err;
+    }
+
     ret = fdt_end_node(kinfo->fdt);
     if ( ret < 0 )
         goto err;
@@ -2552,6 +2610,8 @@ static int __init construct_domU(struct domain *d,
 
     printk("*** LOADING DOMU cpus=%u memory=%luKB ***\n", d->max_vcpus, mem);
 
+    kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
+
     if ( vcpu_create(d, 0, 0) == NULL )
         return -ENOMEM;
     d->max_pages = ~0U;
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 4320f72..33f3e72 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -33,6 +33,9 @@ struct kernel_info {
     paddr_t dtb_paddr;
     paddr_t initrd_paddr;
 
+    /* Enable pl011 emulation */
+    bool vpl011;
+
     /* loader to use for this kernel */
     void (*load)(struct kernel_info *info);
     /* loader specific state */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 20/26] xen/arm: introduce a union in vpl011
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (18 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 19/26] xen/arm: generate vpl011 node on device tree for domU Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 21/26] xen/arm: refactor vpl011_data_avail Stefano Stabellini
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Introduce a union in struct vpl011 to contain the console ring members.
A later patch will add another member of the union for the case where
the backend is in Xen.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v4:
- name union "backend"

Changes in v3:
- rename ring field to dom

Changes in v2:
- new patch
---
 xen/arch/arm/vpl011.c        | 22 ++++++++++++----------
 xen/include/asm-arm/vpl011.h |  8 ++++++--
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index a281eab..ebc045e 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -82,7 +82,7 @@ static uint8_t vpl011_read_data(struct domain *d)
     unsigned long flags;
     uint8_t data = 0;
     struct vpl011 *vpl011 = &d->arch.vpl011;
-    struct xencons_interface *intf = vpl011->ring_buf;
+    struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
     XENCONS_RING_IDX in_cons, in_prod;
 
     VPL011_LOCK(d, flags);
@@ -145,7 +145,7 @@ static uint8_t vpl011_read_data(struct domain *d)
 static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011,
                                          unsigned int fifo_level)
 {
-    struct xencons_interface *intf = vpl011->ring_buf;
+    struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
     unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL;
 
     BUILD_BUG_ON(sizeof(intf->out) < SBSA_UART_FIFO_SIZE);
@@ -164,7 +164,7 @@ static void vpl011_write_data(struct domain *d, uint8_t data)
 {
     unsigned long flags;
     struct vpl011 *vpl011 = &d->arch.vpl011;
-    struct xencons_interface *intf = vpl011->ring_buf;
+    struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
     XENCONS_RING_IDX out_cons, out_prod;
 
     VPL011_LOCK(d, flags);
@@ -382,7 +382,7 @@ static void vpl011_data_avail(struct domain *d)
 {
     unsigned long flags;
     struct vpl011 *vpl011 = &d->arch.vpl011;
-    struct xencons_interface *intf = vpl011->ring_buf;
+    struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
     XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
     XENCONS_RING_IDX in_fifo_level, out_fifo_level;
 
@@ -459,14 +459,14 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
     int rc;
     struct vpl011 *vpl011 = &d->arch.vpl011;
 
-    if ( vpl011->ring_buf )
+    if ( vpl011->backend.dom.ring_buf )
         return -EINVAL;
 
     /* Map the guest PFN to Xen address space. */
     rc =  prepare_ring_for_helper(d,
                                   gfn_x(info->gfn),
-                                  &vpl011->ring_page,
-                                  &vpl011->ring_buf);
+                                  &vpl011->backend.dom.ring_page,
+                                  &vpl011->backend.dom.ring_buf);
     if ( rc < 0 )
         goto out;
 
@@ -495,7 +495,8 @@ out2:
     vgic_free_virq(d, GUEST_VPL011_SPI);
 
 out1:
-    destroy_ring_for_helper(&vpl011->ring_buf, vpl011->ring_page);
+    destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
+			                vpl011->backend.dom.ring_page);
 
 out:
     return rc;
@@ -505,11 +506,12 @@ void domain_vpl011_deinit(struct domain *d)
 {
     struct vpl011 *vpl011 = &d->arch.vpl011;
 
-    if ( !vpl011->ring_buf )
+    if ( !vpl011->backend.dom.ring_buf )
         return;
 
     free_xen_event_channel(d, vpl011->evtchn);
-    destroy_ring_for_helper(&vpl011->ring_buf, vpl011->ring_page);
+    destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
+			                vpl011->backend.dom.ring_page);
 }
 
 /*
diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h
index db95ff8..42d7a24 100644
--- a/xen/include/asm-arm/vpl011.h
+++ b/xen/include/asm-arm/vpl011.h
@@ -31,8 +31,12 @@
 #define SBSA_UART_FIFO_SIZE 32
 
 struct vpl011 {
-    void *ring_buf;
-    struct page_info *ring_page;
+    union {
+        struct {
+            void *ring_buf;
+            struct page_info *ring_page;
+        } dom;
+    } backend;
     uint32_t    uartfr;         /* Flag register */
     uint32_t    uartcr;         /* Control register */
     uint32_t    uartimsc;       /* Interrupt mask register*/
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 21/26] xen/arm: refactor vpl011_data_avail
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (19 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 20/26] xen/arm: introduce a union in vpl011 Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 22/26] xen: support console_switching between Dom0 and DomUs on ARM Stefano Stabellini
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Move the code to calculate in_fifo_level and out_fifo_level out of
vpl011_data_avail, to the caller.
This change will make it possible to reuse vpl011_data_avail with
different ring structures in a later patch.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
Changes in v3:
- remove forward declaration of vpl011_data_avail

Changes in v2:
- new patch
---
 xen/arch/arm/vpl011.c | 64 +++++++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index ebc045e..68452a8 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -378,30 +378,13 @@ static const struct mmio_handler_ops vpl011_mmio_handler = {
     .write = vpl011_mmio_write,
 };
 
-static void vpl011_data_avail(struct domain *d)
+static void vpl011_data_avail(struct domain *d,
+                              XENCONS_RING_IDX in_fifo_level,
+                              XENCONS_RING_IDX in_size,
+                              XENCONS_RING_IDX out_fifo_level,
+                              XENCONS_RING_IDX out_size)
 {
-    unsigned long flags;
     struct vpl011 *vpl011 = &d->arch.vpl011;
-    struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
-    XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
-    XENCONS_RING_IDX in_fifo_level, out_fifo_level;
-
-    VPL011_LOCK(d, flags);
-
-    in_cons = intf->in_cons;
-    in_prod = intf->in_prod;
-    out_cons = intf->out_cons;
-    out_prod = intf->out_prod;
-
-    smp_rmb();
-
-    in_fifo_level = xencons_queued(in_prod,
-                                   in_cons,
-                                   sizeof(intf->in));
-
-    out_fifo_level = xencons_queued(out_prod,
-                                    out_cons,
-                                    sizeof(intf->out));
 
     /**** Update the UART RX state ****/
 
@@ -410,11 +393,11 @@ static void vpl011_data_avail(struct domain *d)
         vpl011->uartfr &= ~RXFE;
 
     /* Set the FIFO_FULL bit if the Xen buffer is full. */
-    if ( in_fifo_level == sizeof(intf->in) )
+    if ( in_fifo_level == in_size )
         vpl011->uartfr |= RXFF;
 
     /* Assert the RX interrupt if the FIFO is more than half way filled. */
-    if ( in_fifo_level >= sizeof(intf->in) - SBSA_UART_FIFO_LEVEL )
+    if ( in_fifo_level >= in_size - SBSA_UART_FIFO_LEVEL )
         vpl011->uartris |= RXI;
 
     /*
@@ -427,7 +410,7 @@ static void vpl011_data_avail(struct domain *d)
 
     /**** Update the UART TX state ****/
 
-    if ( out_fifo_level != sizeof(intf->out) )
+    if ( out_fifo_level != out_size )
     {
         vpl011->uartfr &= ~TXFF;
 
@@ -445,13 +428,38 @@ static void vpl011_data_avail(struct domain *d)
 
     if ( out_fifo_level == 0 )
         vpl011->uartfr |= TXFE;
-
-    VPL011_UNLOCK(d, flags);
 }
 
 static void vpl011_notification(struct vcpu *v, unsigned int port)
 {
-    vpl011_data_avail(v->domain);
+    unsigned long flags;
+    struct domain *d = v->domain;
+    struct vpl011 *vpl011 = &d->arch.vpl011;
+    struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
+    XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod;
+    XENCONS_RING_IDX in_fifo_level, out_fifo_level;
+
+    VPL011_LOCK(d, flags);
+
+    in_cons = intf->in_cons;
+    in_prod = intf->in_prod;
+    out_cons = intf->out_cons;
+    out_prod = intf->out_prod;
+
+    smp_rmb();
+
+    in_fifo_level = xencons_queued(in_prod,
+                                   in_cons,
+                                   sizeof(intf->in));
+
+    out_fifo_level = xencons_queued(out_prod,
+                                    out_cons,
+                                    sizeof(intf->out));
+
+    vpl011_data_avail(v->domain, in_fifo_level, sizeof(intf->in),
+                      out_fifo_level, sizeof(intf->out));
+
+    VPL011_UNLOCK(d, flags);
 }
 
 int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 22/26] xen: support console_switching between Dom0 and DomUs on ARM
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (20 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 21/26] xen/arm: refactor vpl011_data_avail Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-05  9:15   ` Jan Beulich
  2018-11-02 23:45 ` [PATCH v6 23/26] xen/arm: Allow vpl011 to be used by DomU Stefano Stabellini
                   ` (3 subsequent siblings)
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, sstabellini, andrii_anisov, konrad.wilk,
	George.Dunlap, andrew.cooper3, Achin.Gupta, ian.jackson,
	xen-devel, tim, jbeulich, wei.liu2

Today Ctrl-AAA is used to switch between Xen and Dom0. Extend the
mechanism to allow for switching between Xen, Dom0, and any of the
initial DomU created from Xen alongside Dom0 out of information provided
via device tree.

Rename xen_rx to console_rx to match the new behavior.

Clarify existing comment about "notify the guest", making it clear that
it is only about the hardware domain.

Switching the console input to domUs started from Xen at boot is
#ifdef'ed to 0 in this patch. The code will be enabled when
vpl011_rx_char_xen is introduced. For now it is disabled for
bisectability.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
CC: andrew.cooper3@citrix.com
CC: George.Dunlap@eu.citrix.com
CC: ian.jackson@eu.citrix.com
CC: jbeulich@suse.com
CC: konrad.wilk@oracle.com
CC: tim@xen.org
CC: wei.liu2@citrix.com
---
Changes in v6:
- improve in-code comment
- improve commit messahe
- code style

Changes in v5:
- move patch earlier and disable code that calls vpl011_rx_char_xen (not
  defined yet)
- improve comments
- replace ifs with a switch
- code style

Changes in v4:
- handle console_rx == 0 in console_input_domain
- use rcu_lock_by_domain instead of get_domain_by_id
- handle the case where the returned domain is NULL
- send_global_virq(VIRQ_CONSOLE) only when chars are for Dom0
- make console_rx unsigned int
- fix comment
- code readability improvement
- fix opt_conswitch[1] == 'x' case
- move console_input_domain to next patch

Changes in v3:
- only call vpl011 functions ifdef CONFIG_SBSA_VUART_CONSOLE
- add blank line and spaces
- remove xen_rx from print messages
- rename xen_rx to console_rx
- keep switch_serial_input() at boot
- add better comments
- fix existing comment
- add warning if no guests console/uart is available
- add console_input_domain function

Changes in v2:
- only call vpl011_rx_char if the vpl011 has been initialized
---
 xen/drivers/char/console.c | 82 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 68 insertions(+), 14 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 3b75f7a..8a01a43 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -31,10 +31,13 @@
 #include <xen/early_printk.h>
 #include <xen/warning.h>
 #include <xen/pv_console.h>
+#include <asm/setup.h>
 
 #ifdef CONFIG_X86
 #include <xen/consoled.h>
 #include <asm/guest.h>
+#else
+#include <asm/vpl011.h>
 #endif
 
 /* console: comma-separated list of console outputs. */
@@ -391,31 +394,82 @@ static void dump_console_ring_key(unsigned char key)
     free_xenheap_pages(buf, order);
 }
 
-/* CTRL-<switch_char> switches input direction between Xen and DOM0. */
+/*
+ * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
+ * and the DomUs started from Xen at boot.
+ */
 #define switch_code (opt_conswitch[0]-'a'+1)
-static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. */
+/*
+ * console_rx=0 => input to xen
+ * console_rx=1 => input to dom0
+ * console_rx=N => input to dom(N-1)
+ */
+static unsigned int __read_mostly console_rx = 0;
 
 static void switch_serial_input(void)
 {
-    static char *input_str[2] = { "DOM0", "Xen" };
-    xen_rx = !xen_rx;
-    printk("*** Serial input -> %s", input_str[xen_rx]);
+    if ( console_rx == max_init_domid + 1 )
+    {
+        console_rx = 0;
+        printk("*** Serial input to Xen");
+    }
+    else
+    {
+        console_rx++;
+        printk("*** Serial input to DOM%d", console_rx - 1);
+    }
+
     if ( switch_code )
-        printk(" (type 'CTRL-%c' three times to switch input to %s)",
-               opt_conswitch[0], input_str[!xen_rx]);
+        printk(" (type 'CTRL-%c' three times to switch input)",
+               opt_conswitch[0]);
     printk("\n");
 }
 
 static void __serial_rx(char c, struct cpu_user_regs *regs)
 {
-    if ( xen_rx )
+    switch ( console_rx )
+    {
+    case 0:
         return handle_keypress(c, regs);
 
-    /* Deliver input to guest buffer, unless it is already full. */
-    if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
-        serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
-    /* Always notify the guest: prevents receive path from getting stuck. */
-    send_global_virq(VIRQ_CONSOLE);
+    case 1:
+        /*
+         * Deliver input to the hardware domain buffer, unless it is
+         * already full.
+         */
+        if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
+            serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
+
+        /*
+         * Always notify the hardware domain: prevents receive path from
+         * getting stuck.
+         */
+        send_global_virq(VIRQ_CONSOLE);
+        break;
+
+#if 0
+    default:
+    {
+        struct domain *d = rcu_lock_domain_by_any_id(console_rx - 1);
+
+        /*
+         * If we have a properly initialized vpl011 console for the
+         * domain, without a full PV ring to Dom0 (in that case input
+         * comes from the PV ring), then send the character to it.
+         */
+        if ( d != NULL &&
+             !d->arch.vpl011.backend_in_domain &&
+             d->arch.vpl011.backend.xen != NULL )
+            vpl011_rx_char_xen(d, c);
+        else
+            printk("Cannot send chars to Dom%d: no UART available\n",
+                   console_rx - 1);
+
+        if ( d != NULL )
+            rcu_unlock_domain(d);
+    }
+#endif
+    }
 
 #ifdef CONFIG_X86
     if ( pv_shim && pv_console )
@@ -943,7 +997,7 @@ void __init console_endboot(void)
      * a useful 'how to switch' message.
      */
     if ( opt_conswitch[1] == 'x' )
-        xen_rx = !xen_rx;
+        console_rx = max_init_domid + 1;
 
     register_keyhandler('w', dump_console_ring_key,
                         "synchronously dump console ring buffer (dmesg)", 0);
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 23/26] xen/arm: Allow vpl011 to be used by DomU
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (21 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 22/26] xen: support console_switching between Dom0 and DomUs on ARM Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen Stefano Stabellini
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

Make vpl011 being able to be used without a userspace component in Dom0.
In that case, output is printed to the Xen serial and input is received
from the Xen serial one character at a time.

Call domain_vpl011_init during construct_domU if vpl011 is enabled.

Introduce a new ring struct with only the ring array to avoid a waste of
memory. Introduce separate read_data and write_data functions for
initial domains: vpl011_write_data_xen is very simple and just writes
to the console, while vpl011_read_data_xen is a duplicate of
vpl011_read_data. Although textually almost identical, we are forced to
duplicate the functions because the struct layout is different.

Output characters are printed one by one, potentially leading to
intermixed output of different domains on the console. A follow-up patch
will solve the issue by introducing buffering.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v5:
- renable call to vpl011_rx_char_xen from console.c

Changes in v4:
- code style

Changes in v3:
- add in-code comments
- improve existing comments
- remove ifdef around domain_vpl011_init in construct_domU
- add ASSERT
- use SBSA_UART_FIFO_SIZE for in buffer size
- rename ring_enable to backend_in_domain
- rename struct xencons_in to struct vpl011_xen_backend
- rename inring field to xen
- rename helper functions accordingly
- remove unnecessary stub implementation of vpl011_rx_char
- move vpl011_rx_char_xen within the file to avoid the need of a forward
  declaration of vpl011_data_avail
- fix small bug in vpl011_rx_char_xen: increment in_prod before using it
  to check xencons_queued.

Changes in v2:
- only init if vpl011
- rename vpl011_read_char to vpl011_rx_char
- remove spurious change
- fix coding style
- use different ring struct
- move the write_data changes to their own function
  (vpl011_write_data_noring)
- duplicate vpl011_read_data
---
 xen/arch/arm/domain_build.c  |   9 +-
 xen/arch/arm/vpl011.c        | 200 +++++++++++++++++++++++++++++++++++++------
 xen/drivers/char/console.c   |   2 +-
 xen/include/asm-arm/vpl011.h |   8 ++
 4 files changed, 193 insertions(+), 26 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d81ec53..761accb 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2632,7 +2632,14 @@ static int __init construct_domU(struct domain *d,
     if ( rc < 0 )
         return rc;
 
-    return construct_domain(d, &kinfo);
+    rc = construct_domain(d, &kinfo);
+    if ( rc < 0 )
+        return rc;
+
+    if ( kinfo.vpl011 )
+        rc = domain_vpl011_init(d, NULL);
+
+    return rc;
 }
 
 void __init create_domUs(void)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 68452a8..131507e 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct domain *d)
 #endif
 }
 
+/*
+ * vpl011_write_data_xen writes chars from the vpl011 out buffer to the
+ * console. Only to be used when the backend is Xen.
+ */
+static void vpl011_write_data_xen(struct domain *d, uint8_t data)
+{
+    unsigned long flags;
+    struct vpl011 *vpl011 = &d->arch.vpl011;
+
+    VPL011_LOCK(d, flags);
+
+    printk("%c", data);
+    if (data == '\n')
+        printk("DOM%u: ", d->domain_id);
+
+    vpl011->uartris |= TXI;
+    vpl011->uartfr &= ~TXFE;
+    vpl011_update_interrupt_status(d);
+
+    VPL011_UNLOCK(d, flags);
+}
+
+/*
+ * vpl011_read_data_xen reads data when the backend is xen. Characters
+ * are added to the vpl011 receive buffer by vpl011_rx_char_xen.
+ */
+static uint8_t vpl011_read_data_xen(struct domain *d)
+{
+    unsigned long flags;
+    uint8_t data = 0;
+    struct vpl011 *vpl011 = &d->arch.vpl011;
+    struct vpl011_xen_backend *intf = vpl011->backend.xen;
+    XENCONS_RING_IDX in_cons, in_prod;
+
+    VPL011_LOCK(d, flags);
+
+    in_cons = intf->in_cons;
+    in_prod = intf->in_prod;
+
+    smp_rmb();
+
+    /*
+     * It is expected that there will be data in the ring buffer when this
+     * function is called since the guest is expected to read the data register
+     * only if the TXFE flag is not set.
+     * If the guest still does read when TXFE bit is set then 0 will be returned.
+     */
+    if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
+    {
+        unsigned int fifo_level;
+
+        data = intf->in[xencons_mask(in_cons, sizeof(intf->in))];
+        in_cons += 1;
+        smp_mb();
+        intf->in_cons = in_cons;
+
+        fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in));
+
+        /* If the FIFO is now empty, we clear the receive timeout interrupt. */
+        if ( fifo_level == 0 )
+        {
+            vpl011->uartfr |= RXFE;
+            vpl011->uartris &= ~RTI;
+        }
+
+        /* If the FIFO is more than half empty, we clear the RX interrupt. */
+        if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL )
+            vpl011->uartris &= ~RXI;
+
+        vpl011_update_interrupt_status(d);
+    }
+    else
+        gprintk(XENLOG_ERR, "vpl011: Unexpected IN ring buffer empty\n");
+
+    /*
+     * We have consumed a character or the FIFO was empty, so clear the
+     * "FIFO full" bit.
+     */
+    vpl011->uartfr &= ~RXFF;
+
+    VPL011_UNLOCK(d, flags);
+
+    return data;
+}
+
 static uint8_t vpl011_read_data(struct domain *d)
 {
     unsigned long flags;
@@ -240,7 +325,10 @@ static int vpl011_mmio_read(struct vcpu *v,
     case DR:
         if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
 
-        *r = vreg_reg32_extract(vpl011_read_data(d), info);
+        if ( vpl011->backend_in_domain )
+            *r = vreg_reg32_extract(vpl011_read_data(d), info);
+        else
+            *r = vreg_reg32_extract(vpl011_read_data_xen(d), info);
         return 1;
 
     case RSR:
@@ -325,7 +413,10 @@ static int vpl011_mmio_write(struct vcpu *v,
 
         vreg_reg32_update(&data, r, info);
         data &= 0xFF;
-        vpl011_write_data(v->domain, data);
+        if ( vpl011->backend_in_domain )
+            vpl011_write_data(v->domain, data);
+        else
+            vpl011_write_data_xen(v->domain, data);
         return 1;
     }
 
@@ -430,6 +521,39 @@ static void vpl011_data_avail(struct domain *d,
         vpl011->uartfr |= TXFE;
 }
 
+/*
+ * vpl011_rx_char_xen adds a char to a domain's vpl011 receive buffer.
+ * It is only used when the vpl011 backend is in Xen.
+ */
+void vpl011_rx_char_xen(struct domain *d, char c)
+{
+    unsigned long flags;
+    struct vpl011 *vpl011 = &d->arch.vpl011;
+    struct vpl011_xen_backend *intf = vpl011->backend.xen;
+    XENCONS_RING_IDX in_cons, in_prod, in_fifo_level;
+
+    ASSERT(!vpl011->backend_in_domain);
+    VPL011_LOCK(d, flags);
+
+    in_cons = intf->in_cons;
+    in_prod = intf->in_prod;
+    if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == sizeof(intf->in) )
+    {
+        VPL011_UNLOCK(d, flags);
+        return;
+    }
+
+    intf->in[xencons_mask(in_prod, sizeof(intf->in))] = c;
+    intf->in_prod = ++in_prod;
+
+    in_fifo_level = xencons_queued(in_prod,
+                                   in_cons,
+                                   sizeof(intf->in));
+
+    vpl011_data_avail(d, in_fifo_level, sizeof(intf->in), 0, SBSA_UART_FIFO_SIZE);
+    VPL011_UNLOCK(d, flags);
+}
+
 static void vpl011_notification(struct vcpu *v, unsigned int port)
 {
     unsigned long flags;
@@ -470,27 +594,47 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
     if ( vpl011->backend.dom.ring_buf )
         return -EINVAL;
 
-    /* Map the guest PFN to Xen address space. */
-    rc =  prepare_ring_for_helper(d,
-                                  gfn_x(info->gfn),
-                                  &vpl011->backend.dom.ring_page,
-                                  &vpl011->backend.dom.ring_buf);
-    if ( rc < 0 )
-        goto out;
+    /*
+     * info is NULL when the backend is in Xen.
+     * info is != NULL when the backend is in a domain.
+     */
+    if ( info != NULL )
+    {
+        vpl011->backend_in_domain = true;
+
+        /* Map the guest PFN to Xen address space. */
+        rc =  prepare_ring_for_helper(d,
+                                      gfn_x(info->gfn),
+                                      &vpl011->backend.dom.ring_page,
+                                      &vpl011->backend.dom.ring_buf);
+        if ( rc < 0 )
+            goto out;
+
+        rc = alloc_unbound_xen_event_channel(d, 0, info->console_domid,
+                                             vpl011_notification);
+        if ( rc < 0 )
+            goto out1;
+
+        vpl011->evtchn = info->evtchn = rc;
+    }
+    else
+    {
+        vpl011->backend_in_domain = false;
+
+        vpl011->backend.xen = xzalloc(struct vpl011_xen_backend);
+        if ( vpl011->backend.xen == NULL )
+        {
+            rc = -EINVAL;
+            goto out1;
+        }
+    }
 
     rc = vgic_reserve_virq(d, GUEST_VPL011_SPI);
     if ( !rc )
     {
         rc = -EINVAL;
-        goto out1;
-    }
-
-    rc = alloc_unbound_xen_event_channel(d, 0, info->console_domid,
-                                         vpl011_notification);
-    if ( rc < 0 )
         goto out2;
-
-    vpl011->evtchn = info->evtchn = rc;
+    }
 
     spin_lock_init(&vpl011->lock);
 
@@ -503,8 +647,11 @@ out2:
     vgic_free_virq(d, GUEST_VPL011_SPI);
 
 out1:
-    destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
-			                vpl011->backend.dom.ring_page);
+    if ( vpl011->backend_in_domain )
+        destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
+                                vpl011->backend.dom.ring_page);
+    else
+        xfree(vpl011->backend.xen);
 
 out:
     return rc;
@@ -514,12 +661,17 @@ void domain_vpl011_deinit(struct domain *d)
 {
     struct vpl011 *vpl011 = &d->arch.vpl011;
 
-    if ( !vpl011->backend.dom.ring_buf )
-        return;
+    if ( vpl011->backend_in_domain )
+    {
+        if ( !vpl011->backend.dom.ring_buf )
+            return;
 
-    free_xen_event_channel(d, vpl011->evtchn);
-    destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
-			                vpl011->backend.dom.ring_page);
+        free_xen_event_channel(d, vpl011->evtchn);
+        destroy_ring_for_helper(&vpl011->backend.dom.ring_buf,
+                                vpl011->backend.dom.ring_page);
+    }
+    else
+        xfree(vpl011->backend.xen);
 }
 
 /*
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 8a01a43..65fd406 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -447,7 +447,7 @@ static void __serial_rx(char c, struct cpu_user_regs *regs)
         send_global_virq(VIRQ_CONSOLE);
         break;
 
-#if 0
+#ifdef CONFIG_SBSA_VUART_CONSOLE
     default:
     {
         struct domain *d = rcu_lock_domain_by_any_id(console_rx - 1);
diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h
index 42d7a24..5eb6d25 100644
--- a/xen/include/asm-arm/vpl011.h
+++ b/xen/include/asm-arm/vpl011.h
@@ -21,6 +21,7 @@
 
 #include <public/domctl.h>
 #include <public/io/ring.h>
+#include <public/io/console.h>
 #include <asm/vreg.h>
 #include <xen/mm.h>
 
@@ -29,13 +30,19 @@
 #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags)
 
 #define SBSA_UART_FIFO_SIZE 32
+struct vpl011_xen_backend {
+    char in[SBSA_UART_FIFO_SIZE];
+    XENCONS_RING_IDX in_cons, in_prod;
+};
 
 struct vpl011 {
+    bool backend_in_domain;
     union {
         struct {
             void *ring_buf;
             struct page_info *ring_page;
         } dom;
+        struct vpl011_xen_backend *xen;
     } backend;
     uint32_t    uartfr;         /* Flag register */
     uint32_t    uartcr;         /* Control register */
@@ -57,6 +64,7 @@ struct vpl011_init_info {
 int domain_vpl011_init(struct domain *d,
                        struct vpl011_init_info *info);
 void domain_vpl011_deinit(struct domain *d);
+void vpl011_rx_char_xen(struct domain *d, char c);
 #else
 static inline int domain_vpl011_init(struct domain *d,
                                      struct vpl011_init_info *info)
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (22 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 23/26] xen/arm: Allow vpl011 to be used by DomU Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-09 14:32   ` Julien Grall
  2018-11-02 23:45 ` [PATCH v6 25/26] xen/arm: move kernel.h to asm-arm/ Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 26/26] xen/arm: split domain_build.c Stefano Stabellini
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, sstabellini, andrii_anisov, konrad.wilk,
	George.Dunlap, andrew.cooper3, Achin.Gupta, ian.jackson,
	xen-devel, tim, jbeulich, wei.liu2

To avoid mixing the output of different domains on the console, buffer
the output chars and print line by line. Unless the domain has input
from the serial, in which case we want to print char by char for a
smooth user experience.

The size of SBSA_UART_OUT_BUF_SIZE is arbitrary, choose the same size
as VUART_BUF_SIZE used in vuart.c.

Export a function named console_input_domain() to allow others to know
which domains has input at a given time.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
CC: andrew.cooper3@citrix.com
CC: George.Dunlap@eu.citrix.com
CC: ian.jackson@eu.citrix.com
CC: jbeulich@suse.com
CC: konrad.wilk@oracle.com
CC: tim@xen.org
CC: wei.liu2@citrix.com
---
XXX: merge this patch with "xen/arm: Allow vpl011 to be used by DomU" on
     commit

Changes in v6:
- fix typo
- add input != NULL check

Changes in v5:
- use rcu_lock in console_input_domain
- rcu_unlock at the end of vpl011_write_data_xen
- add a comment on top of console_input_domain as a reminder that the
  caller needs to rcu_unlock

Changes in v4:
- make SBSA_UART_OUT_BUF_SIZE the same size of VUART_BUF_SIZE
- rearrange the code to be clearer input and != input cases
- code style
- add \n when printing the out buffer because is full
- don't print prefix for input domain
---
 xen/arch/arm/vpl011.c        | 37 ++++++++++++++++++++++++++++++++++---
 xen/drivers/char/console.c   |  8 ++++++++
 xen/include/asm-arm/vpl011.h |  4 ++++
 xen/include/xen/console.h    |  2 ++
 4 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 131507e..719a339 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -28,6 +28,7 @@
 #include <xen/lib.h>
 #include <xen/mm.h>
 #include <xen/sched.h>
+#include <xen/console.h>
 #include <public/domctl.h>
 #include <public/io/console.h>
 #include <asm/pl011-uart.h>
@@ -85,18 +86,48 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
 {
     unsigned long flags;
     struct vpl011 *vpl011 = &d->arch.vpl011;
+    struct vpl011_xen_backend *intf = vpl011->backend.xen;
+    struct domain *input = console_input_domain();
 
     VPL011_LOCK(d, flags);
 
-    printk("%c", data);
-    if (data == '\n')
-        printk("DOM%u: ", d->domain_id);
+    intf->out[intf->out_prod++] = data;
+    if ( d == input )
+    {
+        if ( intf->out_prod == 1 )
+        {
+            printk("%c", data);
+            intf->out_prod = 0;
+        }
+        else
+        {
+            if ( data != '\n' )
+                intf->out[intf->out_prod++] = '\n';
+            intf->out[intf->out_prod++] = '\0';
+            printk("%s", intf->out);
+            intf->out_prod = 0;
+        }
+    }
+    else
+    {
+        if ( intf->out_prod == SBSA_UART_OUT_BUF_SIZE - 2 ||
+             data == '\n' )
+        {
+            if ( data != '\n' )
+                intf->out[intf->out_prod++] = '\n';
+            intf->out[intf->out_prod++] = '\0';
+            printk("DOM%u: %s", d->domain_id, intf->out);
+            intf->out_prod = 0;
+        }
+    }
 
     vpl011->uartris |= TXI;
     vpl011->uartfr &= ~TXFE;
     vpl011_update_interrupt_status(d);
 
     VPL011_UNLOCK(d, flags);
+    if ( input != NULL )
+        rcu_unlock_domain(input);
 }
 
 /*
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 65fd406..fa6569d 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -406,6 +406,14 @@ static void dump_console_ring_key(unsigned char key)
  */
 static unsigned int __read_mostly console_rx = 0;
 
+/* Make sure to rcu_unlock_domain after use */
+struct domain *console_input_domain(void)
+{
+    if ( console_rx == 0 )
+            return NULL;
+    return rcu_lock_domain_by_id(console_rx - 1);
+}
+
 static void switch_serial_input(void)
 {
     if ( console_rx == max_init_domid + 1 )
diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h
index 5eb6d25..12576a4 100644
--- a/xen/include/asm-arm/vpl011.h
+++ b/xen/include/asm-arm/vpl011.h
@@ -30,9 +30,13 @@
 #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags)
 
 #define SBSA_UART_FIFO_SIZE 32
+/* Same size as VUART_BUF_SIZE, used in vuart.c */
+#define SBSA_UART_OUT_BUF_SIZE 128
 struct vpl011_xen_backend {
     char in[SBSA_UART_FIFO_SIZE];
+    char out[SBSA_UART_OUT_BUF_SIZE];
     XENCONS_RING_IDX in_cons, in_prod;
+    XENCONS_RING_IDX out_prod;
 };
 
 struct vpl011 {
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index 70c9911..c5a85c8 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -31,6 +31,8 @@ void console_end_sync(void);
 void console_start_log_everything(void);
 void console_end_log_everything(void);
 
+struct domain *console_input_domain(void);
+
 /*
  * Steal output from the console. Returns +ve identifier, else -ve error.
  * Takes the handle of the serial line to steal, and steal callback function.
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 25/26] xen/arm: move kernel.h to asm-arm/
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (23 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-02 23:45 ` [PATCH v6 26/26] xen/arm: split domain_build.c Stefano Stabellini
  25 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

It will be #included by a file in a xen/arch/arm subdirectory.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain_build.c  |  2 +-
 xen/arch/arm/kernel.c        |  3 +-
 xen/arch/arm/kernel.h        | 86 --------------------------------------------
 xen/include/asm-arm/kernel.h | 86 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 89 deletions(-)
 delete mode 100644 xen/arch/arm/kernel.h
 create mode 100644 xen/include/asm-arm/kernel.h

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 761accb..32ed5aa 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -17,6 +17,7 @@
 #include <xen/warning.h>
 #include <acpi/actables.h>
 #include <asm/device.h>
+#include <asm/kernel.h>
 #include <asm/setup.h>
 #include <asm/platform.h>
 #include <asm/psci.h>
@@ -25,7 +26,6 @@
 
 #include <xen/irq.h>
 #include <xen/grant_table.h>
-#include "kernel.h"
 
 static unsigned int __initdata opt_dom0_max_vcpus;
 integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d6c810b..6dd9901 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -16,8 +16,7 @@
 #include <xen/vmap.h>
 
 #include <asm/guest_access.h>
-
-#include "kernel.h"
+#include <asm/kernel.h>
 
 #define UIMAGE_MAGIC          0x27051956
 #define UIMAGE_NMLEN          32
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
deleted file mode 100644
index 33f3e72..0000000
--- a/xen/arch/arm/kernel.h
+++ /dev/null
@@ -1,86 +0,0 @@
-/*
- * Kernel image loading.
- *
- * Copyright (C) 2011 Citrix Systems, Inc.
- */
-#ifndef __ARCH_ARM_KERNEL_H__
-#define __ARCH_ARM_KERNEL_H__
-
-#include <xen/device_tree.h>
-#include <asm/setup.h>
-
-struct kernel_info {
-#ifdef CONFIG_ARM_64
-    enum domain_type type;
-#endif
-
-    struct domain *d;
-
-    void *fdt; /* flat device tree */
-    paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
-    struct meminfo mem;
-
-    /* kernel entry point */
-    paddr_t entry;
-
-    /* grant table region */
-    paddr_t gnttab_start;
-    paddr_t gnttab_size;
-
-    /* boot blob load addresses */
-    const struct bootmodule *kernel_bootmodule, *initrd_bootmodule;
-    const char* cmdline;
-    paddr_t dtb_paddr;
-    paddr_t initrd_paddr;
-
-    /* Enable pl011 emulation */
-    bool vpl011;
-
-    /* loader to use for this kernel */
-    void (*load)(struct kernel_info *info);
-    /* loader specific state */
-    union {
-        struct {
-            paddr_t kernel_addr;
-            paddr_t len;
-#ifdef CONFIG_ARM_64
-            paddr_t text_offset; /* 64-bit Image only */
-#endif
-            paddr_t start; /* 32-bit zImage only */
-        } zimage;
-    };
-};
-
-/*
- * Probe the kernel to detemine its type and select a loader.
- *
- * Sets in info:
- *  ->type
- *  ->load hook, and sets loader specific variables ->zimage
- */
-int kernel_probe(struct kernel_info *info, const struct dt_device_node *domain);
-
-/*
- * Loads the kernel into guest RAM.
- *
- * Expects to be set in info when called:
- *  ->mem
- *  ->fdt
- *
- * Sets in info:
- *  ->entry
- *  ->dtb_paddr
- *  ->initrd_paddr
- */
-void kernel_load(struct kernel_info *info);
-
-#endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/asm-arm/kernel.h b/xen/include/asm-arm/kernel.h
new file mode 100644
index 0000000..33f3e72
--- /dev/null
+++ b/xen/include/asm-arm/kernel.h
@@ -0,0 +1,86 @@
+/*
+ * Kernel image loading.
+ *
+ * Copyright (C) 2011 Citrix Systems, Inc.
+ */
+#ifndef __ARCH_ARM_KERNEL_H__
+#define __ARCH_ARM_KERNEL_H__
+
+#include <xen/device_tree.h>
+#include <asm/setup.h>
+
+struct kernel_info {
+#ifdef CONFIG_ARM_64
+    enum domain_type type;
+#endif
+
+    struct domain *d;
+
+    void *fdt; /* flat device tree */
+    paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
+    struct meminfo mem;
+
+    /* kernel entry point */
+    paddr_t entry;
+
+    /* grant table region */
+    paddr_t gnttab_start;
+    paddr_t gnttab_size;
+
+    /* boot blob load addresses */
+    const struct bootmodule *kernel_bootmodule, *initrd_bootmodule;
+    const char* cmdline;
+    paddr_t dtb_paddr;
+    paddr_t initrd_paddr;
+
+    /* Enable pl011 emulation */
+    bool vpl011;
+
+    /* loader to use for this kernel */
+    void (*load)(struct kernel_info *info);
+    /* loader specific state */
+    union {
+        struct {
+            paddr_t kernel_addr;
+            paddr_t len;
+#ifdef CONFIG_ARM_64
+            paddr_t text_offset; /* 64-bit Image only */
+#endif
+            paddr_t start; /* 32-bit zImage only */
+        } zimage;
+    };
+};
+
+/*
+ * Probe the kernel to detemine its type and select a loader.
+ *
+ * Sets in info:
+ *  ->type
+ *  ->load hook, and sets loader specific variables ->zimage
+ */
+int kernel_probe(struct kernel_info *info, const struct dt_device_node *domain);
+
+/*
+ * Loads the kernel into guest RAM.
+ *
+ * Expects to be set in info when called:
+ *  ->mem
+ *  ->fdt
+ *
+ * Sets in info:
+ *  ->entry
+ *  ->dtb_paddr
+ *  ->initrd_paddr
+ */
+void kernel_load(struct kernel_info *info);
+
+#endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v6 26/26] xen/arm: split domain_build.c
  2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
                   ` (24 preceding siblings ...)
  2018-11-02 23:45 ` [PATCH v6 25/26] xen/arm: move kernel.h to asm-arm/ Stefano Stabellini
@ 2018-11-02 23:45 ` Stefano Stabellini
  2018-11-09 14:35   ` Julien Grall
  25 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-02 23:45 UTC (permalink / raw)
  To: julien.grall
  Cc: Stefano Stabellini, Achin.Gupta, sstabellini, andrii_anisov, xen-devel

domain_build.c is too large.

Move all the ACPI specific device tree generating functions from
domain_build.c to acpi/domain_build.c.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---

Changes in v6:
- fix license

Changes in v4:
- rename acpi_dt_build to domain_build.c
- add copyright header
- remove useless #include
- remove acpi_dt_build.h, add domain_build.h
---
 xen/arch/arm/acpi/Makefile         |   1 +
 xen/arch/arm/acpi/domain_build.c   | 591 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/domain_build.c        | 585 +-----------------------------------
 xen/include/asm-arm/domain_build.h |  31 ++
 4 files changed, 628 insertions(+), 580 deletions(-)
 create mode 100644 xen/arch/arm/acpi/domain_build.c
 create mode 100644 xen/include/asm-arm/domain_build.h

diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
index 23963f8..94ae249 100644
--- a/xen/arch/arm/acpi/Makefile
+++ b/xen/arch/arm/acpi/Makefile
@@ -1,2 +1,3 @@
 obj-y += lib.o
+obj-y += domain_build.o
 obj-y += boot.init.o
diff --git a/xen/arch/arm/acpi/domain_build.c b/xen/arch/arm/acpi/domain_build.c
new file mode 100644
index 0000000..ed097d5
--- /dev/null
+++ b/xen/arch/arm/acpi/domain_build.c
@@ -0,0 +1,591 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/mm.h>
+#include <xen/sched.h>
+#include <xen/acpi.h>
+#include <xen/event.h>
+#include <xen/iocap.h>
+#include <xen/device_tree.h>
+#include <xen/libfdt/libfdt.h>
+#include <acpi/actables.h>
+#include <asm/kernel.h>
+#include <asm/domain_build.h>
+
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
+#define ACPI_DOM0_FDT_MIN_SIZE 4096
+
+static int __init acpi_iomem_deny_access(struct domain *d)
+{
+    acpi_status status;
+    struct acpi_table_spcr *spcr = NULL;
+    unsigned long mfn;
+    int rc;
+
+    /* Firstly permit full MMIO capabilities. */
+    rc = iomem_permit_access(d, 0UL, ~0UL);
+    if ( rc )
+        return rc;
+
+    /* TODO: Deny MMIO access for SMMU, GIC ITS */
+    status = acpi_get_table(ACPI_SIG_SPCR, 0,
+                            (struct acpi_table_header **)&spcr);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        printk("Failed to get SPCR table\n");
+        return -EINVAL;
+    }
+
+    mfn = spcr->serial_port.address >> PAGE_SHIFT;
+    /* Deny MMIO access for UART */
+    rc = iomem_deny_access(d, mfn, mfn + 1);
+    if ( rc )
+        return rc;
+
+    /* Deny MMIO access for GIC regions */
+    return gic_iomem_deny_access(d);
+}
+
+static int __init acpi_route_spis(struct domain *d)
+{
+    int i, res;
+    struct irq_desc *desc;
+
+    /*
+     * Route the IRQ to hardware domain and permit the access.
+     * The interrupt type will be set by set by the hardware domain.
+     */
+    for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
+    {
+        /*
+         * TODO: Exclude the SPIs SMMU uses which should not be routed to
+         * the hardware domain.
+         */
+        desc = irq_to_desc(i);
+        if ( desc->action != NULL)
+            continue;
+
+        /* XXX: Shall we use a proper devname? */
+        res = map_irq_to_domain(d, i, true, "ACPI");
+        if ( res )
+            return res;
+    }
+
+    return 0;
+}
+
+static int __init acpi_make_hypervisor_node(const struct kernel_info *kinfo,
+                                            struct membank tbl_add[])
+{
+    const char compat[] =
+        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
+        "xen,xen";
+    int res;
+    /* Convenience alias */
+    void *fdt = kinfo->fdt;
+
+    dt_dprintk("Create hypervisor node\n");
+
+    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
+    res = fdt_begin_node(fdt, "hypervisor");
+    if ( res )
+        return res;
+
+    /* Cannot use fdt_property_string due to embedded nulls */
+    res = fdt_property(fdt, "compatible", compat, sizeof(compat));
+    if ( res )
+        return res;
+
+    res = acpi_make_efi_nodes(fdt, tbl_add);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+/*
+ * Prepare a minimal DTB for Dom0 which contains bootargs, initrd, memory
+ * information, EFI table.
+ */
+static int __init create_acpi_dtb(struct kernel_info *kinfo,
+                                  struct membank tbl_add[])
+{
+    int new_size;
+    int ret;
+
+    dt_dprintk("Prepare a min DTB for DOM0\n");
+
+    /* Allocate min size for DT */
+    new_size = ACPI_DOM0_FDT_MIN_SIZE;
+    kinfo->fdt = xmalloc_bytes(new_size);
+
+    if ( kinfo->fdt == NULL )
+        return -ENOMEM;
+
+    /* Create a new empty DT for DOM0 */
+    ret = fdt_create(kinfo->fdt, new_size);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_finish_reservemap(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_begin_node(kinfo->fdt, "/");
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2);
+    if ( ret )
+        return ret;
+
+    ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1);
+    if ( ret )
+        return ret;
+
+    /* Create a chosen node for DOM0 */
+    ret = make_chosen_node(kinfo);
+    if ( ret )
+        goto err;
+
+    ret = acpi_make_hypervisor_node(kinfo, tbl_add);
+    if ( ret )
+        goto err;
+
+    ret = fdt_end_node(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_finish(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    return 0;
+
+  err:
+    printk("Device tree generation failed (%d).\n", ret);
+    xfree(kinfo->fdt);
+    return -EINVAL;
+}
+
+static void __init acpi_map_other_tables(struct domain *d)
+{
+    int i;
+    unsigned long res;
+    u64 addr, size;
+
+    /* Map all ACPI tables to Dom0 using 1:1 mappings. */
+    for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
+    {
+        addr = acpi_gbl_root_table_list.tables[i].address;
+        size = acpi_gbl_root_table_list.tables[i].length;
+        res = map_regions_p2mt(d,
+                               gaddr_to_gfn(addr),
+                               PFN_UP(size),
+                               maddr_to_mfn(addr),
+                               p2m_mmio_direct_c);
+        if ( res )
+        {
+             panic(XENLOG_ERR "Unable to map ACPI region 0x%"PRIx64
+                   " - 0x%"PRIx64" in domain\n",
+                   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
+        }
+    }
+}
+
+static int __init acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
+{
+
+    struct acpi_table_rsdp *rsdp = NULL;
+    u64 addr;
+    u64 table_size = sizeof(struct acpi_table_rsdp);
+    u8 *base_ptr;
+    u8 checksum;
+
+    addr = acpi_os_get_root_pointer();
+    if ( !addr  )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp = acpi_os_map_memory(addr, table_size);
+    base_ptr = d->arch.efi_acpi_table
+               + acpi_get_table_offset(tbl_add, TBL_RSDP);
+    memcpy(base_ptr, rsdp, table_size);
+    acpi_os_unmap_memory(rsdp, table_size);
+
+    rsdp = (struct acpi_table_rsdp *)base_ptr;
+    /* Replace xsdt_physical_address */
+    rsdp->xsdt_physical_address = tbl_add[TBL_XSDT].start;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, rsdp), table_size);
+    rsdp->checksum = rsdp->checksum - checksum;
+
+    tbl_add[TBL_RSDP].start = d->arch.efi_acpi_gpa
+                              + acpi_get_table_offset(tbl_add, TBL_RSDP);
+    tbl_add[TBL_RSDP].size = table_size;
+
+    return 0;
+}
+
+static void __init acpi_xsdt_modify_entry(u64 entry[],
+                                          unsigned long entry_count,
+                                          char *signature, u64 addr)
+{
+    int i;
+    struct acpi_table_header *table;
+    u64 size = sizeof(struct acpi_table_header);
+
+    for( i = 0; i < entry_count; i++ )
+    {
+        table = acpi_os_map_memory(entry[i], size);
+        if ( ACPI_COMPARE_NAME(table->signature, signature) )
+        {
+            entry[i] = addr;
+            acpi_os_unmap_memory(table, size);
+            break;
+        }
+        acpi_os_unmap_memory(table, size);
+    }
+}
+
+static int __init acpi_create_xsdt(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_xsdt *xsdt = NULL;
+    u64 table_size, addr;
+    unsigned long entry_count;
+    u8 *base_ptr;
+    u8 checksum;
+
+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+
+    /* Add place for STAO table in XSDT table */
+    table_size = table->length + sizeof(u64);
+    entry_count = (table->length - sizeof(struct acpi_table_header))
+                  / sizeof(u64);
+    base_ptr = d->arch.efi_acpi_table
+               + acpi_get_table_offset(tbl_add, TBL_XSDT);
+    memcpy(base_ptr, table, table->length);
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    xsdt = (struct acpi_table_xsdt *)base_ptr;
+    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
+                           ACPI_SIG_FADT, tbl_add[TBL_FADT].start);
+    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
+                           ACPI_SIG_MADT, tbl_add[TBL_MADT].start);
+    xsdt->table_offset_entry[entry_count] = tbl_add[TBL_STAO].start;
+
+    xsdt->header.length = table_size;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, xsdt), table_size);
+    xsdt->header.checksum -= checksum;
+
+    tbl_add[TBL_XSDT].start = d->arch.efi_acpi_gpa
+                              + acpi_get_table_offset(tbl_add, TBL_XSDT);
+    tbl_add[TBL_XSDT].size = table_size;
+
+    return 0;
+}
+
+static int __init acpi_create_stao(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_stao *stao = NULL;
+    u32 table_size = sizeof(struct acpi_table_stao);
+    u32 offset = acpi_get_table_offset(tbl_add, TBL_STAO);
+    acpi_status status;
+    u8 *base_ptr, checksum;
+
+    /* Copy OEM and ASL compiler fields from another table, use MADT */
+    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        const char *msg = acpi_format_exception(status);
+
+        printk("STAO: Failed to get MADT table, %s\n", msg);
+        return -EINVAL;
+    }
+
+    base_ptr = d->arch.efi_acpi_table + offset;
+    memcpy(base_ptr, table, sizeof(struct acpi_table_header));
+
+    stao = (struct acpi_table_stao *)base_ptr;
+    memcpy(stao->header.signature, ACPI_SIG_STAO, 4);
+    stao->header.revision = 1;
+    stao->header.length = table_size;
+    stao->ignore_uart = 1;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, stao), table_size);
+    stao->header.checksum -= checksum;
+
+    tbl_add[TBL_STAO].start = d->arch.efi_acpi_gpa + offset;
+    tbl_add[TBL_STAO].size = table_size;
+
+    return 0;
+}
+
+static int __init acpi_create_madt(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_madt *madt = NULL;
+    struct acpi_subtable_header *header;
+    struct acpi_madt_generic_distributor *gicd;
+    u32 table_size = sizeof(struct acpi_table_madt);
+    u32 offset = acpi_get_table_offset(tbl_add, TBL_MADT);
+    int ret;
+    acpi_status status;
+    u8 *base_ptr, checksum;
+
+    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        const char *msg = acpi_format_exception(status);
+
+        printk("Failed to get MADT table, %s\n", msg);
+        return -EINVAL;
+    }
+
+    base_ptr = d->arch.efi_acpi_table + offset;
+    memcpy(base_ptr, table, table_size);
+
+    /* Add Generic Distributor. */
+    header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
+    if ( !header )
+    {
+        printk("Can't get GICD entry\n");
+        return -EINVAL;
+    }
+    gicd = container_of(header, struct acpi_madt_generic_distributor, header);
+    memcpy(base_ptr + table_size, gicd,
+                sizeof(struct acpi_madt_generic_distributor));
+    table_size += sizeof(struct acpi_madt_generic_distributor);
+
+    /* Add other subtables. */
+    ret = gic_make_hwdom_madt(d, offset + table_size);
+    if ( ret < 0 )
+    {
+        printk("Failed to get other subtables\n");
+        return -EINVAL;
+    }
+    table_size += ret;
+
+    madt = (struct acpi_table_madt *)base_ptr;
+    madt->header.length = table_size;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), table_size);
+    madt->header.checksum -= checksum;
+
+    tbl_add[TBL_MADT].start = d->arch.efi_acpi_gpa + offset;
+    tbl_add[TBL_MADT].size = table_size;
+
+    return 0;
+}
+
+static int __init acpi_create_fadt(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_fadt *fadt = NULL;
+    u64 table_size;
+    acpi_status status;
+    u8 *base_ptr;
+    u8 checksum;
+
+    status = acpi_get_table(ACPI_SIG_FADT, 0, &table);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        const char *msg = acpi_format_exception(status);
+
+        printk("Failed to get FADT table, %s\n", msg);
+        return -EINVAL;
+    }
+
+    table_size = table->length;
+    base_ptr = d->arch.efi_acpi_table
+               + acpi_get_table_offset(tbl_add, TBL_FADT);
+    memcpy(base_ptr, table, table_size);
+    fadt = (struct acpi_table_fadt *)base_ptr;
+
+    /* Set PSCI_COMPLIANT and PSCI_USE_HVC */
+    fadt->arm_boot_flags |= (ACPI_FADT_PSCI_COMPLIANT | ACPI_FADT_PSCI_USE_HVC);
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, fadt), table_size);
+    fadt->header.checksum -= checksum;
+
+    tbl_add[TBL_FADT].start = d->arch.efi_acpi_gpa
+                              + acpi_get_table_offset(tbl_add, TBL_FADT);
+    tbl_add[TBL_FADT].size = table_size;
+
+    return 0;
+}
+
+static int __init estimate_acpi_efi_size(struct domain *d,
+                                         struct kernel_info *kinfo)
+{
+    size_t efi_size, acpi_size, madt_size;
+    u64 addr;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+
+    acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
+    acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
+
+    madt_size = gic_get_hwdom_madt_size(d);
+    acpi_size += ROUNDUP(madt_size, 8);
+
+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    if ( !rsdp_tbl )
+    {
+        printk("Unable to map RSDP table\n");
+        return -EINVAL;
+    }
+
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+    if ( !table )
+    {
+        printk("Unable to map XSDT table\n");
+        return -EINVAL;
+    }
+
+    /* Add place for STAO table in XSDT table */
+    acpi_size += ROUNDUP(table->length + sizeof(u64), 8);
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+
+    acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8);
+    d->arch.efi_acpi_len = PAGE_ALIGN(ROUNDUP(efi_size, 8)
+                                      + ROUNDUP(acpi_size, 8));
+
+    return 0;
+}
+
+static int __init prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+    struct membank tbl_add[TBL_MMAX] = {};
+
+    rc = estimate_acpi_efi_size(d, kinfo);
+    if ( rc != 0 )
+        return rc;
+
+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+    if ( d->arch.efi_acpi_table == NULL )
+    {
+        printk("unable to allocate memory!\n");
+        return -ENOMEM;
+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /*
+     * For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. Also it needs to
+     * check if the size of grant table region is enough for those ACPI tables.
+     */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+    if ( kinfo->gnttab_size < d->arch.efi_acpi_len )
+    {
+        printk("The grant table region is not enough to fit the ACPI tables!\n");
+        return -EINVAL;
+    }
+
+    rc = acpi_create_fadt(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
+    rc = acpi_create_madt(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
+    rc = acpi_create_stao(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
+    rc = acpi_create_xsdt(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
+    rc = acpi_create_rsdp(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
+    acpi_map_other_tables(d);
+    acpi_create_efi_system_table(d, tbl_add);
+    acpi_create_efi_mmap_table(d, &kinfo->mem, tbl_add);
+
+    /* Map the EFI and ACPI tables to Dom0 */
+    rc = map_regions_p2mt(d,
+                          gaddr_to_gfn(d->arch.efi_acpi_gpa),
+                          PFN_UP(d->arch.efi_acpi_len),
+                          virt_to_mfn(d->arch.efi_acpi_table),
+                          p2m_mmio_direct_c);
+    if ( rc != 0 )
+    {
+        printk(XENLOG_ERR "Unable to map EFI/ACPI table 0x%"PRIx64
+               " - 0x%"PRIx64" in domain %d\n",
+               d->arch.efi_acpi_gpa & PAGE_MASK,
+               PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
+               d->domain_id);
+        return rc;
+    }
+
+    /*
+     * Flush the cache for this region, otherwise DOM0 may read wrong data when
+     * the cache is disabled.
+     */
+    clean_and_invalidate_dcache_va_range(d->arch.efi_acpi_table,
+                                         d->arch.efi_acpi_len);
+
+    rc = create_acpi_dtb(kinfo, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
+    rc = acpi_route_spis(d);
+    if ( rc != 0 )
+        return rc;
+
+    rc = acpi_iomem_deny_access(d);
+    if ( rc != 0 )
+        return rc;
+
+    /*
+     * All PPIs have been registered, allocate the event channel
+     * interrupts.
+     */
+    evtchn_allocate(d);
+
+    return 0;
+}
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 32ed5aa..7a1dd56 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -23,6 +23,7 @@
 #include <asm/psci.h>
 #include <asm/setup.h>
 #include <asm/cpufeature.h>
+#include <asm/domain_build.h>
 
 #include <xen/irq.h>
 #include <xen/grant_table.h>
@@ -679,8 +680,6 @@ static int __init make_memory_node(const struct domain *d,
     return res;
 }
 
-static void evtchn_allocate(struct domain *d);
-
 static int __init make_hypervisor_node(struct domain *d,
                                        const struct kernel_info *kinfo,
                                        int addrcells, int sizecells)
@@ -1041,7 +1040,7 @@ static int __init make_timer_node(const struct domain *d, void *fdt,
  * on ACPI systems, and DomUs started directly from Xen based on device
  * tree information.
  */
-static int __init make_chosen_node(const struct kernel_info *kinfo)
+int __init make_chosen_node(const struct kernel_info *kinfo)
 {
     int res;
     const char *bootargs = NULL;
@@ -1082,8 +1081,8 @@ static int __init make_chosen_node(const struct kernel_info *kinfo)
     return res;
 }
 
-static int __init map_irq_to_domain(struct domain *d, unsigned int irq,
-                                    bool need_mapping, const char *devname)
+int __init map_irq_to_domain(struct domain *d, unsigned int irq,
+                             bool need_mapping, const char *devname)
 {
     int res;
 
@@ -1797,580 +1796,6 @@ static int __init prepare_dtb_hwdom(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
-#ifdef CONFIG_ACPI
-#define ACPI_DOM0_FDT_MIN_SIZE 4096
-
-static int __init acpi_iomem_deny_access(struct domain *d)
-{
-    acpi_status status;
-    struct acpi_table_spcr *spcr = NULL;
-    unsigned long mfn;
-    int rc;
-
-    /* Firstly permit full MMIO capabilities. */
-    rc = iomem_permit_access(d, 0UL, ~0UL);
-    if ( rc )
-        return rc;
-
-    /* TODO: Deny MMIO access for SMMU, GIC ITS */
-    status = acpi_get_table(ACPI_SIG_SPCR, 0,
-                            (struct acpi_table_header **)&spcr);
-
-    if ( ACPI_FAILURE(status) )
-    {
-        printk("Failed to get SPCR table\n");
-        return -EINVAL;
-    }
-
-    mfn = spcr->serial_port.address >> PAGE_SHIFT;
-    /* Deny MMIO access for UART */
-    rc = iomem_deny_access(d, mfn, mfn + 1);
-    if ( rc )
-        return rc;
-
-    /* Deny MMIO access for GIC regions */
-    return gic_iomem_deny_access(d);
-}
-
-static int __init acpi_route_spis(struct domain *d)
-{
-    int i, res;
-    struct irq_desc *desc;
-
-    /*
-     * Route the IRQ to hardware domain and permit the access.
-     * The interrupt type will be set by set by the hardware domain.
-     */
-    for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
-    {
-        /*
-         * TODO: Exclude the SPIs SMMU uses which should not be routed to
-         * the hardware domain.
-         */
-        desc = irq_to_desc(i);
-        if ( desc->action != NULL)
-            continue;
-
-        /* XXX: Shall we use a proper devname? */
-        res = map_irq_to_domain(d, i, true, "ACPI");
-        if ( res )
-            return res;
-    }
-
-    return 0;
-}
-
-static int __init acpi_make_hypervisor_node(const struct kernel_info *kinfo,
-                                            struct membank tbl_add[])
-{
-    const char compat[] =
-        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
-        "xen,xen";
-    int res;
-    /* Convenience alias */
-    void *fdt = kinfo->fdt;
-
-    dt_dprintk("Create hypervisor node\n");
-
-    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
-    res = fdt_begin_node(fdt, "hypervisor");
-    if ( res )
-        return res;
-
-    /* Cannot use fdt_property_string due to embedded nulls */
-    res = fdt_property(fdt, "compatible", compat, sizeof(compat));
-    if ( res )
-        return res;
-
-    res = acpi_make_efi_nodes(fdt, tbl_add);
-    if ( res )
-        return res;
-
-    res = fdt_end_node(fdt);
-
-    return res;
-}
-
-/*
- * Prepare a minimal DTB for Dom0 which contains bootargs, initrd, memory
- * information, EFI table.
- */
-static int __init create_acpi_dtb(struct kernel_info *kinfo,
-                                  struct membank tbl_add[])
-{
-    int new_size;
-    int ret;
-
-    dt_dprintk("Prepare a min DTB for DOM0\n");
-
-    /* Allocate min size for DT */
-    new_size = ACPI_DOM0_FDT_MIN_SIZE;
-    kinfo->fdt = xmalloc_bytes(new_size);
-
-    if ( kinfo->fdt == NULL )
-        return -ENOMEM;
-
-    /* Create a new empty DT for DOM0 */
-    ret = fdt_create(kinfo->fdt, new_size);
-    if ( ret < 0 )
-        goto err;
-
-    ret = fdt_finish_reservemap(kinfo->fdt);
-    if ( ret < 0 )
-        goto err;
-
-    ret = fdt_begin_node(kinfo->fdt, "/");
-    if ( ret < 0 )
-        goto err;
-
-    ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2);
-    if ( ret )
-        return ret;
-
-    ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1);
-    if ( ret )
-        return ret;
-
-    /* Create a chosen node for DOM0 */
-    ret = make_chosen_node(kinfo);
-    if ( ret )
-        goto err;
-
-    ret = acpi_make_hypervisor_node(kinfo, tbl_add);
-    if ( ret )
-        goto err;
-
-    ret = fdt_end_node(kinfo->fdt);
-    if ( ret < 0 )
-        goto err;
-
-    ret = fdt_finish(kinfo->fdt);
-    if ( ret < 0 )
-        goto err;
-
-    return 0;
-
-  err:
-    printk("Device tree generation failed (%d).\n", ret);
-    xfree(kinfo->fdt);
-    return -EINVAL;
-}
-
-static void __init acpi_map_other_tables(struct domain *d)
-{
-    int i;
-    unsigned long res;
-    u64 addr, size;
-
-    /* Map all ACPI tables to Dom0 using 1:1 mappings. */
-    for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
-    {
-        addr = acpi_gbl_root_table_list.tables[i].address;
-        size = acpi_gbl_root_table_list.tables[i].length;
-        res = map_regions_p2mt(d,
-                               gaddr_to_gfn(addr),
-                               PFN_UP(size),
-                               maddr_to_mfn(addr),
-                               p2m_mmio_direct_c);
-        if ( res )
-        {
-             panic(XENLOG_ERR "Unable to map ACPI region 0x%"PRIx64
-                   " - 0x%"PRIx64" in domain\n",
-                   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
-        }
-    }
-}
-
-static int __init acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
-{
-
-    struct acpi_table_rsdp *rsdp = NULL;
-    u64 addr;
-    u64 table_size = sizeof(struct acpi_table_rsdp);
-    u8 *base_ptr;
-    u8 checksum;
-
-    addr = acpi_os_get_root_pointer();
-    if ( !addr  )
-    {
-        printk("Unable to get acpi root pointer\n");
-        return -EINVAL;
-    }
-    rsdp = acpi_os_map_memory(addr, table_size);
-    base_ptr = d->arch.efi_acpi_table
-               + acpi_get_table_offset(tbl_add, TBL_RSDP);
-    memcpy(base_ptr, rsdp, table_size);
-    acpi_os_unmap_memory(rsdp, table_size);
-
-    rsdp = (struct acpi_table_rsdp *)base_ptr;
-    /* Replace xsdt_physical_address */
-    rsdp->xsdt_physical_address = tbl_add[TBL_XSDT].start;
-    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, rsdp), table_size);
-    rsdp->checksum = rsdp->checksum - checksum;
-
-    tbl_add[TBL_RSDP].start = d->arch.efi_acpi_gpa
-                              + acpi_get_table_offset(tbl_add, TBL_RSDP);
-    tbl_add[TBL_RSDP].size = table_size;
-
-    return 0;
-}
-
-static void __init acpi_xsdt_modify_entry(u64 entry[],
-                                          unsigned long entry_count,
-                                          char *signature, u64 addr)
-{
-    int i;
-    struct acpi_table_header *table;
-    u64 size = sizeof(struct acpi_table_header);
-
-    for( i = 0; i < entry_count; i++ )
-    {
-        table = acpi_os_map_memory(entry[i], size);
-        if ( ACPI_COMPARE_NAME(table->signature, signature) )
-        {
-            entry[i] = addr;
-            acpi_os_unmap_memory(table, size);
-            break;
-        }
-        acpi_os_unmap_memory(table, size);
-    }
-}
-
-static int __init acpi_create_xsdt(struct domain *d, struct membank tbl_add[])
-{
-    struct acpi_table_header *table = NULL;
-    struct acpi_table_rsdp *rsdp_tbl;
-    struct acpi_table_xsdt *xsdt = NULL;
-    u64 table_size, addr;
-    unsigned long entry_count;
-    u8 *base_ptr;
-    u8 checksum;
-
-    addr = acpi_os_get_root_pointer();
-    if ( !addr )
-    {
-        printk("Unable to get acpi root pointer\n");
-        return -EINVAL;
-    }
-    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
-    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
-                               sizeof(struct acpi_table_header));
-
-    /* Add place for STAO table in XSDT table */
-    table_size = table->length + sizeof(u64);
-    entry_count = (table->length - sizeof(struct acpi_table_header))
-                  / sizeof(u64);
-    base_ptr = d->arch.efi_acpi_table
-               + acpi_get_table_offset(tbl_add, TBL_XSDT);
-    memcpy(base_ptr, table, table->length);
-    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
-    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
-
-    xsdt = (struct acpi_table_xsdt *)base_ptr;
-    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
-                           ACPI_SIG_FADT, tbl_add[TBL_FADT].start);
-    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
-                           ACPI_SIG_MADT, tbl_add[TBL_MADT].start);
-    xsdt->table_offset_entry[entry_count] = tbl_add[TBL_STAO].start;
-
-    xsdt->header.length = table_size;
-    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, xsdt), table_size);
-    xsdt->header.checksum -= checksum;
-
-    tbl_add[TBL_XSDT].start = d->arch.efi_acpi_gpa
-                              + acpi_get_table_offset(tbl_add, TBL_XSDT);
-    tbl_add[TBL_XSDT].size = table_size;
-
-    return 0;
-}
-
-static int __init acpi_create_stao(struct domain *d, struct membank tbl_add[])
-{
-    struct acpi_table_header *table = NULL;
-    struct acpi_table_stao *stao = NULL;
-    u32 table_size = sizeof(struct acpi_table_stao);
-    u32 offset = acpi_get_table_offset(tbl_add, TBL_STAO);
-    acpi_status status;
-    u8 *base_ptr, checksum;
-
-    /* Copy OEM and ASL compiler fields from another table, use MADT */
-    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
-
-    if ( ACPI_FAILURE(status) )
-    {
-        const char *msg = acpi_format_exception(status);
-
-        printk("STAO: Failed to get MADT table, %s\n", msg);
-        return -EINVAL;
-    }
-
-    base_ptr = d->arch.efi_acpi_table + offset;
-    memcpy(base_ptr, table, sizeof(struct acpi_table_header));
-
-    stao = (struct acpi_table_stao *)base_ptr;
-    memcpy(stao->header.signature, ACPI_SIG_STAO, 4);
-    stao->header.revision = 1;
-    stao->header.length = table_size;
-    stao->ignore_uart = 1;
-    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, stao), table_size);
-    stao->header.checksum -= checksum;
-
-    tbl_add[TBL_STAO].start = d->arch.efi_acpi_gpa + offset;
-    tbl_add[TBL_STAO].size = table_size;
-
-    return 0;
-}
-
-static int __init acpi_create_madt(struct domain *d, struct membank tbl_add[])
-{
-    struct acpi_table_header *table = NULL;
-    struct acpi_table_madt *madt = NULL;
-    struct acpi_subtable_header *header;
-    struct acpi_madt_generic_distributor *gicd;
-    u32 table_size = sizeof(struct acpi_table_madt);
-    u32 offset = acpi_get_table_offset(tbl_add, TBL_MADT);
-    int ret;
-    acpi_status status;
-    u8 *base_ptr, checksum;
-
-    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
-
-    if ( ACPI_FAILURE(status) )
-    {
-        const char *msg = acpi_format_exception(status);
-
-        printk("Failed to get MADT table, %s\n", msg);
-        return -EINVAL;
-    }
-
-    base_ptr = d->arch.efi_acpi_table + offset;
-    memcpy(base_ptr, table, table_size);
-
-    /* Add Generic Distributor. */
-    header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
-    if ( !header )
-    {
-        printk("Can't get GICD entry\n");
-        return -EINVAL;
-    }
-    gicd = container_of(header, struct acpi_madt_generic_distributor, header);
-    memcpy(base_ptr + table_size, gicd,
-                sizeof(struct acpi_madt_generic_distributor));
-    table_size += sizeof(struct acpi_madt_generic_distributor);
-
-    /* Add other subtables. */
-    ret = gic_make_hwdom_madt(d, offset + table_size);
-    if ( ret < 0 )
-    {
-        printk("Failed to get other subtables\n");
-        return -EINVAL;
-    }
-    table_size += ret;
-
-    madt = (struct acpi_table_madt *)base_ptr;
-    madt->header.length = table_size;
-    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), table_size);
-    madt->header.checksum -= checksum;
-
-    tbl_add[TBL_MADT].start = d->arch.efi_acpi_gpa + offset;
-    tbl_add[TBL_MADT].size = table_size;
-
-    return 0;
-}
-
-static int __init acpi_create_fadt(struct domain *d, struct membank tbl_add[])
-{
-    struct acpi_table_header *table = NULL;
-    struct acpi_table_fadt *fadt = NULL;
-    u64 table_size;
-    acpi_status status;
-    u8 *base_ptr;
-    u8 checksum;
-
-    status = acpi_get_table(ACPI_SIG_FADT, 0, &table);
-
-    if ( ACPI_FAILURE(status) )
-    {
-        const char *msg = acpi_format_exception(status);
-
-        printk("Failed to get FADT table, %s\n", msg);
-        return -EINVAL;
-    }
-
-    table_size = table->length;
-    base_ptr = d->arch.efi_acpi_table
-               + acpi_get_table_offset(tbl_add, TBL_FADT);
-    memcpy(base_ptr, table, table_size);
-    fadt = (struct acpi_table_fadt *)base_ptr;
-
-    /* Set PSCI_COMPLIANT and PSCI_USE_HVC */
-    fadt->arm_boot_flags |= (ACPI_FADT_PSCI_COMPLIANT | ACPI_FADT_PSCI_USE_HVC);
-    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, fadt), table_size);
-    fadt->header.checksum -= checksum;
-
-    tbl_add[TBL_FADT].start = d->arch.efi_acpi_gpa
-                              + acpi_get_table_offset(tbl_add, TBL_FADT);
-    tbl_add[TBL_FADT].size = table_size;
-
-    return 0;
-}
-
-static int __init estimate_acpi_efi_size(struct domain *d,
-                                         struct kernel_info *kinfo)
-{
-    size_t efi_size, acpi_size, madt_size;
-    u64 addr;
-    struct acpi_table_rsdp *rsdp_tbl;
-    struct acpi_table_header *table;
-
-    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
-
-    acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
-    acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
-
-    madt_size = gic_get_hwdom_madt_size(d);
-    acpi_size += ROUNDUP(madt_size, 8);
-
-    addr = acpi_os_get_root_pointer();
-    if ( !addr )
-    {
-        printk("Unable to get acpi root pointer\n");
-        return -EINVAL;
-    }
-
-    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
-    if ( !rsdp_tbl )
-    {
-        printk("Unable to map RSDP table\n");
-        return -EINVAL;
-    }
-
-    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
-                               sizeof(struct acpi_table_header));
-    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
-    if ( !table )
-    {
-        printk("Unable to map XSDT table\n");
-        return -EINVAL;
-    }
-
-    /* Add place for STAO table in XSDT table */
-    acpi_size += ROUNDUP(table->length + sizeof(u64), 8);
-    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
-
-    acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8);
-    d->arch.efi_acpi_len = PAGE_ALIGN(ROUNDUP(efi_size, 8)
-                                      + ROUNDUP(acpi_size, 8));
-
-    return 0;
-}
-
-static int __init prepare_acpi(struct domain *d, struct kernel_info *kinfo)
-{
-    int rc = 0;
-    int order;
-    struct membank tbl_add[TBL_MMAX] = {};
-
-    rc = estimate_acpi_efi_size(d, kinfo);
-    if ( rc != 0 )
-        return rc;
-
-    order = get_order_from_bytes(d->arch.efi_acpi_len);
-    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
-    if ( d->arch.efi_acpi_table == NULL )
-    {
-        printk("unable to allocate memory!\n");
-        return -ENOMEM;
-    }
-    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
-
-    /*
-     * For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
-     * region. So we use it as the ACPI table mapped address. Also it needs to
-     * check if the size of grant table region is enough for those ACPI tables.
-     */
-    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
-    if ( kinfo->gnttab_size < d->arch.efi_acpi_len )
-    {
-        printk("The grant table region is not enough to fit the ACPI tables!\n");
-        return -EINVAL;
-    }
-
-    rc = acpi_create_fadt(d, tbl_add);
-    if ( rc != 0 )
-        return rc;
-
-    rc = acpi_create_madt(d, tbl_add);
-    if ( rc != 0 )
-        return rc;
-
-    rc = acpi_create_stao(d, tbl_add);
-    if ( rc != 0 )
-        return rc;
-
-    rc = acpi_create_xsdt(d, tbl_add);
-    if ( rc != 0 )
-        return rc;
-
-    rc = acpi_create_rsdp(d, tbl_add);
-    if ( rc != 0 )
-        return rc;
-
-    acpi_map_other_tables(d);
-    acpi_create_efi_system_table(d, tbl_add);
-    acpi_create_efi_mmap_table(d, &kinfo->mem, tbl_add);
-
-    /* Map the EFI and ACPI tables to Dom0 */
-    rc = map_regions_p2mt(d,
-                          gaddr_to_gfn(d->arch.efi_acpi_gpa),
-                          PFN_UP(d->arch.efi_acpi_len),
-                          virt_to_mfn(d->arch.efi_acpi_table),
-                          p2m_mmio_direct_c);
-    if ( rc != 0 )
-    {
-        printk(XENLOG_ERR "Unable to map EFI/ACPI table 0x%"PRIx64
-               " - 0x%"PRIx64" in domain %d\n",
-               d->arch.efi_acpi_gpa & PAGE_MASK,
-               PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
-               d->domain_id);
-        return rc;
-    }
-
-    /*
-     * Flush the cache for this region, otherwise DOM0 may read wrong data when
-     * the cache is disabled.
-     */
-    clean_and_invalidate_dcache_va_range(d->arch.efi_acpi_table,
-                                         d->arch.efi_acpi_len);
-
-    rc = create_acpi_dtb(kinfo, tbl_add);
-    if ( rc != 0 )
-        return rc;
-
-    rc = acpi_route_spis(d);
-    if ( rc != 0 )
-        return rc;
-
-    rc = acpi_iomem_deny_access(d);
-    if ( rc != 0 )
-        return rc;
-
-    /*
-     * All PPIs have been registered, allocate the event channel
-     * interrupts.
-     */
-    evtchn_allocate(d);
-
-    return 0;
-}
-#else
-static int __init prepare_acpi(struct domain *d, struct kernel_info *kinfo)
-{
-    /* Only booting with ACPI will hit here */
-    BUG();
-    return -EINVAL;
-}
-#endif
 static void __init dtb_load(struct kernel_info *kinfo)
 {
     unsigned long left;
@@ -2443,7 +1868,7 @@ static void __init initrd_load(struct kernel_info *kinfo)
  * Note that this should only be called once all PPIs used by the
  * hardware domain have been registered.
  */
-static void __init evtchn_allocate(struct domain *d)
+void __init evtchn_allocate(struct domain *d)
 {
     int res;
     u64 val;
diff --git a/xen/include/asm-arm/domain_build.h b/xen/include/asm-arm/domain_build.h
new file mode 100644
index 0000000..34ceddc
--- /dev/null
+++ b/xen/include/asm-arm/domain_build.h
@@ -0,0 +1,31 @@
+#ifndef __ASM_DOMAIN_BUILD_H__
+#define __ASM_DOMAIN_BUILD_H__
+
+#include <xen/sched.h>
+#include <asm/kernel.h>
+
+int map_irq_to_domain(struct domain *d, unsigned int irq,
+                      bool need_mapping, const char *devname);
+int make_chosen_node(const struct kernel_info *kinfo);
+void evtchn_allocate(struct domain *d);
+
+#ifndef CONFIG_ACPI
+static inline int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG();
+    return -EINVAL;
+}
+#else
+int prepare_acpi(struct domain *d, struct kernel_info *kinfo);
+#endif
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 22/26] xen: support console_switching between Dom0 and DomUs on ARM
  2018-11-02 23:45 ` [PATCH v6 22/26] xen: support console_switching between Dom0 and DomUs on ARM Stefano Stabellini
@ 2018-11-05  9:15   ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2018-11-05  9:15 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Tim Deegan, andrii_anisov, Konrad Rzeszutek Wilk, George Dunlap,
	Andrew Cooper, Achin.Gupta, Ian Jackson, xen-devel, Julien Grall,
	Stefano Stabellini, Wei Liu

>>> On 03.11.18 at 00:45, <sstabellini@kernel.org> wrote:
> Today Ctrl-AAA is used to switch between Xen and Dom0. Extend the
> mechanism to allow for switching between Xen, Dom0, and any of the
> initial DomU created from Xen alongside Dom0 out of information provided
> via device tree.
> 
> Rename xen_rx to console_rx to match the new behavior.
> 
> Clarify existing comment about "notify the guest", making it clear that
> it is only about the hardware domain.
> 
> Switching the console input to domUs started from Xen at boot is
> #ifdef'ed to 0 in this patch. The code will be enabled when
> vpl011_rx_char_xen is introduced. For now it is disabled for
> bisectability.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with two remarks (but the ack applies with or without either
taken care of):

> @@ -391,31 +394,82 @@ static void dump_console_ring_key(unsigned char key)
>      free_xenheap_pages(buf, order);
>  }
>  
> -/* CTRL-<switch_char> switches input direction between Xen and DOM0. */
> +/*
> + * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
> + * and the DomUs started from Xen at boot.
> + */
>  #define switch_code (opt_conswitch[0]-'a'+1)
> -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. */
> +/*
> + * console_rx=0 => input to xen
> + * console_rx=1 => input to dom0
> + * console_rx=N => input to dom(N-1)
> + */
> +static unsigned int __read_mostly console_rx = 0;
>  
>  static void switch_serial_input(void)
>  {
> -    static char *input_str[2] = { "DOM0", "Xen" };
> -    xen_rx = !xen_rx;
> -    printk("*** Serial input -> %s", input_str[xen_rx]);
> +    if ( console_rx == max_init_domid + 1 )

I generally consider it better to use >= in cases like this.

>  static void __serial_rx(char c, struct cpu_user_regs *regs)
>  {
> -    if ( xen_rx )
> +    switch ( console_rx )
> +    {
> +    case 0:
>          return handle_keypress(c, regs);
>  
> -    /* Deliver input to guest buffer, unless it is already full. */
> -    if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
> -        serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> -    /* Always notify the guest: prevents receive path from getting stuck. */
> -    send_global_virq(VIRQ_CONSOLE);
> +    case 1:
> +        /*
> +         * Deliver input to the hardware domain buffer, unless it is
> +         * already full.
> +         */
> +        if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE )
> +            serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c;
> +
> +        /*
> +         * Always notify the hardware domain: prevents receive path from
> +         * getting stuck.
> +         */
> +        send_global_virq(VIRQ_CONSOLE);
> +        break;
> +
> +#if 0
> +    default:
> +    {
> +        struct domain *d = rcu_lock_domain_by_any_id(console_rx - 1);
> +
> +        /*
> +         * If we have a properly initialized vpl011 console for the
> +         * domain, without a full PV ring to Dom0 (in that case input
> +         * comes from the PV ring), then send the character to it.
> +         */
> +        if ( d != NULL &&
> +             !d->arch.vpl011.backend_in_domain &&
> +             d->arch.vpl011.backend.xen != NULL )
> +            vpl011_rx_char_xen(d, c);
> +        else
> +            printk("Cannot send chars to Dom%d: no UART available\n",
> +                   console_rx - 1);
> +
> +        if ( d != NULL )
> +            rcu_unlock_domain(d);
> +    }
> +#endif

As said before, I would have preferred if this code block was
introduced only with the patch making it usable, rather than
with the #if 0 here.

Jan



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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen
  2018-11-02 23:44 ` [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen Stefano Stabellini
@ 2018-11-09 14:01   ` Julien Grall
  2018-11-09 21:38     ` Stefano Stabellini
  0 siblings, 1 reply; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:01 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi,

On 02/11/2018 23:44, Stefano Stabellini wrote:
> Make sure to only look for multiboot compatible nodes only under
> /chosen, not under any other paths (depth <= 3).
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> 
> ---
> 
> Changes in v6:
> - do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
> - remove sizeof, use hardcoded value
> 
> Changes in v5:
> - add patch
> - add check on return value of fdt_get_path
> ---
>   xen/arch/arm/bootfdt.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 8eba42c..a42fe87 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -173,7 +173,15 @@ static void __init process_multiboot_node(const void *fdt, int node,
>       bootmodule_kind kind;
>       paddr_t start, size;
>       const char *cmdline;
> -    int len;
> +    int len = 8; /* sizeof "/chosen" */
> +    char path[8];
> +    int ret;
> +
> +    /* Check that the node is under "chosen" */
> +    ret = fdt_get_path(fdt, node, path, len);
> +    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||

I don't understand why you specifically check for -FDT_ERR_NOSPACE here.

Looking at fdt_get_path(...) there are case where the function may return 
-FDT_ERR_NOSPACE yet not filling up path. So you would end up to compare garbage.

> +         strncmp(path, "/chosen", len - 1) )
> +        return;
>   
>       prop = fdt_get_property(fdt, node, "reg", &len);
>       if ( !prop )
> @@ -286,8 +294,8 @@ static int __init early_scan_node(const void *fdt,
>   {
>       if ( device_tree_node_matches(fdt, node, "memory") )
>           process_memory_node(fdt, node, name, address_cells, size_cells);
> -    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
> -              device_tree_node_compatible(fdt, node, "multiboot,module" ))
> +    else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ||
> +              device_tree_node_compatible(fdt, node, "multiboot,module" )))
>           process_multiboot_node(fdt, node, name, address_cells, size_cells);
>       else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
>           process_chosen_node(fdt, node, name, address_cells, size_cells);
> 
Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 07/26] xen/arm: don't add duplicate boot modules, introduce domU flag
  2018-11-02 23:44 ` [PATCH v6 07/26] xen/arm: don't add duplicate boot modules, introduce domU flag Stefano Stabellini
@ 2018-11-09 14:06   ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:06 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi Stefano,

On 02/11/2018 23:44, Stefano Stabellini wrote:
> Don't add duplicate boot modules (same kind and same start address),
> they are freed later, we don't want to introduce double-free errors.
> 
> Introduce a domU flag in struct bootmodule and struct bootcmdline. Set
> it for kernels and ramdisks of "xen,domain" nodes to avoid getting
> confused in kernel_probe, where we try to guess which is the dom0 kernel
> and initrd to be compatible with all versions of the multiboot spec.
> 
> boot_module_find_by_kind and boot_cmdline_find_by_kind automatically
> check for !domU entries (they are only used for non-domU modules).
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 08/26] xen/arm: probe domU kernels and initrds
  2018-11-02 23:45 ` [PATCH v6 08/26] xen/arm: probe domU kernels and initrds Stefano Stabellini
@ 2018-11-09 14:09   ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:09 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi Stefano,

On 02/11/2018 23:45, Stefano Stabellini wrote:
> -    info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
> +    printk("Loading Dom%d kernel from boot module @ %"PRIpaddr"\n",
> +           info->d->domain_id, info->kernel_bootmodule->start);

NIT: You probably want to use the new %pd format. This will allow you to print a 
domain more consistently and also drop ->domain_id :).

The rest of the code looks good:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 09/26] xen/arm: add start to struct bootcmdline
  2018-11-02 23:45 ` [PATCH v6 09/26] xen/arm: add start to struct bootcmdline Stefano Stabellini
@ 2018-11-09 14:10   ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:10 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi Stefano,

On 02/11/2018 23:45, Stefano Stabellini wrote:
> Add a new start address field to struct bootcmdline to easily match a
> cmdline to the corresponding bootmodule. This is useful for debugging
> (not actually needed for functionalities today, but could be.)
> 
> Instead of printing the index in the cmdline array, print the start
> address of the corresponding bootmodule for each cmdline in
> early_print_info.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
>   xen/arch/arm/bootfdt.c      | 4 ++--
>   xen/arch/arm/setup.c        | 3 ++-
>   xen/include/asm-arm/setup.h | 3 ++-
>   3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 8d9ba47..4f0712a 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -238,7 +238,7 @@ static void __init process_multiboot_node(const void *fdt, int node,
>       if ( !prop )
>           return;
>       add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data,
> -                     kind, domU);
> +                     kind, start, domU);
>   }
>   
>   static void __init process_chosen_node(const void *fdt, int node,
> @@ -335,7 +335,7 @@ static void __init early_print_info(void)
>       }
>       printk("\n");
>       for ( i = 0 ; i < cmds->nr_mods; i++ )
> -        printk("CMDLINE[%d]:%s %s\n", i,
> +        printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start,
>                  cmds->cmdline[i].dt_name,
>                  &cmds->cmdline[i].cmdline[0]);
>       printk("\n");
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index dbba8f3..a819953 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -254,7 +254,7 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
>   }
>   
>   void __init add_boot_cmdline(const char *name, const char *cmdline,
> -                             bootmodule_kind kind, bool domU)
> +                             bootmodule_kind kind, paddr_t start, bool domU)
>   {
>       struct bootcmdlines *cmds = &bootinfo.cmdlines;
>       struct bootcmdline *cmd;
> @@ -268,6 +268,7 @@ void __init add_boot_cmdline(const char *name, const char *cmdline,
>       cmd = &cmds->cmdline[cmds->nr_mods++];
>       cmd->kind = kind;
>       cmd->domU = domU;
> +    cmd->start = start;
>   
>       ASSERT(strlen(name) <= DT_MAX_NAME);
>       safe_strcpy(cmd->dt_name, name);
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 33fb04e..0d787e6 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -49,6 +49,7 @@ struct bootmodule {
>   struct bootcmdline {
>       bootmodule_kind kind;
>       bool domU;
> +    paddr_t start;
>       char dt_name[DT_MAX_NAME];
>       char cmdline[BOOTMOD_MAX_CMDLINE];
>   };
> @@ -104,7 +105,7 @@ struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
>   struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind kind,
>                                                                paddr_t start);
>   void add_boot_cmdline(const char *name, const char *cmdline,
> -                      bootmodule_kind kind, bool domU);
> +                      bootmodule_kind kind, paddr_t start, bool domU);
>   struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind);
>   struct bootcmdline * __init boot_cmdline_find_by_name(const char *name);
>   const char * __init boot_module_kind_as_string(bootmodule_kind kind);
> 

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 12/26] xen/arm: introduce allocate_memory
  2018-11-02 23:45 ` [PATCH v6 12/26] xen/arm: introduce allocate_memory Stefano Stabellini
@ 2018-11-09 14:19   ` Julien Grall
  2018-11-09 21:33     ` Stefano Stabellini
  0 siblings, 1 reply; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:19 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi Stefano,

Most of the code is mine, so it is hard to review it :). Although, I have a few 
comments below.


On 02/11/2018 23:45, Stefano Stabellini wrote:
> Introduce an allocate_memory function able to allocate memory for DomUs
> and map it at the right guest addresses, according to the guest memory
> map: GUEST_RAM0_BASE and GUEST_RAM1_BASE.
> 
> This is under #if 0 as not used for now.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> ---
> Changes in v6:
> - turn dprintks into printk
> - use panic instead of printk+BUG_ON
> - use Julien's implementation of allocate_bank_memory
> 
> Changes in v5:
> - improve commit message
> - code style
> - remove unneeded local var
> - while loop in allocate_bank_memory to allocate memory so that it
>    doesn't have to be contiguos
> - combile while loops in allocate_memory
> 
> Changes in v4:
> - move earlier, add #if 0
> - introduce allocate_bank_memory, remove insert_bank
> - allocate_bank_memory allocate memory and inserts the bank, while
>    allocate_memory specifies where to do that
> 
> Changes in v3:
> - new patch
> ---
>   xen/arch/arm/domain_build.c | 102 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 102 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 66a258a..86abcc6 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -368,6 +368,108 @@ static void __init allocate_memory_11(struct domain *d,
>       }
>   }
>   
> +#if 0
> +static bool __init allocate_bank_memory(struct domain *d,
> +                                       struct kernel_info *kinfo,
> +                                       gfn_t sgfn,
> +                                       unsigned long tot_size)
> +{
> +    int res;
> +    struct page_info *pg;
> +    struct membank *bank;
> +    gfn_t gfn = sgfn;
> +    unsigned long size = tot_size;

The introduction for those 2 variables can be avoided if you pre-populate the 
bank before hand (see more below). I think this would avoid to confuse between 
the different variables.

> +    unsigned int max_order = ~0;
> +
> +    while ( size > 0 )
> +    {
> +        unsigned int order = get_allocation_size(size);
> +
> +        order = min(max_order, order);
> +
> +        pg = alloc_domheap_pages(d, order, 0);
> +        if ( !pg )
> +        {
> +            /*
> +             * If we can't allocate one page, then it is unlikely to
> +             * succeed in the next iteration. So bail out.
> +             */
> +            if ( !order )
> +                return false;
> +
> +            /*
> +             * If we can't allocate memory with order, then it is
> +             * unlikely to succeed in the next iteration.
> +             * Record the order - 1 to avoid re-trying.
> +             */
> +            max_order = order - 1;
> +            continue;
> +        }
> +
> +        res = guest_physmap_add_page(d, gfn, page_to_mfn(pg), order);
> +        if ( res )
> +        {
> +            dprintk(XENLOG_ERR, "Failed map pages to DOMU: %d", res);
> +            return false;
> +        }
> +
> +        gfn = gfn_add(gfn, 1UL << order);
> +        size -= (1ULL << (PAGE_SHIFT + order));
> +    }
> +
> +    bank = &kinfo->mem.bank[kinfo->mem.nr_banks];
> +    bank->start = gfn_to_gaddr(sgfn);
> +    bank->size = tot_size;

In relation with my comment on the variables, this is the 3 lines that could be 
moved earlier.

> +    kinfo->mem.nr_banks++;
> +    kinfo->unassigned_mem -= tot_size;
> +
> +    return true;
> +}
> +
> +static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
> +{
> +    unsigned int i;
> +    unsigned long bank_size;
> +
> +    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for dom%d:\n",

NIT: You can use the recently %pd to print the domain.

> +           /* Don't want format this as PRIpaddr (16 digit hex) */
> +           (unsigned long)(kinfo->unassigned_mem >> 20), d->domain_id);
> +
> +    kinfo->mem.nr_banks = 0;
> +    bank_size = MIN(GUEST_RAM0_SIZE, kinfo->unassigned_mem);
> +    if ( !allocate_bank_memory(d, kinfo, gaddr_to_gfn(GUEST_RAM0_BASE),
> +                               bank_size) )
> +        goto fail;
> +
> +    bank_size = MIN(GUEST_RAM1_SIZE, kinfo->unassigned_mem);
> +    if ( !allocate_bank_memory(d, kinfo, gaddr_to_gfn(GUEST_RAM1_BASE),
> +                               bank_size) )
> +        goto fail;
> +
> +    if ( kinfo->unassigned_mem )
> +        goto fail;
> +
> +    for( i = 0; i < kinfo->mem.nr_banks; i++ )
> +    {
> +        printk(XENLOG_INFO "Dom%d BANK[%d] %#"PRIpaddr"-%#"PRIpaddr" (%ldMB)\n",

Same here.

> +               d->domain_id,
> +               i,
> +               kinfo->mem.bank[i].start,
> +               kinfo->mem.bank[i].start + kinfo->mem.bank[i].size,
> +               /* Don't want format this as PRIpaddr (16 digit hex) */
> +               (unsigned long)(kinfo->mem.bank[i].size >> 20));
> +    }
> +
> +    return;
> +
> +fail:
> +    panic("Failed to allocate requested domain memory."
> +          /* Don't want format this as PRIpaddr (16 digit hex) */
> +          " %ldKB unallocated. Fix the VMs configurations.\n",
> +          (unsigned long)kinfo->unassigned_mem >> 10);
> +}
> +#endif
> +
>   static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
>                                      const struct dt_device_node *node)
>   {
> 

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen
  2018-11-02 23:45 ` [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen Stefano Stabellini
@ 2018-11-09 14:32   ` Julien Grall
  2018-11-09 21:13     ` Stefano Stabellini
  0 siblings, 1 reply; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:32 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, andrii_anisov, konrad.wilk, George.Dunlap,
	andrew.cooper3, Achin.Gupta, ian.jackson, xen-devel, tim,
	jbeulich, wei.liu2

Hi Stefano,

On 02/11/2018 23:45, Stefano Stabellini wrote:
> To avoid mixing the output of different domains on the console, buffer
> the output chars and print line by line. Unless the domain has input
> from the serial, in which case we want to print char by char for a
> smooth user experience.
> 
> The size of SBSA_UART_OUT_BUF_SIZE is arbitrary, choose the same size
> as VUART_BUF_SIZE used in vuart.c.
> 
> Export a function named console_input_domain() to allow others to know
> which domains has input at a given time.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

With one question below:

Reviewed-by: Julien Grall <julien.grall@arm.com>

> CC: andrew.cooper3@citrix.com
> CC: George.Dunlap@eu.citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: jbeulich@suse.com
> CC: konrad.wilk@oracle.com
> CC: tim@xen.org
> CC: wei.liu2@citrix.com
> ---
> XXX: merge this patch with "xen/arm: Allow vpl011 to be used by DomU" on
>       commit

Could you provide a commit message that will be used after merge?

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 26/26] xen/arm: split domain_build.c
  2018-11-02 23:45 ` [PATCH v6 26/26] xen/arm: split domain_build.c Stefano Stabellini
@ 2018-11-09 14:35   ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2018-11-09 14:35 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi Stefano,

On 02/11/2018 23:45, Stefano Stabellini wrote:
> domain_build.c is too large.
> 
> Move all the ACPI specific device tree generating functions from
> domain_build.c to acpi/domain_build.c.
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> 
> ---
> 
> Changes in v6:
> - fix license
> 
> Changes in v4:
> - rename acpi_dt_build to domain_build.c
> - add copyright header
> - remove useless #include
> - remove acpi_dt_build.h, add domain_build.h
> ---
>   xen/arch/arm/acpi/Makefile         |   1 +
>   xen/arch/arm/acpi/domain_build.c   | 591 +++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/domain_build.c        | 585 +-----------------------------------
>   xen/include/asm-arm/domain_build.h |  31 ++
>   4 files changed, 628 insertions(+), 580 deletions(-)
>   create mode 100644 xen/arch/arm/acpi/domain_build.c
>   create mode 100644 xen/include/asm-arm/domain_build.h
> 
> diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> index 23963f8..94ae249 100644
> --- a/xen/arch/arm/acpi/Makefile
> +++ b/xen/arch/arm/acpi/Makefile
> @@ -1,2 +1,3 @@
>   obj-y += lib.o
> +obj-y += domain_build.o

One minor comment. It would be good if we check all symbols in this file belongs 
to init. Can you use domain_build.init.o here?

With that:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen
  2018-11-09 14:32   ` Julien Grall
@ 2018-11-09 21:13     ` Stefano Stabellini
  0 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-09 21:13 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Stefano Stabellini, andrii_anisov,
	konrad.wilk, George.Dunlap, andrew.cooper3, Achin.Gupta,
	ian.jackson, xen-devel, tim, jbeulich, wei.liu2

On Fri, 9 Nov 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 02/11/2018 23:45, Stefano Stabellini wrote:
> > To avoid mixing the output of different domains on the console, buffer
> > the output chars and print line by line. Unless the domain has input
> > from the serial, in which case we want to print char by char for a
> > smooth user experience.
> > 
> > The size of SBSA_UART_OUT_BUF_SIZE is arbitrary, choose the same size
> > as VUART_BUF_SIZE used in vuart.c.
> > 
> > Export a function named console_input_domain() to allow others to know
> > which domains has input at a given time.
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> 
> With one question below:
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>
> 
> > CC: andrew.cooper3@citrix.com
> > CC: George.Dunlap@eu.citrix.com
> > CC: ian.jackson@eu.citrix.com
> > CC: jbeulich@suse.com
> > CC: konrad.wilk@oracle.com
> > CC: tim@xen.org
> > CC: wei.liu2@citrix.com
> > ---
> > XXX: merge this patch with "xen/arm: Allow vpl011 to be used by DomU" on
> >       commit
> 
> Could you provide a commit message that will be used after merge?

Yes, I read both commit messages, and a simple combination of the two
messages (removing the "Output characters are printed one by one"
sentence of course) should work well:

---

xen/arm: Allow vpl011 to be used by DomU

Make vpl011 being able to be used without a userspace component in Dom0.
In that case, output is printed to the Xen serial and input is received
from the Xen serial one character at a time.

Call domain_vpl011_init during construct_domU if vpl011 is enabled.

Introduce a new ring struct with only the ring array to avoid a waste of
memory. Introduce separate read_data and write_data functions for
initial domains: vpl011_write_data_xen is very simple and just writes
to the console, while vpl011_read_data_xen is a duplicate of
vpl011_read_data. Although textually almost identical, we are forced to
duplicate the functions because the struct layout is different.

To avoid mixing the output of different domains on the console, buffer
the output chars and print line by line. Unless the domain has input
from the serial, in which case we want to print char by char for a
smooth user experience.

The size of SBSA_UART_OUT_BUF_SIZE is arbitrary, choose the same size
as VUART_BUF_SIZE used in vuart.c.

Export a function named console_input_domain() to allow others to know
which domains has input at a given time.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 12/26] xen/arm: introduce allocate_memory
  2018-11-09 14:19   ` Julien Grall
@ 2018-11-09 21:33     ` Stefano Stabellini
  0 siblings, 0 replies; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-09 21:33 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Achin.Gupta, Stefano Stabellini,
	andrii_anisov, xen-devel

On Fri, 9 Nov 2018, Julien Grall wrote:
> Hi Stefano,
> 
> Most of the code is mine, so it is hard to review it :). Although, I have a
> few comments below.
> 
> 
> On 02/11/2018 23:45, Stefano Stabellini wrote:
> > Introduce an allocate_memory function able to allocate memory for DomUs
> > and map it at the right guest addresses, according to the guest memory
> > map: GUEST_RAM0_BASE and GUEST_RAM1_BASE.
> > 
> > This is under #if 0 as not used for now.
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > ---
> > Changes in v6:
> > - turn dprintks into printk
> > - use panic instead of printk+BUG_ON
> > - use Julien's implementation of allocate_bank_memory
> > 
> > Changes in v5:
> > - improve commit message
> > - code style
> > - remove unneeded local var
> > - while loop in allocate_bank_memory to allocate memory so that it
> >    doesn't have to be contiguos
> > - combile while loops in allocate_memory
> > 
> > Changes in v4:
> > - move earlier, add #if 0
> > - introduce allocate_bank_memory, remove insert_bank
> > - allocate_bank_memory allocate memory and inserts the bank, while
> >    allocate_memory specifies where to do that
> > 
> > Changes in v3:
> > - new patch
> > ---
> >   xen/arch/arm/domain_build.c | 102
> > ++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 102 insertions(+)
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 66a258a..86abcc6 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -368,6 +368,108 @@ static void __init allocate_memory_11(struct domain
> > *d,
> >       }
> >   }
> >   +#if 0
> > +static bool __init allocate_bank_memory(struct domain *d,
> > +                                       struct kernel_info *kinfo,
> > +                                       gfn_t sgfn,
> > +                                       unsigned long tot_size)
> > +{
> > +    int res;
> > +    struct page_info *pg;
> > +    struct membank *bank;
> > +    gfn_t gfn = sgfn;
> > +    unsigned long size = tot_size;
> 
> The introduction for those 2 variables can be avoided if you pre-populate the
> bank before hand (see more below). I think this would avoid to confuse between
> the different variables.

OK


> > +    unsigned int max_order = ~0;
> > +
> > +    while ( size > 0 )
> > +    {
> > +        unsigned int order = get_allocation_size(size);
> > +
> > +        order = min(max_order, order);
> > +
> > +        pg = alloc_domheap_pages(d, order, 0);
> > +        if ( !pg )
> > +        {
> > +            /*
> > +             * If we can't allocate one page, then it is unlikely to
> > +             * succeed in the next iteration. So bail out.
> > +             */
> > +            if ( !order )
> > +                return false;
> > +
> > +            /*
> > +             * If we can't allocate memory with order, then it is
> > +             * unlikely to succeed in the next iteration.
> > +             * Record the order - 1 to avoid re-trying.
> > +             */
> > +            max_order = order - 1;
> > +            continue;
> > +        }
> > +
> > +        res = guest_physmap_add_page(d, gfn, page_to_mfn(pg), order);
> > +        if ( res )
> > +        {
> > +            dprintk(XENLOG_ERR, "Failed map pages to DOMU: %d", res);
> > +            return false;
> > +        }
> > +
> > +        gfn = gfn_add(gfn, 1UL << order);
> > +        size -= (1ULL << (PAGE_SHIFT + order));
> > +    }
> > +
> > +    bank = &kinfo->mem.bank[kinfo->mem.nr_banks];
> > +    bank->start = gfn_to_gaddr(sgfn);
> > +    bank->size = tot_size;
> 
> In relation with my comment on the variables, this is the 3 lines that could
> be moved earlier.

understood


> > +    kinfo->mem.nr_banks++;
> > +    kinfo->unassigned_mem -= tot_size;
> > +
> > +    return true;
> > +}
> > +
> > +static void __init allocate_memory(struct domain *d, struct kernel_info
> > *kinfo)
> > +{
> > +    unsigned int i;
> > +    unsigned long bank_size;
> > +
> > +    printk(XENLOG_INFO "Allocating mappings totalling %ldMB for dom%d:\n",
> 
> NIT: You can use the recently %pd to print the domain.

Sure


> > +           /* Don't want format this as PRIpaddr (16 digit hex) */
> > +           (unsigned long)(kinfo->unassigned_mem >> 20), d->domain_id);
> > +
> > +    kinfo->mem.nr_banks = 0;
> > +    bank_size = MIN(GUEST_RAM0_SIZE, kinfo->unassigned_mem);
> > +    if ( !allocate_bank_memory(d, kinfo, gaddr_to_gfn(GUEST_RAM0_BASE),
> > +                               bank_size) )
> > +        goto fail;
> > +
> > +    bank_size = MIN(GUEST_RAM1_SIZE, kinfo->unassigned_mem);
> > +    if ( !allocate_bank_memory(d, kinfo, gaddr_to_gfn(GUEST_RAM1_BASE),
> > +                               bank_size) )
> > +        goto fail;
> > +
> > +    if ( kinfo->unassigned_mem )
> > +        goto fail;
> > +
> > +    for( i = 0; i < kinfo->mem.nr_banks; i++ )
> > +    {
> > +        printk(XENLOG_INFO "Dom%d BANK[%d] %#"PRIpaddr"-%#"PRIpaddr"
> > (%ldMB)\n",
> 
> Same here.

OK


> > +               d->domain_id,
> > +               i,
> > +               kinfo->mem.bank[i].start,
> > +               kinfo->mem.bank[i].start + kinfo->mem.bank[i].size,
> > +               /* Don't want format this as PRIpaddr (16 digit hex) */
> > +               (unsigned long)(kinfo->mem.bank[i].size >> 20));
> > +    }
> > +
> > +    return;
> > +
> > +fail:
> > +    panic("Failed to allocate requested domain memory."
> > +          /* Don't want format this as PRIpaddr (16 digit hex) */
> > +          " %ldKB unallocated. Fix the VMs configurations.\n",
> > +          (unsigned long)kinfo->unassigned_mem >> 10);
> > +}
> > +#endif
> > +
> >   static int __init write_properties(struct domain *d, struct kernel_info
> > *kinfo,
> >                                      const struct dt_device_node *node)
> >   {
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen
  2018-11-09 14:01   ` Julien Grall
@ 2018-11-09 21:38     ` Stefano Stabellini
  2018-11-09 22:35       ` Julien Grall
  0 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-09 21:38 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Achin.Gupta, Stefano Stabellini,
	andrii_anisov, xen-devel

On Fri, 9 Nov 2018, Julien Grall wrote:
> Hi,
> 
> On 02/11/2018 23:44, Stefano Stabellini wrote:
> > Make sure to only look for multiboot compatible nodes only under
> > /chosen, not under any other paths (depth <= 3).
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > 
> > ---
> > 
> > Changes in v6:
> > - do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
> > - remove sizeof, use hardcoded value
> > 
> > Changes in v5:
> > - add patch
> > - add check on return value of fdt_get_path
> > ---
> >   xen/arch/arm/bootfdt.c | 14 +++++++++++---
> >   1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > index 8eba42c..a42fe87 100644
> > --- a/xen/arch/arm/bootfdt.c
> > +++ b/xen/arch/arm/bootfdt.c
> > @@ -173,7 +173,15 @@ static void __init process_multiboot_node(const void
> > *fdt, int node,
> >       bootmodule_kind kind;
> >       paddr_t start, size;
> >       const char *cmdline;
> > -    int len;
> > +    int len = 8; /* sizeof "/chosen" */
> > +    char path[8];
> > +    int ret;
> > +
> > +    /* Check that the node is under "chosen" */
> > +    ret = fdt_get_path(fdt, node, path, len);
> > +    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||
> 
> I don't understand why you specifically check for -FDT_ERR_NOSPACE here.
> 
> Looking at fdt_get_path(...) there are case where the function may return
> -FDT_ERR_NOSPACE yet not filling up path. So you would end up to compare
> garbage.

I am explicitely checking for -FDT_ERR_NOSPACE because it is a valid
condition in this case: -FDT_ERR_NOSPACE is returned where `path' is not
big enough to contain the full device tree path. It is OK and expected,
given that path is only 8 chars long. So, in case of -FDT_ERR_NOSPACE,
we should continue and do the comparison with "/chosen". For other
errors we should return.


> > +         strncmp(path, "/chosen", len - 1) )
> > +        return;
> >         prop = fdt_get_property(fdt, node, "reg", &len);
> >       if ( !prop )
> > @@ -286,8 +294,8 @@ static int __init early_scan_node(const void *fdt,
> >   {
> >       if ( device_tree_node_matches(fdt, node, "memory") )
> >           process_memory_node(fdt, node, name, address_cells, size_cells);
> > -    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module"
> > ) ||
> > -              device_tree_node_compatible(fdt, node, "multiboot,module" ))
> > +    else if ( depth <= 3 && (device_tree_node_compatible(fdt, node,
> > "xen,multiboot-module" ) ||
> > +              device_tree_node_compatible(fdt, node, "multiboot,module" )))
> >           process_multiboot_node(fdt, node, name, address_cells,
> > size_cells);
> >       else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen")
> > )
> >           process_chosen_node(fdt, node, name, address_cells, size_cells);
> > 

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen
  2018-11-09 21:38     ` Stefano Stabellini
@ 2018-11-09 22:35       ` Julien Grall
  2018-11-12 21:13         ` Stefano Stabellini
  0 siblings, 1 reply; 41+ messages in thread
From: Julien Grall @ 2018-11-09 22:35 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin.Gupta, andrii_anisov, xen-devel

Hi Stefano,

On 11/9/18 9:38 PM, Stefano Stabellini wrote:
> On Fri, 9 Nov 2018, Julien Grall wrote:
>> Hi,
>>
>> On 02/11/2018 23:44, Stefano Stabellini wrote:
>>> Make sure to only look for multiboot compatible nodes only under
>>> /chosen, not under any other paths (depth <= 3).
>>>
>>> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
>>>
>>> ---
>>>
>>> Changes in v6:
>>> - do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
>>> - remove sizeof, use hardcoded value
>>>
>>> Changes in v5:
>>> - add patch
>>> - add check on return value of fdt_get_path
>>> ---
>>>    xen/arch/arm/bootfdt.c | 14 +++++++++++---
>>>    1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
>>> index 8eba42c..a42fe87 100644
>>> --- a/xen/arch/arm/bootfdt.c
>>> +++ b/xen/arch/arm/bootfdt.c
>>> @@ -173,7 +173,15 @@ static void __init process_multiboot_node(const void
>>> *fdt, int node,
>>>        bootmodule_kind kind;
>>>        paddr_t start, size;
>>>        const char *cmdline;
>>> -    int len;
>>> +    int len = 8; /* sizeof "/chosen" */
>>> +    char path[8];
>>> +    int ret;
>>> +
>>> +    /* Check that the node is under "chosen" */
>>> +    ret = fdt_get_path(fdt, node, path, len);
>>> +    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||
>>
>> I don't understand why you specifically check for -FDT_ERR_NOSPACE here.
>>
>> Looking at fdt_get_path(...) there are case where the function may return
>> -FDT_ERR_NOSPACE yet not filling up path. So you would end up to compare
>> garbage.
> 
> I am explicitely checking for -FDT_ERR_NOSPACE because it is a valid
> condition in this case: -FDT_ERR_NOSPACE is returned where `path' is not
> big enough to contain the full device tree path. It is OK and expected,
> given that path is only 8 chars long. So, in case of -FDT_ERR_NOSPACE,
> we should continue and do the comparison with "/chosen". For other
> errors we should return.

Let's take an example with a path called /deadbeef. This will not hold 
in the variable path. Do you agree that fdt_get_path will return 
-FDT_ERR_NO_SPACE in that case?

AFAIU the function fdt_get_path, the buffer will contain the character
/ followed by garbage as '\0' is only added in successful path.

This also fit with the description of fdt_get_path when 
-FDT_ERR_NO_SPACE. It does not promise you the buffer will contain 
anything. It only tells you that the path on the given node will not fit 
in the buffer.

So I still don't think you can assume the behavior you described above.

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen
  2018-11-09 22:35       ` Julien Grall
@ 2018-11-12 21:13         ` Stefano Stabellini
  2018-11-12 21:37           ` Julien Grall
  0 siblings, 1 reply; 41+ messages in thread
From: Stefano Stabellini @ 2018-11-12 21:13 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Achin.Gupta, Stefano Stabellini,
	andrii_anisov, xen-devel

On Fri, 9 Nov 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 11/9/18 9:38 PM, Stefano Stabellini wrote:
> > On Fri, 9 Nov 2018, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 02/11/2018 23:44, Stefano Stabellini wrote:
> > > > Make sure to only look for multiboot compatible nodes only under
> > > > /chosen, not under any other paths (depth <= 3).
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > > > 
> > > > ---
> > > > 
> > > > Changes in v6:
> > > > - do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
> > > > - remove sizeof, use hardcoded value
> > > > 
> > > > Changes in v5:
> > > > - add patch
> > > > - add check on return value of fdt_get_path
> > > > ---
> > > >    xen/arch/arm/bootfdt.c | 14 +++++++++++---
> > > >    1 file changed, 11 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > > > index 8eba42c..a42fe87 100644
> > > > --- a/xen/arch/arm/bootfdt.c
> > > > +++ b/xen/arch/arm/bootfdt.c
> > > > @@ -173,7 +173,15 @@ static void __init process_multiboot_node(const
> > > > void
> > > > *fdt, int node,
> > > >        bootmodule_kind kind;
> > > >        paddr_t start, size;
> > > >        const char *cmdline;
> > > > -    int len;
> > > > +    int len = 8; /* sizeof "/chosen" */
> > > > +    char path[8];
> > > > +    int ret;
> > > > +
> > > > +    /* Check that the node is under "chosen" */
> > > > +    ret = fdt_get_path(fdt, node, path, len);
> > > > +    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||
> > > 
> > > I don't understand why you specifically check for -FDT_ERR_NOSPACE here.
> > > 
> > > Looking at fdt_get_path(...) there are case where the function may return
> > > -FDT_ERR_NOSPACE yet not filling up path. So you would end up to compare
> > > garbage.
> > 
> > I am explicitely checking for -FDT_ERR_NOSPACE because it is a valid
> > condition in this case: -FDT_ERR_NOSPACE is returned where `path' is not
> > big enough to contain the full device tree path. It is OK and expected,
> > given that path is only 8 chars long. So, in case of -FDT_ERR_NOSPACE,
> > we should continue and do the comparison with "/chosen". For other
> > errors we should return.
> 
> Let's take an example with a path called /deadbeef. This will not hold in the
> variable path. Do you agree that fdt_get_path will return -FDT_ERR_NO_SPACE in
> that case?
> 
> AFAIU the function fdt_get_path, the buffer will contain the character
> / followed by garbage as '\0' is only added in successful path.
> 
> This also fit with the description of fdt_get_path when -FDT_ERR_NO_SPACE. It
> does not promise you the buffer will contain anything. It only tells you that
> the path on the given node will not fit in the buffer.
> 
> So I still don't think you can assume the behavior you described above.

The lack of '\0' is not an issue, we can still compare the content with
strncmp even if '\0' is missing. But you are right, the description is
not clear about the content of `path' if -FDT_ERR_NO_SPACE is returned.
It doesn't clearly say that the content is still guaranteed to be
properly populated until the end of `buf'.

If we don't want to rely on the implementation of fdt_get_path to still
populate the content in case of -FDT_ERR_NO_SPACE, then we'll have to
allocate roughtly DT_MAX_NAME*3 = 41*3 chars for path.
Technically I think it would be DT_MAX_NAME*2+6 ("chosen") +3 ('/' three
times) + ('\0') = 92. We could use 100 chars to stay on the safe side.
Is that OK for you?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen
  2018-11-12 21:13         ` Stefano Stabellini
@ 2018-11-12 21:37           ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2018-11-12 21:37 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Stefano Stabellini, Achin Gupta, nd, andrii_anisov, xen-devel

Hi,

On 12/11/2018 21:13, Stefano Stabellini wrote:
> On Fri, 9 Nov 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 11/9/18 9:38 PM, Stefano Stabellini wrote:
>>> On Fri, 9 Nov 2018, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> On 02/11/2018 23:44, Stefano Stabellini wrote:
>>>>> Make sure to only look for multiboot compatible nodes only under
>>>>> /chosen, not under any other paths (depth <= 3).
>>>>>
>>>>> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
>>>>>
>>>>> ---
>>>>>
>>>>> Changes in v6:
>>>>> - do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
>>>>> - remove sizeof, use hardcoded value
>>>>>
>>>>> Changes in v5:
>>>>> - add patch
>>>>> - add check on return value of fdt_get_path
>>>>> ---
>>>>>     xen/arch/arm/bootfdt.c | 14 +++++++++++---
>>>>>     1 file changed, 11 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
>>>>> index 8eba42c..a42fe87 100644
>>>>> --- a/xen/arch/arm/bootfdt.c
>>>>> +++ b/xen/arch/arm/bootfdt.c
>>>>> @@ -173,7 +173,15 @@ static void __init process_multiboot_node(const
>>>>> void
>>>>> *fdt, int node,
>>>>>         bootmodule_kind kind;
>>>>>         paddr_t start, size;
>>>>>         const char *cmdline;
>>>>> -    int len;
>>>>> +    int len = 8; /* sizeof "/chosen" */
>>>>> +    char path[8];
>>>>> +    int ret;
>>>>> +
>>>>> +    /* Check that the node is under "chosen" */
>>>>> +    ret = fdt_get_path(fdt, node, path, len);
>>>>> +    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||
>>>>
>>>> I don't understand why you specifically check for -FDT_ERR_NOSPACE here.
>>>>
>>>> Looking at fdt_get_path(...) there are case where the function may return
>>>> -FDT_ERR_NOSPACE yet not filling up path. So you would end up to compare
>>>> garbage.
>>>
>>> I am explicitely checking for -FDT_ERR_NOSPACE because it is a valid
>>> condition in this case: -FDT_ERR_NOSPACE is returned where `path' is not
>>> big enough to contain the full device tree path. It is OK and expected,
>>> given that path is only 8 chars long. So, in case of -FDT_ERR_NOSPACE,
>>> we should continue and do the comparison with "/chosen". For other
>>> errors we should return.
>>
>> Let's take an example with a path called /deadbeef. This will not hold in the
>> variable path. Do you agree that fdt_get_path will return -FDT_ERR_NO_SPACE in
>> that case?
>>
>> AFAIU the function fdt_get_path, the buffer will contain the character
>> / followed by garbage as '\0' is only added in successful path.
>>
>> This also fit with the description of fdt_get_path when -FDT_ERR_NO_SPACE. It
>> does not promise you the buffer will contain anything. It only tells you that
>> the path on the given node will not fit in the buffer.
>>
>> So I still don't think you can assume the behavior you described above.
> 
> The lack of '\0' is not an issue, we can still compare the content with
> strncmp even if '\0' is missing.

The problem is not really because of '\0' is missing but the fact you 
may not have 8 valid characters in the buffer. In some case, you will 
only have one, yet you compare with a 8 characters string.

> But you are right, the description is
> not clear about the content of `path' if -FDT_ERR_NO_SPACE is returned.
> It doesn't clearly say that the content is still guaranteed to be
> properly populated until the end of `buf'.
> 
> If we don't want to rely on the implementation of fdt_get_path to still
> populate the content in case of -FDT_ERR_NO_SPACE, 

I would have been happy to just update the documentation but the code 
does not seem match that behavior (see above). If you can prove me the 
case I gave can work perfectly, then I might reconsider it.

then we'll have to
> allocate roughtly DT_MAX_NAME*3 = 41*3 chars for path.
> Technically I think it would be DT_MAX_NAME*2+6 ("chosen") +3 ('/' three
> times) + ('\0') = 92. We could use 100 chars to stay on the safe side.

I would prefer the 92 characters with a comment on top. This is quite 
unlikely to hit it.

Cheers,

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2018-11-12 21:37 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 23:45 [PATCH v6 00/25] dom0less step1: boot multiple domains from device tree Stefano Stabellini
2018-11-02 23:44 ` [PATCH v6 01/26] xen: allow console_io hypercalls from certain DomUs Stefano Stabellini
2018-11-02 23:44 ` [PATCH v6 02/26] xen/arm: extend device tree based multiboot protocol Stefano Stabellini
2018-11-02 23:44 ` [PATCH v6 03/26] xen/arm: document dom0less Stefano Stabellini
2018-11-02 23:44 ` [PATCH v6 04/26] xen/arm: increase MAX_MODULES Stefano Stabellini
2018-11-02 23:44 ` [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen Stefano Stabellini
2018-11-09 14:01   ` Julien Grall
2018-11-09 21:38     ` Stefano Stabellini
2018-11-09 22:35       ` Julien Grall
2018-11-12 21:13         ` Stefano Stabellini
2018-11-12 21:37           ` Julien Grall
2018-11-02 23:44 ` [PATCH v6 06/26] xen/arm: introduce bootcmdlines Stefano Stabellini
2018-11-02 23:44 ` [PATCH v6 07/26] xen/arm: don't add duplicate boot modules, introduce domU flag Stefano Stabellini
2018-11-09 14:06   ` Julien Grall
2018-11-02 23:45 ` [PATCH v6 08/26] xen/arm: probe domU kernels and initrds Stefano Stabellini
2018-11-09 14:09   ` Julien Grall
2018-11-02 23:45 ` [PATCH v6 09/26] xen/arm: add start to struct bootcmdline Stefano Stabellini
2018-11-09 14:10   ` Julien Grall
2018-11-02 23:45 ` [PATCH v6 10/26] xen/arm: rename get_11_allocation_size to get_allocation_size Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 11/26] xen/arm: rename allocate_memory to allocate_memory_11 Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 12/26] xen/arm: introduce allocate_memory Stefano Stabellini
2018-11-09 14:19   ` Julien Grall
2018-11-09 21:33     ` Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 13/26] xen/arm: refactor construct_dom0 Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 14/26] xen/arm: move unregister_init_virtual_region to init_done Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 15/26] xen/arm: introduce create_domUs Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 16/26] xen/arm: implement construct_domU Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 17/26] xen/arm: generate a simple device tree for domUs Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 18/26] xen/arm: make set_interrupt_ppi able to handle non-PPI Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 19/26] xen/arm: generate vpl011 node on device tree for domU Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 20/26] xen/arm: introduce a union in vpl011 Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 21/26] xen/arm: refactor vpl011_data_avail Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 22/26] xen: support console_switching between Dom0 and DomUs on ARM Stefano Stabellini
2018-11-05  9:15   ` Jan Beulich
2018-11-02 23:45 ` [PATCH v6 23/26] xen/arm: Allow vpl011 to be used by DomU Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 24/26] xen/vpl011: buffer out chars when the backend is xen Stefano Stabellini
2018-11-09 14:32   ` Julien Grall
2018-11-09 21:13     ` Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 25/26] xen/arm: move kernel.h to asm-arm/ Stefano Stabellini
2018-11-02 23:45 ` [PATCH v6 26/26] xen/arm: split domain_build.c Stefano Stabellini
2018-11-09 14:35   ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.