xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: Shannon Zhao <zhaoshenglong@huawei.com>, xen-devel@lists.xen.org
Cc: stefano.stabellini@citrix.com, shannon.zhao@linaro.org,
	peter.huangpeng@huawei.com
Subject: Re: [PATCH v6 14/22] arm/acpi: Create min DT stub for Dom0
Date: Tue, 22 Mar 2016 01:07:41 +0000	[thread overview]
Message-ID: <56F09ADD.3040203@arm.com> (raw)
In-Reply-To: <1458207668-12012-15-git-send-email-zhaoshenglong@huawei.com>



On 17/03/2016 09:41, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Create a DT for Dom0 for ACPI-case only. DT contains minimal required
> informations such as Dom0 bootargs, initrd, efi description table and
> address of uefi memory table.
>
> Also port the document of this device tree bindings from Linux.

Porting means the document already exist in Linux and therefore there is 
a commit associated.

In this case, the document didn't reach Linux and this series will 
likely reach upstream and the Linux one.

So I think this should be the other way around.

>
> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>   docs/misc/arm/device-tree/uefi.txt |  58 +++++++++++++++
>   xen/arch/arm/domain_build.c        | 143 +++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/efi/efi-dom0.c        |  48 +++++++++++++
>   xen/include/asm-arm/setup.h        |   2 +
>   4 files changed, 251 insertions(+)
>   create mode 100644 docs/misc/arm/device-tree/uefi.txt
>
> diff --git a/docs/misc/arm/device-tree/uefi.txt b/docs/misc/arm/device-tree/uefi.txt
> new file mode 100644
> index 0000000..41a8be0
> --- /dev/null
> +++ b/docs/misc/arm/device-tree/uefi.txt

Looking at the content, this file describe the bindings to notify the 
kernel that it runs on Xen. This is not UEFI specific.

So guest.txt would be a better name.

> @@ -0,0 +1,58 @@
> +* Xen hypervisor device tree bindings
> +
> +Xen ARM virtual platforms shall have a top-level "hypervisor" node with
> +the following properties:
> +
> +- compatible:
> +	compatible = "xen,xen-<version>", "xen,xen";
> +  where <version> is the version of the Xen ABI of the platform.
> +
> +- reg: specifies the base physical address and size of a region in
> +  memory where the grant table should be mapped to, using an
> +  HYPERVISOR_memory_op hypercall. The memory region is large enough to map
> +  the whole grant table (it is larger or equal to gnttab_max_grant_frames()).
> +
> +- interrupts: the interrupt used by Xen to inject event notifications.
> +  A GIC node is also required.

The properties "reg" and "interrupts" are not created by Xen when using 
ACPI. However, based on the bindings description they are mandatory.

Please update the description to reflect that the properties doesn't 
exist when booting using ACPI.

> +
> +To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
> +under /hypervisor with following parameters:
> +
> +________________________________________________________________________________
> +Name                      | Size   | Description
> +================================================================================
> +xen,uefi-system-table     | 64-bit | Guest physical address of the UEFI System
> +                          |        | Table.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-start       | 64-bit | Guest physical address of the UEFI memory
> +                          |        | map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
> +                          |        | pointed to in previous entry.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
> +                          |        | memory map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
> +--------------------------------------------------------------------------------
> +
> +Example (assuming #address-cells = <2> and #size-cells = <2>):
> +
> +hypervisor {
> +	compatible = "xen,xen-4.3", "xen,xen";
> +	reg = <0 0xb0000000 0 0x20000>;
> +	interrupts = <1 15 0xf08>;
> +	uefi {
> +		xen,uefi-system-table = <0xXXXXXXXX>;
> +		xen,uefi-mmap-start = <0xXXXXXXXX>;
> +		xen,uefi-mmap-size = <0xXXXXXXXX>;
> +		xen,uefi-mmap-desc-size = <0xXXXXXXXX>;
> +		xen,uefi-mmap-desc-ver = <0xXXXXXXXX>;
> +        };
> +};
> +
> +The format and meaning of the "xen,uefi-*" parameters are similar to those in
> +Documentation/arm/uefi.txt in Linux, which are provided by the regular Linux
> +UEFI stub. However they differ because they are provided by the Xen hypervisor,
> +together with a set of UEFI runtime services implemented via hypercalls, see
> +xen/include/public/platform.h.
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e036887..6726e45 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1357,6 +1357,145 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>   }
>
>   #ifdef CONFIG_ACPI
> +#define ACPI_DOM0_FDT_MIN_SIZE 4096
> +
> +static int make_chosen_node(const struct kernel_info *kinfo,

Please prefix it with acpi_ to make clear it's ACPI specific.

> +                            struct membank tbl_add[])

Why do you pass tbl_add here? You don't use it within the function.

> +{
> +    int res;
> +    const char *bootargs = NULL;
> +    const struct bootmodule *mod = kinfo->kernel_bootmodule;
> +    void *fdt = kinfo->fdt;
> +
> +    DPRINT("Create chosen node\n");
> +    res = fdt_begin_node(fdt, "chosen");
> +    if ( res )
> +        return res;
> +
> +    if ( mod && mod->cmdline[0] )
> +    {
> +        bootargs = &mod->cmdline[0];
> +        res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
> +        if ( res )
> +           return res;
> +    }
> +
> +    /*
> +     * If the bootloader provides an initrd, we must create a placeholder
> +     * for the initrd properties. The values will be replaced later.
> +     */
> +    if ( mod && mod->size )
> +    {
> +        u64 a = 0;
> +        res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a));
> +        if ( res )
> +            return res;
> +    }
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}

[...]

> @@ -1706,6 +1845,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>           return rc;
>       }
>
> +    rc = create_acpi_dtb(kinfo, tbl_add);
> +    if ( rc != 0 )
> +        return rc;
> +
>       return 0;
>   }
>   #else
> diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
> index 3ffde94..688fa8b 100644
> --- a/xen/arch/arm/efi/efi-dom0.c
> +++ b/xen/arch/arm/efi/efi-dom0.c
> @@ -24,6 +24,7 @@
>   #include "efi.h"
>   #include "efi-dom0.h"
>   #include <xen/pfn.h>
> +#include <xen/libfdt/libfdt.h>
>   #include <asm/setup.h>
>   #include <asm/acpi.h>
>
> @@ -132,3 +133,50 @@ void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>       tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
>                                * (mem->nr_banks + acpi_mem.nr_banks + 1);
>   }
> +
> +/* Create place holder for efi values. */

Placeholder means we will create an empty space and replace by proper 
values later.

However, you directly create the properties with proper values.

> +int __init arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[])

It's odd to have this function in efi-dom0.c. We want to keep all the 
device tree creation together.

Also, to stay consistent with the other name. Please rename the function 
into acpi_make_efi_nodes.

> +{
> +    u64 fdt_val64;
> +    u32 fdt_val32;
> +    int desc_ver = 1;
> +    int res;
> +
> +    res = fdt_begin_node(fdt, "uefi");
> +    if ( res )
> +        return res;
> +
> +    fdt_val64 = cpu_to_fdt64(tbl_add[TBL_EFIT].start);
> +    res = fdt_property(fdt, "xen,uefi-system-table",
> +                       &fdt_val64, sizeof(fdt_val64));

Those two lines could be replaced by fdt_property_u64.

> +    if ( res )
> +        return res;
> +
> +    fdt_val64 = cpu_to_fdt64(tbl_add[TBL_MMAP].start);
> +    res = fdt_property(fdt, "xen,uefi-mmap-start",
> +                       &fdt_val64,  sizeof(fdt_val64));

Ditto

> +    if ( res )
> +        return res;
> +
> +    fdt_val32 = cpu_to_fdt32(tbl_add[TBL_MMAP].size);
> +    res = fdt_property(fdt, "xen,uefi-mmap-size",
> +                       &fdt_val32,  sizeof(fdt_val32));

Here by fdt_property_u32.

> +    if ( res )
> +        return res;
> +
> +    fdt_val32 = cpu_to_fdt32(sizeof(EFI_MEMORY_DESCRIPTOR));
> +    res = fdt_property(fdt, "xen,uefi-mmap-desc-size",
> +                         &fdt_val32, sizeof(fdt_val32));

Ditto

> +    if ( res )
> +        return res;
> +
> +    fdt_val32 = cpu_to_fdt32(desc_ver);
> +    res = fdt_property(fdt, "xen,uefi-mmap-desc-ver",
> +                         &fdt_val32, sizeof(fdt_val32));

Ditto

> +    if ( res )
> +        return res;
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index b2899f2..05f0210 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -61,6 +61,8 @@ void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>                                   const struct meminfo *mem,
>                                   struct membank tbl_add[]);
>
> +int arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
> +
>   int construct_dom0(struct domain *d);
>
>   void discard_initial_modules(void);
>

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-03-22  1:07 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-17  9:40 [PATCH v6 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
2016-03-17  9:40 ` [PATCH v6 01/22] arm/acpi: Estimate memory required for acpi/efi tables Shannon Zhao
2016-03-18 18:44   ` Julien Grall
2016-03-21 13:01     ` Jan Beulich
2016-03-22 13:01     ` Shannon Zhao
2016-03-22  0:28   ` Julien Grall
2016-03-24 12:36   ` Stefano Stabellini
2016-03-17  9:40 ` [PATCH v6 02/22] arm/acpi: Add a helper function to get the acpi table offset Shannon Zhao
2016-03-21 12:16   ` Julien Grall
2016-03-24 12:32   ` Stefano Stabellini
2016-03-17  9:40 ` [PATCH v6 03/22] arm/acpi: Prepare FADT table for Dom0 Shannon Zhao
2016-03-21 12:25   ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 04/22] arm/gic: Add a new callback for creating MADT " Shannon Zhao
2016-03-21 15:18   ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 05/22] arm/acpi: Prepare " Shannon Zhao
2016-03-21 15:26   ` Julien Grall
2016-03-21 15:51     ` Jan Beulich
2016-03-17  9:40 ` [PATCH v6 06/22] arm/acpi: Prepare STAO " Shannon Zhao
2016-03-21 15:31   ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 07/22] arm/acpi: Prepare XSDT " Shannon Zhao
2016-03-21 15:43   ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 08/22] arm/acpi: Prepare RSDP " Shannon Zhao
2016-03-21 15:47   ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 09/22] arm/p2m: Add helper functions to map memory regions Shannon Zhao
2016-03-21 15:52   ` Julien Grall
2016-03-22 13:05     ` Shannon Zhao
2016-03-22 15:59       ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 10/22] arm/acpi: Map all other tables for Dom0 Shannon Zhao
2016-03-21 15:57   ` Julien Grall
2016-03-17  9:40 ` [PATCH v6 11/22] arm/acpi: Prepare EFI system table " Shannon Zhao
2016-03-21 16:10   ` Julien Grall
2016-03-22 13:06     ` Shannon Zhao
2016-03-22 13:24       ` Jan Beulich
2016-03-17  9:40 ` [PATCH v6 12/22] arm/acpi: Prepare EFI memory descriptor " Shannon Zhao
2016-03-21 16:51   ` Julien Grall
2016-03-22 13:16     ` Shannon Zhao
2016-03-22 16:04       ` Julien Grall
2016-03-24 15:06         ` Shannon Zhao
2016-03-24 15:23           ` Julien Grall
2016-03-24 12:39   ` Stefano Stabellini
2016-03-17  9:40 ` [PATCH v6 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0 Shannon Zhao
2016-03-22  0:42   ` Julien Grall
2016-03-22 13:18     ` Shannon Zhao
2016-03-22 16:16       ` Julien Grall
2016-03-24 14:59         ` Shannon Zhao
2016-03-24 15:35           ` Julien Grall
2016-03-17  9:41 ` [PATCH v6 14/22] arm/acpi: Create min DT stub for Dom0 Shannon Zhao
2016-03-22  1:07   ` Julien Grall [this message]
2016-03-22 13:23     ` Shannon Zhao
2016-03-17  9:41 ` [PATCH v6 15/22] arm/acpi: Permit access all Xen unused SPIs " Shannon Zhao
2016-03-22 18:18   ` Julien Grall
2016-03-24 15:01     ` Shannon Zhao
2016-03-24 15:37       ` Julien Grall
2016-03-25  7:13         ` Shannon Zhao
2016-03-17  9:41 ` [PATCH v6 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically Shannon Zhao
2016-03-22 19:17   ` Julien Grall
2016-03-24 12:41   ` Stefano Stabellini
2016-03-17  9:41 ` [PATCH v6 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions Shannon Zhao
2016-03-22 19:33   ` Julien Grall
2016-03-24 12:45     ` Stefano Stabellini
2016-03-24 15:03       ` Shannon Zhao
2016-03-24 15:39         ` Julien Grall
2016-03-17  9:41 ` [PATCH v6 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0 Shannon Zhao
2016-03-23 11:50   ` Julien Grall
2016-03-17  9:41 ` [PATCH v6 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ Shannon Zhao
2016-03-17 10:42   ` Jan Beulich
2016-03-17 11:04     ` Shannon Zhao
2016-03-17 11:29       ` Jan Beulich
2016-03-17 11:56         ` Shannon Zhao
2016-03-17 12:42           ` Jan Beulich
2016-03-17  9:41 ` [PATCH v6 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI Shannon Zhao
2016-03-23 12:15   ` Julien Grall
2016-03-17  9:41 ` [PATCH v6 21/22] xen/arm: Add a hypercall for device mmio mapping Shannon Zhao
2016-03-17  9:41 ` [PATCH v6 22/22] xen/arm64: Add ACPI support Shannon Zhao
2016-03-17 10:52   ` Jan Beulich
2016-03-17 11:03     ` Shannon Zhao
2016-03-17 11:31       ` Jan Beulich
2016-03-17 13:10         ` Shannon Zhao
2016-03-17 13:24           ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56F09ADD.3040203@arm.com \
    --to=julien.grall@arm.com \
    --cc=peter.huangpeng@huawei.com \
    --cc=shannon.zhao@linaro.org \
    --cc=stefano.stabellini@citrix.com \
    --cc=xen-devel@lists.xen.org \
    --cc=zhaoshenglong@huawei.com \
    --subject='Re: [PATCH v6 14/22] arm/acpi: Create min DT stub for Dom0' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).